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 doesn't correctly narrow union types #54270

Closed
matthew-dean opened this issue May 16, 2023 · 6 comments
Closed

TypeScript doesn't correctly narrow union types #54270

matthew-dean opened this issue May 16, 2023 · 6 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@matthew-dean
Copy link

Bug Report

Given these interfaces:

interface Book {
  author: string
}
interface Collection {
  type: 'collection'
  title: string
}

export const doSomething = (item: Book | Collection) => {
}

TypeScript correctly infers the type from an if statement, but doesn't, by process of elimination, infer the other type in the union.

export const doSomething = (item: Book | Collection) => {
  if ('type' in item && item.type === 'collection') {
    console.log(item.title) // this is fine
  } else {
    console.log(item.author) // this throws an error
  }
}

This is an illogical error. If there are only 2 possible types, then if the first type is correctly inferred, then the else statement must be the other type, as the other interface does not have a type key nor a type key with a value of collection.

🔎 Search Terms

So, I searched, and this could be a duplicate of #52272 which was marked a duplicate of #43026, but it's not clear its a duplicate, as those describe an a / b / c case where this is an a / b case.

Is the problem that the other object does not have a type key? Is there a scenario where it could be narrowed?

🕗 Version & Regression Information

  • This is a crash
  • This changed between versions ______ and _______
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
  • I was unable to test this on prior versions because _______

⏯ Playground Link

Playground link with relevant code

💻 Code

Shown above

🙁 Actual behavior

Types were not narrowed

🙂 Expected behavior

Union types are narrowed

@matthew-dean
Copy link
Author

Hmm it looks like I can get the above code to work with the following change:

interface Book {
  type: undefined
  author: string
}
interface Collection {
  type: 'collection'
  title: string
}

So, even though type does not exist as a key at runtime, and I'm explicitly checking for it, TypeScript seems to be satisfied that these can be narrowed. Maybe this falls under the realm of a feature request? Could TypeScript not correctly infer the type without an explicit key set to undefined?

@RyanCavanaugh
Copy link
Member

Broadly speaking, control flow analysis considers there to be two kinds of conditions: those that affect the type of a variable, and those that don't.

In this case, the in operator on the first clause is a narrowing operation and thus can narrow the operand to the Collection type. In the second part of the clause, the equality comparison is not something that can induce a narrowing because there's nothing to narrow to. Control flow analysis assumes that you must have written that code on purpose and that therefore it must not always be true.

If it's not always true, then we have to entertain the possibility that indeed some values that have a type field aren't actually Collections after all. In that case, in the else clause, it's indeed not safe to assume that the value is a Book.

@RyanCavanaugh
Copy link
Member

In other words, TypeScript is expecting you to uniformly either acknowledge or reject the possible existence of the BookCollection type shown here:

interface BookCollection extends Book {
  type: 'collection';
}

interface Collection {
  type: 'collection';
  title: string;
}

@matthew-dean
Copy link
Author

matthew-dean commented May 16, 2023

@RyanCavanaugh

I don't really understand at all, but that's okay; I'd probably need some kind of YouTube course to understand how TypeScript understands types.

Somewhat related: The following would probably be better written as a feature request (if it hasn't already), but I feel like there are a bunch of times where I want to just assert to TypeScript that something is of a certain type within a scope, without having to use parentheses with each reference, like:

if (condition) {
  assert item is Book;
  console.log(item.prop1, item.prop2)
} else {
  assert item is Collection;
  console.log(item.prop3, item.prop4)
}

I know you can do type-narrowing assertions with functions or variable assignment, it would just be nice to have something that's erasable at runtime.

I used to run into this all the time when working with the React.Children API, where nodes can be of different types, and you have to essentially assign the node to another variable just to assert the type for the rest of the scope of the function.

@matthew-dean
Copy link
Author

matthew-dean commented May 16, 2023

feel like there are a bunch of times where I want to just assert to TypeScript that something is of a certain type within a scope

Ah looks like the second part is a (partial) duplicate of #53201

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 16, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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