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

Add support for Optional Chaining #33294

Merged
merged 25 commits into from Sep 30, 2019

Conversation

@rbuckton
Copy link
Member

commented Sep 7, 2019

This PR adds support for the ECMAScript Optional Chaining proposal which is now at stage 3.

Optional Expressions

An optional expression is an expression involving a property access, element access, or call, in which the expression is guarded against null and undefined through the use of the ?. token:

a?.b // optional property access
a?.[x] // optional element access
a?.() // optional call

In these cases, when a is either null or undefined, the result of the expression becomes undefined. When this happens, any side effects to the right of the ?. operator are not evaluated.

When emit down-level, the above expressions are translated into the following JavaScript:

var _a, _b, _c;
(_a = a) === null || _a === void 0 ? void 0 : _a.b; // optional property access 'a?.b'
(_b = a) === null || _b === void 0 ? void 0 : _b[x]; // optional element access 'a?.[x]'
(_c = a) === null || _c === void 0 ? void 0 : _c(); // optional call 'a?.()'

Optional Chains

An optional chain is an optional expression followed by one or more subsequent regular property access, element access, or call argument lists:

a?.b.c; // property access chain
a?.b[x]; // element access chain
a?.b(); // call chain

When a is either null or undefined above, the rest of the chain is also not evaluated. However, if a is any other value, the expression is evaluated as normal. In the above cases, if a.b were either null or undefined, the expression would still throw as the ?. token only guards the value of a.

When emit down-level, the above expressions are translated into the following JavaScript:

var _a, _b, _c;
(_a = a) === null || _a === void 0 ? void 0 : _a.b.c; // property access chain 'a?.b.c'
(_b = a) === null || _b === void 0 ? void 0 : _b.b[x]; // element access chain 'a?.b[x]'
(_c = a) === null || _c === void 0 ? void 0 : _c.b(); // call chain `a?.b()`

Optional Chaining and Generics

In addition to supporting ?.( for an optional call, we also support supplying generic type arguments to an optional call:

a?.<string>(b)

Optional Chaining and the Type System

When running outside of strictNullChecks, optional chaining should have no observable effect on the type system, as null and undefined in most cases are removed or widened to any.

When running inside of strictNullChecks, optional chaining introduces a special internal optional marker type. The optional type is a special case of the undefined type, and is treated as undefined for most type checking operations. Whenever the type checker encounters an optional expression, the null and undefined types are removed from the type of the expression and the optional type is added in its place. In this way, an optional chain can detect whether its preceding expression's type introduced a null or undefined in any step of the chain to the right of the optional expression.

For example:

declare let a: { b: { c: number } | undefined } | undefined;
a?.b?.c; // ok: has type `number | undefined`
a?.b.c; // error: object is possibly undefined (at `a?.b`)

In the first case, the expression a?.b?.c is safe to evaluate as both the a variable and the b property are guarded. In the second case, we can determine that it is not safe to evaluate the c property on a?.b as the b property is possibly undefined.

Fixes #16

@mariusschulz

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

I've been looking forward to Optional Chaining in TypeScript for a long time! Looking forward to seeing this PR land. :)

I'm curious, what was the reason that you decided to emit (_a = a) === null || _a === void 0 instead of the shorter (_a = a) == null?

var _a, _b, _c;
(_a = a) == null ? void 0 : _a.b; // optional property access 'a?.b'
(_b = a) == null ? void 0 : _b[x]; // optional element access 'a?.[x]'
(_c = a) == null ? void 0 : _c(); // optional call 'a?.()'
@SimenB

This comment has been minimized.

Copy link

commented Sep 7, 2019

This is super exciting 🎉 Any plans to surface a type error if the value is not either null or undefined? In which case it's unnecessary, so having that be an error would be sorta like TS2367, and we can avoid the runtime overhead

@jwbay

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

@fbartho

This comment has been minimized.

Copy link

commented Sep 7, 2019

@SimenB surfacing an error there would only help perfectly typed projects avoid runtime slowdowns, but it would also impose a syntax tax on projects that have to deal with the real world — infinite node_modules that are only partially typed, or where the types aren’t accurate. I wouldn’t want to have to cast (improperlyTyped as any)?.foo?.bar As this kind of would defeat some of the value of this operator.

I’d support a flag defaulting to off, --strictOptionalChaining, however!

@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2019

Any plans to surface a type error if the value is not either null or undefined?

Not at this time, no. Such an error would only be valid under strict mode (since ?. has no effect on types in non-strict mode), and would prevent you from writing defensive code against external APIs that are possibly unsafe. Also, we do not currently do this for the non-null assertion operator (!), so it would not make sense for us to do that here as well.

That kind of rule may be better implemented as a lint rule rather than a TypeScript-specific flag.

@ExE-Boss

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

Actually,

a?.b // optional property access
a?.[x] // optional element access
a?.() // optional call

should get transpiled to:

var _a, _a2, _a3;

(_a = a) === null || _a === void 0 ? void 0 : _a.b; // optional property access
(_a2 = a) === null || _a2 === void 0 ? void 0 : _a2[x]; // optional element access
(_a3 = a) === null || _a3 === void 0 ? void 0 : _a3(); // optional call
@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

@ExE-Boss: How does your example differ from my example in the description?

@ExE-Boss

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

Yours uses:

var _a, _b, _c; // Implies optional access from different variables `a`, `b` and `c`.

Whereas mine uses

var _a, _a2, _a3; // Implies distinct optional accesses from variable `a`.
@Jessidhia

This comment has been minimized.

Copy link

commented Sep 12, 2019

There is, in fact, a lint rule for that, though it will need to be updated for the ?. case. tslint / eslint

@orta

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

@typescript-bot pack this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2019

Heya @orta, I've started to run the tarball bundle task on this PR at c2f53fc. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2019

Hey @orta, 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/45184/artifacts?artifactName=tgz&fileId=4C24B3EBE47F04661C6F0C3769DECEBF2B693C39E432217A9786260BDF23992502&fileName=/typescript-3.7.0-insiders.20190923.tgz"
    }
}

and then running npm install.

@orta

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

Here's a playground

Copy link
Member

left a comment

I'd like to see some control flow tests - I'm pretty sure the control flow graph for

const q = Math.random() > 0.5 ? undefined : { b: 12 };
let b: number;
const a = q?.[b = 12, "b"];
b.toFixed(); // should fail under `strict` as `b` is not definitely assigned

needs to be updated.

@dragomirtitian

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2019

To use the new PR in playground feature 😊, it seems intelisense has no suggestions ?. for primitive type:

declare let x: string | undefined;
let l = x?.length // no intelisense for length

Play

@sandersn sandersn self-assigned this Sep 23, 2019
src/compiler/checker.ts Outdated Show resolved Hide resolved
@@ -8779,6 +8788,9 @@ namespace ts {
signature.unionSignatures ? getUnionType(map(signature.unionSignatures, getReturnTypeOfSignature), UnionReduction.Subtype) :
getReturnTypeFromAnnotation(signature.declaration!) ||
(nodeIsMissing((<FunctionLikeDeclaration>signature.declaration).body) ? anyType : getReturnTypeFromBody(<FunctionLikeDeclaration>signature.declaration));
if (signature.isOptionalCall) {

This comment has been minimized.

Copy link
@weswigham

weswigham Sep 23, 2019

Member

I think there's enough of these fields now that isOptionalCall, hasLiteralTypes, and hasRestParameter should probably be combined into a Flags field of some kind.

This comment has been minimized.

Copy link
@weswigham

weswigham Sep 23, 2019

Member

Related: instantiateSignature doesn't propagate this forward, so rather than a direct access, this needs to follow .target's to check if the uninstantiated signature has a isOptionalCall field, no?

This comment has been minimized.

Copy link
@rbuckton

rbuckton Sep 24, 2019

Author Member

That is correct.

Copy link
Member

left a comment

Also tests with generic type parameters on the signature being chained on, eg

declare function absorb<T>(): T;
declare const a: { m?<T>(obj: {x: T}): T } | undefined;
const n1: number = a?.m?({x: 12)}); // should be an error (`undefined` is not assignable to `number`)
const n2: number = a?.m?({x: absorb()}); // likewise
const n3: number | undefined = a?.m?({x: 12)}); // should be ok
const n4: number | undefined = a?.m?({x: absorb()}); // likewise

// Also a test showing `!` vs `?` for good measure
let t1 = a?.m?({x: 12});
t1 = a!.m!({x: 12});
@andrewbranch

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

Are we planning on including completions that convert . to ?. later, or is there a reason not to? I pulled down this PR and did a quick prototype:

Kapture 2019-09-23 at 14 46 21

Copy link
Member

left a comment

Found a crash in completions. In createCompletionEntry, when converting property access to element access, there’s a line that searches the property access for a DotToken and continues under the assumption that it must exist.

image

TypeError: Cannot read property 'getStart' of undefined
    at createCompletionEntry (tsserver.js:103011:67)
    at getCompletionEntriesFromSymbols (tsserver.js:103078:29)
    at completionInfoFromData (tsserver.js:102945:17)
    at Object.getCompletionsAtPosition (tsserver.js:102900:28)
@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2019

@typescript-bot perf test this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

commented Sep 29, 2019

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

Update: The results are in!

@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2019

@ahejlsberg Should I be using getTypeWithFacts for the non-null case, or should I be using getNonNullableType (which produces a NonNullable<T> for type variables)?

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

commented Sep 29, 2019

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

Here they are:

Comparison Report - master..33294

Metric master 33294 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 331,498k (± 0.02%) 331,674k (± 0.03%) +176k (+ 0.05%) 331,482k 331,828k
Parse Time 1.56s (± 0.61%) 1.57s (± 0.72%) +0.01s (+ 0.90%) 1.55s 1.60s
Bind Time 0.80s (± 0.62%) 0.82s (± 0.68%) +0.02s (+ 2.11%) 0.81s 0.83s
Check Time 4.34s (± 0.28%) 4.45s (± 0.51%) +0.11s (+ 2.56%) 4.40s 4.52s
Emit Time 5.25s (± 0.70%) 5.43s (± 0.89%) +0.18s (+ 3.53%) 5.37s 5.61s
Total Time 11.95s (± 0.36%) 12.27s (± 0.44%) +0.33s (+ 2.73%) 12.19s 12.41s
Monaco - node (v12.1.0, x64)
Memory used 346,484k (± 0.01%) 346,552k (± 0.02%) +68k (+ 0.02%) 346,394k 346,679k
Parse Time 1.23s (± 0.76%) 1.23s (± 0.59%) -0.00s (- 0.16%) 1.22s 1.25s
Bind Time 0.70s (± 0.63%) 0.71s (± 0.81%) +0.01s (+ 1.14%) 0.70s 0.72s
Check Time 4.40s (± 0.43%) 4.44s (± 0.54%) +0.04s (+ 0.79%) 4.38s 4.50s
Emit Time 2.88s (± 0.75%) 3.02s (± 0.96%) +0.15s (+ 5.04%) 2.96s 3.10s
Total Time 9.21s (± 0.39%) 9.40s (± 0.48%) +0.18s (+ 2.01%) 9.26s 9.49s
TFS - node (v12.1.0, x64)
Memory used 304,629k (± 0.02%) 304,722k (± 0.02%) +93k (+ 0.03%) 304,601k 304,826k
Parse Time 0.95s (± 0.63%) 0.96s (± 0.99%) +0.00s (+ 0.10%) 0.93s 0.97s
Bind Time 0.66s (± 0.68%) 0.67s (± 0.60%) +0.01s (+ 1.98%) 0.66s 0.68s
Check Time 3.97s (± 0.64%) 4.00s (± 0.38%) +0.04s (+ 0.93%) 3.97s 4.04s
Emit Time 2.97s (± 0.55%) 3.09s (± 0.93%) +0.11s (+ 3.84%) 3.02s 3.15s
Total Time 8.55s (± 0.33%) 8.72s (± 0.39%) +0.17s (+ 2.00%) 8.63s 8.80s
Angular - node (v8.9.0, x64)
Memory used 350,502k (± 0.01%) 350,694k (± 0.02%) +192k (+ 0.05%) 350,564k 350,774k
Parse Time 2.11s (± 0.42%) 2.11s (± 0.48%) +0.00s (+ 0.05%) 2.10s 2.14s
Bind Time 0.87s (± 0.77%) 0.90s (± 0.53%) +0.02s (+ 2.75%) 0.89s 0.91s
Check Time 5.19s (± 0.47%) 5.33s (± 0.56%) +0.15s (+ 2.83%) 5.30s 5.44s
Emit Time 5.95s (± 1.41%) 6.24s (± 0.95%) +0.29s (+ 4.88%) 6.09s 6.33s
Total Time 14.12s (± 0.67%) 14.58s (± 0.60%) +0.46s (+ 3.27%) 14.38s 14.79s
Monaco - node (v8.9.0, x64)
Memory used 364,301k (± 0.01%) 364,434k (± 0.01%) +134k (+ 0.04%) 364,330k 364,490k
Parse Time 1.56s (± 0.47%) 1.57s (± 0.45%) +0.01s (+ 0.45%) 1.55s 1.58s
Bind Time 0.90s (± 0.68%) 0.91s (± 0.38%) +0.01s (+ 0.67%) 0.90s 0.91s
Check Time 5.23s (± 1.03%) 5.52s (± 0.52%) +0.29s (+ 5.53%) 5.47s 5.59s
Emit Time 3.28s (± 2.45%) 3.06s (± 0.88%) -0.22s (- 6.59%) 2.96s 3.10s
Total Time 10.97s (± 0.37%) 11.06s (± 0.26%) +0.09s (+ 0.79%) 11.01s 11.13s
TFS - node (v8.9.0, x64)
Memory used 321,047k (± 0.01%) 321,221k (± 0.01%) +175k (+ 0.05%) 321,112k 321,315k
Parse Time 1.27s (± 0.71%) 1.27s (± 0.46%) +0.00s (+ 0.16%) 1.26s 1.28s
Bind Time 0.78s (± 6.89%) 0.72s (± 0.62%) -0.06s (- 7.95%) 0.71s 0.73s
Check Time 4.52s (± 1.93%) 4.67s (± 0.39%) +0.16s (+ 3.43%) 4.64s 4.73s
Emit Time 3.08s (± 0.73%) 3.22s (± 0.40%) +0.13s (+ 4.28%) 3.20s 3.25s
Total Time 9.64s (± 0.56%) 9.88s (± 0.28%) +0.23s (+ 2.38%) 9.81s 9.93s
Angular - node (v8.9.0, x86)
Memory used 198,645k (± 0.02%) 198,864k (± 0.02%) +219k (+ 0.11%) 198,800k 198,983k
Parse Time 2.04s (± 0.61%) 2.04s (± 0.79%) 0.00s ( 0.00%) 2.01s 2.09s
Bind Time 0.99s (± 1.01%) 1.00s (± 0.81%) +0.02s (+ 1.62%) 0.99s 1.03s
Check Time 4.72s (± 0.35%) 4.84s (± 0.62%) +0.12s (+ 2.52%) 4.79s 4.94s
Emit Time 5.78s (± 1.35%) 6.13s (± 1.39%) +0.35s (+ 6.04%) 6.00s 6.33s
Total Time 13.52s (± 0.61%) 14.00s (± 0.59%) +0.48s (+ 3.59%) 13.84s 14.17s
Monaco - node (v8.9.0, x86)
Memory used 203,827k (± 0.02%) 203,956k (± 0.02%) +129k (+ 0.06%) 203,878k 204,053k
Parse Time 1.62s (± 0.54%) 1.61s (± 0.71%) -0.01s (- 0.37%) 1.59s 1.64s
Bind Time 0.72s (± 0.68%) 0.73s (± 0.71%) +0.01s (+ 0.97%) 0.72s 0.74s
Check Time 5.32s (± 1.07%) 5.38s (± 0.59%) +0.06s (+ 1.13%) 5.30s 5.44s
Emit Time 2.81s (± 2.88%) 2.89s (± 0.87%) +0.07s (+ 2.63%) 2.85s 2.96s
Total Time 10.47s (± 0.47%) 10.61s (± 0.54%) +0.14s (+ 1.32%) 10.51s 10.74s
TFS - node (v8.9.0, x86)
Memory used 180,612k (± 0.01%) 180,739k (± 0.02%) +127k (+ 0.07%) 180,686k 180,829k
Parse Time 1.30s (± 0.46%) 1.31s (± 0.74%) +0.01s (+ 0.54%) 1.29s 1.34s
Bind Time 0.67s (± 0.86%) 0.68s (± 1.20%) +0.01s (+ 1.94%) 0.67s 0.71s
Check Time 4.39s (± 0.49%) 4.46s (± 0.45%) +0.07s (+ 1.55%) 4.40s 4.49s
Emit Time 2.84s (± 0.83%) 2.95s (± 0.78%) +0.11s (+ 3.80%) 2.90s 2.99s
Total Time 9.20s (± 0.25%) 9.40s (± 0.47%) +0.20s (+ 2.15%) 9.27s 9.50s
Angular - node (v9.0.0, x64)
Memory used 350,051k (± 0.02%) 350,319k (± 0.01%) +268k (+ 0.08%) 350,263k 350,438k
Parse Time 1.83s (± 0.73%) 1.83s (± 0.51%) +0.01s (+ 0.33%) 1.81s 1.85s
Bind Time 0.82s (± 0.46%) 0.83s (± 0.98%) +0.02s (+ 2.21%) 0.82s 0.86s
Check Time 4.99s (± 0.52%) 5.09s (± 0.39%) +0.10s (+ 2.00%) 5.04s 5.13s
Emit Time 5.71s (± 1.57%) 5.96s (± 0.47%) +0.25s (+ 4.38%) 5.89s 6.02s
Total Time 13.35s (± 0.69%) 13.72s (± 0.33%) +0.37s (+ 2.79%) 13.63s 13.79s
Monaco - node (v9.0.0, x64)
Memory used 364,056k (± 0.02%) 364,268k (± 0.01%) +211k (+ 0.06%) 364,173k 364,347k
Parse Time 1.31s (± 0.37%) 1.32s (± 0.42%) +0.00s (+ 0.30%) 1.31s 1.33s
Bind Time 0.84s (± 0.73%) 0.86s (± 0.90%) +0.02s (+ 2.14%) 0.84s 0.88s
Check Time 5.25s (± 1.00%) 5.35s (± 1.39%) +0.10s (+ 1.96%) 5.06s 5.42s
Emit Time 2.93s (± 3.80%) 3.03s (± 3.64%) +0.10s (+ 3.51%) 2.97s 3.48s
Total Time 10.34s (± 0.74%) 10.57s (± 0.47%) +0.22s (+ 2.18%) 10.51s 10.75s
TFS - node (v9.0.0, x64)
Memory used 320,733k (± 0.02%) 320,948k (± 0.02%) +215k (+ 0.07%) 320,799k 321,020k
Parse Time 1.04s (± 0.72%) 1.04s (± 0.54%) -0.01s (- 0.48%) 1.03s 1.05s
Bind Time 0.66s (± 0.67%) 0.68s (± 0.86%) +0.01s (+ 2.27%) 0.66s 0.69s
Check Time 4.58s (± 0.45%) 4.83s (± 0.42%) +0.25s (+ 5.37%) 4.79s 4.88s
Emit Time 3.23s (± 0.93%) 3.06s (± 0.67%) -0.17s (- 5.35%) 3.03s 3.12s
Total Time 9.52s (± 0.47%) 9.61s (± 0.40%) +0.09s (+ 0.90%) 9.54s 9.69s
Angular - node (v9.0.0, x86)
Memory used 198,782k (± 0.04%) 198,951k (± 0.03%) +169k (+ 0.08%) 198,845k 199,113k
Parse Time 1.73s (± 0.61%) 1.74s (± 0.67%) +0.01s (+ 0.58%) 1.72s 1.78s
Bind Time 0.92s (± 0.53%) 0.94s (± 0.98%) +0.02s (+ 1.95%) 0.93s 0.97s
Check Time 4.41s (± 0.68%) 4.52s (± 0.82%) +0.11s (+ 2.43%) 4.42s 4.58s
Emit Time 5.55s (± 0.77%) 5.69s (± 0.65%) +0.14s (+ 2.52%) 5.63s 5.78s
Total Time 12.61s (± 0.43%) 12.89s (± 0.44%) +0.28s (+ 2.23%) 12.73s 13.01s
Monaco - node (v9.0.0, x86)
Memory used 203,908k (± 0.02%) 204,104k (± 0.02%) +196k (+ 0.10%) 204,045k 204,199k
Parse Time 1.35s (± 0.48%) 1.35s (± 0.39%) +0.00s (+ 0.07%) 1.34s 1.36s
Bind Time 0.66s (± 0.51%) 0.68s (± 0.82%) +0.01s (+ 2.26%) 0.67s 0.69s
Check Time 5.14s (± 0.36%) 5.06s (± 2.37%) -0.08s (- 1.63%) 4.88s 5.31s
Emit Time 2.72s (± 0.72%) 3.05s (± 4.33%) +0.33s (+12.20%) 2.82s 3.27s
Total Time 9.88s (± 0.31%) 10.14s (± 0.47%) +0.26s (+ 2.67%) 10.01s 10.24s
TFS - node (v9.0.0, x86)
Memory used 180,553k (± 0.02%) 180,739k (± 0.02%) +186k (+ 0.10%) 180,694k 180,789k
Parse Time 1.06s (± 0.73%) 1.07s (± 1.14%) +0.01s (+ 0.94%) 1.05s 1.11s
Bind Time 0.63s (± 1.32%) 0.64s (± 1.01%) +0.01s (+ 1.92%) 0.63s 0.66s
Check Time 4.31s (± 0.35%) 4.36s (± 0.84%) +0.05s (+ 1.16%) 4.28s 4.47s
Emit Time 2.83s (± 0.77%) 2.94s (± 1.84%) +0.11s (+ 4.00%) 2.85s 3.13s
Total Time 8.82s (± 0.30%) 9.00s (± 0.73%) +0.18s (+ 2.05%) 8.88s 9.15s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-161-generic
Architecturex64
Available Memory16 GB
Available Memory9 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 33294 10
Baseline master 10

@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2019

I've made a few improvements to the Debug.formatControlFlowGraph function. Its getting a bit big, so I'm planning to move it out of src/compiler and into a separate script that can be loaded on demand while debugging.

It prints out a control flow graph in the following format:

// if (o?.x.y) {
//   o.x.y; // reference
// }
> reference.flowNode.__debugToString()
Branch ┬ True (o?.x.y) ┬ True (o) ─ Start  
       ╰ True (o?.x.y) ╯              
// try {
//   if (o?.x.y) {
//     o.x.y;
//    }
// }
// finally {
//   o.x.y; // reference
// }
> reference.flowNode.__debugToString()
Branch ┬ Branch ────┬ True (o?.x.y) ─────────────────┬ True (o) ┬ Start
       │            ╰ Branch ───────┬ False (o?.x.y) ╯          │
       │                            ╰ False (o) ────────────────╯
       ╰ PreFinally

It doesn't print the antecedents of a PreFinally since they would require crossing over connections, but this should be helpful for us when debugging CFA.

@rbuckton rbuckton force-pushed the optionalChainingStage3 branch from 96a91c9 to 0828674 Sep 30, 2019
@rbuckton

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2019

@typescript-bot perf test this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

commented Sep 30, 2019

Heya @rbuckton, I've started to run the perf test suite on this PR at 0828674. 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

commented Sep 30, 2019

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

Here they are:

Comparison Report - master..33294

Metric master 33294 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 331,498k (± 0.02%) 332,910k (± 0.05%) +1,412k (+ 0.43%) 332,347k 333,118k
Parse Time 1.56s (± 0.61%) 1.57s (± 0.48%) +0.01s (+ 0.58%) 1.56s 1.59s
Bind Time 0.80s (± 0.62%) 0.84s (± 0.66%) +0.03s (+ 4.10%) 0.83s 0.85s
Check Time 4.34s (± 0.28%) 4.45s (± 0.60%) +0.11s (+ 2.58%) 4.40s 4.52s
Emit Time 5.25s (± 0.70%) 5.47s (± 0.92%) +0.22s (+ 4.27%) 5.39s 5.59s
Total Time 11.95s (± 0.36%) 12.33s (± 0.57%) +0.38s (+ 3.18%) 12.20s 12.48s
Monaco - node (v12.1.0, x64)
Memory used 346,484k (± 0.01%) 348,075k (± 0.02%) +1,592k (+ 0.46%) 347,970k 348,244k
Parse Time 1.23s (± 0.76%) 1.23s (± 0.56%) -0.00s (- 0.33%) 1.21s 1.24s
Bind Time 0.70s (± 0.63%) 0.72s (± 0.72%) +0.02s (+ 2.71%) 0.71s 0.73s
Check Time 4.40s (± 0.43%) 4.43s (± 0.34%) +0.03s (+ 0.68%) 4.39s 4.46s
Emit Time 2.88s (± 0.75%) 2.99s (± 0.54%) +0.11s (+ 3.93%) 2.94s 3.02s
Total Time 9.21s (± 0.39%) 9.37s (± 0.24%) +0.16s (+ 1.73%) 9.31s 9.42s
TFS - node (v12.1.0, x64)
Memory used 304,629k (± 0.02%) 306,800k (± 0.02%) +2,171k (+ 0.71%) 306,693k 306,902k
Parse Time 0.95s (± 0.63%) 0.96s (± 0.79%) +0.00s (+ 0.10%) 0.94s 0.97s
Bind Time 0.66s (± 0.68%) 0.69s (± 0.83%) +0.03s (+ 4.86%) 0.68s 0.71s
Check Time 3.97s (± 0.64%) 4.01s (± 0.55%) +0.04s (+ 1.08%) 3.98s 4.07s
Emit Time 2.97s (± 0.55%) 3.11s (± 1.10%) +0.14s (+ 4.61%) 3.04s 3.21s
Total Time 8.55s (± 0.33%) 8.77s (± 0.50%) +0.22s (+ 2.54%) 8.66s 8.87s
Angular - node (v8.9.0, x64)
Memory used 350,502k (± 0.01%) 352,094k (± 0.02%) +1,592k (+ 0.45%) 351,853k 352,241k
Parse Time 2.11s (± 0.42%) 2.10s (± 0.53%) -0.01s (- 0.57%) 2.07s 2.12s
Bind Time 0.87s (± 0.77%) 0.90s (± 0.83%) +0.03s (+ 3.09%) 0.89s 0.92s
Check Time 5.19s (± 0.47%) 5.31s (± 0.58%) +0.13s (+ 2.41%) 5.25s 5.40s
Emit Time 5.95s (± 1.41%) 6.27s (± 1.00%) +0.32s (+ 5.42%) 6.13s 6.46s
Total Time 14.12s (± 0.67%) 14.58s (± 0.55%) +0.46s (+ 3.24%) 14.48s 14.87s
Monaco - node (v8.9.0, x64)
Memory used 364,301k (± 0.01%) 366,035k (± 0.01%) +1,735k (+ 0.48%) 365,931k 366,151k
Parse Time 1.56s (± 0.47%) 1.57s (± 0.48%) +0.01s (+ 0.83%) 1.56s 1.59s
Bind Time 0.90s (± 0.68%) 0.92s (± 0.32%) +0.02s (+ 1.89%) 0.91s 0.92s
Check Time 5.23s (± 1.03%) 5.47s (± 0.45%) +0.24s (+ 4.63%) 5.42s 5.54s
Emit Time 3.28s (± 2.45%) 3.06s (± 0.57%) -0.22s (- 6.65%) 3.02s 3.10s
Total Time 10.97s (± 0.37%) 11.02s (± 0.34%) +0.05s (+ 0.47%) 10.97s 11.13s
TFS - node (v8.9.0, x64)
Memory used 321,047k (± 0.01%) 323,267k (± 0.01%) +2,220k (+ 0.69%) 323,199k 323,327k
Parse Time 1.27s (± 0.71%) 1.27s (± 0.64%) +0.01s (+ 0.63%) 1.26s 1.29s
Bind Time 0.78s (± 6.89%) 0.75s (± 1.04%) -0.03s (- 3.97%) 0.73s 0.76s
Check Time 4.52s (± 1.93%) 4.67s (± 0.51%) +0.15s (+ 3.37%) 4.63s 4.74s
Emit Time 3.08s (± 0.73%) 3.22s (± 0.49%) +0.14s (+ 4.41%) 3.18s 3.25s
Total Time 9.64s (± 0.56%) 9.91s (± 0.35%) +0.26s (+ 2.74%) 9.84s 9.99s
Angular - node (v8.9.0, x86)
Memory used 198,645k (± 0.02%) 199,514k (± 0.03%) +869k (+ 0.44%) 199,363k 199,648k
Parse Time 2.04s (± 0.61%) 2.04s (± 0.57%) +0.01s (+ 0.44%) 2.02s 2.08s
Bind Time 0.99s (± 1.01%) 1.04s (± 1.02%) +0.05s (+ 5.07%) 1.02s 1.07s
Check Time 4.72s (± 0.35%) 4.83s (± 0.41%) +0.12s (+ 2.46%) 4.79s 4.87s
Emit Time 5.78s (± 1.35%) 6.05s (± 1.36%) +0.27s (+ 4.71%) 5.95s 6.37s
Total Time 13.52s (± 0.61%) 13.96s (± 0.62%) +0.45s (+ 3.29%) 13.88s 14.30s
Monaco - node (v8.9.0, x86)
Memory used 203,827k (± 0.02%) 204,774k (± 0.02%) +947k (+ 0.46%) 204,680k 204,861k
Parse Time 1.62s (± 0.54%) 1.60s (± 0.36%) -0.02s (- 1.11%) 1.59s 1.61s
Bind Time 0.72s (± 0.68%) 0.76s (± 0.45%) +0.04s (+ 5.39%) 0.76s 0.77s
Check Time 5.32s (± 1.07%) 5.34s (± 0.47%) +0.02s (+ 0.43%) 5.29s 5.39s
Emit Time 2.81s (± 2.88%) 2.87s (± 0.44%) +0.06s (+ 2.10%) 2.85s 2.91s
Total Time 10.47s (± 0.47%) 10.57s (± 0.27%) +0.10s (+ 0.98%) 10.52s 10.65s
TFS - node (v8.9.0, x86)
Memory used 180,612k (± 0.01%) 181,832k (± 0.01%) +1,220k (+ 0.68%) 181,787k 181,890k
Parse Time 1.30s (± 0.46%) 1.32s (± 0.90%) +0.01s (+ 1.07%) 1.29s 1.34s
Bind Time 0.67s (± 0.86%) 0.71s (± 0.78%) +0.04s (+ 5.67%) 0.70s 0.72s
Check Time 4.39s (± 0.49%) 4.43s (± 0.46%) +0.04s (+ 0.87%) 4.38s 4.47s
Emit Time 2.84s (± 0.83%) 2.98s (± 0.91%) +0.14s (+ 4.79%) 2.92s 3.06s
Total Time 9.20s (± 0.25%) 9.43s (± 0.35%) +0.23s (+ 2.48%) 9.38s 9.54s
Angular - node (v9.0.0, x64)
Memory used 350,051k (± 0.02%) 351,729k (± 0.02%) +1,678k (+ 0.48%) 351,586k 351,873k
Parse Time 1.83s (± 0.73%) 1.84s (± 0.58%) +0.01s (+ 0.71%) 1.82s 1.87s
Bind Time 0.82s (± 0.46%) 0.84s (± 0.77%) +0.03s (+ 3.31%) 0.83s 0.86s
Check Time 4.99s (± 0.52%) 5.06s (± 0.70%) +0.07s (+ 1.40%) 5.01s 5.17s
Emit Time 5.71s (± 1.57%) 5.92s (± 0.67%) +0.21s (+ 3.62%) 5.85s 6.00s
Total Time 13.35s (± 0.69%) 13.66s (± 0.23%) +0.31s (+ 2.35%) 13.55s 13.71s
Monaco - node (v9.0.0, x64)
Memory used 364,056k (± 0.02%) 365,826k (± 0.01%) +1,769k (+ 0.49%) 365,723k 365,941k
Parse Time 1.31s (± 0.37%) 1.31s (± 0.72%) -0.00s (- 0.00%) 1.30s 1.34s
Bind Time 0.84s (± 0.73%) 0.87s (± 1.70%) +0.03s (+ 3.80%) 0.85s 0.92s
Check Time 5.25s (± 1.00%) 5.30s (± 1.26%) +0.05s (+ 0.88%) 5.12s 5.38s
Emit Time 2.93s (± 3.80%) 3.10s (± 4.82%) +0.17s (+ 5.83%) 2.98s 3.53s
Total Time 10.34s (± 0.74%) 10.59s (± 0.98%) +0.25s (+ 2.38%) 10.48s 10.91s
TFS - node (v9.0.0, x64)
Memory used 320,733k (± 0.02%) 323,032k (± 0.02%) +2,299k (+ 0.72%) 322,950k 323,182k
Parse Time 1.04s (± 0.72%) 1.04s (± 0.50%) -0.00s (- 0.19%) 1.03s 1.05s
Bind Time 0.66s (± 0.67%) 0.69s (± 0.32%) +0.03s (+ 4.08%) 0.68s 0.69s
Check Time 4.58s (± 0.45%) 4.83s (± 0.45%) +0.24s (+ 5.30%) 4.78s 4.87s
Emit Time 3.23s (± 0.93%) 3.06s (± 0.50%) -0.17s (- 5.14%) 3.03s 3.10s
Total Time 9.52s (± 0.47%) 9.62s (± 0.35%) +0.10s (+ 1.05%) 9.55s 9.67s
Angular - node (v9.0.0, x86)
Memory used 198,782k (± 0.04%) 199,632k (± 0.02%) +850k (+ 0.43%) 199,531k 199,749k
Parse Time 1.73s (± 0.61%) 1.74s (± 0.45%) +0.01s (+ 0.75%) 1.72s 1.75s
Bind Time 0.92s (± 0.53%) 0.96s (± 1.13%) +0.04s (+ 4.11%) 0.94s 0.99s
Check Time 4.41s (± 0.68%) 4.53s (± 0.84%) +0.13s (+ 2.84%) 4.45s 4.60s
Emit Time 5.55s (± 0.77%) 5.72s (± 0.91%) +0.17s (+ 3.06%) 5.65s 5.85s
Total Time 12.61s (± 0.43%) 12.96s (± 0.43%) +0.35s (+ 2.78%) 12.83s 13.07s
Monaco - node (v9.0.0, x86)
Memory used 203,908k (± 0.02%) 204,879k (± 0.02%) +971k (+ 0.48%) 204,756k 204,979k
Parse Time 1.35s (± 0.48%) 1.34s (± 0.65%) -0.01s (- 0.45%) 1.33s 1.36s
Bind Time 0.66s (± 0.51%) 0.68s (± 0.65%) +0.02s (+ 2.87%) 0.67s 0.69s
Check Time 5.14s (± 0.36%) 5.09s (± 1.91%) -0.06s (- 1.13%) 4.89s 5.24s
Emit Time 2.72s (± 0.72%) 2.99s (± 4.95%) +0.26s (+ 9.66%) 2.80s 3.25s
Total Time 9.88s (± 0.31%) 10.09s (± 0.59%) +0.21s (+ 2.18%) 9.99s 10.23s
TFS - node (v9.0.0, x86)
Memory used 180,553k (± 0.02%) 181,748k (± 0.02%) +1,195k (+ 0.66%) 181,654k 181,836k
Parse Time 1.06s (± 0.73%) 1.07s (± 0.77%) +0.01s (+ 0.76%) 1.05s 1.09s
Bind Time 0.63s (± 1.32%) 0.65s (± 0.86%) +0.02s (+ 3.51%) 0.64s 0.66s
Check Time 4.31s (± 0.35%) 4.32s (± 0.57%) +0.01s (+ 0.23%) 4.24s 4.35s
Emit Time 2.83s (± 0.77%) 2.92s (± 1.30%) +0.09s (+ 3.22%) 2.81s 2.99s
Total Time 8.82s (± 0.30%) 8.95s (± 0.44%) +0.13s (+ 1.51%) 8.86s 9.04s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-161-generic
Architecturex64
Available Memory16 GB
Available Memory9 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 33294 10
Baseline master 10

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.7.0 milestone Sep 30, 2019
rbuckton added 2 commits Sep 30, 2019
# Conflicts:
#	src/compiler/checker.ts
#	src/compiler/diagnosticMessages.json
@rbuckton rbuckton merged commit fcd9334 into master Sep 30, 2019
4 of 5 checks passed
4 of 5 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
license/cla All CLA requirements met.
Details
node10 Build #46586 succeeded
Details
node12 Build #46584 succeeded
Details
node8 Build #46585 succeeded
Details
@RyanCavanaugh RyanCavanaugh deleted the optionalChainingStage3 branch Sep 30, 2019
@kube

This comment has been minimized.

Copy link

commented Oct 1, 2019

❤️

@ekfn

This comment has been minimized.

Copy link

commented Oct 1, 2019

@andrewbranch

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

@ekfn the TS version there is from an earlier commit in this PR. That issue was fixed—you can see it give a proper error in the nightly: https://www.typescriptlang.org/play/?ts=Nightly#code/MYewdgzgLgBAZiEMC8MAUYBcMwFcC2ARgKYBOAlCgHw4DcAsAFCiSyECGpK6YA-NniJlKyGghAZeAOnK0gA

Wait sorry, it looks like the playground doesn’t have the new nightly yet. Here’s the commit where it was fixed, at any rate: e073c05

And here’s a screenshot of it working in master:

image

@j-oliveras

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2019

@andrewbranch now it works on playground too.

@andrewbranch

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

For me, the playground does show a syntax error, but it’s because it starts to parse as a conditional expression:

image

Subtle difference, but you can tell because the JavaScript emit in the right pane emits a conditional expression with empty true and false sides, and none of the var _a helper variables associated with optional expressions 😄

@j-oliveras

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2019

Sorry, my comment was that nightly works.

I understand that the last nightly have this PR and #32883 included (or maybe it loads the previous nightly). Something its wrong because it try to parse ?. and ?? as ?: (the last one as a two ?: operators).

Edit: locally with 3.7.0-dev.20191001 I see the same errors as on playground.

@andrewbranch

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

image

3.7.0-dev.20191001, shown here, definitely works, but for me loading Nightly on the playground is clearly loading an older build. I’m not sure how to tell for sure what build it is. /cc @orta?

@j-oliveras

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2019

Tested again locally and it works both (this PR and #32883). Probably I was using another version.

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