parse/check/emit shorthand property assignment in destructuring#5121
Conversation
There was a problem hiding this comment.
When would you have a question token in a shorthand?
There was a problem hiding this comment.
AFAIR we currently parse it and issue the grammar error later
|
Looks good, but probably someone on the team should sign off. |
|
Thanks @JsonFreeman :) |
|
@mhegazy can you take a look? |
|
I actually posted comments earlier, but GitHub went down as I was posting them. Sorry about that @vladima |
There was a problem hiding this comment.
Why not just error on the objectAssignmentInitializer?
There was a problem hiding this comment.
I think the idea is to error on the first incorrect token. Also, this controls the span size so it is just 1.
There was a problem hiding this comment.
the comment is referring to both equalsToken and objectAssignmentInitializer - this pair matches Initializer part from CoverInitializerName production. My reasoning of reporting error on = instead of entire range [equalsToken, objectAssignmentInitializer] was:
- having large ranges of squigglies is annoying
- I can certainly see cases when by mistake people type
{x = 1}instead of{x: 1}and personally for me it will look confusing if we squiggle= 1instead of just =`
There was a problem hiding this comment.
Okay; in that case, can you amend the comment so it specifically says "The below are used". and just make it "It is a grammar error".
Other than that LGTM.
parse/check/emit shorthand property assignment in destructuring
parse/check/emit shorthand property assignment in destructuring
fixes #5035