Skip to content

Narrowing by checking for existence of key leads to never. #38608

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
MicahZoltu opened this issue May 16, 2020 · 7 comments · Fixed by #38610
Closed

Narrowing by checking for existence of key leads to never. #38608

MicahZoltu opened this issue May 16, 2020 · 7 comments · Fixed by #38610
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@MicahZoltu
Copy link
Contributor

MicahZoltu commented May 16, 2020

TypeScript Version: 3.9.2

Search Terms:
instanceof narrow derived

Code

class CustomError extends Error { extra: string = 'hello' }
const error: Error = new CustomError()
if ('extra' in error) {
    // code path reached at runtime
    error // type: never
} else {
    // code path not reached at runtime
    error // type: Error
}

Expected behavior:
error inside the if block is still of type Error

Actual behavior:
error inside the if block is of type never

Playground Link: https://www.typescriptlang.org/play?#code/MYGwhgzhAEDCCuEAuB7AtgUQE5ZV6ApgB5IEB2AJjNrvgN6ElZgBc0yWAlmQObQC80AOQALAiBAoh0AL4AoYCjLJCOPGxp4B0MgQDucRKkxqsACgCUczgDNoZocSTNp3VbQvQ6c6L-daAegDoJABPAAcCNl0ANwIsORlCEAgCLx8-eNpoIJCIqOhNBJkgA

Related Issues:
None that seemed to match.

@MicahZoltu
Copy link
Contributor Author

MicahZoltu commented May 16, 2020

It would be super cool if the type inside the if statement were Error & { extra: unknown }, but I know TS doesn't currently narrow objects like this which I have accepted, so I would be fine with just Error as the type inside the if block.

@jcalz
Copy link
Contributor

jcalz commented May 16, 2020

Duplicate of #21732? Or related anyway

@MicahZoltu
Copy link
Contributor Author

#21732 would be very cool to have and resolving that may (as a side effect) resolve this (depending how it is implemented), but I don't think this is a duplicate of that or needs to block on that necessarily.

jack-williams added a commit to jack-williams/TypeScript that referenced this issue May 16, 2020
@jack-williams
Copy link
Collaborator

This looks to be similar to #21517. There was some discussion here.

It's not quite clear what the behaviour should be for the singleton case. For a union I think it is clear you want to narrow down to never, but for declared singletons there is some ambiguity. Then there is the case where you start with a singleton, narrow to a union, then back to a singleton from the union.

A proposed fix is to not apply in narrowing to singleton declared types. That is, in narrowing gets applied when either the type is a union, or the type is not the declared type (because of narrowing).

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 8, 2020
@RyanCavanaugh
Copy link
Member

@ahejlsberg thoughts on desired behavior?

@MicahZoltu MicahZoltu changed the title Narrowing with instanceof by checking for existence of key leads to never. Narrowing by checking for existence of key leads to never. Jun 9, 2020
@MicahZoltu
Copy link
Contributor Author

Further simplified repro case:

const apple: {} = { a: 'hello' }
if ('a' in apple) {
    apple // type: never
} else {
    apple // type: {}
}

The issue here is that just because apple has a key a doesn't mean it is no longer of type {}.

@ahejlsberg
Copy link
Member

@RyanCavanaugh I think our current behavior is quite odd. There's some good discussion in #21732, and I like @jcalz's suggestion of extending the current behavior to intersect with Record<K, unknown> when the current behavior would otherwise have produced never. There are no doubt some details to work out to get that in place without breaking changes, but I think it makes sense to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants