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

Type narrowing does not survive after applying suggested 'extract to constant' refactoring #43333

Closed
fpintos opened this issue Mar 21, 2021 · 2 comments · Fixed by #44730
Closed
Labels
Duplicate An existing issue was already created

Comments

@fpintos
Copy link
Member

fpintos commented Mar 21, 2021

Bug Report

Applying the 'extract to constant' refactoring on an ?: operator can cause type narrowing to get lost, making subsequent calls in the true or false expressions of the ?: operator invalid from typescript's perspective, if the ?: operator is being used to detect null/undefined cases.

🔎 Search Terms

type narrowing, undefined, boolean, ? operator
See also #35260

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about "narrow"

⏯ Playground Link

Playground link with relevant code

💻 Code

export default function actOnDataOrUndefined(data?: string[]) {
  // This is fine
  const ok = data && data.length != 0 ? data.length : null;

  // Refactor the condition to a local variable and typescript 
  // reports that data can be undefined in the true-case expression
  // when it clearly(?) cannot be, since hasData is true only when data is not undefined.
  const hasData = data && data.length != 0;
  const not_ok = hasData ? data.length : null;
}

🙁 Actual behavior

In the 'ok' case, pre-refactoring, typescript compiles correctly since it determines that data is not undefined in the true expression. Selecting the boolean expression of the ?: operator, Visual Studio suggests that we can extract the expression into a local variable. Doing so, creates the 'not_ok' case, and then typescript believes "data" to be string[]|undefined.

🙂 Expected behavior

The best case would be for typescript to correctly identify that in the true expression of the ?: operator type type of data is only string[] and not string[] | undefined, since code analysis can prove that, when hasData is true, then data is not undefined.

If, for some reason, that is not possible (or it is expensive), then perhaps the refactoring should not be available or it needs to suggest changes in other places in the code, since the expectation is that refactoring will keep resulting code working as before.

In my particular production code I simply "fixed" the issue by using data!, but that still cost me a PR-build cycle since I didn't realize the mistake when I first did the refactoring (in my case, data was being passed to a separate function instead of used directly, and vscode was not fast enough to show the error before the change was submitted as PR).

@MartinJohns
Copy link
Contributor

The issue with the narrowing is tracked by #12184.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Mar 22, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' 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
Duplicate An existing issue was already created
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants