Skip to content
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

Support re-aliasing of type alias instantiations #42284

Merged
merged 5 commits into from
Jan 11, 2021

Conversation

ahejlsberg
Copy link
Member

This PR continues the work in #42149 by supporting re-aliasing of type alias instantiations. For example:

type Thing<T> = { value: T };
type ArrayThing<U> = Thing<U[]>;
type StringArrayThing = ArrayThing<string>;

declare let t1: Thing<string[]>;     // Thing<string[]>
declare let t2: ArrayThing<string>;  // ArrayThing<string> (previously Thing<string[]>)
declare let t3: StringArrayThing;    // StringArrayThing (previously Thing<string[]>)

interface Item<T> { value: T }
type ArrayItem<U> = Item<U[]>;
type NumberArrayItem = ArrayItem<number>;

declare let t4: Item<number[]>;     // Item<number[]>
declare let t5: ArrayItem<number>;  // ArrayItem<number>
declare let t6: NumberArrayItem;    // NumberArrayItem (previously ArrayItem<number>)

function foo<T, K extends keyof T>(obj: T, key: K) {
    let x: Omit<T, K>;  // Omit<T, K> (previously Pick<T, Exclude<keyof T, K>>)
}

Previously, instantiations of aliased anonymous object, mapped, or conditional types with a new type alias would continue to display the original type alias in diagnostics, quick info, and declaration files. Now, re-aliasing of such types consistently adopts and displays the new type alias.

In addition, instantiations of distributive types (such as homomorphic mapped types and distributive conditional types) now properly preserve the original type alias when the instantiation results in a union type:

type A = { a: string };
type B = { b: string };

declare let p: Partial<A | B>;  // Partial<A | B> (previously Partial<A> | Partial<B>)

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 11, 2021
@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 8ac7030. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 8ac7030. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 8ac7030. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 8ac7030. You can monitor the build here.

@ahejlsberg
Copy link
Member Author

@robpalme This is the continuation PR I mentioned in #42149. Give it a try and let me know how it goes.

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..42284

