Don't re-alias top-level type aliases with local type aliases#43701
Don't re-alias top-level type aliases with local type aliases#43701ahejlsberg merged 2 commits intomasterfrom
Conversation
amcasey
left a comment
There was a problem hiding this comment.
This code appears to match the explanation you gave at the design meeting, but I don't understand the feature well enough to predict possible consequences. Is there anything specific you wanted me to test/check?
| Type 'string | number' is not assignable to type 'ZeroOf<T>'. | ||
| Type 'string' is not assignable to type 'ZeroOf<T>'. | ||
| tests/cases/conformance/types/conditional/conditionalTypes1.ts(263,9): error TS2403: Subsequent variable declarations must have the same type. Variable 'z' must be of type 'T1', but here has type 'T2'. | ||
| tests/cases/conformance/types/conditional/conditionalTypes1.ts(263,9): error TS2403: Subsequent variable declarations must have the same type. Variable 'z' must be of type 'T1', but here has type 'Foo<T & U>'. |
There was a problem hiding this comment.
This seems worse, but I believe I understood you to say it's just a return to our old behavior?
There was a problem hiding this comment.
Yes, it's a return to the old behavior, the rationale being that Foo<xxx> is accessible outside the function whereas T2 is not. I think very little code will be affected by this change, as it is relatively rare to have local type aliases.
Not really. I suppose the debatable part is whether we just want to point users at the workaround (i.e. changing |
Is there some way we could supplement tracing to make it clear when a bad expansion happens so that we can at least point users to the relevant type declarations? If we can't assist users in finding where to apply that workaround, I'd be pretty reluctant to recommend they do so. |
|
@typescript-bot perf test this faster |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at e8f86b3. You can monitor the build here. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at e8f86b3. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here they are:Comparison Report - master..43701
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@DanielRosenwasser Here they are:Comparison Report - master..43701
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
I can't think of any. If we preserved all possible type aliases then perhaps, but in that case we'd probably just add logic to pick the first accessible alias. But as we discussed in the design meeting, I don't think the additional complexity is merited. Overall, I think we're better off with the behavior in this PR so I'll merge it. |
TS 4.3 avoids re-aliasing type aliases needlessly, in microsoft/TypeScript#43701. This changes the way that a couple of tests in redux-orm print their type aliases.
TS 4.3 avoids re-aliasing type aliases needlessly, in microsoft/TypeScript#43701. This changes the way that a couple of tests in redux-orm print their type aliases.
Fixes #43622.