Fix destructuring and narrowing interaction #47337

gabritto · 2022-01-06T21:15:39Z

Background

In 4.3, we improved control-flow narrowing of generic types (#43183). When we are about to go into control-flow narrowing, we call a function getNarrowableTypeForReference, that returns the type we are going to use in narrowing.
As of 4.3, when we call getNarrowableTypeForReference on something of type T extends C, we often get type C (the constraint) instead of the generic type T, which allows us to do narrowing when we couldn't. As an example:

type C = { kind: "a", a: string } | { kind: "b", b: string }

function func<T extends C>(value: T) {
    if (value.kind === "a") {
        value.a; // In 4.3: value.a: string
    } else {
        value.b; // In 4.3: value.b: string
    }
}

Inside the if branch above, to get the type of value.a, we first need the type of value. So we call getNarrowableTypeForReference(value), and get type C (instead of the generic T, which we can't narrow). Then we can further narrow C into { kind: "a", a: string } due to the if condition. Finally, we get the type of property a of type { kind: "a", a: string }, which is type string, and that is the type of value.a.

In 4.3, the same thing happened for binding elements in binding patterns:

type C = { kind: "a", a: string } | { kind: "b", b: string }

function func<T extends C>(value: T) {
    if (value.kind === "a") {
        const { a } = value; // In 4.3: a: string
    } else {
        const { b } = value; // In 4.3: b: string
    }
}

However, that meant other things broke, like #43941:

type Params = {
    foo: string;
} & ({ tag: 'a'; type: number } | { tag: 'b'; type: string });

const getType = <P extends Params>(params: P) => {
    const {
        foo,
        ...rest // <4.3: rest: Omit<P, "foo"> -- 4.3: rest: { tag: 'a'; type: number } | { tag: 'b'; type: string }
    } = params;

    return rest;
};

declare const params: Params;

switch (params.tag) {
    case 'a': {
        const result = getType(params).type; // <4.3: result: number -- 4.3: result: string | number
        break;
    }
    case 'b': {
        const result = getType(params).type; // <4.3: result: string -- 4.3: result: string | number
        break;
    }
}

In the example above, to determine the type of ...rest, we once again call getNarrowableTypeForReference(params), determine that type, and then compute whatever properties are left after we remove foo.
Before 4.3, getNarrowableTypeForReference(params) returned type P, so rest would have generic type Omit<P, "foo">. In 4.3, getNarrowableTypeForReference(params)returned typeParams, so restwould have non-generic type{ tag: 'a'; type: number } | { tag: 'b'; type: string }`.

That distinction between rest having a generic versus non-generic type means we can or can't narrow other uses of rest.
The problem in the example above surfaces in the switch statement, when we try to call the function getType, that returns rest.
If we keep rest generic, we can "flow" the narrowing of the switch statement into the return type of getType.
If rest is instead the non-generic type, we can't do that narrowing, because there's no longer a connection between getType's type parameter P and the type of rest.

Change in 4.4

To fix the issue #43941 above, we reverted the getNarrowableTypeForReference(ref) behavior, making it return the type parameter instead of the type parameter's constraint when the reference ref was contextually typed by binding patterns.
So the result was like this:

type C = { kind: "a", a: string } | { kind: "b", b: string }

function func<T extends C>(value: T) {
    if (value.kind === "a") {
        value.a; // In 4.4: value.a: string
        const { a } = value; // In 4.4: error, "Property 'a' does not exist on type 'C'".
    } else {
        value.b; // In 4.4: value.b: string
        const { b } = value; // In 4.4: error, "Property 'a' does not exist on type 'C'".
    }
}

Issue

It seems we are at odds on whether we should have getNarrowableTypeForReference return the generic type or the constraint when we have binding patterns/destructuring, because the most useful behavior can go either way depending on the code in question.

In some cases, we really expect the binding pattern access to behave like the regular property access:

if (value.kind === "a") {
        value.a; // string
        const { a } = value; // error, but should be string???
}

On the other hand, in this case:

const getType = <P extends Params>(params: P) => {
    const {
        foo,
        ...rest
    } = params;

    return rest;
};

when rest has the generic type Omit<P, "foo">, it becomes possibly more useful for callers of that function, because we can narrow the return type based on the argument type.

Proposed fix

A compromise for this issue with binding patterns is to have getNarrowableTypeForReference return the generic type if we are retrieving the type in order to use it for a rest binding element, and to have getNarrowableTypeForReference return the constraint type for non-rest binding elements. Like this:

type C = { kind: "a", a: string } | { kind: "b", b: string }

function func<T extends C>(value: T) {
    if (value.kind === "a") {
        const { a } = value; // getNarrowableTypeForReference(value) = C, a: string
        const { ...rest } = value; // getNarrowableTypeForReference(value) = T, rest: T (or Omit<T, something>)
    }
}

Implementation

To implement this special casing for binding elements, we need a way to know *inside getNarrowableTypeForReference if we are returning a type to be used for determining the type of an object rest binding element.
This PR changes getNarrowableTypeForReference (and some other checker functions) to take in a checkMode parameter.
We also have a new CheckMode flag, CheckMode.ObjectRestBindingElement, that indicates precisely this.

Questions

Do we want to move forward with this fix? It's a bit weird and seems like we're stacking a special case on top of a special case, but there might not be a better way to solve this, because an argument can be made for using a generic type's constraint, and an argument can also be made for keeping the generic type in the situations above.
Is there a better way to implement this, without passing around CheckMode?
Do we want this special, generic-preserving behavior for rest binding elements in array binding patterns too?

gabritto · 2022-01-10T22:51:28Z

src/compiler/checker.ts

-        function getTypeForVariableLikeDeclaration(declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement | JSDocPropertyLikeTag, includeOptionality: boolean): Type | undefined {
+        function getTypeForVariableLikeDeclaration(
+            declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement | JSDocPropertyLikeTag,
+            includeOptionality: boolean, // >> TODO: change this to use a flag in check mode; figure out how this interacts with caching


If we decide to move forward with this PR, I'm going to replace the use of includeOptionality parameter by a new check mode flag.

gabritto · 2022-01-11T01:18:55Z

tests/cases/compiler/narrowingDestructuring.ts

+function func2<T extends Z>(value: T) {
+    if (value.kind === "f") {
+        const { f: f1 } = value;
+        const { f: { a, ...spread } } = value;


I'm not sure if this case is any more weird than this whole thing already is, but... ...spread here is not going to have a generic type, since we first get the non-generic type of value, then find out the non-generic type of property f on that type, and that's the type we use to compute the type of spread.

gabritto · 2022-01-11T01:19:45Z

tests/cases/compiler/narrowingDestructuring.ts

+        const { kind, ...r1 } = t;
+        const r2 = (({ kind, ...rest }) => rest)(t);


That's an inconsistency that was already present. We have:
r1 : Omit<T, "kind">, but r2 : { a: string; }.

gabritto · 2022-01-11T01:34:06Z

@typescript-bot perf test this

orta · 2022-01-21T09:47:16Z

@typescript-bot perf test this

typescript-bot · 2022-01-21T09:47:20Z

Heya @orta, I've started to run the perf test suite on this PR at 23c38be. You can monitor the build here.

Update: The results are in!

typescript-bot · 2022-01-21T10:25:46Z

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

Here they are:

Comparison Report - main..47337

_Metric	_main	₄₇₃₃₇	_Delta	_Best	_Worst
_{Angular - node (v10.16.3, x64)}
_{Memory used}	_{355,936k (± 0.02%)}	_{355,949k (± 0.03%)}	_{+13k (+ 0.00%)}	_355,786k	_356,217k
_{Parse Time}	_{1.97s (± 0.48%)}	_{1.97s (± 0.66%)}	_{+0.00s (+ 0.05%)}	_1.93s	_1.99s
_{Bind Time}	_{0.86s (± 1.02%)}	_{0.86s (± 0.65%)}	_{-0.01s (- 0.58%)}	_0.85s	_0.87s
_{Check Time}	_{5.57s (± 0.52%)}	_{5.58s (± 0.42%)}	_{+0.01s (+ 0.18%)}	_5.53s	_5.64s
_{Emit Time}	_{5.93s (± 0.58%)}	_{5.91s (± 0.43%)}	_{-0.02s (- 0.32%)}	_5.86s	_5.95s
_{Total Time}	_{14.32s (± 0.48%)}	_{14.31s (± 0.29%)}	_{-0.01s (- 0.10%)}	_14.21s	_14.40s
_{Compiler-Unions - node (v10.16.3, x64)}
_{Memory used}	_{204,215k (± 0.02%)}	_{204,203k (± 0.02%)}	_{-11k (- 0.01%)}	_204,141k	_204,300k
_{Parse Time}	_{0.79s (± 1.11%)}	_{0.80s (± 0.84%)}	_{+0.00s (+ 0.50%)}	_0.78s	_0.81s
_{Bind Time}	_{0.53s (± 1.32%)}	_{0.52s (± 1.00%)}	_{-0.01s (- 1.51%)}	_0.51s	_0.53s
_{Check Time}	_{7.86s (± 0.46%)}	_{7.88s (± 0.82%)}	_{+0.01s (+ 0.18%)}	_7.75s	_8.05s
_{Emit Time}	_{2.46s (± 0.56%)}	_{2.46s (± 0.73%)}	_{-0.00s (- 0.08%)}	_2.43s	_2.52s
_{Total Time}	_{11.65s (± 0.37%)}	_{11.65s (± 0.71%)}	_{+0.01s (+ 0.06%)}	_11.51s	_11.86s
_{Monaco - node (v10.16.3, x64)}
_{Memory used}	_{342,654k (± 0.01%)}	_{342,598k (± 0.02%)}	_{-56k (- 0.02%)}	_342,442k	_342,734k
_{Parse Time}	_{1.49s (± 0.41%)}	_{1.49s (± 0.58%)}	_{-0.00s (- 0.20%)}	_1.48s	_1.52s
_{Bind Time}	_{0.76s (± 0.45%)}	_{0.76s (± 0.68%)}	_{+0.00s (+ 0.26%)}	_0.75s	_0.77s
_{Check Time}	_{5.55s (± 0.74%)}	_{5.55s (± 0.68%)}	_{+0.00s (+ 0.02%)}	_5.43s	_5.64s
_{Emit Time}	_{3.20s (± 1.01%)}	_{3.23s (± 1.28%)}	_{+0.03s (+ 0.87%)}	_3.14s	_3.35s
_{Total Time}	_{11.00s (± 0.41%)}	_{11.04s (± 0.49%)}	_{+0.03s (+ 0.29%)}	_10.89s	_11.13s
_{TFS - node (v10.16.3, x64)}
_{Memory used}	_{305,702k (± 0.02%)}	_{305,684k (± 0.04%)}	_{-19k (- 0.01%)}	_305,492k	_306,094k
_{Parse Time}	_{1.20s (± 0.77%)}	_{1.21s (± 0.37%)}	_{+0.01s (+ 0.50%)}	_1.20s	_1.22s
_{Bind Time}	_{0.71s (± 0.96%)}	_{0.72s (± 1.03%)}	_{+0.01s (+ 1.12%)}	_0.70s	_0.73s
_{Check Time}	_{5.04s (± 0.74%)}	_{5.07s (± 0.27%)}	_{+0.03s (+ 0.60%)}	_5.05s	_5.10s
_{Emit Time}	_{3.38s (± 0.91%)}	_{3.40s (± 1.42%)}	_{+0.02s (+ 0.50%)}	_3.31s	_3.48s
_{Total Time}	_{10.34s (± 0.45%)}	_{10.39s (± 0.52%)}	_{+0.06s (+ 0.57%)}	_10.31s	_10.51s
_{material-ui - node (v10.16.3, x64)}
_{Memory used}	_{471,728k (± 0.01%)}	_{471,756k (± 0.01%)}	_{+28k (+ 0.01%)}	_471,696k	_471,859k
_{Parse Time}	_{1.78s (± 0.53%)}	_{1.78s (± 0.50%)}	_{+0.00s (+ 0.11%)}	_1.77s	_1.80s
_{Bind Time}	_{0.67s (± 1.43%)}	_{0.66s (± 1.59%)}	_{-0.00s (- 0.30%)}	_0.65s	_0.69s
_{Check Time}	_{14.25s (± 0.52%)}	_{14.29s (± 0.61%)}	_{+0.05s (+ 0.34%)}	_14.13s	_14.48s
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_{0.00s ( NaN%)}	_0.00s	_0.00s
_{Total Time}	_{16.70s (± 0.46%)}	_{16.74s (± 0.58%)}	_{+0.05s (+ 0.28%)}	_16.56s	_16.94s
_{xstate - node (v10.16.3, x64)}
_{Memory used}	_{577,149k (± 1.84%)}	_{570,076k (± 0.02%)}	_{-7,073k (- 1.23%)}	_569,877k	_570,320k
_{Parse Time}	_{2.56s (± 0.37%)}	_{2.56s (± 0.39%)}	_{-0.00s (- 0.00%)}	_2.54s	_2.58s
_{Bind Time}	_{1.01s (± 0.36%)}	_{1.02s (± 0.80%)}	_{+0.01s (+ 0.59%)}	_1.00s	_1.04s
_{Check Time}	_{1.51s (± 0.63%)}	_{1.50s (± 0.44%)}	_{-0.00s (- 0.13%)}	_1.49s	_1.52s
_{Emit Time}	_{0.07s (± 0.00%)}	_{0.07s (± 0.00%)}	_{0.00s ( 0.00%)}	_0.07s	_0.07s
_{Total Time}	_{5.14s (± 0.30%)}	_{5.15s (± 0.18%)}	_{+0.01s (+ 0.16%)}	_5.12s	_5.16s
_{Angular - node (v12.1.0, x64)}
_{Memory used}	_{333,741k (± 0.02%)}	_{333,720k (± 0.02%)}	_{-21k (- 0.01%)}	_333,508k	_333,825k
_{Parse Time}	_{1.95s (± 0.65%)}	_{1.96s (± 0.70%)}	_{+0.00s (+ 0.26%)}	_1.93s	_1.98s
_{Bind Time}	_{0.84s (± 0.59%)}	_{0.84s (± 0.90%)}	_{-0.00s (- 0.12%)}	_0.83s	_0.86s
_{Check Time}	_{5.38s (± 0.41%)}	_{5.39s (± 0.42%)}	_{+0.00s (+ 0.06%)}	_5.34s	_5.45s
_{Emit Time}	_{6.16s (± 0.56%)}	_{6.13s (± 0.34%)}	_{-0.04s (- 0.58%)}	_6.08s	_6.19s
_{Total Time}	_{14.35s (± 0.28%)}	_{14.32s (± 0.27%)}	_{-0.03s (- 0.20%)}	_14.22s	_14.39s
_{Compiler-Unions - node (v12.1.0, x64)}
_{Memory used}	_{191,739k (± 0.07%)}	_{191,599k (± 0.14%)}	_{-140k (- 0.07%)}	_190,750k	_191,981k
_{Parse Time}	_{0.78s (± 0.79%)}	_{0.78s (± 0.61%)}	_{+0.00s (+ 0.51%)}	_0.77s	_0.79s
_{Bind Time}	_{0.53s (± 0.63%)}	_{0.53s (± 1.12%)}	_{+0.00s (+ 0.94%)}	_0.52s	_0.55s
_{Check Time}	_{7.31s (± 0.56%)}	_{7.40s (± 0.32%)}	_{+0.09s (+ 1.19%)}	_7.34s	_7.44s
_{Emit Time}	_{2.49s (± 0.99%)}	_{2.48s (± 0.92%)}	_{-0.02s (- 0.60%)}	_2.42s	_2.51s
_{Total Time}	_{11.11s (± 0.44%)}	_{11.19s (± 0.30%)}	_{+0.08s (+ 0.76%)}	_11.11s	_11.26s
_{Monaco - node (v12.1.0, x64)}
_{Memory used}	_{325,619k (± 0.03%)}	_{325,615k (± 0.04%)}	_{-4k (- 0.00%)}	_325,411k	_326,080k
_{Parse Time}	_{1.48s (± 0.46%)}	_{1.48s (± 1.05%)}	_{+0.01s (+ 0.61%)}	_1.46s	_1.52s
_{Bind Time}	_{0.74s (± 0.63%)}	_{0.74s (± 0.70%)}	_{+0.00s (+ 0.14%)}	_0.73s	_0.75s
_{Check Time}	_{5.40s (± 0.48%)}	_{5.41s (± 0.42%)}	_{+0.00s (+ 0.09%)}	_5.35s	_5.46s
_{Emit Time}	_{3.22s (± 0.52%)}	_{3.25s (± 0.71%)}	_{+0.03s (+ 1.06%)}	_3.20s	_3.29s
_{Total Time}	_{10.83s (± 0.36%)}	_{10.88s (± 0.39%)}	_{+0.05s (+ 0.47%)}	_10.77s	_10.99s
_{TFS - node (v12.1.0, x64)}
_{Memory used}	_{290,345k (± 0.02%)}	_{290,386k (± 0.01%)}	_{+41k (+ 0.01%)}	_290,310k	_290,498k
_{Parse Time}	_{1.21s (± 0.71%)}	_{1.23s (± 0.97%)}	_{+0.02s (+ 1.32%)}	_1.20s	_1.26s
_{Bind Time}	_{0.70s (± 0.71%)}	_{0.70s (± 0.83%)}	_{+0.01s (+ 1.01%)}	_0.69s	_0.71s
_{Check Time}	_{4.99s (± 0.54%)}	_{4.97s (± 0.61%)}	_{-0.02s (- 0.40%)}	_4.92s	_5.03s
_{Emit Time}	_{3.42s (± 0.68%)}	_{3.45s (± 0.88%)}	_{+0.02s (+ 0.64%)}	_3.40s	_3.54s
_{Total Time}	_{10.32s (± 0.29%)}	_{10.34s (± 0.48%)}	_{+0.02s (+ 0.23%)}	_10.24s	_10.49s
_{material-ui - node (v12.1.0, x64)}
_{Memory used}	_{450,445k (± 0.01%)}	_{450,372k (± 0.06%)}	_{-73k (- 0.02%)}	_449,238k	_450,585k
_{Parse Time}	_{1.78s (± 0.37%)}	_{1.79s (± 0.43%)}	_{+0.01s (+ 0.51%)}	_1.77s	_1.81s
_{Bind Time}	_{0.64s (± 0.53%)}	_{0.65s (± 1.08%)}	_{+0.01s (+ 0.93%)}	_0.64s	_0.67s
_{Check Time}	_{12.81s (± 0.81%)}	_{12.89s (± 0.79%)}	_{+0.08s (+ 0.62%)}	_12.64s	_13.17s
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_{0.00s ( NaN%)}	_0.00s	_0.00s
_{Total Time}	_{15.24s (± 0.68%)}	_{15.33s (± 0.63%)}	_{+0.09s (+ 0.62%)}	_15.08s	_15.59s
_{xstate - node (v12.1.0, x64)}
_{Memory used}	_{539,525k (± 1.44%)}	_{536,032k (± 0.02%)}	_{-3,493k (- 0.65%)}	_535,897k	_536,324k
_{Parse Time}	_{2.49s (± 0.64%)}	_{2.49s (± 0.55%)}	_{0.00s ( 0.00%)}	_2.46s	_2.53s
_{Bind Time}	_{1.05s (± 0.67%)}	_{1.05s (± 0.89%)}	_{+0.00s (+ 0.19%)}	_1.04s	_1.08s
_{Check Time}	_{1.44s (± 0.89%)}	_{1.45s (± 0.70%)}	_{+0.00s (+ 0.21%)}	_1.42s	_1.47s
_{Emit Time}	_{0.07s (± 0.00%)}	_{0.07s (± 0.00%)}	_{0.00s ( 0.00%)}	_0.07s	_0.07s
_{Total Time}	_{5.06s (± 0.42%)}	_{5.06s (± 0.30%)}	_{-0.00s (- 0.02%)}	_5.01s	_5.08s
_{Angular - node (v14.15.1, x64)}
_{Memory used}	_{332,065k (± 0.00%)}	_{332,092k (± 0.00%)}	_{+27k (+ 0.01%)}	_332,052k	_332,132k
_{Parse Time}	_{1.96s (± 0.56%)}	_{1.96s (± 0.60%)}	_{-0.00s (- 0.05%)}	_1.93s	_1.98s
_{Bind Time}	_{0.88s (± 0.88%)}	_{0.88s (± 0.85%)}	_{+0.00s (+ 0.23%)}	_0.87s	_0.90s
_{Check Time}	_{5.43s (± 0.48%)}	_{5.42s (± 0.60%)}	_{-0.00s (- 0.06%)}	_5.36s	_5.50s
_{Emit Time}	_{6.22s (± 0.67%)}	_{6.23s (± 0.60%)}	_{+0.01s (+ 0.16%)}	_6.15s	_6.29s
_{Total Time}	_{14.48s (± 0.43%)}	_{14.49s (± 0.40%)}	_{+0.01s (+ 0.07%)}	_14.36s	_14.61s
_{Compiler-Unions - node (v14.15.1, x64)}
_{Memory used}	_{193,280k (± 0.37%)}	_{192,547k (± 0.63%)}	_{-734k (- 0.38%)}	_189,417k	_193,714k
_{Parse Time}	_{0.81s (± 0.71%)}	_{0.81s (± 0.76%)}	_{-0.00s (- 0.12%)}	_0.80s	_0.83s
_{Bind Time}	_{0.56s (± 0.60%)}	_{0.56s (± 0.53%)}	_{-0.00s (- 0.89%)}	_0.55s	_0.56s
_{Check Time}	_{7.46s (± 0.66%)}	_{7.48s (± 0.94%)}	_{+0.02s (+ 0.29%)}	_7.38s	_7.68s
_{Emit Time}	_{2.47s (± 0.34%)}	_{2.49s (± 1.15%)}	_{+0.03s (+ 1.09%)}	_2.44s	_2.58s
_{Total Time}	_{11.29s (± 0.46%)}	_{11.34s (± 0.77%)}	_{+0.04s (+ 0.39%)}	_11.21s	_11.63s
_{Monaco - node (v14.15.1, x64)}
_{Memory used}	_{324,443k (± 0.00%)}	_{324,440k (± 0.01%)}	_{-3k (- 0.00%)}	_324,396k	_324,473k
_{Parse Time}	_{1.51s (± 0.51%)}	_{1.51s (± 0.75%)}	_{+0.00s (+ 0.07%)}	_1.49s	_1.54s
_{Bind Time}	_{0.77s (± 0.75%)}	_{0.77s (± 0.86%)}	_{-0.00s (- 0.00%)}	_0.76s	_0.79s
_{Check Time}	_{5.36s (± 0.85%)}	_{5.38s (± 0.42%)}	_{+0.02s (+ 0.39%)}	_5.32s	_5.41s
_{Emit Time}	_{3.28s (± 0.75%)}	_{3.30s (± 0.69%)}	_{+0.02s (+ 0.67%)}	_3.26s	_3.34s
_{Total Time}	_{10.92s (± 0.56%)}	_{10.96s (± 0.39%)}	_{+0.05s (+ 0.42%)}	_10.88s	_11.05s
_{TFS - node (v14.15.1, x64)}
_{Memory used}	_{289,220k (± 0.00%)}	_{289,222k (± 0.01%)}	_{+2k (+ 0.00%)}	_289,146k	_289,261k
_{Parse Time}	_{1.24s (± 0.90%)}	_{1.23s (± 0.60%)}	_{-0.01s (- 0.49%)}	_1.21s	_1.24s
_{Bind Time}	_{0.75s (± 1.19%)}	_{0.74s (± 0.64%)}	_{-0.00s (- 0.40%)}	_0.74s	_0.76s
_{Check Time}	_{4.97s (± 0.46%)}	_{4.99s (± 0.63%)}	_{+0.02s (+ 0.36%)}	_4.93s	_5.05s
_{Emit Time}	_{3.55s (± 0.72%)}	_{3.56s (± 0.50%)}	_{+0.01s (+ 0.39%)}	_3.51s	_3.60s
_{Total Time}	_{10.50s (± 0.49%)}	_{10.52s (± 0.42%)}	_{+0.02s (+ 0.23%)}	_10.39s	_10.60s
_{material-ui - node (v14.15.1, x64)}
_{Memory used}	_{448,666k (± 0.00%)}	_{448,541k (± 0.06%)}	_{-125k (- 0.03%)}	_447,459k	_448,698k
_{Parse Time}	_{1.84s (± 0.67%)}	_{1.84s (± 0.76%)}	_{0.00s ( 0.00%)}	_1.82s	_1.88s
_{Bind Time}	_{0.68s (± 0.54%)}	_{0.69s (± 0.84%)}	_{+0.01s (+ 1.17%)}	_0.68s	_0.70s
_{Check Time}	_{12.88s (± 0.68%)}	_{12.90s (± 0.79%)}	_{+0.03s (+ 0.22%)}	_12.67s	_13.08s
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_{0.00s ( NaN%)}	_0.00s	_0.00s
_{Total Time}	_{15.40s (± 0.56%)}	_{15.43s (± 0.70%)}	_{+0.03s (+ 0.21%)}	_15.18s	_15.62s
_{xstate - node (v14.15.1, x64)}
_{Memory used}	_{533,709k (± 0.00%)}	_{533,719k (± 0.01%)}	_{+10k (+ 0.00%)}	_533,669k	_533,793k
_{Parse Time}	_{2.55s (± 0.40%)}	_{2.56s (± 0.57%)}	_{+0.01s (+ 0.39%)}	_2.52s	_2.59s
_{Bind Time}	_{1.17s (± 0.91%)}	_{1.17s (± 0.81%)}	_{+0.01s (+ 0.86%)}	_1.16s	_1.20s
_{Check Time}	_{1.48s (± 0.57%)}	_{1.49s (± 0.59%)}	_{+0.01s (+ 0.61%)}	_1.47s	_1.51s
_{Emit Time}	_{0.07s (± 0.00%)}	_{0.07s (± 0.00%)}	_{0.00s ( 0.00%)}	_0.07s	_0.07s
_{Total Time}	_{5.27s (± 0.25%)}	_{5.30s (± 0.37%)}	_{+0.03s (+ 0.57%)}	_5.26s	_5.33s

System

_{Machine Name}	_ts-ci-ubuntu
_Platform	_{linux 4.4.0-210-generic}
_Architecture	_x64
_{Available Memory}	_{16 GB}
_{Available Memory}	_{7 GB}
_CPUs	_{4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz}

Hosts

node (v10.16.3, x64)
node (v12.1.0, x64)
node (v14.15.1, x64)

Scenarios

Angular - node (v10.16.3, x64)
Angular - node (v12.1.0, x64)
Angular - node (v14.15.1, x64)
Compiler-Unions - node (v10.16.3, x64)
Compiler-Unions - node (v12.1.0, x64)
Compiler-Unions - node (v14.15.1, x64)
Monaco - node (v10.16.3, x64)
Monaco - node (v12.1.0, x64)
Monaco - node (v14.15.1, x64)
TFS - node (v10.16.3, x64)
TFS - node (v12.1.0, x64)
TFS - node (v14.15.1, x64)
material-ui - node (v10.16.3, x64)
material-ui - node (v12.1.0, x64)
material-ui - node (v14.15.1, x64)
xstate - node (v10.16.3, x64)
xstate - node (v12.1.0, x64)
xstate - node (v14.15.1, x64)

_Benchmark	_Name	_Iterations
_Current	₄₇₃₃₇	₁₀
_Baseline	_main	₁₀

Developer Information:

Download Benchmark

gabritto · 2022-01-21T20:01:14Z

@typescript-bot pack this

typescript-bot · 2022-01-21T20:01:17Z

Heya @gabritto, I've started to run the tarball bundle task on this PR at 23c38be. You can monitor the build here.

typescript-bot · 2022-01-21T20:20:17Z

Hey @gabritto, 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/118069/artifacts?artifactName=tgz&fileId=5E7679E65EC7843039E23306BDAFB943423F455FE7823C179CC050430C6167E202&fileName=/typescript-4.6.0-insiders.20220121.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/pr-build@4.6.0-pr-47337-6".;

gabritto · 2022-01-28T20:34:37Z

src/compiler/checker.ts

+        RestBindingElement = 1 << 5,       // Checking a type that is going to be used to determine the type of a rest binding element
+                                           //   e.g. in `const { a, ...rest } = foo`, when checking the type of `foo` to determine the type of `rest`,
+                                           //   we need to preserve generic types instead of substituting them for constraints
+        IncludeOptionality = 1 << 6,       // >> TODO: description, replace bool param with flag


I'll add this flag instead of the includeOptionality parameter in another commit, once we settle on the other changes of this PR

We should definitely get this change in the PR as well.

Since replacing the includeOptionality by a CheckMode flag affects our caching behavior in checkExpressionCached (or we'll have to add an exception for that flag only), I'm gonna merge this without that part for the 4.6 RC, and I can add this change in another PR if we really want this.

RyanCavanaugh · 2022-02-07T21:58:04Z

@ahejlsberg any concerns? 4.6 RC is fast approaching and we'd like to have this in

ahejlsberg

Looks good in general, but would like to see a few changes first.

ahejlsberg · 2022-02-08T13:46:06Z

src/compiler/checker.ts

+        RestBindingElement = 1 << 5,       // Checking a type that is going to be used to determine the type of a rest binding element
+                                           //   e.g. in `const { a, ...rest } = foo`, when checking the type of `foo` to determine the type of `rest`,
+                                           //   we need to preserve generic types instead of substituting them for constraints
+        IncludeOptionality = 1 << 6,       // >> TODO: description, replace bool param with flag


We should definitely get this change in the PR as well.

src/compiler/checker.ts

ahejlsberg · 2022-02-08T14:02:59Z

src/compiler/checker.ts

            const symbol = getSymbolOfNode(node);
-            return symbol && getSymbolLinks(symbol).type || getTypeForVariableLikeDeclaration(node, /*includeOptionality*/ false);
+            return symbol && getSymbolLinks(symbol).type || getTypeForVariableLikeDeclaration(node, /*includeOptionality*/ false, checkMode);


Yeah, we should definitely fold the includeOptionality into CheckMode.

src/compiler/checker.ts

gabritto added 3 commits December 13, 2021 17:32

WIP: pass in checkmode to getNarrowableTypeForReference

e9b9f2d

add tests

a89fa72

Merge branch 'main' into gabritto/issue45762

b4eab71

typescript-bot assigned gabritto Jan 6, 2022

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 6, 2022

gabritto added 4 commits January 6, 2022 16:20

another pass through new check mode argument

789c249

rename new check mode

1675422

only use rest flag for rest elements in objects

e385038

add and update tests

a831ccd

typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 10, 2022

gabritto changed the title ~~Gabritto/issue45762~~ Fix destructuring and narrowing interaction Jan 10, 2022

gabritto added 4 commits January 10, 2022 14:02

Merge branch 'main' into gabritto/issue45762

16c8a2d

change check mode flag name

68276e4

restore package-lock.json

5383454

fix comments

30b7589

gabritto commented Jan 10, 2022

View reviewed changes

gabritto added 2 commits January 10, 2022 14:58

get rid of fourslash tests

28f667d

fix caching in checkExpressionCached when checkMode is not normal

23c38be

gabritto commented Jan 11, 2022

View reviewed changes

gabritto requested review from weswigham and ahejlsberg January 11, 2022 01:32

gabritto marked this pull request as ready for review January 11, 2022 23:16

gabritto added 2 commits January 21, 2022 12:58

Don't distinguish between object and array rest elements

aad8852

Merge branch 'main' into gabritto/issue45762

e8ba399

gabritto commented Jan 28, 2022

View reviewed changes

sandersn added this to Not started in PR Backlog via automation Jan 31, 2022

andrewbranch mentioned this pull request Feb 3, 2022

Type guard of union with conditional type not working since 4.3 #44382

Closed

DanielRosenwasser mentioned this pull request Feb 3, 2022

Design Meeting Notes, 1/21/2022 #47726

Closed

ahejlsberg requested changes Feb 8, 2022

View reviewed changes

PR Backlog automation moved this from Not started to Waiting on author Feb 8, 2022

gabritto added 2 commits February 8, 2022 11:50

get rid of undefined check mode

20bd98c

don't make includeOptionality into checkmode flag

6ba2667

gabritto merged commit 2d0a00d into main Feb 9, 2022

PR Backlog automation moved this from Waiting on author to Done Feb 9, 2022

gabritto deleted the gabritto/issue45762 branch February 9, 2022 20:25

RyanCavanaugh mentioned this pull request Feb 14, 2022

Unable to read properties off object spread in generic function parameter (regression with 4.6.1-rc) #47865

Closed

gabritto mentioned this pull request Feb 15, 2022

Fix incorrect rest element type inside contextually typed parameter #47909

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix destructuring and narrowing interaction #47337

Fix destructuring and narrowing interaction #47337

gabritto commented Jan 6, 2022 •

edited

gabritto Jan 10, 2022

gabritto Jan 11, 2022 •

edited

gabritto Jan 11, 2022

gabritto commented Jan 11, 2022

orta commented Jan 21, 2022

typescript-bot commented Jan 21, 2022 •

edited

typescript-bot commented Jan 21, 2022

gabritto commented Jan 21, 2022

typescript-bot commented Jan 21, 2022 •

edited

typescript-bot commented Jan 21, 2022 •

edited

gabritto Jan 28, 2022

ahejlsberg Feb 8, 2022

gabritto Feb 9, 2022

RyanCavanaugh commented Feb 7, 2022

ahejlsberg left a comment

ahejlsberg Feb 8, 2022

ahejlsberg Feb 8, 2022

		const { kind, ...r1 } = t;
		const r2 = (({ kind, ...rest }) => rest)(t);

Fix destructuring and narrowing interaction #47337

Fix destructuring and narrowing interaction #47337

Conversation

gabritto commented Jan 6, 2022 • edited

Background

Change in 4.4

Issue

Proposed fix

Implementation

Questions

gabritto Jan 10, 2022

Choose a reason for hiding this comment

gabritto Jan 11, 2022 • edited

Choose a reason for hiding this comment

gabritto Jan 11, 2022

Choose a reason for hiding this comment

gabritto commented Jan 11, 2022

orta commented Jan 21, 2022

typescript-bot commented Jan 21, 2022 • edited

typescript-bot commented Jan 21, 2022

gabritto commented Jan 21, 2022

typescript-bot commented Jan 21, 2022 • edited

typescript-bot commented Jan 21, 2022 • edited

gabritto Jan 28, 2022

Choose a reason for hiding this comment

ahejlsberg Feb 8, 2022

Choose a reason for hiding this comment

gabritto Feb 9, 2022

Choose a reason for hiding this comment

RyanCavanaugh commented Feb 7, 2022

ahejlsberg left a comment

Choose a reason for hiding this comment

ahejlsberg Feb 8, 2022

Choose a reason for hiding this comment

ahejlsberg Feb 8, 2022

Choose a reason for hiding this comment

gabritto commented Jan 6, 2022 •

edited

gabritto Jan 11, 2022 •

edited

typescript-bot commented Jan 21, 2022 •

edited

typescript-bot commented Jan 21, 2022 •

edited

typescript-bot commented Jan 21, 2022 •

edited