Skip to content

Type guard not applied based on indexed access with string literal #26232

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

Closed
dalen opened this issue Aug 6, 2018 · 6 comments
Closed

Type guard not applied based on indexed access with string literal #26232

dalen opened this issue Aug 6, 2018 · 6 comments
Assignees
Labels
Add a Flag Any problem can be solved by flags, except for the problem of having too many flags Bug A bug in TypeScript

Comments

@dalen
Copy link

dalen commented Aug 6, 2018

TypeScript Version: 3.1.0-dev.20180804

Search Terms:
dash, bracket

Code

Very similar to documentation example code, but with a discriminant containing a dash. Likely not because of the dash itself, but because the lookup is using s['kind-of'] instead of s.kind.

interface Square {
  'kind-of': 'square';
  size: number;
}

interface Rectangle {
  'kind-of': 'rectangle';
  width: number;
  height: number;
}
type Shape = Square | Rectangle;

function area(s: Shape): number {
  if (s['kind-of'] === 'square') {
    // Now TypeScript *knows* that `s` must be a square ;)
    // So you can use its members safely :)
    return s.size * s.size;
  } else {
    // Wasn't a square? So TypeScript will figure out that it must be a Rectangle ;)
    // So you can use its members safely :)
    return s.width * s.height;
  }
}

Expected behavior:

No errors

Actual behavior:

test.ts:17:14 - error TS2339: Property 'size' does not exist on type 'Shape'.
  Property 'size' does not exist on type 'Rectangle'.

17     return s.size * s.size;
                ~~~~

test.ts:17:23 - error TS2339: Property 'size' does not exist on type 'Shape'.
  Property 'size' does not exist on type 'Rectangle'.

17     return s.size * s.size;
                         ~~~~

test.ts:21:14 - error TS2339: Property 'width' does not exist on type 'Shape'.
  Property 'width' does not exist on type 'Square'.

21     return s.width * s.height;
                ~~~~~

test.ts:21:24 - error TS2339: Property 'height' does not exist on type 'Shape'.
  Property 'height' does not exist on type 'Square'.

21     return s.width * s.height;
                          ~~~~~~

My real world use case is to check type of AWS Cloudwatch event using the detail-type field on the event, so can't just change the name to get around the restriction easily.

@RyanCavanaugh
Copy link
Member

It's the use of bracket notation (x["a"]) rather than dot (x.a) that causes this

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.1 milestone Aug 6, 2018
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 6, 2018
@RyanCavanaugh RyanCavanaugh changed the title Discriminated unions not working when discriminant contains dash Type guard not applied based on indexed access with string literal Aug 6, 2018
@ahejlsberg
Copy link
Member

@sandersn already looked at this, I think there is a PR still open. The issue is that supporting indexed access (x["foo"]) as well as property access (x.foo) in control flow analysis has significant negative performance impact because we need to include many more nodes (i.e. all indexed access operations) in the control flow graph.

@sandersn
Copy link
Member

sandersn commented Aug 6, 2018

It was #10565. It might not be too hard to revive this PR and see how bad the slowdown is now, but I doubt it will be much different.

@RyanCavanaugh
Copy link
Member

I'm surprised a syntactic check could be that expensive. There doesn't seem to be much going on in that PR that wasn't happening before and the number of indexed-by-string-literal property accesses in a given program shouldn't be that large?

@benneq
Copy link

benneq commented Aug 13, 2018

Could you at least add this feature with some feature flag in tsconfig?

I don't care if compilation becomes slower. But I do care about type errors. That's what this language is all about.

@RyanCavanaugh RyanCavanaugh added the Add a Flag Any problem can be solved by flags, except for the problem of having too many flags label Aug 13, 2018
@sandersn
Copy link
Member

This should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add a Flag Any problem can be solved by flags, except for the problem of having too many flags Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants