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

Unnecessary strictNullChecks errors for nested property access protected by type predicate #50566

Closed
ashsearle opened this issue Aug 31, 2022 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@ashsearle
Copy link

Bug Report

TypeScript shows strictNullChecks errors for "possible 'null'" and "possible 'undefined'" values along a property access chain, even though the code is protected by a type predicate using optional chaining that guarantees the property access chain is valid.

🔎 Search Terms

NonNullable type predicate optional chaining

🕗 Version & Regression Information

  • This is the behavior in every version I tried (from 3.7.5. to 4.9.0 nightly), and I reviewed the FAQ for entries about type system behaviour, type guards and save navigation operator.

⏯ Playground Link

Playground Link: Provided

💻 Code

type SomeType = {
  prop?: {
    nestedProp?: string
  }
}
declare function someRandomFunc(...args: any[]): SomeType | null;
const isDefinedAndNotNull = <T,>(val: T | null | undefined): val is NonNullable<T> => val != null;

const instance = someRandomFunc();

if (isDefinedAndNotNull(instance?.prop?.nestedProp)) {
  // Unexpected strictNullChecks errors on the line below:
  // - instance: Object is possibly 'null'
  // - instance: Object is possibly 'undefined'
  // - prop: Object is possibly 'undefined'
  someRandomFunc(instance.prop.nestedProp);
}

if (
  isDefinedAndNotNull(instance) &&
  isDefinedAndNotNull(instance?.prop) &&
  isDefinedAndNotNull(instance?.prop?.nestedProp)
  ) {
  // No strictNullChecks errors here, but too much unnecessary code above
  someRandomFunc(instance.prop.nestedProp);
}
Compiler Options
{
  "compilerOptions": {
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "target": "ES2017",
    "jsx": "react",
    "module": "ESNext",
    "moduleResolution": "node"
  }
}

🙁 Actual behavior

TypeScript emits errors about possible null / undefined values along a property access chain.

This is unexpected as the code is guarded by a type predicate.

🙂 Expected behavior

No strictNullChecks errors.

@fatcerberus
Copy link

fatcerberus commented Aug 31, 2022

This seems like an expected limitation, given that checking the value of a property doesn't narrow the containing object in general (except for discriminated unions). I note that there are potentially multiple steps of inference required:

* instance?.prop?.nestedProp is known to not be undefined, therefore
* instance?.prop must also be non-nullish, therefore
* instance must also be non-nullish

It would indeed be nice if this pattern worked, but I don't know if the control-flow analysis machinery as it exists today is capable of making these inferences.

edit: I'll leave this here for posterity but this analysis of the situation was wrong - see below.

@MartinJohns
Copy link
Contributor

Duplicate of #34974.

@fatcerberus
Copy link

As pointed out in the dupe'd issue, this works if you write the type check inline:

if (instance?.prop?.nestedProp != null) {
  someRandomFunc(instance.prop.nestedProp);  // saul goodman
}

So ignore my comment above - TS can do the necessary inferences, it's just that NonNullable<T> is opaque to it.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Aug 31, 2022
@ashsearle
Copy link
Author

Thanks for the comments. @RyanCavanaugh's comments on the original issue gives me hope this might be fixed in the future, and there's no need for me to leave this duplicate issue open.

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

No branches or pull requests

4 participants