Fix assignment of intersections to objects with optional properties#37195
Fix assignment of intersections to objects with optional properties#37195ahejlsberg merged 8 commits intomasterfrom
Conversation
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f902f8a. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at f902f8a. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at f902f8a. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at f902f8a. You can monitor the build here. |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@ahejlsberg Here they are:Comparison Report - master..37195
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot run dt |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f902f8a. You can monitor the build here. |
|
@typescript-bot user test this |
|
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 1a5911a. You can monitor the build here. |
|
So, notably, this does not fix #36637 because that has a generic |
That's right. |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@weswigham Can you take a look at the last user test suite run and tell me which of those are preexisting conditions, or how I might find out? The |
Looking over #37219 and #37203 , looks like just the |
|
Summarizing test runs: No change in RWC and DT and no appreciable perf impact. New errors in the So, this PR is technically a breaking change because we find new issues. |
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 1a5911a. You can monitor the build here. |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build. |
|
CC @dzearing @JasonGore, it looks like this change will cause issues for Fabric. It looks like tightening the declarations of to use |
We consider any intersection
A & Bassignable to a typeTif eitherAorBis assignable toT. This can be problematic whenTis an object type with optional properties. For example:The issue here is that because
{ b: string }is assignable to{ a?: number, b: string }we also consider any intersection that includes{ b: string }to be assignable--even if the intersection includes another type that conflicts with the optionalaproperty.With this PR we now require intersections of object types as a whole to be assignable to the target type. In other words, we treat
{ a: null } & { b: string}identically to{ a: null, b: string }in relationships.Fixes #36604.