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

Recursive type references #33050

Merged
merged 29 commits into from Sep 25, 2019

Conversation

@ahejlsberg
Copy link
Member

ahejlsberg commented Aug 23, 2019

This PR implements support for recursive type references. For example:

type ValueOrArray<T> = T | Array<ValueOrArray<T>>;

const a0: ValueOrArray<number> = 1;
const a1: ValueOrArray<number> = [1, [2, 3], [4, [5, [6, 7]]]];

type HypertextNode = string | [string, { [key: string]: any }, ...HypertextNode[]];

const hypertextNode: HypertextNode =
    ["div", { id: "parent" },
        ["div", { id: "first-child" }, "I'm the first child"],
        ["div", { id: "second-child" }, "I'm the second child"]
    ];

type Json = string | number | boolean | null | Json[] | { [key: string]: Json };

let data: Json = {
  caption: "Test",
  location: { x: 10, y: 20 },
  values: [0, 10, 20]
}

Previously, the above would have multiple circularity errors.

The specific change made by this PR is that type arguments are permitted to make circular references in aliased types of the following kinds:

  • Instantiations of generic class and interface types (for example Array<Foo>).
  • Array types (for example Foo[]).
  • Tuple types (for example [string, Foo?]).

A type T is said to be an aliased type when

  • T is the right hand side of a type alias declaration, or
  • T is a constituent of an aliased union, intersection, indexed access, or conditional type, or
  • T is an aliased parenthesized type.

For example, in the type alias declaration

type ValueOrArray<T> = T | Array<ValueOrArray<T>>;

the type argument ValueOrArray<T> occurs in an aliased type instantiation that is a constituent of an aliased union type that is the right hand side of a type alias declaration, and the type argument is therefore permitted to be a circular reference.

An additional change in this PR is that when an array type, a tuple type, or an instantiation of a generic class or interface type is the right hand side of a type alias declaration, that type alias becomes the name by which the type is referenced in compiler diagnostics and quick info. Previously, such aliased types were always shown in their expanded form.

NOTE: For consumers of the TypeScript compiler APIs this PR is potentially a breaking change because it removes the typeArguments property from the TypeReference interface and provides a getTypeArguments method on the TypeChecker interface to be used instead. This change is necessary because resolution of type arguments in type references is now deferred.

Fixes #3496.
Fixes #6230.
Fixes #14174.
Fixes #28929.
Fixes #32967.

@ahejlsberg ahejlsberg mentioned this pull request Aug 23, 2019
5 of 5 tasks complete
@falsandtru

This comment has been minimized.

Copy link
Contributor

falsandtru commented Aug 23, 2019

with additional work it can be expanded to all type references.

To be clear, do you intend to resolve nested Promise types? Another, do you also intend to improve Array#{flat,flatMap}?

@weswigham

This comment has been minimized.

Copy link
Member

weswigham commented Aug 23, 2019

OK, so I expanded enough on this locally to try the

type HypertextNode = string | [string, { [key: string]: any }, ...HypertextNode[]];

const hypertextNode: HypertextNode =
    ["div", { id: "parent" },
        ["div", { id: "first-child" }, "I'm the first child"],
        ["div", { id: "second-child" }, "I'm the second child"]
    ];

example out (which is a motivator here), and the approach quickly hits its limits - namely, when we compare tuples, eventually we compare the array methods on the tuple, which have this types - this this type for the recursive alias then expands forever in instantiation, because getAnonymousTypeInstantiation eagerly applies type arguments to the object's captured type parameters (which un-defers them, which isn't an immediate issue, since their containing types now exist, but their structure is then problematic as instantiation doesn't keep any kind of "visited" stack, so will happily spin forever on the type (not knowing it needs to introduce a deferral anywhere). The un-deferal occurs because the getIndexTypeOfType call required for a rest parameter forces the type argument to the array to resolve before it's passed forward (while when the reference within the type argument is what is deferred, as in the other PR, this is no problem). I've opened this with the diff of what I've added

@fatcerberus

This comment has been minimized.

Copy link

fatcerberus commented Aug 24, 2019

type ValueOrArray<T> = T | ValueOrArray<T>[];

So is this an alternative to #33018?

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 26, 2019

@weswigham Latest commit changes deferred type reference instantiation to use the same logic as anonymous types. This ensures that deferred type references continue to be deferred when instantiated, and it fixes the issues you were seeing with infinite instantiations. The motivating example

type HypertextNode = string | [string, Record<string, unknown>, ...HypertextNode[]];

const hypertextNode: HypertextNode =
  ["div", {id: "parent"},
    ["div", {id: "first-child"}, "I'm the first child"],
    ["div", {id: "second-child"}, "I'm the second child"]
  ];

now works as expected.

I still need to look at pulling in the other changes in your diff. The depth stuff was just a quick fix I put in place, I agree it isn't the correct solution.

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 26, 2019

@weswigham Printback now fixed by cherry picking some of your changes.

A bit more context on the latest commits... The instantiation logic for array and tuple type references is now similar to that of anonymous types and mapped types: We track which outer type parameters are in scope at the location of the reference and cache instantiations based on the type arguments of those outer type parameters. This means self-referential array and tuple types are no more expensive than self-referential object types.

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 26, 2019

is this an alternative to #33018?

I think this PR is the right way to solve the issue in #32967.

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 27, 2019

@typescript-bot perf test this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Aug 27, 2019

Heya @ahejlsberg, I've started to run the perf test suite on this PR at b18c70f. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Aug 27, 2019

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

Here they are:

Comparison Report - master..33050

Metric master 33050 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 325,602k (± 0.02%) 359,124k (± 0.04%) +33,523k (+10.30%) 358,591k 359,283k
Parse Time 1.48s (± 0.66%) 1.47s (± 0.63%) -0.01s (- 0.67%) 1.45s 1.49s
Bind Time 0.76s (± 0.89%) 0.76s (± 0.59%) -0.01s (- 0.79%) 0.75s 0.77s
Check Time 4.21s (± 0.47%) 4.39s (± 0.35%) +0.18s (+ 4.20%) 4.36s 4.43s
Emit Time 5.30s (± 1.06%) 5.26s (± 0.76%) -0.04s (- 0.77%) 5.18s 5.36s
Total Time 11.75s (± 0.61%) 11.87s (± 0.40%) +0.12s (+ 1.02%) 11.78s 12.00s
Monaco - node (v12.1.0, x64)
Memory used 345,852k (± 0.03%) 360,491k (± 0.02%) +14,640k (+ 4.23%) 360,365k 360,727k
Parse Time 1.23s (± 0.63%) 1.23s (± 0.73%) -0.00s (- 0.41%) 1.20s 1.24s
Bind Time 0.68s (± 1.17%) 0.68s (± 0.85%) -0.00s (- 0.29%) 0.67s 0.69s
Check Time 4.27s (± 0.47%) 4.36s (± 0.57%) +0.09s (+ 2.11%) 4.29s 4.40s
Emit Time 2.85s (± 0.55%) 2.86s (± 1.02%) +0.00s (+ 0.07%) 2.80s 2.95s
Total Time 9.04s (± 0.31%) 9.12s (± 0.48%) +0.08s (+ 0.93%) 9.02s 9.24s
TFS - node (v12.1.0, x64)
Memory used 301,389k (± 0.02%) 307,834k (± 0.01%) +6,446k (+ 2.14%) 307,753k 307,969k
Parse Time 0.95s (± 0.62%) 0.95s (± 0.74%) -0.00s (- 0.31%) 0.94s 0.97s
Bind Time 0.62s (± 0.79%) 0.62s (± 1.04%) -0.00s (- 0.32%) 0.61s 0.64s
Check Time 3.85s (± 0.66%) 3.92s (± 0.55%) +0.07s (+ 1.82%) 3.85s 3.96s
Emit Time 2.95s (± 0.36%) 2.99s (± 1.06%) +0.04s (+ 1.49%) 2.93s 3.05s
Total Time 8.37s (± 0.41%) 8.48s (± 0.51%) +0.11s (+ 1.30%) 8.39s 8.57s
Angular - node (v8.9.0, x64)
Memory used 344,224k (± 0.02%) 378,076k (± 0.01%) +33,852k (+ 9.83%) 377,963k 378,173k
Parse Time 1.98s (± 0.44%) 2.00s (± 0.88%) +0.02s (+ 0.81%) 1.97s 2.04s
Bind Time 0.82s (± 0.98%) 0.82s (± 1.06%) 0.00s ( 0.00%) 0.80s 0.84s
Check Time 5.01s (± 0.52%) 5.38s (± 0.43%) +0.37s (+ 7.41%) 5.33s 5.44s
Emit Time 6.11s (± 0.53%) 6.37s (± 1.51%) +0.26s (+ 4.22%) 6.17s 6.61s
Total Time 13.91s (± 0.33%) 14.56s (± 0.79%) +0.65s (+ 4.65%) 14.36s 14.89s
Monaco - node (v8.9.0, x64)
Memory used 363,679k (± 0.02%) 378,332k (± 0.01%) +14,653k (+ 4.03%) 378,242k 378,441k
Parse Time 1.56s (± 0.48%) 1.56s (± 0.32%) -0.01s (- 0.32%) 1.54s 1.56s
Bind Time 0.89s (± 0.93%) 0.88s (± 0.93%) -0.00s (- 0.34%) 0.87s 0.91s
Check Time 5.15s (± 1.40%) 5.41s (± 0.79%) +0.25s (+ 4.87%) 5.32s 5.47s
Emit Time 3.15s (± 4.30%) 2.98s (± 1.01%) -0.16s (- 5.24%) 2.91s 3.06s
Total Time 10.75s (± 0.86%) 10.82s (± 0.43%) +0.08s (+ 0.71%) 10.74s 10.95s
TFS - node (v8.9.0, x64)
Memory used 317,623k (± 0.02%) 324,123k (± 0.01%) +6,500k (+ 2.05%) 324,046k 324,178k
Parse Time 1.26s (± 0.75%) 1.26s (± 0.58%) +0.00s (+ 0.08%) 1.24s 1.27s
Bind Time 0.71s (± 5.63%) 0.69s (± 5.55%) -0.02s (- 2.81%) 0.66s 0.80s
Check Time 4.41s (± 1.44%) 4.49s (± 1.11%) +0.08s (+ 1.84%) 4.33s 4.55s
Emit Time 3.05s (± 0.97%) 3.08s (± 0.55%) +0.03s (+ 0.88%) 3.02s 3.11s
Total Time 9.43s (± 0.39%) 9.52s (± 0.40%) +0.09s (+ 0.92%) 9.41s 9.58s
Angular - node (v8.9.0, x86)
Memory used 194,997k (± 0.03%) 211,984k (± 0.01%) +16,987k (+ 8.71%) 211,929k 212,017k
Parse Time 1.93s (± 0.65%) 1.93s (± 0.99%) +0.00s (+ 0.05%) 1.90s 1.99s
Bind Time 0.95s (± 0.87%) 0.94s (± 1.07%) -0.01s (- 1.15%) 0.93s 0.98s
Check Time 4.60s (± 0.60%) 4.91s (± 0.73%) +0.31s (+ 6.72%) 4.81s 4.98s
Emit Time 5.83s (± 0.56%) 6.14s (± 2.24%) +0.30s (+ 5.23%) 5.83s 6.45s
Total Time 13.31s (± 0.40%) 13.92s (± 1.32%) +0.61s (+ 4.54%) 13.47s 14.28s
Monaco - node (v8.9.0, x86)
Memory used 203,157k (± 0.02%) 210,526k (± 0.02%) +7,368k (+ 3.63%) 210,418k 210,624k
Parse Time 1.62s (± 0.98%) 1.61s (± 0.58%) -0.01s (- 0.37%) 1.59s 1.63s
Bind Time 0.72s (± 0.86%) 0.72s (± 0.55%) 0.00s ( 0.00%) 0.71s 0.73s
Check Time 4.88s (± 0.66%) 5.05s (± 0.69%) +0.17s (+ 3.40%) 4.99s 5.15s
Emit Time 3.20s (± 0.82%) 3.23s (± 0.72%) +0.03s (+ 0.81%) 3.19s 3.29s
Total Time 10.43s (± 0.55%) 10.60s (± 0.38%) +0.18s (+ 1.71%) 10.49s 10.71s
TFS - node (v8.9.0, x86)
Memory used 178,520k (± 0.02%) 181,759k (± 0.02%) +3,239k (+ 1.81%) 181,683k 181,827k
Parse Time 1.32s (± 0.79%) 1.31s (± 0.94%) -0.01s (- 0.46%) 1.29s 1.34s
Bind Time 0.64s (± 1.97%) 0.64s (± 1.32%) 0.00s ( 0.00%) 0.63s 0.67s
Check Time 4.31s (± 0.44%) 4.34s (± 0.47%) +0.04s (+ 0.81%) 4.32s 4.40s
Emit Time 2.87s (± 0.98%) 2.89s (± 2.01%) +0.02s (+ 0.73%) 2.77s 3.03s
Total Time 9.14s (± 0.56%) 9.19s (± 0.79%) +0.05s (+ 0.57%) 9.01s 9.34s
Angular - node (v9.0.0, x64)
Memory used 343,821k (± 0.02%) 377,603k (± 0.01%) +33,782k (+ 9.83%) 377,520k 377,693k
Parse Time 1.72s (± 0.42%) 1.71s (± 0.76%) -0.01s (- 0.41%) 1.70s 1.76s
Bind Time 0.76s (± 0.48%) 0.76s (± 0.88%) -0.00s (- 0.13%) 0.75s 0.78s
Check Time 4.80s (± 0.41%) 5.09s (± 0.84%) +0.30s (+ 6.17%) 4.99s 5.18s
Emit Time 5.67s (± 1.75%) 6.02s (± 1.18%) +0.35s (+ 6.11%) 5.79s 6.17s
Total Time 12.96s (± 0.77%) 13.59s (± 0.56%) +0.63s (+ 4.87%) 13.47s 13.75s
Monaco - node (v9.0.0, x64)
Memory used 363,410k (± 0.03%) 378,075k (± 0.01%) +14,665k (+ 4.04%) 377,979k 378,196k
Parse Time 1.32s (± 0.46%) 1.31s (± 0.67%) -0.00s (- 0.15%) 1.30s 1.34s
Bind Time 0.84s (± 0.86%) 0.84s (± 1.60%) -0.01s (- 0.71%) 0.81s 0.86s
Check Time 4.99s (± 1.69%) 5.09s (± 1.29%) +0.10s (+ 1.94%) 4.95s 5.22s
Emit Time 3.21s (± 5.27%) 3.18s (± 4.62%) -0.03s (- 0.81%) 2.91s 3.36s
Total Time 10.36s (± 0.98%) 10.42s (± 1.03%) +0.06s (+ 0.56%) 10.20s 10.65s
TFS - node (v9.0.0, x64)
Memory used 317,383k (± 0.01%) 323,904k (± 0.01%) +6,520k (+ 2.05%) 323,838k 323,979k
Parse Time 1.04s (± 0.53%) 1.04s (± 0.66%) -0.01s (- 0.58%) 1.02s 1.05s
Bind Time 0.62s (± 1.00%) 0.62s (± 0.64%) +0.00s (+ 0.32%) 0.61s 0.63s
Check Time 4.39s (± 0.57%) 4.44s (± 0.55%) +0.06s (+ 1.30%) 4.38s 4.49s
Emit Time 3.17s (± 0.72%) 3.19s (± 1.02%) +0.02s (+ 0.73%) 3.09s 3.23s
Total Time 9.21s (± 0.43%) 9.29s (± 0.43%) +0.08s (+ 0.87%) 9.19s 9.38s
Angular - node (v9.0.0, x86)
Memory used 195,115k (± 0.04%) 212,005k (± 0.04%) +16,891k (+ 8.66%) 211,741k 212,124k
Parse Time 1.64s (± 0.43%) 1.64s (± 0.74%) -0.00s (- 0.18%) 1.61s 1.66s
Bind Time 0.90s (± 1.10%) 0.89s (± 0.90%) -0.01s (- 0.67%) 0.87s 0.91s
Check Time 4.24s (± 0.27%) 4.51s (± 0.67%) +0.27s (+ 6.32%) 4.45s 4.60s
Emit Time 5.52s (± 1.03%) 5.61s (± 0.59%) +0.09s (+ 1.65%) 5.54s 5.68s
Total Time 12.30s (± 0.54%) 12.65s (± 0.50%) +0.35s (+ 2.84%) 12.56s 12.83s
Monaco - node (v9.0.0, x86)
Memory used 203,236k (± 0.02%) 210,581k (± 0.02%) +7,345k (+ 3.61%) 210,496k 210,650k
Parse Time 1.35s (± 0.64%) 1.34s (± 0.45%) -0.00s (- 0.22%) 1.33s 1.36s
Bind Time 0.65s (± 0.92%) 0.65s (± 1.28%) -0.00s (- 0.00%) 0.63s 0.66s
Check Time 4.70s (± 1.00%) 4.81s (± 0.33%) +0.11s (+ 2.45%) 4.78s 4.86s
Emit Time 3.08s (± 1.25%) 3.13s (± 0.66%) +0.04s (+ 1.43%) 3.07s 3.18s
Total Time 9.77s (± 0.39%) 9.93s (± 0.31%) +0.16s (+ 1.63%) 9.87s 10.01s
TFS - node (v9.0.0, x86)
Memory used 178,591k (± 0.02%) 181,779k (± 0.02%) +3,188k (+ 1.78%) 181,677k 181,856k
Parse Time 1.07s (± 0.66%) 1.07s (± 1.25%) -0.00s (- 0.28%) 1.04s 1.10s
Bind Time 0.58s (± 0.95%) 0.58s (± 0.89%) -0.00s (- 0.17%) 0.57s 0.60s
Check Time 4.16s (± 0.71%) 4.18s (± 0.57%) +0.02s (+ 0.58%) 4.13s 4.23s
Emit Time 2.81s (± 1.28%) 2.79s (± 1.13%) -0.02s (- 0.71%) 2.71s 2.87s
Total Time 8.62s (± 0.67%) 8.62s (± 0.47%) -0.00s (- 0.01%) 8.56s 8.74s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-142-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
  • node (v9.0.0, x64)
  • node (v9.0.0, x86)
Scenarios
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Angular - node (v9.0.0, x64)
  • Angular - node (v9.0.0, x86)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • Monaco - node (v9.0.0, x64)
  • Monaco - node (v9.0.0, x86)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • TFS - node (v9.0.0, x64)
  • TFS - node (v9.0.0, x86)
Benchmark Name Iterations
Current 33050 10
Baseline master 10

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 27, 2019

Performance tests above show a worst case 5% check time increase and 10% memory consumption increase. That's a bit too expensive. With the latest commit we only defer type argument resolution for aliased array and tuple types. That should improve the numbers.

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 27, 2019

@typescript-bot perf test this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Aug 27, 2019

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 8f3a917. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Aug 27, 2019

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

Here they are:

Comparison Report - master..33050

Metric master 33050 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 325,554k (± 0.02%) 325,824k (± 0.03%) +270k (+ 0.08%) 325,584k 326,089k
Parse Time 1.48s (± 0.64%) 1.49s (± 0.46%) +0.00s (+ 0.13%) 1.47s 1.50s
Bind Time 0.76s (± 0.81%) 0.76s (± 0.88%) -0.00s (- 0.26%) 0.74s 0.77s
Check Time 4.19s (± 0.58%) 4.21s (± 0.62%) +0.02s (+ 0.45%) 4.15s 4.26s
Emit Time 5.25s (± 0.88%) 5.25s (± 0.46%) +0.00s (+ 0.02%) 5.21s 5.33s
Total Time 11.69s (± 0.54%) 11.71s (± 0.34%) +0.02s (+ 0.17%) 11.64s 11.82s
Monaco - node (v12.1.0, x64)
Memory used 345,878k (± 0.02%) 345,853k (± 0.02%) -25k (- 0.01%) 345,746k 345,940k
Parse Time 1.22s (± 0.78%) 1.22s (± 0.60%) -0.01s (- 0.49%) 1.20s 1.23s
Bind Time 0.68s (± 0.50%) 0.68s (± 0.77%) +0.00s (+ 0.30%) 0.67s 0.69s
Check Time 4.27s (± 0.40%) 4.26s (± 0.48%) -0.01s (- 0.19%) 4.21s 4.30s
Emit Time 2.87s (± 0.77%) 2.85s (± 1.00%) -0.01s (- 0.52%) 2.81s 2.94s
Total Time 9.03s (± 0.34%) 9.01s (± 0.53%) -0.03s (- 0.29%) 8.91s 9.15s
TFS - node (v12.1.0, x64)
Memory used 301,359k (± 0.02%) 301,409k (± 0.02%) +50k (+ 0.02%) 301,321k 301,576k
Parse Time 0.95s (± 0.52%) 0.94s (± 0.81%) -0.01s (- 0.94%) 0.93s 0.97s
Bind Time 0.63s (± 1.53%) 0.63s (± 1.10%) -0.00s (- 0.64%) 0.61s 0.64s
Check Time 3.86s (± 0.58%) 3.85s (± 0.48%) -0.01s (- 0.16%) 3.83s 3.90s
Emit Time 2.96s (± 0.58%) 2.97s (± 0.92%) +0.01s (+ 0.24%) 2.89s 3.03s
Total Time 8.40s (± 0.41%) 8.39s (± 0.49%) -0.01s (- 0.11%) 8.29s 8.48s
Angular - node (v8.9.0, x64)
Memory used 344,222k (± 0.01%) 344,607k (± 0.02%) +385k (+ 0.11%) 344,424k 344,709k
Parse Time 2.00s (± 1.18%) 1.99s (± 0.60%) -0.00s (- 0.15%) 1.97s 2.03s
Bind Time 0.82s (± 0.71%) 0.82s (± 0.54%) +0.00s (+ 0.12%) 0.81s 0.83s
Check Time 5.01s (± 0.49%) 5.06s (± 0.44%) +0.04s (+ 0.88%) 5.02s 5.13s
Emit Time 6.13s (± 0.38%) 6.10s (± 0.59%) -0.03s (- 0.46%) 6.00s 6.16s
Total Time 13.96s (± 0.31%) 13.97s (± 0.34%) +0.01s (+ 0.10%) 13.87s 14.06s
Monaco - node (v8.9.0, x64)
Memory used 363,615k (± 0.01%) 363,679k (± 0.01%) +64k (+ 0.02%) 363,581k 363,816k
Parse Time 1.56s (± 0.38%) 1.56s (± 0.72%) -0.00s (- 0.06%) 1.54s 1.59s
Bind Time 0.89s (± 0.50%) 0.89s (± 1.30%) +0.01s (+ 0.68%) 0.87s 0.92s
Check Time 5.15s (± 2.16%) 5.09s (± 1.81%) -0.06s (- 1.11%) 4.97s 5.38s
Emit Time 3.13s (± 4.31%) 3.26s (± 3.01%) +0.13s (+ 4.29%) 2.96s 3.35s
Total Time 10.72s (± 0.66%) 10.81s (± 0.54%) +0.09s (+ 0.79%) 10.65s 10.92s
TFS - node (v8.9.0, x64)
Memory used 317,606k (± 0.01%) 317,633k (± 0.01%) +26k (+ 0.01%) 317,541k 317,741k
Parse Time 1.26s (± 0.54%) 1.25s (± 0.38%) -0.00s (- 0.24%) 1.24s 1.26s
Bind Time 0.69s (± 4.68%) 0.68s (± 3.86%) -0.01s (- 1.74%) 0.65s 0.78s
Check Time 4.46s (± 1.01%) 4.48s (± 0.97%) +0.02s (+ 0.47%) 4.35s 4.56s
Emit Time 3.09s (± 0.52%) 3.09s (± 0.48%) -0.00s (- 0.10%) 3.06s 3.12s
Total Time 9.50s (± 0.38%) 9.50s (± 0.39%) +0.00s (+ 0.04%) 9.44s 9.58s
Angular - node (v8.9.0, x86)
Memory used 194,996k (± 0.02%) 195,135k (± 0.02%) +139k (+ 0.07%) 195,058k 195,235k
Parse Time 1.93s (± 1.03%) 1.94s (± 0.96%) +0.00s (+ 0.21%) 1.90s 1.99s
Bind Time 0.94s (± 0.61%) 0.94s (± 0.59%) +0.00s (+ 0.21%) 0.93s 0.95s
Check Time 4.58s (± 0.45%) 4.59s (± 0.49%) +0.01s (+ 0.26%) 4.54s 4.63s
Emit Time 5.84s (± 0.86%) 5.83s (± 1.01%) -0.01s (- 0.12%) 5.71s 6.02s
Total Time 13.29s (± 0.51%) 13.30s (± 0.40%) +0.01s (+ 0.06%) 13.16s 13.40s
Monaco - node (v8.9.0, x86)
Memory used 203,173k (± 0.01%) 203,172k (± 0.02%) -1k (- 0.00%) 203,115k 203,249k
Parse Time 1.61s (± 0.68%) 1.61s (± 0.55%) +0.00s (+ 0.12%) 1.59s 1.63s
Bind Time 0.72s (± 1.22%) 0.72s (± 1.08%) -0.00s (- 0.28%) 0.71s 0.74s
Check Time 4.88s (± 0.43%) 4.91s (± 0.83%) +0.03s (+ 0.68%) 4.85s 5.04s
Emit Time 3.19s (± 0.53%) 3.20s (± 1.17%) +0.01s (+ 0.35%) 3.13s 3.31s
Total Time 10.40s (± 0.28%) 10.45s (± 0.60%) +0.04s (+ 0.43%) 10.32s 10.59s
TFS - node (v8.9.0, x86)
Memory used 178,481k (± 0.02%) 178,525k (± 0.01%) +44k (+ 0.02%) 178,504k 178,561k
Parse Time 1.31s (± 0.58%) 1.32s (± 1.20%) +0.01s (+ 0.61%) 1.30s 1.38s
Bind Time 0.64s (± 1.16%) 0.64s (± 0.81%) -0.00s (- 0.16%) 0.63s 0.65s
Check Time 4.29s (± 0.56%) 4.29s (± 0.29%) +0.00s (+ 0.09%) 4.26s 4.32s
Emit Time 2.86s (± 1.01%) 2.86s (± 0.71%) -0.01s (- 0.21%) 2.82s 2.91s
Total Time 9.10s (± 0.44%) 9.11s (± 0.24%) +0.01s (+ 0.10%) 9.06s 9.18s
Angular - node (v9.0.0, x64)
Memory used 343,776k (± 0.01%) 344,198k (± 0.01%) +421k (+ 0.12%) 344,031k 344,270k
Parse Time 1.72s (± 0.34%) 1.72s (± 0.55%) -0.01s (- 0.35%) 1.70s 1.74s
Bind Time 0.76s (± 1.16%) 0.77s (± 0.78%) +0.00s (+ 0.26%) 0.75s 0.78s
Check Time 4.74s (± 1.10%) 4.79s (± 0.70%) +0.05s (+ 1.16%) 4.72s 4.86s
Emit Time 5.71s (± 2.22%) 5.71s (± 1.58%) +0.00s (+ 0.05%) 5.53s 5.86s
Total Time 12.93s (± 1.02%) 12.99s (± 0.65%) +0.06s (+ 0.44%) 12.77s 13.16s
Monaco - node (v9.0.0, x64)
Memory used 363,435k (± 0.01%) 363,351k (± 0.02%) -85k (- 0.02%) 363,225k 363,455k
Parse Time 1.32s (± 0.72%) 1.32s (± 0.63%) -0.00s (- 0.08%) 1.30s 1.34s
Bind Time 0.83s (± 1.13%) 0.83s (± 1.45%) +0.00s (+ 0.48%) 0.81s 0.86s
Check Time 5.06s (± 1.76%) 5.03s (± 1.65%) -0.03s (- 0.55%) 4.88s 5.16s
Emit Time 3.04s (± 5.42%) 3.11s (± 5.79%) +0.07s (+ 2.47%) 2.85s 3.38s
Total Time 10.24s (± 0.99%) 10.29s (± 1.13%) +0.05s (+ 0.49%) 10.11s 10.53s
TFS - node (v9.0.0, x64)
Memory used 317,428k (± 0.01%) 317,446k (± 0.01%) +19k (+ 0.01%) 317,359k 317,522k
Parse Time 1.04s (± 0.59%) 1.04s (± 0.50%) -0.00s (- 0.19%) 1.03s 1.05s
Bind Time 0.62s (± 0.72%) 0.62s (± 0.54%) -0.00s (- 0.32%) 0.61s 0.63s
Check Time 4.38s (± 0.42%) 4.38s (± 0.70%) +0.00s (+ 0.09%) 4.31s 4.46s
Emit Time 3.20s (± 0.59%) 3.20s (± 0.31%) +0.00s (+ 0.13%) 3.19s 3.23s
Total Time 9.24s (± 0.26%) 9.24s (± 0.31%) +0.01s (+ 0.06%) 9.20s 9.32s
Angular - node (v9.0.0, x86)
Memory used 195,045k (± 0.02%) 195,200k (± 0.03%) +156k (+ 0.08%) 195,100k 195,338k
Parse Time 1.64s (± 0.58%) 1.63s (± 0.46%) -0.00s (- 0.18%) 1.62s 1.65s
Bind Time 0.89s (± 1.00%) 0.89s (± 0.38%) -0.01s (- 0.78%) 0.88s 0.89s
Check Time 4.24s (± 0.37%) 4.28s (± 0.52%) +0.04s (+ 0.83%) 4.23s 4.33s
Emit Time 5.52s (± 0.85%) 5.54s (± 0.57%) +0.02s (+ 0.31%) 5.48s 5.62s
Total Time 12.29s (± 0.37%) 12.33s (± 0.29%) +0.04s (+ 0.31%) 12.25s 12.42s
Monaco - node (v9.0.0, x86)
Memory used 203,229k (± 0.01%) 203,229k (± 0.02%) +0k (+ 0.00%) 203,146k 203,345k
Parse Time 1.35s (± 0.63%) 1.34s (± 0.46%) -0.01s (- 0.59%) 1.33s 1.36s
Bind Time 0.64s (± 0.90%) 0.64s (± 0.76%) +0.00s (+ 0.16%) 0.63s 0.65s
Check Time 4.68s (± 0.38%) 4.70s (± 0.51%) +0.02s (+ 0.36%) 4.66s 4.76s
Emit Time 3.10s (± 0.74%) 3.08s (± 0.67%) -0.01s (- 0.42%) 3.06s 3.16s
Total Time 9.77s (± 0.34%) 9.76s (± 0.38%) -0.00s (- 0.01%) 9.69s 9.86s
TFS - node (v9.0.0, x86)
Memory used 178,567k (± 0.02%) 178,563k (± 0.02%) -4k (- 0.00%) 178,487k 178,634k
Parse Time 1.07s (± 1.47%) 1.06s (± 0.82%) -0.01s (- 0.84%) 1.04s 1.08s
Bind Time 0.58s (± 0.81%) 0.58s (± 0.86%) -0.00s (- 0.69%) 0.57s 0.59s
Check Time 4.15s (± 0.27%) 4.14s (± 0.97%) -0.01s (- 0.22%) 4.05s 4.27s
Emit Time 2.78s (± 0.89%) 2.79s (± 0.61%) +0.00s (+ 0.11%) 2.75s 2.82s
Total Time 8.58s (± 0.51%) 8.56s (± 0.46%) -0.02s (- 0.23%) 8.45s 8.64s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-142-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
  • node (v9.0.0, x64)
  • node (v9.0.0, x86)
Scenarios
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Angular - node (v9.0.0, x64)
  • Angular - node (v9.0.0, x86)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • Monaco - node (v9.0.0, x64)
  • Monaco - node (v9.0.0, x86)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • TFS - node (v9.0.0, x64)
  • TFS - node (v9.0.0, x86)
Benchmark Name Iterations
Current 33050 10
Baseline master 10

@weswigham

This comment has been minimized.

Copy link
Member

weswigham commented Aug 27, 2019

Rather than creating a(nother) location with an observable inline vs not inlined checking change (which we usually attempt to remove), would it really be so bad to only make the deferred type in cases where we find the type to be circular (ie, when we spot it on the circularity stack)? It's not the first place where we'd be doing something other than returning an errorType upon discovering a circularity - getTypeOfVariableOrParameterOrPropertyWorker has fallbacks on initial circularity in some circumstances already, and operating in that way would guarantee the perf and memory costs are near zero unless the feature is actually used.

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Aug 27, 2019

Performance numbers now look great. Basically zero cost to cover the known scenarios!

would it really be so bad to only make the deferred type in cases where we find the type to be circular (ie, when we spot it on the circularity stack)?

That might be fine too, though we're already at zero cost for the feature. I like the current syntactic solution because it is easy to reason about when it kicks in. I wasn't aware that we do something other than return errorType upon discovering circularities. I'm not convinced that works in all cases because we potentially invalidate multiple circularity stack entries upon detecting a circularity, but we only do the non-errorType logic for some of them.

@weswigham

This comment has been minimized.

Copy link
Member

weswigham commented Aug 28, 2019

Yeah, I've wondered about that, too, but it has yet to come up. In any case, that's why I cautiously wrote the peek method instead in the other PR, to avoid invalidating the whole stack. But yeah, I know this roughly covers the important bits via syntax, and in that way, means there's always a way to make a thing work by transforming it into some kind of extracted alias, but we repeatedly got issues about how conditionals, when inlined, don't act the same (because if changes to distributivity, which is intentional), and any time an inlined type doesn't behave correctly because of an issue in variance, it's invariably reported. While this is easier to reason about implementation-wise, the behavior on usage is actually harder to justify, imo.

@RWalkling

This comment has been minimized.

Copy link

RWalkling commented Oct 6, 2019

@Himself65
Additionally, if you want to not only unpack the inner most type, but also keep the containers along the way, you can generalize the Flatten type like this:

type Flatten<T> = T extends Recursive<infer R> ?
    R extends Set<infer S> ? Set<Flatten<S>> :
    R extends Promise<infer S> ? Promise<Flatten<S>> :
    R extends SomeType<infer S> ? SomeType<Flatten<S>> :
    // add any other types to pass through here
    R : never

This type removes all Plain type constructors and passes through Set, Promise and SomeType:

type ResultOfRecursiveType = Plain<Promise<Plain<Set<Plain<Promise<Promise<Plain<Set<Set<string>>>>>>>>>>
type FlattenedType = Flatten<ResultOfRecursiveType> 
// above is Promise<Set<Promise<Promise<Set<Set<string>>>>>>
@Himself65

This comment has been minimized.

Copy link

Himself65 commented Oct 6, 2019

@RWalkling thanks a lot. I got the point now

@ahejlsberg

This comment has been minimized.

Copy link
Member Author

ahejlsberg commented Oct 7, 2019

@RWalkling @jack-williams @jcalz Make sure to use the actual 3.7-beta in the examples being discussed above. In #33678 we fixed our type inference logic to guard against runaway recursion caused by recursive types, and that definitely has an impact on the patterns above.

@acewert

This comment has been minimized.

Copy link

acewert commented Oct 17, 2019

I'm probably misunderstanding either Record or what cases this PR is supposed to fix, but is there a reason why this compiles on 3.7.0-beta:

type StringOrKeyedString =
    | string
    | { [key: string]: StringOrKeyedString };

But this does not?

type StringOrKeyedString =
    | string
    | Record<string, StringOrKeyedString>;

The latter still gives a "Type alias 'StringOrKeyedString' circularly references itself."

@jack-williams

This comment has been minimized.

Copy link
Collaborator

jack-williams commented Oct 17, 2019

In the first example the recursive reference is guarded within an object.

In the second example the recursive reference is an argument to a type alias and is not known to be guarded. TypeScript wont peek inside the type alias to see if it binds an object type.

If you want to get rid of the object literal you can use an interface type instead:

interface RecordI<T> {
    [k: string]: T;
}

type StringOrKeyedString =
    | string
    | RecordI<StringOrKeyedString>;

This basically falls under the requirement:

Instantiations of generic class and interface types (for example Array).

@RWalkling

This comment has been minimized.

Copy link

RWalkling commented Oct 17, 2019

@acewert
As per the specification (first post):

The specific change made by this PR is that type arguments are permitted to make circular references in aliased types of the following kinds:

  • Instantiations of generic class and interface types (for example Array).
  • Array types (for example Foo[]).
  • Tuple types (for example [string, Foo?]).

The first example you posted is an interface type, so it works. The Record type however is just a type alias (it is defined with type), so it does not fulfill the criteria as outlined above. I can only speculate as to why this decision was made, but I guess it's because type aliases can act as type operators (for example they can map over another type), while interfaces are purely declarative, so this way the compiler doesn't have to handle recursive type operators, which are too good to be true and so aren't allowed to
exist... 😢
To be fair though, these beasts are actually pretty hard to tame; there are tons of papers published about recursive type operators and I guess the TS team does not want to get into that rabbit hole, as therein madness lies

@pleerock

This comment has been minimized.

Copy link

pleerock commented Oct 20, 2019

@ahejlsberg unfortunately this PR doesn't fix what I expected (and I guess I'm not alone here). I would like something like this to work:

type Model<T> = {
}

type Schema<T> =
    T extends String ? string :
    T extends Number ? number :
    T extends Boolean ? boolean :
    T extends Model<infer U> ? Schema<U> :
    never

is there opened feature request for this type of recursion or it's not possible to implement it at the moment? Here is a real world example how this feature can improve experience (duplication, easy of maintenance and refactoring) by removing half of the code in this file.

@Himself65

This comment has been minimized.

Copy link

Himself65 commented Oct 20, 2019

@pleerock

It's the same thing with my issue #34555 🤔

@jcalz

This comment has been minimized.

Copy link
Contributor

jcalz commented Oct 20, 2019

I think #26980 is the open issue for circular conditional types.

@loganpowell

This comment has been minimized.

Copy link

loganpowell commented Oct 23, 2019

Up until this PR, I've been a TypeScript skeptic. Count me in.

@Anber

This comment has been minimized.

Copy link

Anber commented Nov 2, 2019

Why it's not possible to do something like this?

interface IState<TResolvables extends {}, TParent extends IState<any, any>> {
  parent?: TParent;
  resolve?: {
    [T in keyof TResolvables]: TResolvables[T];
  };
}

type Resolves<T> = T extends IState<infer TResolvables, infer TParent>
  ? (TResolvables | Resolves<TParent>)
  : never;

It generates two errors:

  1. Type alias 'Resolves' circularly references itself.(2456)
  2. Type 'Resolves' is not generic.(2315)
@zheeeng

This comment has been minimized.

Copy link

zheeeng commented Nov 6, 2019

Is it possible that inferring a type that is referenced by/fed to itself?

type RefSelf<Input extends Record<string, (...args: any[]) => unknown>> = {
  [k in keyof Input]: () => ReturnType<Input[k]>
}

declare function calculate <
  Input extends Record<string, (self: RefSelf<Input>) => unknown>,
>(input: Input): RefSelf<Input>

// const fourtyTwo2: () => any
const { fourtyTwo, fourtyTwo2 } = calculate({
  fourtyTwo: () => 42,
  // (parameter) self: RefSelf<{fourtyTwo: unknown; fourtyTwo2: unknown;}>
  // (property) fourtyTwo: () => any
  fourtyTwo2: (self) => self.fourtyTwo(),
})
@SamuraiJack

This comment has been minimized.

Copy link

SamuraiJack commented Nov 7, 2019

I believe 5% performance is very acceptable trade for this feature (is it outside of the margin of error at all?). The recursive type references should just be enabled for all cases.

@icehaunter

This comment has been minimized.

Copy link

icehaunter commented Nov 9, 2019

@RWalkling hey, sorry to be a bother, but I have a question in regards to your unwarp construct. I have successfully converted a tree-like structure into a recursive nesting of Plain<Plain<...> | MyType<'a', 'b'>> values. Unfortunately, using released TS 3.7, your recursive inference

type Recursive<T> = 
    | T 
    | Plain<Recursive<T>> 

export type Flatten<T> = T extends Recursive<infer R> ? R : never;

did not work for me, instead producing the same type I passed into the flatten. Could you advise as to how can I unwrap the nested Plain objects? I need a union of all MyType instances in the tree.
Thanks a lot!

@isiahmeadows

This comment has been minimized.

Copy link

isiahmeadows commented Nov 9, 2019

@icehaunter Try recursively extracting in Flatten<T> instead. That's what I would've normally done before this PR, and it's also simpler.

export type Flatten<T> = T extends Plain<infer U> ? Flatten<U> : T;
@icehaunter

This comment has been minimized.

Copy link

icehaunter commented Nov 10, 2019

@isiahmeadows that doesn't work, because of error "Type alias 'Flatten' circularly references itself". Because the recursive type usage is not inside an interface.

@isiahmeadows

This comment has been minimized.

Copy link

isiahmeadows commented Nov 10, 2019

@icehaunter 🤦‍♀️ Try this. (That was untested, obviously, and so is this.)

type Flatten<T> = {
	0: T,
	1: T extends Plain<infer U> ? Flatten<U> : never,
}[T extends Plain<any> ? 1 : 0]

If it weren't so difficult and time-consuming (as basically every type system more complicated than C or Hindley-Milner is), I'd reimplement TS myself using proper deductive types with infer being proper mathematical induction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.