-
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
.every() .find() .filter() methods fail with "This expression is not callable." error on union of array types #44373
Comments
Hi @DanielRosenwasser, please check out my comment on that issue (also linked in the writeup above). This issue is a followup to #36390, not a duplicate. That issue was closed when #31023 fixed the majority of array functions to work with unions of array types. This issue (and #44063) track the handful of array functions which still don't work post-#31023. If you'd rather reopen #36390 and rewrite the title and description I suppose we could dedupe this in, but given that there was so much activity there dating back years, and that most of the usecases motivating that issue have in fact been solved for, it seemed much clearer to me to track the remaining bits of cleanup separately here. |
@weswigham what's the current state of what we might be able to do here? |
Well, we'd need to come up with sensible rules for merging overload lists. Conceptually, we could (attempt to) make a list of 4 overloads by combining each of the input signatures, except there's no inherent ordering to those 4 signatures, unlike a normal overload set, so it's unclear how we'd choose which signature to resolve against, ultimately. |
As a workaround for anyone searching lodash's |
@weswigham Is there anything in particular that makes methods like |
Maybe something could be done specifically for arrays instead of worrying about merging arbitrary signatures? Like, widen |
Can we add const m2 = [] as Array<number> | Array<string>;
m2.map(it=>it); // why does that work ?
m2.reduce((it) => it); // error
m2.concat(m2); // error |
Probably known, but also affects:
|
Strengthen forEach, filter, map, from, find, findIndex to be more stricter, so that the TypeScript compiler can understand the type of this when thisArg is used but the predicate does not explicitly mention the type of this and only be implicit. Although methods like some, every also have the thisArg as its parameter but is not included in this change, because in some places those are called like e.g. array.some(isNaN) and in these cases, the TSC does not understand the type of the isNaN is actually a subtype. Also, this excludes to improve the methods cases where the interface has overloads of the same method, due to the bug microsoft#44373, as the current TSC cannot resolve multiple declarations properly for some cases.
Strengthen forEach, filter, map, from, find, findIndex to be more stricter, so that the TypeScript compiler can understand the type of this when thisArg is used but the predicate does not explicitly mention the type of this and only be implicit. Although methods like some, every also have the thisArg as its parameter but is not included in this change, because in some places those are called like e.g. array.some(isNaN) and in these cases, the TSC does not understand the type of the isNaN is actually a subtype. Also, this excludes to improve the methods cases where the interface has overloads of the same method, due to the bug microsoft#44373, as the current TSC cannot resolve multiple declarations properly for cases which these functions are called on a union of types which all are arrays. e.g. (string[]|number[]).forEach(predicate, ...);
Hey @DanielRosenwasser, any estimates when could that be fixed? |
For anyone looking for an easy workaround: interface Fizz {
id: number;
fizz: string;
}
interface Buzz {
id: number;
buzz: string;
}
const arr: Fizz[] | Buzz[] = [];
arr.filter(item => item.id < 5); // This is an error
const arrFixed: Array<typeof arr[number]> = arr; // This is the fix
arrFixed.filter(item => item.id < 5); // This works! It's not exactly equivalent but should work for most cases. Hope this helps! |
@gaberogan this just makes arrFixed type to |
Why would that result in declare const a: string[] | number[]
// Error. Should work and result in `string[] | number[]`
const b = a.filter((_i /* type is `any` */) => true)
// Works, but wrong type. `c` is (string | number)[]
const c = (a as typeof a[number][]).filter((_i /* type is `string | number` */) => true)
// Works but has two typecasts. Cumbersome.
const d = (a as typeof a[number][]).filter((_i /* type is `string | number` */) => true) as typeof a |
@weswigham let's take another bite at the apple |
Hello @weswigham, please has this issue been resolved? |
@Emmahchinonso you can see a few lines above your comment is a linked pull request that will close the issue. This issue is still open, meaning it is still open, but you can subscribe to notifications if you want. |
will this also gonna work after this is merged? const arr: string[] | number[] = [];
const val: string | number = 5;
const index = arr.indexOf(val); |
@gegxen why don’t you clone the commit in the linked PR and try it? |
Copying snippet from @gaberogan It's better to union the type before typing it as Array.
|
@kkumawat333 this isn't a problem anymore as of 5.2 |
@ericbf No it seems fine to me, |
Or just do a simple change like this: |
Hello @Jv7680, this issue has been closed for a while now. But, for the record, the problem with that suggestion is that the type would not actually be correct. But that's been discussed already further up in the thread. |
Bug Report
🔎 Search Terms
🕗 Version & Regression Information
⏯ Playground Link
💻 Code
🙁 Actual behavior
Compilation fails with the following error:
🙂 Expected behavior
Compilation succeeds, as it does for map and several other array functions.
🔖 Useful links
The text was updated successfully, but these errors were encountered: