Skip to content

Conditional type narrowing ignores nullish coalescing to falsy value #43705

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
aaronadamsCA opened this issue Apr 16, 2021 · 7 comments
Closed
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@aaronadamsCA
Copy link

Bug Report

if (a.b?.c ?? false) a.b.c does not narrow the type of a.b to remove nullish types.

🔎 Search Terms

conditional type narrowing, nullish coalescing operator

🕗 Version & Regression Information

Occurs in v4.2.3 and v4.3.0-beta. Behaviour appears consistent since nullish coalescing was first introduced in v3.7.

⏯ Playground Link

Playground link with relevant code

💻 Code

interface A {
  b?: B;
}

interface B {
  c: string;
}

(a: A): void => {
  // Works, but intended behaviour is unclear
  if (a.b?.c) a.b.c.length;

  // Doesn't work 🙁
  if (a.b?.c ?? "") a.b.c.length;
  //                  ^ Object is possibly 'undefined'. ts(2532)

  // Not very concise 🙁
  if (a.b?.c !== undefined && a.b.c) a.b.c.length;
};

It's an edge case with a good use case. @typescript-eslint/strict-boolean-expressions helps avoid tricky bugs by requiring code to separately handle nullish vs. falsy values. Nullish coalescing would be the simplest way to explicitly treat nullish values the same as an empty string.

This might be related to #38136 but I'm unsure.

@Zzzen
Copy link
Contributor

Zzzen commented Apr 16, 2021

why don't you use if (a.b?.c != undefined) a.b.c.length; ?

@andrewbranch
Copy link
Member

Why is if (a.b?.c) a.b.c.length unclear? When I see if (a.b?.c ?? "") or if (a.b?.c ?? false), I get very confused as to why something already falsey (null or undefined) would need to be coalesced to something else falsey in a conditional.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Apr 19, 2021
@RyanCavanaugh
Copy link
Member

It doesn't seem like this particular lint rule configuration is helping code clarity (for humans or machines). If this turns out to be a popular thing to write we can see about adding detection for it but generally we don't spend perf budget on handling constructs that don't change the code behavior.

@andrewbranch
Copy link
Member

As an additional workaround, if (!!a.b?.c) seems like it would satisfy both your lint rule and our control flow analysis, though I’m generally in favor of turning off painful lint rules.

@aaronadamsCA
Copy link
Author

I can change the use case, sure, but I wanted to point out the gap anyway. Never know when a little thing points to a bigger thing.

@JeanRemiDelteil
Copy link

As an additional workaround, if (!!a.b?.c) seems like it would satisfy both your lint rule and our control flow analysis, though I’m generally in favor of turning off painful lint rules.

This is not testing only for null || undefined, but anything falsey. So the behavior changes.

Here is a simple code example that output an error in typescript, when it should not:

const readProp = (property: string | undefined | null): string => {
  if ((property ?? undefined) === undefined) {
    return ''
  }

  return property;
  //     ~~~~~~~~
  // Type 'string | null | undefined' is not assignable to type 'string'.
  // Type 'undefined' is not assignable to type 'string'.
}

Playground link

Our Context:
We have a rule not to use null in our project, and writing the test like this avoid usage of null, while still checking in javascript against potential null values (that can happen due to external libraries).

Anyway, there is a problem in the type checking / type guard when using nullish coalescing operator "??".

@aaronadamsCA
Copy link
Author

aaronadamsCA commented Dec 28, 2021

FWIW, I long ago reconfigured our ESLint rules to use eqeqeq: ["error", "smart"] so that we could use the pattern if (a.b?.c != null) a.b.c.length. Works fine for us.

@aaronadamsCA aaronadamsCA closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants