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

Add module-expression #238

Merged
merged 2 commits into from Feb 25, 2021
Merged

Conversation

sosukesuzuki
Copy link
Contributor

Copy link
Contributor

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

LGTM with a nit.

type: "ModuleExpression";
body: Program;
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider add a note that the sourceType of body must be "module".

Copy link
Member

Choose a reason for hiding this comment

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

We can probably subtype Program to a Module for that. But it's fine for experimental.

@sosukesuzuki
Copy link
Contributor Author

babel/babel#12469 has been merged and Babel 7.13 has been released! Can we merge this?

@JLHwung
Copy link
Contributor

JLHwung commented Feb 25, 2021

ping @RReverser

@RReverser RReverser merged commit dca169a into estree:master Feb 25, 2021
@sosukesuzuki sosukesuzuki deleted the add-js-module-blocks branch February 25, 2021 15:18
@fisker
Copy link

fisker commented Apr 27, 2021

@nzakas Don't you think this will break losts of ESLint plugins? Many rule listen on Program or Program:exit and assume it's the AST root.

@nzakas
Copy link
Contributor

nzakas commented Apr 28, 2021

Yes, that’s a good point, which is why we probably want to go with @RReverser’s suggestion in the final version: #238 (comment)

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.

None yet

5 participants