-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allow computed class properties #5206
Allow computed class properties #5206
Conversation
@@ -222,6 +222,7 @@ grammar = | |||
ObjAssignable: [ | |||
o 'SimpleObjAssignable' | |||
o '[ Expression ]', -> new Value new ComputedPropertyName $2 | |||
o '@ [ Expression ]', -> new Value LOC(1)(new ThisLiteral $1), [LOC(3)(new ComputedPropertyName($3))], 'this' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly copied this grammar rule from the rule for ThisProperty
@@ -2007,7 +2007,11 @@ exports.ExecutableClassBody = class ExecutableClassBody extends Base | |||
# The class scope is not available yet, so return the assignment to update later | |||
assign = @externalCtor = new Assign new Value, value | |||
else if not assign.variable.this | |||
name = new (if base.shouldCache() then Index else Access) base | |||
name = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix for the invalid JS for non-static computed class properties (extracted from #5205)
@@ -118,14 +118,21 @@ exports.Rewriter = class Rewriter | |||
# The lexer has tagged the opening bracket of an indexing operation call. | |||
# Match it with its paired close. | |||
closeOpenIndexes: -> | |||
startToken = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allowing static computed class properties/methods was a bit more complex. In addition to the new grammar rule above, in the rewriter here we first want to make sure that the [
/]
in eg @[b]: ...
are not tagged as INDEX_START
/INDEX_END
since that would be inaccurate and creates an ambiguous grammar
I believe this is safe because there's not a scenario where an actual INDEX_END
would be immediately followed by a :
token
@@ -321,7 +328,12 @@ exports.Rewriter = class Rewriter | |||
if tag is ':' | |||
# Go back to the (implicit) start of the object. | |||
s = switch | |||
when @tag(i - 1) in EXPRESSION_END then start[1] | |||
when @tag(i - 1) in EXPRESSION_END |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, we have to make sure that for an implicit object (whether an actual implicit object or a class body) starting with a static computed property eg @[b]: ->
, the start of the implicit object comes before the @
Thanks, this looks great. Seeing how complicated this got, I’m glad we split it off from the AST work. |
Fixes #5204
@GeoffreyBooth this PR contains two things (both raised in #5204):
I extracted this fix from #5205 to target it here against
master