Skip to content

Control flow not working as expected outside of if block #10706

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
watzon opened this issue Sep 4, 2016 · 9 comments
Closed

Control flow not working as expected outside of if block #10706

watzon opened this issue Sep 4, 2016 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@watzon
Copy link

watzon commented Sep 4, 2016

TypeScript Version: 2.0.2 and nightly (2.0.0-dev.20160904)

Code

// A *self-contained* demonstration of the problem follows...
public test(records: IRecord | IRecord[]) {
  if (!Array.isArray(records)) {
    records = Array(records); // or records = [records]
  }
  return records.length;
}

Expected behavior:

Length should return the length of the array since I casted records to an array if it comes in as a single IRecord.

Actual behavior:

I get the error: Property length does not exist on type 'IRecord | Record[]'.

Workaround:
Not ideal

public test(records: IRecord | IRecord[]) {
  let _records: IRecord[];
  if (!Array.isArray(records)) {
    _records = Array(records);
  } else {
    _records = records;
  }
  return _records.length;
}
@yortus
Copy link
Contributor

yortus commented Sep 5, 2016

@iDev0urer your original example compiles fine for me with TS2.0.2. However your repro is not self-contained since IRecord is not defined, and public test ... is invalid syntax. This is what I used:

interface IRecord { someProp }

function test(records: IRecord | IRecord[]) {
  if (!Array.isArray(records)) {
    records = Array(records); // or records = [records]
  }
  return records.length;
}

...and that works as expected.

However if you declared interface IRecord { } (i.e. an empty interface), then the problem occurs as you describe. But the reason in that case is that IRecord[] would be a structural subtype of IRecord, and that affects the narrowing logic.

@yortus
Copy link
Contributor

yortus commented Sep 5, 2016

Here's an equivalent example to the OP, showing how narrowing behaviour depends on whether the types in the union are unrelated or supertype/subtype.

But I don't know why the compiler acts differently at (1) and (2) below, and whether the compiler could do better here, since it seems inconsistent. Someone from the team, probably @ahejlsberg, would be best able to answer that.

class Animal {species}
class Cat extends Animal {meow}
class Dog extends Animal {woof}
declare function isDog(x): x is Dog;


function test1(x: Cat | Dog) {
  if (!isDog(x)) {
    x // x is Cat
    x = new Dog();
    x // x is Dog                                       <===== (1)
  }
  return x.woof; // OK
}


function test2(x: Animal | Dog) {
  if (!isDog(x)) {
    x // x is Animal
    x = new Dog();
    x // x is Animal|Dog  ---  but why not Dog here?    <===== (2)
  }
  return x.woof; // ERROR: property 'woof' does not exist on type 'Animal | Dog'
}

@watzon
Copy link
Author

watzon commented Sep 5, 2016

@yortus do you think it might make a difference that my examples were done inside of a class rather than as regular functions?

@ahejlsberg
Copy link
Member

@iDev0urer I suspect the issue in the OP is that you've declared interface IRecord { } in which case an IRecord[] is also an IRecord (because every type is a subtype of the empty interface).

@yortus The same is true at (2) in your example. A Dog is also an Animal, so when you assign a Dog to Animal | Dog you still have an Animal | Dog. In general, types like Animal | Dog are kind of non-sensical. You really can only expect meaningful results when the types in a union are truly disjoint.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 6, 2016
@yortus
Copy link
Contributor

yortus commented Sep 6, 2016

@ahejlsberg yeah that was the intention as I had already pointed out the supertype/subtype problem in the OP. I was more interested in why the compiler can't make the test2 example (which corresponds to the OP) work. I suppose it is because subtype reduction on the (2) line doesn't remove Animal from the union because all dogs are animals?

@yortus
Copy link
Contributor

yortus commented Sep 6, 2016

@iDev0urer the narrowing behaviour is the same in both class methods and regular functions.

@watzon
Copy link
Author

watzon commented Sep 6, 2016

@ahejlsberg actually my IRecord looks like this:

export interface IRecord {
  Id?: string;
  type?: string;
  attributes?: IRecordAttributes;
  extIdField?: string;
}

but I have a feeling that I am running into the issue you are describing since all of the params in IRecord are all optional it is basically an empty object. Thanks for the clarification!

@yortus
Copy link
Contributor

yortus commented Sep 6, 2016

types like Animal | Dog are kind of non-sensical.

@ahejlsberg not really. IRecord | IRecord[] is just like Animal | Dog to the compiler, and the OP naturally expected it to work. Unions like this occur regularly in valid code, where the subtype/supertype relationship is incidental and unintentional. #10471 is another case of the same problem with call signatures. Two more examples are here.

I'd rather describe this situation as a limitation of structural typing, rather than a problem with the source code, which is often perfectly acceptable runtime-valid code complete with valid type guards.

Perhaps it's not possible, but it would be great for this to be given further attention to see if there is some way to get narrowing to work the way the programmer expects (and to match runtime behaviour) for unions of unintentionally related types like in this issue. Or at least an FAQ entry to point people to the indicators and workarounds for this situation.

@watzon
Copy link
Author

watzon commented Sep 6, 2016

@yortus thanks for the help!

@mhegazy mhegazy closed this as completed Feb 28, 2017
@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
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants