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

instanceof bug with condition #20393

Closed
cevek opened this issue Dec 1, 2017 · 6 comments
Closed

instanceof bug with condition #20393

cevek opened this issue Dec 1, 2017 · 6 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@cevek
Copy link

cevek commented Dec 1, 2017

TypeScript Version: 2.6.1

Code

interface Foo {
    bar: Bar;
}
interface Bar {
    baz: number;
}

function abc(foo: Foo) {
    if (foo.bar instanceof Array) {
        abc(foo.bar[0]); // no error
    }   else {
       abc(foo.bar); // no error
    }
    
}
@jcalz
Copy link
Contributor

jcalz commented Dec 1, 2017

Can you explain why you think this is a bug? I can see why you might be surprised that the narrowing of foo.bar to any[] causes the any element type to devour the whole expression, but that's how any works. Do you want the type guard for instanceof Array to narrow to an array of a top type like Array<{}> instead of to any[]? If so, maybe this is a duplicate of #12085 or some related issue?

In any case you can work around this yourself:

function abc(foo: Foo) {
  // Arr is a specifically constructor for Array<{}>
  const Arr: { new(): Array<{}> } = Array;   
  // foo.bar[0] is now typed as {}, and the expected error appears
  abc(foo.bar instanceof Arr ? foo.bar[0] : foo.bar);  // {} isn't assignable to Foo
}

@cevek
Copy link
Author

cevek commented Dec 1, 2017

I don't follow you, instanceof Array don't narrow to any[]

const a:number[] = [];
if (a instanceof Array) {
    a[0].foo = 1; // error foo doesn't exist on number
}

@jcalz
Copy link
Contributor

jcalz commented Dec 1, 2017

Not if it already knows that it's possibly narrowing an array, I guess. Someone more familiar with the control flow analysis will have to say how that works... maybe #9999 is related?

But in your case, foo.bar is definitely narrowed from Bar to Bar & any[] in the "then" clause of the conditional expression:

narrowed

Are you suggesting that this is a bug? If so, what do you want it to do instead?

@ahejlsberg
Copy link
Member

Here's the behavior I'm seeing:

function abc(foo: Foo) {
  if (foo.bar instanceof Array) {
    abc(foo.bar[0]);  // Ok, type of foo.bar narrowed to Bar & any[]
  }
  else {
    abc(foo.bar);  // Error, foo.bar is of type Bar
  }
}

In the if branch the type of foo.bar is narrowed to Bar & any[] which, when indexed, produces an any. In the else branch the type of foo.bar is unchanged and an error occurs.

Best I can tell this all is working as intended.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 1, 2017
@cevek
Copy link
Author

cevek commented Dec 1, 2017

@ahejlsberg, Sorry, it is incorrect example
Originally it was this example

function abc(foo: Foo) {
    abc(foo.bar instanceof Array ? foo.bar[0] : foo.bar); // no error
}

This expression foo.bar instanceof Array ? foo.bar[0] : foo.bar converts to any, so I think this is bug, because it is implicitly convertion to any.
But logically it is ok, because any | Bar = any

So, this is not bug, this is feature :(

@cevek
Copy link
Author

cevek commented Dec 1, 2017

So, I think instanceof guard is very bad decision, it doesn't work as expected. So I really need to rewrite all my code to use my custom guard function to do that. Also it concerns to new Array(5) producing any[], it is very annoying implicit behavior

@cevek cevek closed this as completed Dec 1, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants