Skip to content

add import.meta #943

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

Merged
merged 2 commits into from
Apr 2, 2020
Merged

add import.meta #943

merged 2 commits into from
Apr 2, 2020

Conversation

mysticatea
Copy link
Contributor

@mysticatea mysticatea commented Apr 2, 2020

@mysticatea mysticatea mentioned this pull request Apr 2, 2020
4 tasks
@marijnh
Copy link
Member

marijnh commented Apr 2, 2020

Thanks! I'm waiting for the ESTree thing to merge again.

(Yeah, I know, #890 and #890 are also still stuck, after way too long, but supposedly there's interest from @nzakas and @RReverser to try and find a way to move ESTree decisions forward more effectively.)

@RReverser
Copy link
Member

We have already agreed on using MetaProperty for meta properties, so there is nothing really new on ESTree side here and this should be good to go. (the only upstream question is whether we want to list every meta property explicitly in the spec)

if (node.property.name !== "meta")
this.raiseRecoverable(node.property.start, "The only valid meta property for import is import.meta")
if (containsEsc)
this.raiseRecoverable(node.start, "'import.meta' must not contain escaped characters")
Copy link
Member

Choose a reason for hiding this comment

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

Do we have same error in new.target? We probably would want these error messages to align.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have pushed 0164a93 to align the error messages.

For a reference, "'import.meta' must not contain escaped characters" is the same message as Chrome 80.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it matters much to align with some engine, but internal consistency is important. Thanks for fixing!

@marijnh marijnh merged commit b4a6e52 into acornjs:master Apr 2, 2020
@mysticatea mysticatea deleted the import-meta branch April 2, 2020 12:55
@wizzfile
Copy link

wizzfile commented Apr 9, 2020

mysticatea:import-meta

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.

4 participants