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

Dynamic import #5169

Merged
merged 6 commits into from
Mar 20, 2019
Merged

Dynamic import #5169

merged 6 commits into from
Mar 20, 2019

Conversation

helixbass
Copy link
Collaborator

Fixes #4834

@GeoffreyBooth I came across dynamic import() while working on the Prettier plugin and read #4834

This PR adds support for dynamic import() (requiring explicit call parentheses per the apparent consensus in #4834, which I tend to agree with)

It mimics Babel's error message ("import() requires exactly one argument") when supplying less than or more than one argument

It looks like I should do a follow-on PR against ast since Babel appears to use type: 'Import' for dynamic import

src/lexer.coffee Outdated Show resolved Hide resolved
src/lexer.coffee Outdated Show resolved Hide resolved
@@ -240,6 +240,7 @@ grammar =
o 'Super'
o 'This'
o 'SUPER Arguments', -> new SuperCall LOC(1)(new Super), $2, no, $1
o 'DYNAMIC_IMPORT Arguments', -> new DynamicImportCall LOC(1)(new DynamicImport), $2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically it looks like dynamic imports are allowed to be object spread (eg {...import('module')} compiles in Babel)

Added a corresponding test

Copy link
Collaborator

Choose a reason for hiding this comment

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

It evaluates in browsers too:

image

Though it makes no sense, as the import() returns a Promise. Not sure how that’s supposed to interact with object spread, and apparently it doesn’t 😄but still the code compiles even if it’s useless.

@@ -891,7 +893,7 @@ operators = [
['right', 'YIELD']
['right', '=', ':', 'COMPOUND_ASSIGN', 'RETURN', 'THROW', 'EXTENDS']
['right', 'FORIN', 'FOROF', 'FORFROM', 'BY', 'WHEN']
['right', 'IF', 'ELSE', 'FOR', 'WHILE', 'UNTIL', 'LOOP', 'SUPER', 'CLASS', 'IMPORT', 'EXPORT']
['right', 'IF', 'ELSE', 'FOR', 'WHILE', 'UNTIL', 'LOOP', 'SUPER', 'CLASS', 'IMPORT', 'EXPORT', 'DYNAMIC_IMPORT']
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if this is technically necessary but it looked like DYNAMIC_IMPORT belonged at this precedence level (with SUPER, IMPORT and EXPORT)

src/lexer.coffee Outdated Show resolved Hide resolved
GeoffreyBooth and others added 2 commits March 16, 2019 03:47
Co-Authored-By: helixbass <julian@helixbass.net>
@GeoffreyBooth GeoffreyBooth merged commit ff24e5c into jashkenas:master Mar 20, 2019
@GeoffreyBooth
Copy link
Collaborator

Okay, so before we cut a new release we need to update the docs not just to describe import() but to also describe the new policy.

If we’re going to add provisional support for Stage 3 things, class fields could be next.

@jtenner
Copy link

jtenner commented Mar 21, 2019

@GeoffreyBooth This is a pull request years in the making. Thank you!

@helixbass helixbass deleted the dynamic-import branch March 21, 2019 19:03
@voxsoftware
Copy link

voxsoftware commented Mar 23, 2019

when will be this released?

@helixbass
Copy link
Collaborator Author

@GeoffreyBooth do you mind merging latest master into ast? Then I can open a PR for dynamic import AST

@GeoffreyBooth
Copy link
Collaborator

@helixbass Done.

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.

5 participants