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

Narrowing behaviour differs with guard order #7045

Closed
yortus opened this issue Feb 12, 2016 · 7 comments
Closed

Narrowing behaviour differs with guard order #7045

yortus opened this issue Feb 12, 2016 · 7 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@yortus
Copy link
Contributor

yortus commented Feb 12, 2016

Whilst looking at @RyanCavanaugh's example code in this comment I noticed odd narrowing behaviour depending how the type guards are arranged. Is this expected?

function isActuallyThing(x: any): x is Thing {
  return /* my special way of determining Thingness */
}

function doSomething(x: Thing|string) {
  if (typeof x === 'string') {
    //...
  } else if (isActuallyThing(x)) {
    //...
  } else {
    x // type of x here is Thing               <== WHY NOT Thing|string?
  }
}


function sameButDifferent(x: Thing|string) {
  if (isActuallyThing(x)) {
    //...
  }
  else if (typeof x === 'string') {
    //...
  } else {
    x // type of x here is Thing|string
  }
}
@weswigham
Copy link
Member

Consider:

function doSomething(x: Thing|string) {
  if (typeof x === 'string') {
    //...
  } else if (isActuallyThing(x)) {
    //...
  } else {
    x // type of x here is Thing               <== WHY NOT Thing|string?
  }
}


//Is really the same thing as
function doSomething(x: Thing|string) {
  if (typeof x === 'string') {
    //x is a string, dur
  } else {
    //x must be a Thing here
    if (isActuallyThing(x)) {
      //bit of a redundant guard - we're no longer in a union type so no narrowing gets done
    } else {
      x // type of x here is Thing               <== WHY NOT Thing|string?
      // also not in a union type, the above guard does nothing
    }
  }
}

At least that's why it currently works as it does - you could certainly argue that if we aren't in a union anymore, we should still remove types if possible just so we can revert to the original type if possible.

@yortus
Copy link
Contributor Author

yortus commented Feb 12, 2016

@weswigham applying the same logic to sameButDifferent:

function sameButDifferent(x: Thing|string) {
  if (isActuallyThing(x)) {
    //...
  }
  else if (typeof x === 'string') {
    //...
  } else {
    x // type of x here is Thing|string
  }
}

//Is really the same thing as
function sameButDifferent(x: Thing|string) {
  if (isActuallyThing(x)) {
    //x is a Thing, dur
  } else {
    //x must be a string here
    if (typeof x === 'string') {
      //bit of a redundant guard - we're no longer in a union type so no narrowing gets done
    } else {
      x // type of x here is Thing|string                            <== WHY NOT string?
      // type widened back out to Thing|string - different behaviour to doSomething but why?
    }
  }
}

So why not just string if the final else?

@tinganho
Copy link
Contributor

@yortus how is Thing defined?

@yortus
Copy link
Contributor Author

yortus commented Feb 12, 2016

@tinganho I defined it as interface Thing {thing} so it's not a superset/subset of string.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

The issue here is not about ordder, as much as it about behaviour. the two type guards do not behave the same way; the typeof type guards are much "stronger", the user defined and instance of are "weaker", so:

if (typeof x === "string") {
    x // string

    if (isThing(x)) {
        x // still string
    }
}
else {
    x // thing

    if (typeof x === "string") {
        x /// string | Thing
    }
}

@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

I should add the rational is, that at runtime it is impossible to get a value whose typeof is both "string" and "number", but it is possible to get something that is compatible with both types A and B.

@mhegazy mhegazy closed this as completed Feb 19, 2016
@mhegazy mhegazy added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Feb 19, 2016
@yortus
Copy link
Contributor Author

yortus commented Feb 20, 2016

Right, you can have a value that is both Thing and string. So to match runtime behaviour your above example could be better modeled by the compiler as:

if (typeof x === "string") {
    x // string

    if (isThing(x)) {
        x // Thing&string (but currently string)
    }
}
else {
    x // Thing

    if (typeof x === "string") {
        x // Thing&string (but currently Thing|string)
    }
}

The current narrowing behaviour leads to compile-time errors on code that is valid and works at runtime.

@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
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

4 participants