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

Update: support new export syntax #56

Merged
merged 2 commits into from Jun 4, 2020
Merged

Update: support new export syntax #56

merged 2 commits into from Jun 4, 2020

Conversation

mysticatea
Copy link
Member

This PR fixes eslint-scope to support new export * as foo from "source" syntax in ES2020.

The syntax has added the exported property into the ExportAllDeclaration node for as foo. The foo identifier makes no references, so eslint-scope must ignore the exported property.

In more general, eslint-scope should ignore the Export(All|Default|Named)Declaration nodes if the node has their source property (FromClause). Currently eslint-scope does so for only ExportNamedDeclaration, therefore this PR adds Export(All|Default)Declaration, too.

Copy link
Member

@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

ExportDeclaration(node) {
this.visitExportDeclaration(node);
}

ExportAllDeclaration(node) {
this.visitExportDeclaration(node);

Choose a reason for hiding this comment

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

shouldn't this just be empty?
ExportAllDeclaration is specced to always have a source (eg it always exports from an external module and defines no local variables).

I know that visitExportDeclaration does if (node.source) { do nothing }, but why bother with the function call?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I thought that people expect the method visitExportDeclaration to be called on all export declarations because of the name.

@mysticatea mysticatea marked this pull request as ready for review June 4, 2020 11:18
@mysticatea mysticatea merged commit d4a3764 into master Jun 4, 2020
@mysticatea mysticatea deleted the export-star-ns branch June 4, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants