Skip to content

Add ImportExpression node type (ES2020) #365

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 5 commits into from
Mar 27, 2020

Conversation

mkubilayk
Copy link
Contributor

ES2020 grammar has ImportExpression node type for dynamic import(). Based on ESTree spec, this PR adds support for it in ast-types.

@mkubilayk
Copy link
Contributor Author

Hi @benjamn - I'd appreciate if we can get this in. Do you have time to take a look?

@GeeWizWow
Copy link

GeeWizWow commented Mar 17, 2020

@benjamn Hi, any plans to merge this CL ?

Copy link
Owner

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @mkubilayk!

I'm going to release a patch version with this change, and then a minor version after consolidating the def/es* modules into a single def/esNext module, so that we can stop worrying about the versions/years.

@benjamn benjamn merged commit b180525 into benjamn:master Mar 27, 2020
@benjamn
Copy link
Owner

benjamn commented Mar 27, 2020

Please note that Babel already had an Import node that was used as the callee of a CallExpression to represent dynamic import(...) calls. That was changed without much discussion in estree/estree#197, which was shipped along with estree/estree#198. I must be missing some context around how changes like these get approved and shipped, but for now ast-types can just support both versions.

benjamn added a commit to benjamn/recast that referenced this pull request Mar 27, 2020
@mkubilayk mkubilayk deleted the import-expression branch March 30, 2020 07:38
@mkubilayk
Copy link
Contributor Author

Thanks for merging this @benjamn!

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.

3 participants