Fix for relating covered discriminants in unions - #39393
Conversation
|
TypeScript Bot (@typescript-bot) perf test |
|
Heya Ron Buckton (@rbuckton), I've started to run the perf test suite on this PR at 8eba362. You can monitor the build here. Update: The results are in! |
|
Heya Ron Buckton (@rbuckton), I've started to run the parallelized Definitely Typed test suite on this PR at 8eba362. You can monitor the build here. |
|
Heya Ron Buckton (@rbuckton), I've started to run the extended test suite on this PR at 8eba362. You can monitor the build here. |
|
Heya Ron Buckton (@rbuckton), I've started to run the parallelized community code test suite on this PR at 8eba362. 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. |
|
Ron Buckton (@rbuckton) Here they are:Comparison Report - master..39393
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I noticed some failures in the DT test run. I'm trying to determine if these are existing errors or are due to this change. |
|
It looks like the DT failures are issues in DT:
|
In #30779 we added the ability to relate a source type to a target when the target is a discriminated union and the source is covered by the target. This did not work when the target was a union of tuple types as the relationship check would fail when comparing the synthesized index types for each tuple in
indexTypesRelatedTo. This was further complicated by the fact that the new variadic tuple type logic added topropertiesRelatedTofails to exclude relationship checks for non-variable elements whose indices are included in theexcludedPropertiesmap.This PR does two things to address these issues:
propertiesRelatedToso that it aligns with the existing, non-tuple-specific logic.indexTypesRelatedTocheck when both the source type and the target type are tuples, as the index type check should be sufficiently covered by the tuple-specific branch inpropertiesRelatedTo.Alternatively, rather than skipping the
indexTypesRelatedTocheck, we could synthesize a copy of the source and target tuple types with the excluded discriminants erased toneverso that they are excluded from the index type. That would result in the allocation of types purely to accommodateindexTypesRelatedTo, which would then be subsequently discarded. This seems like an unnecessary cost, aspropertiesRelatedTosufficiently covers the numeric index type case.Fixes #39357
Fixes #34967