Skip to content
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

MetaProperty AST (new.target) #5170

Merged
merged 4 commits into from
Mar 17, 2019

Conversation

helixbass
Copy link
Collaborator

@GeoffreyBooth as mentioned in #5169 (comment), new.target should generate a MetaProperty AST node

children: ['meta', 'property']

checkValid: (o) ->
if @meta.value is 'new'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When reading about MetaProperty, I came across import.meta. So makes sense to make MetaProperty a little bit "generic" so that we can plan on adding other meta-properties

-> new.something
''', '''
[stdin]:1:4: error: the only valid meta property for new is new.target
-> new.something
^^^^^^^^^^^^^
'''

test "`new.target` cannot be assigned", ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, it wasn't throwing a compilation error when eg assigning to new.target (since it was just being treated like a normal object-property access)

So this is a benefit of using a separate grammar construct for MetaProperty, since new.target shouldn't be able to be used as an "assignable"

@GeoffreyBooth GeoffreyBooth merged commit 3f5abb3 into jashkenas:ast Mar 17, 2019
@GeoffreyBooth
Copy link
Collaborator

Looks good. We should add support for import.meta to the main branch too, after import().

@helixbass helixbass deleted the new-target-meta-property branch March 17, 2019 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants