Uh oh!
There was an error while loading. Please reload this page.
Preserve type aliases for union and intersection types - #42149
Conversation
ahejlsberg
commented
Dec 29, 2020
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 237e9ca. 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 237e9ca. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 237e9ca. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 237e9ca. You can monitor the build here. |
typescript-bot
commented
Dec 29, 2020
@ahejlsberg Here they are:Comparison Report - master..42149
System
Hosts
Scenarios
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
typescript-bot
commented
Dec 29, 2020
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
ahejlsberg
commented
Jan 1, 2021
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at e0d4774. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the perf test suite on this PR at e0d4774. You can monitor the build here. Update: The results are in! |
robpalme
commented
Jan 9, 2021
I have tested
I reviewed every emit change. It's spot on every single time, resulting in ideal emit, i.e. an expanded union is replaced by a reference to an identifier that is already in scope. As an example, the emit introduced by this now-merged PR... // dts emit from PR#42149
myColorProp: MakeEnum<{readonlyred: "red";readonlygreen: "green";readonlyblue: "blue";}>;...is now replaced with this... // dts emit from e794cb9 "Allow new alias to be associated with type alias instantiation"
myColorProp: Color;This makes the declaration files smaller and easier to read. And it helps prevent inter-project staleness issues. And it will help our documentation generation pipeline that reads the .d.ts files to generate correctly linked HTML. Your work on this is highly appreciated. |
* Create separate types for equivalent aliased unions * Accept new baselines * Preserve original types for union types * Accept new baselines * Preserve intersection origin for union types * Accept new baselines * Accept new baselines * Preserve aliases during relationship checks * Accept new baselines * Preserve aliases for intersection and indexed access types * Accept new baselines * Compute intersection-of-unions cross product without recursion * Accept new baselines * Use denormalized type objects for origin / support 'keyof' origins * Accept new baselines * Fix fourslash test * Recursively extract named union types * Accept new baselines * Map on union origin in mapType to better preserve aliases and origins * Remove redundant call * Accept new baselines * Revert back to declared type when branches produce equivalent union * Accept new baselines * Don't include denormal origin types in regular type statistics * Fix issue with unions not being marked primitive-only * Allow new alias to be associated with type alias instantiation * Accept new baselines * Revert "Accept new baselines" This reverts commit 4507270. * Revert "Allow new alias to be associated with type alias instantiation" This reverts commit 2c2d06d.
FYI: I'm upgrading some projects to use TypeScript 4.2 RC and this change is causing issues with Reduced test case (this worked in TS 4.1): typeCompact<A>=AextendsFunction ? A : {[KinkeyofA]: A[K]}&{};typeDistributiveOmit<T,KextendskeyofT>=Textendsunknown ? Omit<T,K> : never;typeBase<T>=T&{base: string};typeA={a: string};typeB={b: string};typeC=(Base<A>|Base<B>)&{toRemove: string};// Given properties outside of the union, it should omit// $ExpectType { a: string; base: string; } | { b: string; base: string; }typeTest1=Compact<DistributiveOmit<C,'toRemove'>>;… but it fails in TS 4.2: I'm not sure there is any way for me to rewrite this test? I think this is going to cause issues for tests in the DefinitelyTyped repo. |
DanielRosenwasser
commented
Feb 16, 2021
@ahejlsberg@RyanCavanaugh@orta@sandersn, I think @OliverJAsh has a point. It looks like we still haven't provided API users with a way to opt out of printing origin types. This means we're probably unprepared for divergences in type display between 4.1 and 4.2+ in DefinitelyTyped. |
OliverJAsh
commented
Feb 24, 2021
@DanielRosenwasser Now that 4.2 is out I guess we might start running into issues… |
sandersn
commented
Feb 24, 2021
I don't understand the problem well enough yet: does this only cause a problem for API users who have been using text equality on the output of type display? I feel like that's a bad idea anyway, and something that dtslint should stop eventually, switching instead to something like tsd. |
I mean, DT does type text comparison (heh), but it does support specifying multiple allowable representations, since we have textual representation changes somewhat regularly. (On DT, we can say |
I completely agree, but as it stands this is what DefinitelyTyped is using—so after TS 4.2, many tests in DefinitelyTyped will no longer function correctly, and there's no way to rewrite them without significantly changing the behaviour of the test. In my example above, we want to test the structure, not the name of the test. This was possible before because TS printed the whole structure, but now it does not. Until dtslint changes its testing infrastructure, I feel like we need an escape hatch so that these tests can continue to function. |
sandersn
commented
Mar 16, 2021
So far DT has taken the other option of adding synonyms, so I don't think an escape hatch is needed for dtslint at least [1]. A quick scan shows 667 uses of the [1] An interactive expansion of aliases in quickinfo would be nice though. |
OliverJAsh
commented
Mar 17, 2021
I saw your PR which added a bunch of these (DefinitelyTyped/DefinitelyTyped#50566). I'm concerned that this could cause problems in the future. These synonyms are not testing the same thing, so in the future when the tests are only ran with TypeScript versions 4.2 and above, these tests could pass when they should fail. For example, I could modify my example test from above: type Compact<A> = A extends Function ? A : { [K in keyof A]: A[K] } & {};
-type DistributiveOmit<T, K extends keyof T> = T extends unknown ? Omit<T, K> : never;+type DistributiveOmit<T, K extends keyof T> = T;
type Base<T> = T & { base: string };
type A = { a: string };
type B = { b: string };
type C = (Base<A> | Base<B>) & { toRemove: string };
// Given properties outside of the union, it should omit
-// $ExpectType { a: string; base: string; } | { b: string; base: string; }+// $ExpectType { a: string; base: string; } | { b: string; base: string; } || Test1
type Test1 = Compact<DistributiveOmit<C, 'toRemove'>>;I've deliberately broken |
sandersn
commented
Mar 17, 2021
The reason I'm not worried about DefinitelyTyped/DefinitelyTyped#50566 is that those As you point out in your original example, type testing via type alias becomes tautological after this PR. |
@ahejlsberg Here's another situation where this behavior may be undesirable. Sometimes I'll use a type as a sort of lookup for related sub types (where I can't justify creating a named type for each). For example: I can then create a type that behaves like an enum of the different animals: In TS 4.1 if I hover my cursor over But in TS 4.2 it's now obviously going to become the less desirable: Fortunately when I type the following I still get autocompleted suggestions (which really is why I'm doing this in the first place): So some sort of opt out would be very valuable for this case too. This is the first idea off the top of my head. Could such a helper be created today? |
ahejlsberg
commented
Jun 21, 2021
The following should do the trick: typeResolve<T>=TextendsT ? T : never; |
captain-yossarian
commented
Nov 30, 2022
There is might be a bug which was provided between TS 4.1.5 and 4.2.3. |
With this PR we improve preservation of type aliases for union and intersection types. Previously, an alias associated with a union type was lost when that type was made part of another union. For example:
Previously, the types of all but the
shapevariable would be displayed as fully expanded unions (see in playground). Now, the types are displayed as they were written. This makes diagnostics involving these types significantly easier to comprehend.This PR also makes it possible to have distinct aliases for the same union or intersection types. For example:
Previously we'd show the alias
StrOrNum1for bothsn1andsn2.The key implementation change in the PR is that we now include alias symbols in the key used to intern union, intersection, and indexed access types. Thus, aliased forms of these types get a separate type identity. We additionally include an "origin" description in union types when the originating declaration has fewer constituents than the fully expanded and normalized union. This origin may actually be an intersection, as is the case in the type
(A | B | C) & (D | E | F). A slight performance impact is expected because we now create more type identities for equivalent unions.Fixes#35654.