-
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
Propagate 'undefined' instead of the optional type marker at an optional chain boundary #34588
Conversation
…nal chain boundary
# Conflicts: # src/compiler/utilities.ts
@@ -45,9 +45,9 @@ const n4: number | undefined = a?.m?.({x: absorb()}); // likewise | |||
>a?.m : (<T>(obj: { x: T; }) => T) | undefined | |||
>a : { m?<T>(obj: { x: T; }): T; } | undefined | |||
>m : (<T>(obj: { x: T; }) => T) | undefined | |||
>{x: absorb()} : { x: number | undefined; } | |||
>x : number | undefined | |||
>absorb() : number | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting change to inference - I assume this is because previously this undefined
was the optional chain type, and so managed to skip out on the "cleave matching types" logic in inference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. The resulting type now aligns with other examples in the file.
Co-Authored-By: Nathan Shively-Sanders <[email protected]>
@RyanCavanaugh should we cherry-pick this into release-3.7? |
The issue this PR resolves (#34579) was added to the 3.7.2 milestone https://github.com/microsoft/TypeScript/milestone/106, so I think this should be cherry-picked to 3.7 |
@typescript-bot port this to release-3.7 |
@typescript-bot cherry-pick this to release-3.7 bot reference for reference |
Me bad at bot |
Component commits: 99328e9 Propagate 'undefined' instead of the optional type marker at an optional chain boundary 7aa6eee Merge branch 'master' into fix34579 # Conflicts: # src/compiler/utilities.ts 61e6765 Update src/compiler/types.ts Co-Authored-By: Nathan Shively-Sanders <[email protected]>
Hey @weswigham, I've opened #34988 for you. |
Thanks for the quick response 👍 |
Component commits: 99328e9 Propagate 'undefined' instead of the optional type marker at an optional chain boundary 7aa6eee Merge branch 'master' into fix34579 # Conflicts: # src/compiler/utilities.ts 61e6765 Update src/compiler/types.ts Co-Authored-By: Nathan Shively-Sanders <[email protected]>
This ensures that the
optionalType
marker type does not escape the boundary of an optional chain. This causes unintended side effects even thoughoptionalType
is essentially the same asundefinedType
.Fixes #34579