TypeScript: Optional chaining with non null operator is unsafe, because it could throw an exception

You’re asserting that a?.b will always be defined, but that won’t be the case when a is possibly undefined.

I disagree. I think I’m asserting that if a is not nullish, then it is guaranteed to have a non-nullish b property. @proteriax’s example matches my feeling perfectly. If we desugar:

// Input:
a?.b!.c

// Output:
a == null ? undefined : a.b!.c

@rbuckton The code should run the same with or without TS-specific syntax, right? Currently it won’t; the output changes depending on the presence of the !. This is because TS (incorrectly) implicitly groups it as (a?.b)!.c, which through type-elision becomes (a?.b).c, which is clearly not the same as a?.b.c.

This further means that the code with run differently when targeting ESNext (a?.b.c;) vs anything else (((_a = a) === null || _a === void 0 ? void 0 : _a.b).c;).

If we change the precedence of ! to be lower than OptionalChain, then that changes the type of x2 above to be number | undefined*, which will be surprising for users.

I think this is where we disagree. I expect number | undefined* as the result.

If I wanted to unconditionally assert the return type, I would have written (foo?.bar)!.baz to begin with.

Downleveling foo?.bar!.baz into (foo?.bar).baz is really weird. It’s not meaningfully different than downleveling to foo.bar.baz. Whether .bar is the thing throwing (because foo is null) or .baz throws (because foo?.bar returned undefined) is useless.

After our most recent design meeting, we’ve decided that our solution should be a slightly odd hybrid to satisfy both the a?.b!.c case and the a?.b.c! cases.

! will be special-cased so that when the next token could continue an optional chain, the ! will be parsed as part of the current chain expression (the behavior that users on this issue have wanted). Otherwise, it will still work “as expected” at the end of an optional chain expression and remove undefined from the resulting type of the entire chain.

@DanielRosenwasser

I don’t think that’s the right way to think about it because ultimately it’s not really the same code, it’s a distinct syntax that was parsed in a meaningfully different way.

That’s all well and good if you have the TS shift-reduce conflict resolution table memorized, but if you’re a random developer who encounters an error like this:

https://www.typescriptlang.org/play/?ssl=3&ssc=2&pln=1&pc=1#code/BQMw9mBcAEB2CuAbR0A+0De0BGBDATgPwxZ4BeMwAlNALwB8mAvtC0zQ5gLABQ0-0cGEIA6PPjG4y1XkyA

Realizes that they know bar will be defined any time foo is, so adds a !. The error disappears and all seems well, but actually they’ve taken safe JS and transformed it into error-prone JS because of TS’s “warning”. This is a really quite bad developer experience.

And this isn’t just some hypothetical example, VSCode 1.42 will ship tomorrow with an instance of this exact bug: https://github.com/microsoft/vscode/blob/master/src/vs/base/browser/ui/tree/asyncDataTree.ts#L272

How would you propose people address the cant invoke an object which is possibly undefined error in the below:

declare const foo: { bar?: { baz?: () => {} } }
foo.bar?.baz();

given they know baz will be defined when bar is in this case (but the type system doesn’t for whatever reason). The answer has always been add a !, but now that will actually add a runtime error that wasn’t in the original code. And given there’s no ?() syntax, and a as type assertion would have the same issues, I don’t see how a fix is even representable with the current parse strategy.

This is my best shot:

declare const foo: { bar?: { baz?: () => {} } }
if (foo.bar) {
    (foo.bar.baz as Exclude<Exclude<typeof foo.bar, undefined>['baz'], undefined>)() 
}

…which is to say, get rid of the optional chaining entirely.

If I wanted to unconditionally assert the return type, I would have written (foo?.bar)!.baz to begin with.

Exactly, and given there’s no way to represent what a?.b!.c looks like it means, wouldn’t it make the most sense to have it default to “parse A”, then users can add parens to force “parse B” – rather than it defaults to “parse B”, users can add parens to force “parse B”, and “parse A” is unrepresentable?

I’ll look into it. While the syntax is valid to parse, it is a bit nonsensical. You’re asserting that a?.b will always be defined, but that won’t be the case when a is possibly undefined.