tc39 / proposal-optional-chaining

https://tc39.github.io/proposal-optional-chaining/
4.94k stars 75 forks source link

Unwelcome grammar surprise with tagged template literals #56

Closed waldemarhorwat closed 6 years ago

waldemarhorwat commented 6 years ago

Consider the following:

a.b
`c`

This is a single expression statement that uses template with the result of a.b. Now make the . optional:

a?.b
`c`

It suddenly splits into two expression statements thanks to semicolon insertion. Whoops!

This also means that, unless you address this now, you won't be able to orthogonally support templates in the future without breaking compatibility.

ljharb commented 6 years ago

Is there a way you can see to change the grammar to avoid this surprise?

claudepache commented 6 years ago

Good catch.

The obvious solution is to incorporate TemplateLiteral in the OptionalChain production and to add an early error condition when it matches.

Or do you have another suggestion?

littledan commented 6 years ago

@claudepache That suggestion sounds perfect to me. Are there any other similar hazards?

ljharb commented 6 years ago

Why would it be a good idea to make a?.b`template literal` an early error? I wouldn't expect that to be any different than (a == null ? a : a.b`template literal`).

claudepache commented 6 years ago

@ljharb I’m not sure it will be obvious for everyone that a?.b`template literal` would not result in (a == null ? `template literal` : a.b `template literal`). But if you think it should be supported nevertheless, please make your point in #54.

claudepache commented 6 years ago

Are there any other similar hazards?

Considering what may occur at the end of MemberExpression or CallExpression, I think not. But we should watch any future syntactical addition that would be valid both at the end of a LeftHandSideExpression and at the beginning of a statement.

tenbits commented 6 years ago

May I ask, which rule of ASI will cause the parser to insert the semicolon? I thought the optional chaining prefix (token, operator, whatever ) just modifies the binary operator from the example, so that the parser after b on LineTerminator will proceed the stream as usual.

ScottRudiger commented 6 years ago

@waldemarhorwat

Consider the following:

a.b
`c`

This is a single expression statement that uses template with the result of a.b. Now make the . optional:

a?.b
`c`

It suddenly splits into two expression statements thanks to semicolon insertion. Whoops!

I'm a bit confused by this. I agree automatic semi-colon insertion does not apply in the first case (as it's the way JS currently works):

// when 'a' is defined
let a = { b([_]) { console.log(_) } };

a.b
`thing` // logs 'thing'

// when 'a' is null
a = null;

a.b // 'TypeError: Cannot read property 'b' of null
`thing` // not evaluated as 'TypeError' was already thrown

I'm not sure why in the second case, with optional chaining, ASI would act any differently. There would still be no ASI applied:

// when 'a' is defined
let a = { b([_]) { console.log(_) } };

a?.b
`thing` // logs 'thing' - same as above

// when 'a' is null
a = null;

a?.b
`thing` // *nothing logged because a is null* - short circuits to 'undefined'

Maybe it's me misunderstanding, but are you saying that there's something in the spec that calls for automatic semi-colon insertion after optional chains (if so, I was not aware). If that's the case it'd be something like:

// when 'a' is defined
let a = { b([_]) { console.log(_) } };

a?.b // ASI inserted ';' - this line evaluates to '[Function: b]'
`thing` // *nothing logged* - but this line evaluates to 'thing'

// when 'a' is null
a = null;

a?.b // ASI inserted ';' - this line evaluates to 'undefined' due to short-circuiting
`thing` // *nothing logged either* - but this line also evaluates to 'thing'

If that is in fact what we're talking about, I don't think it makes much sense for the optional chaining spec to arbitrarily call for ASI after optional chains. That would change the behavior of how ASI currently works in JS--an entirely separate proposal-able topic.

Since it's probable I'm misunderstanding the premise of this issue, I'll take another stab at it. Is the problem actually that some developers might mistakenly assume that ASI should apply when they write <optional chain> <newline> <template literal>? If so, I don't see how that's any different than their mistaken assumption that ASI will smooth things over for them after writing <static property lookup> <newline> <template literal>.

In any case, I think it's important that semi-colons are manually inserted where automatic semi-colon insertion does not act, for example, semi-colons are needed just as much when the next line starts with ` as if it starts with ( in order to avoid unintended behavior (if unintended).

This also means that, unless you address this now, you won't be able to orthogonally support templates in the future without breaking compatibility.

Despite my misunderstanding (apologies), I love your suggestion that tagged template literals should be addressed now. See #54 for further πŸ’—! I'll leave further pro-template literal support on my part for that issue. 😜

jridgewell commented 6 years ago

are you saying that there's something in the spec that calls for automatic semi-colon insertion after optional chains

It's not specific to optional chains. Per 11.9.1.1, a semi colon is inserted whenever a continuation is invalid grammar.

In this case, the grammar doesn't allow for an optional chain in the tag of a tagged template literal. So, it reads the valid a?.b, then sees a newline. It then sees a template literal `, which is invalid grammar. So it inserts a semi colon, and tries to reparse the template literal.

Is the problem actually that some developers might mistakenly assume that ASI should apply

No, the issue is that people will assume it won't apply (a semi will not be inserted, like how the current tagged template literal works).

ljharb commented 6 years ago

So, can we make an optional chain allowed in the tag of a template literal?

waldemarhorwat commented 6 years ago

Yeah, that's what I would recommend.

claudepache commented 6 years ago

@tenbits I thought the optional chaining prefix (token, operator, whatever ) just modifies the binary operator from the example, so that the parser after b on LineTerminator will proceed the stream as usual.

This is not how it is currently specced (an optional chain supports only a limited set of operators). But I concede that this may be counterintuitive, and it could be adjusted if there is consensus for it. (However, I am very reluctant to accept things like new a?.b() for technical reason, namely because it would lead to significant complication of the grammar.)

tenbits commented 6 years ago

@claudepache an optional chain supports only a limited set of operators

This means, that syntactical grammar doesn't allow (assumed <prfx> for optional prefix) a . b <prfx>`c`, but this does: a <prfx>. b `c`. So, being a valid grammar the parser won't insert the semicolon. That's why I don't quite understand the ASI issue from the example above. Thanks Claude. I see in the readme now, that a<prfx>.b`c` is also not allowed.

claudepache commented 6 years ago

@tenbits That means that a.b`c` is supported, but not a?.b`c`. Indeed, the optional chain would be intuitively ?.b`c`, but the grammar does not support it: it may only be parsed up to ?.b, then an automatic semicolon is inserted because it can’t be parsed further.

ljharb commented 6 years ago

Then that sounds like a grammar bug; it should be supported.

waldemarhorwat commented 6 years ago

My last comment raced with others here so it's unclear what I was referring to. To clarify, I'd recommend that the grammar productions for .? be analogous to those of . to avoid the semicolon insertion surprise. If we want, we can then choose to make the combination of .? with templates a syntax error in the semantics, after the grammar is run.

ScottRudiger commented 6 years ago

@jridgewell

In this case, the grammar doesn't allow for an optional chain in the tag of a tagged template literal. So, it reads the valid a?.b, then sees a newline. It then sees a template literal `, which is invalid grammar. So it inserts a semi colon, and tries to reparse the template literal.

Thanks, I think I get it now. A semicolon is inserted because: 1) (as currently specced) a template literal is not allowed following the example optional chain 2) there's a LineTerminator separating the optional chain from the template literal

I feel a bit foolish with my previous comment when I simply could have just asked, "[W]hich rule of ASI will cause the parser to insert the semicolon?" Thanks @tenbits. πŸ˜…

So to put it in simple terms, for someone not intimately familiar with the language specification, one possible solution is to make \<optional chain> \<template literal> valid syntax (no matter if the optional chaining operator follows the base object, a nested property, or right before the template literal) so that ASI does not occur, right?