Metric master 42284 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 324,053k (± 0.02%) 324,025k (± 0.02%) -28k (- 0.01%) 323,871k 324,182k
Parse Time 1.98s (± 0.66%) 1.99s (± 0.84%) +0.01s (+ 0.50%) 1.95s 2.01s
Bind Time 0.81s (± 0.92%) 0.82s (± 1.35%) +0.00s (+ 0.49%) 0.81s 0.86s
Check Time 4.88s (± 0.33%) 4.88s (± 0.58%) +0.01s (+ 0.21%) 4.82s 4.94s
Emit Time 5.51s (± 0.77%) 5.51s (± 0.61%) -0.00s (- 0.02%) 5.46s 5.59s
Total Time 13.18s (± 0.43%) 13.21s (± 0.50%) +0.03s (+ 0.21%) 13.06s 13.36s
Compiler-Unions - node (v12.1.0, x64)
Memory used 200,247k (± 0.08%) 200,294k (± 0.07%) +47k (+ 0.02%) 199,982k 200,507k
Parse Time 0.78s (± 0.77%) 0.77s (± 0.98%) -0.00s (- 0.00%) 0.76s 0.79s
Bind Time 0.50s (± 0.66%) 0.50s (± 1.04%) -0.00s (- 0.20%) 0.49s 0.51s
Check Time 9.94s (± 1.06%) 9.92s (± 0.79%) -0.02s (- 0.17%) 9.75s 10.15s
Emit Time 2.37s (± 0.97%) 2.36s (± 1.29%) -0.01s (- 0.42%) 2.30s 2.46s
Total Time 13.58s (± 0.74%) 13.55s (± 0.70%) -0.03s (- 0.23%) 13.34s 13.75s
Monaco - node (v12.1.0, x64)
Memory used 337,446k (± 0.03%) 337,473k (± 0.01%) +27k (+ 0.01%) 337,335k 337,541k
Parse Time 1.59s (± 0.30%) 1.58s (± 0.56%) -0.00s (- 0.31%) 1.57s 1.61s
Bind Time 0.70s (± 1.08%) 0.70s (± 0.64%) -0.01s (- 0.99%) 0.69s 0.71s
Check Time 4.95s (± 0.46%) 4.94s (± 0.43%) -0.01s (- 0.12%) 4.88s 4.98s
Emit Time 2.88s (± 0.60%) 2.89s (± 0.42%) +0.01s (+ 0.38%) 2.86s 2.91s
Total Time 10.12s (± 0.24%) 10.11s (± 0.36%) -0.01s (- 0.06%) 9.99s 10.16s
TFS - node (v12.1.0, x64)
Memory used 292,356k (± 0.02%) 292,396k (± 0.02%) +41k (+ 0.01%) 292,282k 292,533k
Parse Time 1.25s (± 0.76%) 1.25s (± 0.71%) -0.01s (- 0.64%) 1.23s 1.26s
Bind Time 0.65s (± 0.73%) 0.65s (± 0.86%) -0.01s (- 0.77%) 0.64s 0.66s
Check Time 4.54s (± 0.70%) 4.52s (± 0.54%) -0.02s (- 0.51%) 4.47s 4.58s
Emit Time 2.95s (± 0.89%) 2.94s (± 0.72%) -0.00s (- 0.14%) 2.90s 2.98s
Total Time 9.40s (± 0.52%) 9.36s (± 0.42%) -0.04s (- 0.37%) 9.28s 9.45s
material-ui - node (v12.1.0, x64)
Memory used 478,950k (± 0.07%) 473,781k (± 0.01%) -5,169k (- 1.08%) 473,585k 473,886k
Parse Time 2.08s (± 0.73%) 2.07s (± 0.54%) -0.02s (- 0.91%) 2.04s 2.09s
Bind Time 0.65s (± 0.73%) 0.64s (± 0.69%) -0.00s (- 0.77%) 0.63s 0.65s
Check Time 12.49s (± 0.90%) 12.51s (± 0.71%) +0.02s (+ 0.14%) 12.28s 12.72s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.23s (± 0.65%) 15.22s (± 0.62%) -0.00s (- 0.03%) 14.98s 15.44s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-197-generic
Architecturex64
Available Memory16 GB
Available Memory4 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v12.1.0, x64)
Scenarios
  • Angular - node (v12.1.0, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Monaco - node (v12.1.0, x64)
  • TFS - node (v12.1.0, x64)
  • material-ui - node (v12.1.0, x64)
Benchmark Name Iterations
Current 42284 10
Baseline master 10

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@ahejlsberg
Copy link
Member Author

Tests all look good, only changes are improved diagnostics. Performance is unaffected.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 8ac7030. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 11, 2021

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/93139/artifacts?artifactName=tgz&fileId=4FE49FBE4115C3F38EC02DE524EF9087A6729DD9744C9B257007FE5D69F77D1202&fileName=/typescript-4.2.0-insiders.20210111.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

Comment on lines +209 to +214
!!! error TS2322: Type 'keyof T' is not assignable to type 'T[keyof T] extends Function ? never : keyof T'.
!!! error TS2322: Type 'string | number | symbol' is not assignable to type 'T[keyof T] extends Function ? never : keyof T'.
!!! error TS2322: Type 'string' is not assignable to type 'T[keyof T] extends Function ? never : keyof T'.
!!! error TS2322: Type 'keyof T' is not assignable to type 'never'.
!!! error TS2322: Type 'string | number | symbol' is not assignable to type 'never'.
!!! error TS2322: Type 'string' is not assignable to type 'never'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's this happening? What was avoiding the elaboration before?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like we can't avoid re-elaboration based on type IDs

@ahejlsberg ahejlsberg merged commit c456bbd into master Jan 11, 2021
@DanielRosenwasser DanielRosenwasser deleted the newAliasesForInstantiatedTypes branch January 12, 2021 00:20
@Kingwl Kingwl mentioned this pull request Jan 12, 2021
10 tasks
@robpalme
Copy link

robpalme commented Jan 12, 2021

@ahejlsberg This is even better than the previous attempt 🎉

As before, there are no inlining/size regressions - I reviewed every changed line and this PR is a pure win. This PR improves upon 2c2d06d by also handling aliases to Record and Pick<obj, union>.

// Previous: 2c2d06
  myRecordProp: Record<string, any>;
  myObjectProp: Pick<{
      lotsOfProps,
    }, "red" | "green" | "blue
  >;

...is replaced by...

// Master: Using PR#42284
  myRecordProp: MyRecordType;
  myObjectProp: MyColorsOnly<{
      lotsOfProps,
    }
  >;

On the large package I referred to in previous reports, this PR results in a further 10KB decrease vs the previous attempt, i.e the Record & Pick changes explains the 10KB win. Meaning the net effect of this PR is now:

  • 50 declaration files out of 394 were affected by this PR
  • Net size of all declaration files shrank by 41KB
    • 50 files shrank (total size of these files reduced by 41KB from 585KB to 544KB)
    • 0 files grew

Zzzen pushed a commit to Zzzen/TypeScript that referenced this pull request Jan 16, 2021
* New aliases for type alias instantiations

* New aliases for conditional, mapped, and anonymous object type instantiations

* Accept new baselines

* Fix issues with re-aliasing

* Accept new baselines
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.2.0 milestone Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants