Skip to content

TS 3.2.0-rc seems to forget union type refinements more eagerly #28568

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
Jessidhia opened this issue Nov 16, 2018 · 7 comments
Closed

TS 3.2.0-rc seems to forget union type refinements more eagerly #28568

Jessidhia opened this issue Nov 16, 2018 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Jessidhia
Copy link

Jessidhia commented Nov 16, 2018

TypeScript Version: 3.2.0-rc

Search Terms:
refinement

Code

(EDIT: incorrect reproduction, see correct example at #28568 (comment))

function test(x: { type: 'a'; body: string | null } | { type: 'b'; body: number | null }): string {
  if (x.body === null) {
    return 'null'
  }
  x.body.toString() // ok

  if (x.type === 'a') {
    return x.body.toString() // x.body may be null
  } else {
    return x.body.toString() // x.body may be null
  }
}

Expected behavior:
The refinement of x.body should persist (it does in 3.1)

Actual behavior:
TS forgets that it has already refined x.body to be not null.

@mmis1000
Copy link

Maybe it guess x.body.toString() may has side effect and fail the refinements?

@Jessidhia
Copy link
Author

Oh, no, removing that line has no effect. I just added it to demonstrate that the initial refinement is working.

@ahejlsberg
Copy link
Member

I can't reproduce a difference between 3.1 and 3.2. In --strictNullChecks mode the example errors on both, and without --strictNullChecks the example passes on both.

As an aside, the error in --strictNullChecks mode is due to a known design limitation in the control flow analyzer--upon encountering a discriminant check we narrow based on that check but forget about other narrowings that were already in effect.

@ahejlsberg ahejlsberg added the Needs More Info The issue still hasn't been fully clarified label Nov 18, 2018
@Jessidhia
Copy link
Author

That is odd. This is an error I only encountered in existing code once I updated to TS 3.2, but my snippet indeed also displays the error in 3.1...

@Jessidhia
Copy link
Author

Jessidhia commented Nov 19, 2018

Oh, I found the version that errors in 3.2 but not 3.1; both with --strict:

3.1:
image

3.2:
image

function test(x: { type: 'a'; body: string | null } | { type: 'b'; body: number | null }): string {
  if (x.body === null) {
    return 'null'
  }

  switch (x.type) {
    case 'a':
      return x.body.toString() // x.body may be null
    case 'b':
      return x.body.toString() // x.body may be null
  }
}

@ahejlsberg
Copy link
Member

ahejlsberg commented Nov 19, 2018

@Kovensky Yes, that is due to #27612. We weren't correctly resetting our narrowing assumptions in switch statements. It happened to work in some cases, including yours, but generally was wrong and inconsistent with corresponding if statements.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs More Info The issue still hasn't been fully clarified labels Nov 19, 2018
@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

4 participants