Skip to content

Array.forEach incorrectly narrows the inferred type when elements are annotated with union type #55201

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

Closed
shogunsea opened this issue Jul 29, 2023 · 5 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@shogunsea
Copy link

Bug Report

πŸ”Ž Search Terms

Array.forEach
union type

πŸ•— Version & Regression Information

all versions

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about array and foreach

⏯ Playground Link

Playground link with relevant **code**

πŸ’» Code

interface FooValue {
  value: string;
}

interface BarValue {
  value: string;
  name: string;
}

interface Props {
  values: BarValue[] | FooValue[];
}

export function foo({ values }: Props) {
  return values.forEach((value) => value);
}

πŸ™ Actual behavior

The inferred type of value is simply FooValue:
image

πŸ™‚ Expected behavior

But really it should be FooValue | BarValue, which is correct when using .map:
image

I suspect this issue happens because forEach isn't annotated with generic return type U while map is:

TypeScript/src/lib/es5.d.ts

Lines 1235 to 1246 in 250065e

/**
* Performs the specified action for each element in an array.
* @param callbackfn A function that accepts up to three arguments. forEach calls the callbackfn function one time for each element in the array.
* @param thisArg An object to which the this keyword can refer in the callbackfn function. If thisArg is omitted, undefined is used as the this value.
*/
forEach(callbackfn: (value: T, index: number, array: readonly T[]) => void, thisArg?: any): void;
/**
* Calls a defined callback function on each element of an array, and returns an array that contains the results.
* @param callbackfn A function that accepts up to three arguments. The map method calls the callbackfn function one time for each element in the array.
* @param thisArg An object to which the this keyword can refer in the callbackfn function. If thisArg is omitted, undefined is used as the this value.
*/
map<U>(callbackfn: (value: T, index: number, array: readonly T[]) => U, thisArg?: any): U[];

In fact after changing forEach to forEach<U> fixes this problem locally:
image

@MartinJohns
Copy link
Contributor

Subtype reduction, working as intended.

@jcalz
Copy link
Contributor

jcalz commented Jul 30, 2023

Essentially a duplicate of #46449 and many others: if A extends B then TS will sometimes reduce A | B to B. This is working as intended. In a perfectly sound type system such types A | B and B would be identical and the reduction from the former to the latter would have no observable effect. Unfortunately TS doesn't have a perfectly sound type system, so the operation is indeed observable and sometimes produces surprising or undesired results for users. But it's not considered a bug.

@fatcerberus
Copy link

fatcerberus commented Jul 30, 2023

In a perfectly sound type system such types A | B and B would be identical and the reduction from the former to the latter would have no observable effect.

Essentially a restatement of #53425 (comment), fwiw
(and in case that isn't clear: objects are allowed to have extra properties, so writing FooValue | BarValue doesn't add any additional constraints on which values are allowed compared to just FooValue by itself).

@shogunsea
Copy link
Author

@jcalz @fatcerberus thanks for the explanation, but why this is not an issue with .map? I'd expect .map and .forEach behave identically except the return value (per the language spec).

But it's not considered a bug

I agree in general based on typescript not being "a perfectly sound system", however this did feel like a bug in our use case: we were using https://typescript-eslint.io/ to lint the data usage based on the type information, and when a piece of data was transformed with Array#forEach it produced wrong result due to this sub type reduction behavior.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Jul 31, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
6 participants