Skip to content

[no-non-null-asserted-optional-chain] support 3.9's new interaction between optional chains and non-null assertions #1976

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
fregante opened this issue May 5, 2020 · 2 comments · Fixed by #2036
Labels
enhancement New feature or request has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@fregante
Copy link
Contributor

fregante commented May 5, 2020

From: https://devblogs.microsoft.com/typescript/announcing-typescript-3-9-beta/#breaking-changes

Specifically, in previous versions, the code

foo?.bar!.baz

was interpreted to be equivalent to the following JavaScript.

(foo?.bar).baz

In the above code the parentheses stop the “short-circuiting” behavior of optional chaining, so if foo is undefined, accessing baz will cause a runtime error.

@typescript-eslint/no-non-null-asserted-optional-chain made sense in that case but TS 3.9 will change the behavior to this:

foo?.bar.baz

which just evaluates to undefined when foo is undefined.

So foo?.bar!.baz becomes perfectly reasonable to catch an undefined foo but ignore an undefined foo.bar

Versions

package version
@typescript-eslint/eslint-plugin 2.27.0
@typescript-eslint/parser 2.27.0
TypeScript 3.9 (future)
ESLint 6.8.0
node 12.15
npm 6.13.6
@fregante fregante added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels May 5, 2020
@fregante fregante changed the title [rulename] issue title Deprecate @typescript-eslint/no-non-null-asserted-optional-chain in TS 3.9 May 5, 2020
@fregante fregante changed the title Deprecate @typescript-eslint/no-non-null-asserted-optional-chain in TS 3.9 Deprecate no-non-null-asserted-optional-chain in TS 3.9 May 5, 2020
@fregante fregante changed the title Deprecate no-non-null-asserted-optional-chain in TS 3.9 [no-non-null-asserted-optional-chain] Deprecate rule in TS 3.9 May 5, 2020
@bradzacher
Copy link
Member

Interesting. When reading the release notes, I didn't quite understand that point correctly.

This makes it much clearer:

TS 3.8

type T = { a: { b: { c: string } } };
declare const x: T | null;

const a = x;       // typeof === T | null
const b = x?.a;    // typeof === T['a'] | undefined;
const c = x?.a!;   // typeof === T['a']
const d = x?.a.b;  // typeof === T['b'] | undefined
const e = x?.a!.b; // typeof === T['b']
const f = x?.a.b!; // typeof === T['b']

repl

TS 3.9

type T = { a: { b: { c: string } } };
declare const x: T | null;

const a = x;       // typeof === T | null
const b = x?.a;    // typeof === T['a'] | undefined;
const c = x?.a!;   // typeof === T['a']
const d = x?.a.b;  // typeof === T['b'] | undefined
const e = x?.a!.b; // typeof === T['b'] | undefined
const f = x?.a.b!; // typeof === T['b']

repl

So if you're on TS 3.9, then it makes sense to remove the functionality that stops non-null assertions within an optional chain.

HOWEVER, the rule isn't useless. It's still completely incorrect to use a non-null assertion at the end of an optional chain x?.a.b!.
This was actually the case that the rule was intended to catch.

I think it's reasonable to add a check to allow a non-null-assertion around an optional chain if, and only if all of the following are matched:

  • the TS version is 3.9
  • the non-null-assertion parent is:
    • an OptionalMemberExpression with optional: false
    • an OptionalCallExpression with optional: false

That will allow x?.a!.b and x?.a!(), but it will still ban:

  • x?.a!?.b and x?.a!?.()
  • x?.a! and x?.a()!
  • (x?.a)! and (x?.a)!()

@bradzacher bradzacher added enhancement New feature or request and removed triage Waiting for team members to take a look labels May 5, 2020
@bradzacher bradzacher changed the title [no-non-null-asserted-optional-chain] Deprecate rule in TS 3.9 [no-non-null-asserted-optional-chain] support 3.9's new interaction between optional chains and non-null assertions May 5, 2020
@bradzacher bradzacher added the has pr there is a PR raised to close this label May 18, 2020
@fregante
Copy link
Contributor Author

Awesome!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants