-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix TS2783 false positive for union types in object spread expressions #62656
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
Conversation
Co-authored-by: RyanCavanaugh <[email protected]>
Co-authored-by: RyanCavanaugh <[email protected]>
|
@typescript-bot test it |
|
Hey @RyanCavanaugh, the results of running the DT tests are ready. Everything looks the same! |
|
@RyanCavanaugh Here are the results of running the user tests with tsc comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@RyanCavanaugh Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@RyanCavanaugh Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Fix issue ref
Fixes #61223
Problem
TypeScript incorrectly reported error TS2783 ("'x' is specified more than once, so this usage will be overwritten") when spreading union types in object literals, even when the property wasn't guaranteed to exist in all constituents of the union.
For example, this code incorrectly produced an error:
The error claimed
idwould "always" be overwritten, but this was incorrect. The spread expression evaluates toThing | { label: string }, and the fallback object{ label: 'Foo' }doesn't have anidproperty at all, soidis only overwritten whenfind()returns aThing.Root Cause
The
checkSpreadPropOverridesfunction only checked if a property hadSymbolFlags.Optionalbefore reporting the error. It didn't account for properties that exist in some constituents of a union type but not others. These properties are marked withCheckFlags.Partial(specificallyReadPartialorWritePartial), indicating they're only present in a subset of the union's constituents.Solution
Added an additional check for
CheckFlags.PartialincheckSpreadPropOverrides. Properties marked as partial in union types should not trigger the "always overwrites" error since they don't exist in all code paths.Test Coverage
Added comprehensive test file
spreadUnionPropOverride.tsthat verifies:Thing | { label: string }no longer errors onidThe fix preserves all existing correct behavior while eliminating the false positive for partial properties in union types.
Original prompt
Fixes #62655
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.