-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider re-ordering Array#reduce overloads in lib.d.ts #7014
Comments
Simpler example: let arrayOfObjs: {}[] = [/*...*/];
let countOfProps = arrayOfObjs.reduce((sum, obj) => sum + Object.keys(obj).length, 0); This doesn't compile, tsc reports In fact the code is fine and runs fine. tsc took the first overload since it works with In general, the second overload will always be skipped if Swapping the order of the two |
I believe it's safe to do this now -- the order we had was dependent on behavior that is no longer the case (ancient history, see http://typescript.codeplex.com/workitem/2352). |
Wow, nice archaeological find 😄 Those were the days... |
Glad this can now be resolved, and I really appreciate the effort the team takes in polishing these corner cases. As a side not, I really dislike the argument ordering that TC39 chose for |
Just stumbled on this - look forward to this being resolved! For anyone else wondering what to do in the interim just supply the type parameter. So to take @yortus example, this: let arrayOfObjs: {}[] = [/*...*/];
let countOfProps = arrayOfObjs.reduce((sum, obj) => sum + Object.keys(obj).length, 0); Becomes: let arrayOfObjs: {}[] = [/*...*/];
let countOfProps = arrayOfObjs.reduce<number>((sum, obj) => sum + Object.keys(obj).length, 0); |
Taking PRs on this with the caveat we're still not 100% sure this is going to be an overall improvement, so reserving the right to reject it if it turns out to be bad on net. |
|
Looks like @vilic took care of this issue. maybe this issue should be closed? It's been around for awhile. @RyanCavanaugh |
@bdurrani Just noticed that I closed the PR myself. Might had encountered some problem but forgot to update the comment here. |
agreed, we encounter this with typescript 3.7.3 const A = [1, '2', 3]
const str: string = A.reduce((str, a) => `${str} ${a.toString()}`, '') which results in this compilation error: Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
2 const str: string = A.reduce((str, a) => `${str} ${a.toString}`, '')
~~~ there is an easy enough workaround for now, i.e. |
An easier workaround.We provide the generic paramater here, so it matches the second overload. const A = [1, '2', 3]
const str: string = A.reduce<string>((str, a) => `${str} ${a.toString()}`, '') |
This comment has been minimized.
This comment has been minimized.
may be fix this with typescript 4.2.2? |
Example code:
The call to
Array#reduce
in the above example definitely returns a unary function, but the type system erroneously infers the return type as a binary function.This seems to be caused by the declaration order of the two overloads of
Array#reduce
inlib.d.ts
. If the declaration order is reversed, the problem is solved.The two overloaded declarations in
lib.d.ts
are as follows:The first overload matches the example with
T=BinaryFunction
, since it satisfies the compiler's assignability checks. So the second overload is never considered, even though it is a strictly better match, withT=BinaryFunction
andU=UnaryFunction
.Would it be possible to swap the overload order for
Array#reduce
(andArray#reduceRight
) inlib.d.ts
to resolve this issue?The text was updated successfully, but these errors were encountered: