Skip to content

Check keys in code flow analysis #10515

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
donaldpipowitch opened this issue Aug 24, 2016 · 4 comments
Closed

Check keys in code flow analysis #10515

donaldpipowitch opened this issue Aug 24, 2016 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Aug 24, 2016

TypeScript Version: playground / 2.1.0-dev

Code

interface Foo {
  bar: string;
}

interface CoolFoo extends Foo {
  baz: string;
}

interface OtherFoo extends Foo {
  zip: string;
}

type SomeFoo = CoolFoo | OtherFoo;

function run(foo: SomeFoo): number {
  if ('baz' in foo) {
    return foo.baz.length;  // error
  } else if ('zip' in foo) {
    return foo.zip.length;  // error
  }
  throw 'Wrong foo!';
}

Expected behavior:

Checking keys should be sufficient to check if foo is either CoolFoo or OtherFoo.

In contrast checking by value works (at least in 2.1.0-dev, not playground):

interface Foo {
  bar: string;
}

interface CoolFoo extends Foo {
  type: 'cool';
  baz: string;
}

interface OtherFoo extends Foo {
  type: 'other';
  zip: string;
}

type SomeFoo = CoolFoo | OtherFoo;

function run(foo: SomeFoo): number {
  if (foo.type === 'cool') {
    return foo.baz.length;  // error
  } else if (foo.type === 'other') {
    return foo.zip.length;  // error
  }
  throw 'Wrong foo!';
}

But even with 2.1.0-dev I cannot check for existence by value. This does not work:

function run(foo: SomeFoo): number {
  if (foo.baz !== undefined) {  // error
    return foo.baz.length;  // error
  } else if (foo.zip !== undefined) {  // error
    return foo.zip.length;  // error
  }
  throw 'Wrong foo!';
}

Not even that:

function run(foo: SomeFoo): number {
  if (typeof foo.baz === 'string') {  // error
    return foo.baz.length;  // error
  } else if (typeof foo.zip === 'string') {  // error
    return foo.zip.length;  // error
  }
  throw 'Wrong foo!';
}

Actual behavior:

Checking keys is not enough to check if foo is either CoolFoo or OtherFoo. It doesn't compile, because TypeScripts treats foo as CoolFoo | OtherFoo even after checking keys.

@DanielRosenwasser
Copy link
Member

Looks like a duplicate of the recently-filed #10515.

@DanielRosenwasser DanielRosenwasser added the Duplicate An existing issue was already created label Aug 24, 2016
@jesseschalken
Copy link
Contributor

@DanielRosenwasser #10515 is this issue.

@kitsonk
Copy link
Contributor

kitsonk commented Aug 24, 2016

There are a few things here.

First, playground is 1.8.0 at the moment, therefore does not have any code flow analysis, nor will 1.8.0 ever.

Second, checking the keys is not a sufficient guard usually:

const foo = { baz: undefined };

if ('baz' in foo) {
    foo.baz.length; // Ooops!
}

Second, when narrowing types, there are limits to the abilities for TypeScript to figure things out. Given you have two types, you should use a custom type guard to narrow it, versus relying upon other aspects. That way your type guard is attesting to the shape, instead of TypeScript having to try to track all the potential differences that would make it work:

interface Foo {
  bar: string;
}

interface CoolFoo extends Foo {
  type: 'cool';
  baz: string;
}

interface OtherFoo extends Foo {
  type: 'other';
  zip: string;
}

type SomeFoo = CoolFoo | OtherFoo;

function isCoolFoo(value: any): value is CoolFoo {
    return value && value.type === 'cool';
}

function run(foo: SomeFoo): number {
  if (isCoolFoo(foo)) {
    return foo.baz.length;
  } else {
    return foo.zip.length;
  }
}

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 24, 2016

Sorry, too busy trying to track down the root duplicate of #10515. I feel like I've read this thread before.

P.S. I meant it's a duplicate of #10485. 😄

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants