Skip to content
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

Closed
ialexryan opened this issue Jun 1, 2021 · 27 comments · Fixed by #53489
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@ialexryan
Copy link
Contributor

ialexryan commented Jun 1, 2021

Bug Report

🔎 Search Terms

  • This expression is not callable
  • none of those signatures are compatible with each other

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about union types. I also reviewed previous related issues.

⏯ Playground Link

💻 Code

interface Fizz {
    id: number;
    fizz: string;
}

interface Buzz {
    id: number;
    buzz: string;
}

([] as Fizz[] | Buzz[]).filter(item => item.id < 5); 

🙁 Actual behavior

Compilation fails with the following error:

This expression is not callable.
  Each member of the union type '{ <S extends Fizz>(predicate: (value: Fizz, index: number, array: Fizz[]) => value is S, thisArg?: any): S[]; (predicate: (value: Fizz, index: number, array: Fizz[]) => unknown, thisArg?: any): Fizz[]; } | { ...; }' has signatures, but none of those signatures are compatible with each other.

🙂 Expected behavior

Compilation succeeds, as it does for map and several other array functions.

🔖 Useful links

@ialexryan
Copy link
Contributor Author

ialexryan commented Jun 1, 2021

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.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 2, 2021
@RyanCavanaugh
Copy link
Member

@weswigham what's the current state of what we might be able to do here?

@weswigham
Copy link
Member

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.

@jfamousket
Copy link

As a workaround for anyone searching lodash's filter function works as expected

@ITenthusiasm
Copy link

@weswigham Is there anything in particular that makes methods like filter difficult to work with compared to map? I haven't looked at the type definition in TS. But from a JS standpoint, the two signatures are the same; so it's a bit surprising that the methods involving predicates are more complex.

@jcalz
Copy link
Contributor

jcalz commented Jan 27, 2022

Maybe something could be done specifically for arrays instead of worrying about merging arbitrary signatures? Like, widen A[] | B[] to readonly (A | B)[] for these failing method calls? 🤷‍♂️

@dderjoel
Copy link

dderjoel commented Jan 27, 2022

Can we add concat to that list above, too?
I have a typescript-playground showing this here.

const m2 = [] as Array<number> | Array<string>;
m2.map(it=>it); // why does that work ?
m2.reduce((it) => it); // error
m2.concat(m2); // error

@vassudanagunta
Copy link

vassudanagunta commented Jun 23, 2022

  • .reduce() also doesn't work, but there is already an issue for that one. (Perhaps we should merge it into this?)

I think it should be merged; if not then its description "Array.reduce doesn't work" needs to be improved. See 44063.

@ericblade
Copy link

Probably known, but also affects:

const arr = [] as Array<number> | Uint8Array;
arr.reduce(...)

h-joo added a commit to h-joo/TypeScript that referenced this issue Oct 5, 2022
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.
h-joo added a commit to h-joo/TypeScript that referenced this issue Oct 5, 2022
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, ...);
@Buslowicz
Copy link

Hey @DanielRosenwasser, any estimates when could that be fixed?

@gaberogan
Copy link

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!

@bmiakoun
Copy link

bmiakoun commented Feb 4, 2023

@gaberogan this just makes arrFixed type to any

@ericbf
Copy link
Contributor

ericbf commented Mar 13, 2023

@bmiakoun

@gaberogan this just makes arrFixed type to any

Why would that result in any?

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

Playground link

@RyanCavanaugh
Copy link
Member

@weswigham let's take another bite at the apple

@Emmahchinonso
Copy link

Hello @weswigham, please has this issue been resolved?

@ericbf
Copy link
Contributor

ericbf commented Apr 11, 2023

@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.

@gegxen
Copy link

gegxen commented Apr 28, 2023

will this also gonna work after this is merged?

const arr: string[] | number[] = [];
const val: string | number = 5;
const index = arr.indexOf(val);

@gegxen
Copy link

gegxen commented Apr 28, 2023

@mon-jai the PR is not merged yet

@ericbf
Copy link
Contributor

ericbf commented Apr 28, 2023

@gegxen why don’t you clone the commit in the linked PR and try it?

@kkumawat333
Copy link

Copying snippet from @gaberogan

It's better to union the type before typing it as Array.

interface Fizz {
    id: number;
    fizz: string;
}

interface Buzz {
    id: number;
    buzz: string;
}

TFizzBuzz = Fizz | Buzz
const arr: TFizzBuzz[] = [];

arr.filter(item => item.id < 5); // This works for me

@joealden
Copy link

joealden commented Oct 9, 2023

@kkumawat333 this isn't a problem anymore as of 5.2

@gaberogan
Copy link

@ericbf No it seems fine to me, arrFixed is not any

Screenshot 2023-12-11 at 4 07 42 PM

@ericbf
Copy link
Contributor

ericbf commented Dec 12, 2023

@ericbf No it seems fine to me, arrFixed is not any

Exactly my point—I stated that arrFixed in the example was not any, contrary to @bmiakoun's comment above. Anyways, this issue is closed now—if you find something that needs to be discussed or fixed, probably open a new issue for that.

@Jv7680
Copy link

Jv7680 commented Jun 29, 2024

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!

Or just do a simple change like this:
_ Use this: const arr: (Fizz | Buzz)[] = [];
_ Instead of this: const arr: Fizz[] | Buzz[] = [];

@ericbf
Copy link
Contributor

ericbf commented Jun 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.