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

TypeScript ignore expression if (object instanceof SomeClass) inside for(){} and inside ()=>{}. #13180

Closed
KostyaTretyak opened this issue Dec 27, 2016 · 5 comments

Comments

@KostyaTretyak
Copy link

TypeScript Version: 2.1.4

Code

class ClassA
{
  propA: any;
}

class ClassB
{
  propB: any;
}

function fn( arr: Array<ClassA | ClassB> )
{
  for( let element of arr )
  {
    if( element instanceof ClassA )
    {
      element.propA = true; // Work as expected

      () =>
      {
        element.propA; // Unexpected error
      }
    }
  }
}

Expected behavior:

Works without error

Actual behavior:

Throw error:

Property 'propA' does not exist on type 'ClassA | ClassB'.
Property 'propA' does not exist on type 'ClassB'.

When I remove the loop for(){}. Works as expected:

class ClassA
{
  propA: any;
}

class ClassB
{
  propB: any;
}

function fn( element: ClassA | ClassB )
{
  if( element instanceof ClassA )
  {
    element.propA = true; // Work as expected

    () =>
    {
      element.propA; // Work as expected
    }
  }
}
@aluanhaddad
Copy link
Contributor

aluanhaddad commented Dec 27, 2016

for (let element of arr) {
  if (element instanceof ClassA) {
    element.propA = true;

   () => {
      element.propA;
    }
  }
}

Your inner function is closing over a mutable binding that is scoped outside the if block and is thus not always going to be an instance of ClassA.

To make this work, use a const binding:

function fn(arr: (ClassA | ClassB)[]) {
  for(const element of arr) {

@KostyaTretyak
Copy link
Author

KostyaTretyak commented Dec 27, 2016

@aluanhaddad, thanks for the answer.

Mutable - possible, but in fact this code is no mutation. TypeScript checks so deeply?

@aluanhaddad
Copy link
Contributor

I think the question comes down to scope of the variable. A constant binding could not be scoped to the for of declarator.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Dec 27, 2016

Also, CFA doesn't track what happens inside of closures to make inferences about types in the outer scope.

My recommendation, and this is just my personal opinion, is that all bindings should be const unless they need to be let or var. I think it makes code a lot easier to read and is real boon to the maintenance programmer.

@KostyaTretyak
Copy link
Author

@aluanhaddad, thank you.

@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
None yet
Projects
None yet
Development

No branches or pull requests

2 participants