Discriminant property type guard not applied with bracket notation

[jvilk]

TypeScript Version: 2.0.0

Code

interface Square {
    kind: "square";
    size: number;
}

interface Rectangle {
    kind: "rectangle";
    width: number;
    height: number;
}

interface Circle {
    kind: "circle";
    radius: number;
}

type Shape = Square | Rectangle | Circle;

function area(s: Shape) {
    // In the following switch statement, the type of s is narrowed in each case clause
    // according to the value of the discriminant property, thus allowing the other properties
    // of that variant to be accessed without a type assertion.
    switch (s['kind']) {
        case "square": return s.size * s.size;
        case "rectangle": return s.width * s.height;
        case "circle": return Math.PI * s.radius * s.radius;
    }
}

Expected behavior:

The code compiles without errors.

Actual behavior:

sample.ts(24,33): error TS2339: Property 'size' does not exist on type 'Square | Rectangle | Circle'.
sample.ts(24,42): error TS2339: Property 'size' does not exist on type 'Square | Rectangle | Circle'.
sample.ts(25,36): error TS2339: Property 'width' does not exist on type 'Square | Rectangle | Circle'.
sample.ts(25,46): error TS2339: Property 'height' does not exist on type 'Square | Rectangle | Circle'.
sample.ts(26,43): error TS2339: Property 'radius' does not exist on type 'Square | Rectangle | Circle'.
sample.ts(26,54): error TS2339: Property 'radius' does not exist on type 'Square | Rectangle | Circle'.

Why this is bad:

I am trying to work with Dropbox's new 2.0 SDK, which heavily uses tagged union types (especially for API errors). The discriminant property is named .tag, so it can only be accessed via bracket notation. I generated TypeScript typings for their new JavaScript SDK, and discovered this bug the hard way. :(

[RyanCavanaugh]

:+1: Bracketed property access should work the same as dotted property access for the purposes of type guards when the indexed name matches a known property

[sandersn]

Fix is up at #10565

[sandersn]

Update from the PR at #10530: narrowing on element access, even just for strings and not numbers, adds 6.5% time to the checker, so this is not likely to go into 2.1 unless we come up with a way to reduce its cost.

[jvilk]

@sandersn thanks for the update. That's unfortunate, but understandable. :(

[GoToLoop]

... when the indexed name matches a known property.

Then I can rest assured that we can still continue to use unmatched index names? So we can still hack the type by adding new properties using the [] syntax, right? :fearful:

[xnnkmd]

@sandersn Can something else be done about this ? It is a really annoying issue which is evident from the large number of referenced bugs.

[michaeljota]

Does still adds only 6.5% at checking? Cause, I know it's a lot, but this is a must have when working in strict mode. It wouldn't compile the code otherwise.

[the-ress]

This also negatively affects usability of the new optional tuple elements:

function foo(bar: [number, string, Function?]) {
    bar[2] && bar[2](); // Cannot invoke an object which is possibly 'undefined'.
}

[DanielRosenwasser]

Keywords: narrow narrowing element access index indexer elementaccessexpression cfa control flow analysis

[DavidGriswoldTeacher]

I'm having an issue that I think is related to this, but i'm not certain but I don't want to open a new bug report if it is.

Todo is a subclass of Note.Todo contains a toggleComplete() method and Note does not.

let notes:Note[] = [new Todo("todo")];
if (notes[0] instanceof Todo) {
   notes[0].toggleComplete(); 
   //should work. Instead, gives an error that toggleComplete does not exist on type Note.
}

It works fine if I assign notes[0] to a variable first with let n = notes[0] but that's an annoying workaround that doesn't transition well to indexed for loops (I'm teaching new programmers and don't want to add for..of to the mix).

Is this issue the same as this simple problem? I was told on stackoverflow that it was, but it feels like a simpler problem than the one being discussed here and in the various duplicates, since my issue is limited to arrays with numeric literal indices.

[dgieselaar]

TS doesn't narrow the type if I use a variable as index instead of a string literal, even if the variable has the same type as the string literal (e.g., both have the type of destination.address). Here's a REPL: https://www.typescriptlang.org/play/index.html?ssl=1&ssc=1&pln=21&pc=2#code/MYewdgzgLgBAIgUQMoBUCSA5Agug8hgfSzjgCVkkYBeGAcgBMBTaASzAEMoXwA6d++gCdmEWgG4AUKEiwkCUgDU0AYQQFsAWQTU6ERoIBuLYIx4cAto3ESJUAJ4AHRjAwgmAIXZ6dAbxgt6AC4YaEE2AHMYAF9JeycYNDAofQ4AG1cmHQzGT28AMhg-AG05RRU1TQQAXWDQiOjYx2cEAA9kwTTsrLccr2cC4sRUTBw0fCISciQkGpCoMLBImNsmmGVwMEZgLnAumkT2zp6YAB8YVsP2dJ7JKXBoGHDGKDR6HQAKMB7g9bBN7e4YGyAEpChIYBD-AAzGDvIbobB4QjEMgUfxgGBfJignzgyH44RQACuHUxPSK8JGSImqOmVUk+KieIhhJJGKxjBK8iUqnUWC09IkTLuMkez1eACYPhyfhstjsgT0cczobCGMwuBwFXwBMIIKJ0WTsWD8QTnmyjZzKYixsjJhRBYyVazSRyuWVeZVBVEgA

[PurpleMagick]

I also found out that variables in bracket notation seem to not work in type guards. Very minimal reproduction:

const obj: { prop: string | null }  = { prop: "hello" };

if (typeof obj.prop === "string")
    obj.prop.length; //OK

if (typeof obj["prop"] === "string")
    obj["prop"].length; //OK

const key = "prop" as const;
if (typeof obj[key] === "string")
    obj[key].length; //error - object is possibly 'null'

Playground Link

It seems like using hardcoded values in bracket notation works, which is a fix made in https://github.com/microsoft/TypeScript/issues/28081 however, using a variable doesn't offer the same behaviour, even if that variable is a const. So, it seems inconsistent. I'd understand if the variable was mutable or maybe even an object (perhaps its toString() doesn't return the same thing every time) but for an immutable string variable, this should work the same as a hard-coded value.

[MatAtBread]

I have a very similar question, in the following example lookup[n] doesn't narrow, but a const P = lookup[n] does, with no intervening yield/await/call side-effects.

Is this the same issue?

declare const lookup: { [P in string]?: string  }

function f(n: string) {
    const P = lookup[n] ;   // typeof P = string | undefined (correct)
    if (lookup[n]) {
        const Q = lookup[n] // typeof Q = string | undefined (huh?)
    }
    if (P) {
        const R = P ;       // typeof R = string (correct)
    }
}

I note from other issues that the case where the expression in the square-bracket is a literal has been fixed (eg https://github.com/microsoft/TypeScript/pull/31478), but this is not the same case. This is the same as @PurpleMagick 's above: the case where the expression within the brackets is invariant between accesses. This will be true in JS as long as there is no intervening yield, await or function call and all the accesses are to locally defined variables (let, const, var, parameter)

The reason is for this request is the very common "test and access" case

[cruhl]

I'm also running into this while trying to safely access Redux Reducers via their type property.

[Danielku15]

I have here another case using the feature where a function does the type check.

Playground link

enum StatementKind { Block }
interface Statement { kind: StatementKind }
interface Block extends Statement { statements:Statement[] }

function isBlock(n: Statement): n is Block { return n.kind === StatementKind.Block }

function handleStatements(statements: Statement[]) {
    if (isBlock(statements[0])) {
        statements = statements[0].statements;
    }
}

function handleStatement(statement: Statement) {
    if (isBlock(statement)) {
        statement = statement.statements[0];
    }
}

On the element access expression of the first function the type is not narrowed.

[marcin-wlodarczyk]

I have a problem with bracket notation and User-Defined Type Guards

Simplified problem reproduction:

class OpenPeriod {
    from: string;

    public isOpen(): this is OpenPeriod {
        return true;
    }

    public isClosed(): this is ClosedPeriod {
        return false;
    }
}

class ClosedPeriod {
    from: string;
    to: string;

    public isOpen(): this is OpenPeriod {
        return false;
    }

    public isClosed(): this is ClosedPeriod {
        return true;
    }
}

class Connection {
    period: OpenPeriod | ClosedPeriod;

    public static getOverlap(a: ClosedPeriod): void {}
}

const a = new Connection();

if(a.period.isClosed()) {
    Connection.getOverlap(a.period); // OK
}

const arr = [a];

if(arr[0].period.isClosed()) {
    Connection.getOverlap(arr[0].period); //  OK
}

for(let i = 0; i < arr.length; i++) {
    if(arr[i].period.isClosed()) {
        Connection.getOverlap(arr[i].period); // error TS2345: Argument of type 'OpenPeriod | ClosedPeriod' is not assignable to parameter of type 'ClosedPeriod'
    }
}

$ tsc --version
Version 4.4.2

[MatAtBread]

I believe this is common case of the type guard not working on array expressions (and maybe some other computed references)

Try assigning the array element to a local variable and see if it is correctly narrowed

[marcin-wlodarczyk]

I believe this is common case of the type guard not working on array expressions (and maybe some other computed references)

Try assigning the array element to a local variable and see if it is correctly narrowed

Yes, it worked. Thanks! However, still I believe that array expressions should be fixed.

for(let i = 0; i < arr.length; i++) {
    const tmp = arr[i];
    if(tmp.period.isClosed()) {
        Connection.getOverlap(tmp.period); // OK
    }
}

[MatAtBread]

Me too! But it's very long standing. I believe the issue is doing the control flow analysis to ensure than the array index expression remains constant under all circumstances

[easyrider]

Is anybody working on that? Is this just a bug or a TS design flaw? Any ideas/hints how it can be fixed? Do you need any help on that?

[MatAtBread]

I'm not involved in TS development, so I've not really investigated. I think it's a limitation rather than by design. Some time ago I did try to pin it down, and expressions that are simple identifiers or objects dereferenced with a . seem to work pretty much everywhere (except closures....different issue), but square brackets and computed expressions (probably logically the same thing) don't.

I'd guess the compiler's table of identifiers and their current type (ie narrower than the declared type) doesn't include expressions, which is potentially a very large set.

[MatAtBread]

I think my guess may be correct: https://github.com/microsoft/TypeScript/issues/11483#issuecomment-257436774

[easyrider]

@MatAtBread, I'm not sure if it is the same issue - here TS throws at a property not at an array element:

// A: an array index
 if (objectsArray[index].prop) objectsArray[index].prop++; 
// ERR: Object is possibly 'undefined'.ts(2532)

// type checking doesn't help here:
if (typeof objectsArray[index].prop === 'number') objectsArray[index].prop++;

// however this works without a type check when no incrementing:
personas[i].failures;

// B: using local var
 let local = objectsArray[index];
 if (local.prop) local.prop++;           // this works

where the prop is defined as:

failures?: number

[MatAtBread]

Yes, I think that's the same case. Computed expressions like objectsArray[index].prop aren't narrowed, but simple ones like local.prop are.

[sandersn]

@easyrider Here's the relevant comment from 2016: https://github.com/microsoft/TypeScript/issues/10530#issuecomment-253651206

[easyrider]

@easyrider Here's the relevant comment from 2016: #10530 (comment)

Thank you @sandersn, what time impact is acceptable (6.5% was in 2016 - now it may be marginal)? Maybe that extra check can be used behind a flag (looks like it was already suggested in 2018: https://github.com/microsoft/TypeScript/pull/10565#issuecomment-397276283)?

@sandersn, did you investigate more after reconstituting the PR in 2018 as mentioned in: https://github.com/microsoft/TypeScript/pull/10565#issuecomment-412657164?

[easyrider]

FYI: I've just created a PR that addresses this issue: https://github.com/microsoft/TypeScript/pull/46121 Help needed with performance testing.

[a-tarasyuk]

There is experimental PR which allows using identifiers (string/number/symbol) in element access expressions with the Perf results

[KunalTanwar]

Occurring when using generic objects

function attrs(element: HTMLElement, attrs: { [key: string]: string | number }): void {
  for (let attr in attrs) {
    element.setAttribute(attr, typeof attrs[attr] === 'number' ? attrs[attr].toString() : attrs[attr])
  }
}

The error I am getting is :

Argument of type 'string | number' is not assignable to parameter of type 'string'.
Type 'number' is not assignable to type 'string'.

[DetachHead]

the original example from this issue seems to have been fixed at some point between typescript 2.0 and 3.3.3333

playground

the other examples in this thread seem to be slightly different, perhaps the original issue should be updated with a more up to date example or closed in favor of a different issue (such as #51368)?

[Andarist]

@RyanCavanaugh, as @DetachHead mentioned - the original example from this issue has been fixed a long time ago. A lot of other examples posted here are also working just fine today. I would propose closing this issue in favor of other ones that are reporting more specific things.

[jcalz]

If we're not going to close this, could someone edit the title and description to reflect the current purpose of the issue? Right now it's just generating noise in every issue closed as a duplicate of this one.

[MicahZoltu]

As others have mentioned, this issue title/description isn't particularly clear, but from the issues marked as duplicate I'm guessing this issue is the same root cause as the following?

declare const apple: { [key: string]: boolean | undefined }
const banana: Record<string, boolean> = apple // error
// Type '{ [key: string]: boolean | undefined; }' is not assignable to type 'Record<string, boolean>'.
//   'string' index signatures are incompatible.
//     Type 'boolean | undefined' is not assignable to type 'boolean'.
//       Type 'undefined' is not assignable to type 'boolean'.(2322)

[joshkel]

@MicahZoltu I don't think that's the same issue, and I don't think the example that you shared would be considered an issue in TypeScript. Although a Record<string, boolean> and a Record<string, boolean | undefined> act very similarly, they differ in whether TS allows undefined to be explicitly assigned to properties, and they're consistently different in how TS does type checks of property values. See this playground link.

[RyanCavanaugh]

Closing since the examples cited here work as expected

[DetachHead]

@RyanCavanaugh in that case can any of the issues that were closed as a duplicate of this one be re-opened? Eg. #51368

[RyanCavanaugh]

@DetachHead let's just get a fresh issue with a clear statement of the problem for clarity

[jcalz]

Well, I opened #56389... not sure if it covers all the loose ends, though.

[phil294]

To anybody following this issue and confused like me, also the more complicated topic of variable index access type guards was fixed this June by ahejlsberg himself with #57847 in TS 5.5 :)

[DetachHead]

the issue i raised (#51368) which was closed as a duplicate of this one still doesn't work:

interface Data {
    a?: number
}

declare const data: Data

declare let key: 'a'

if (data.a !== undefined) {
    key // "a"
    const a = data[key] // number | undefined
    const b = data['a'] // number
}

playground

the difference being that key is a let instead of a const. in #57847 it mentions that it should work on both const and let:

With this PR we perform control flow analysis for element access expressions obj[key] where key is a const variable, or a let variable or parameter that is never targeted in an assignment.

or is there something i'm missing?

[jcalz]

You're missing that the let is in the global scope and so other files might affect it, see https://github.com/microsoft/TypeScript/issues/58972#issuecomment-2184919765

[DetachHead]

i see, thanks. it also seems like attempting it to narrow it with data.a then accessing it with data[key] also doesn't work, so i had to change that too:

+ export {}
+   
  interface Data {
      a?: number
  }

  declare const data: Data

  declare let key: 'a'

- if (data.a !== undefined) {
+ if (data[key] !== undefined) {
      key // "a"
      const a = data[key] // number | undefined
      const b = data['a'] // number
  }