Add dynamic import #834
Add dynamic import #834
Conversation
| @@ -178,6 +178,12 @@ lp.parseStatement = function() { | |||
| return this.parseClass(true) | |||
|
|
|||
| case tt._import: | |||
| if (this.options.ecmaVersion > 10 && this.lookAhead(1).type === tt.parenL) { | |||
There was a problem hiding this comment.
Any chance we could move this into parseExport and do away with the lookAhead (by simply consuming the import token and looking at the next token)?
There was a problem hiding this comment.
That seems to make cases like var dynImport = import; work in acorn loose mode, which probably isn't desirable?
There was a problem hiding this comment.
No, that's not what I mean—inside parseImport, after calling .next() for the import token, you'd first look for (, and parse a dynamic import only if you see it (which would change the signature of parseDynamicImport to require a start position and expect the first token to already be consumed).
test/tests-dynamic-import.js
Outdated
| @@ -0,0 +1,95 @@ | |||
| // Tests for ECMAScript 2020 dynaimic import | |||
There was a problem hiding this comment.
| // Tests for ECMAScript 2020 dynaimic import | |
| // Tests for ECMAScript 2020 dynamic import |
|
|
||
| testFail("var dynImport = import; dynImport('http');", 'Unexpected token (1:22)', { | ||
| ecmaVersion: 11 | ||
| }); |
There was a problem hiding this comment.
Would you add more fail tests?
import();... unexpected token ')'.import(a, b);... unexpected token ','.import(source,);... unexpected token ','.var a = import(source);... unexpected token 'import' ifecmaVersion < 11.
There was a problem hiding this comment.
Updated to consider these as errors and added test cases as well.
|
Thanks for the PR! Please also remove |
|
Updated to remove dynamic import from test 262 unsupported features. |
|
By the way, do you think about estree/estree#197? |
|
Is this PR good to go, other than the conflict on |
|
Yes, I think so. I've merged it as e7c7102 |
|
Wait. ESTree may be going to change the AST of dynamic imports: estree/estree#198 |
|
Ugh, I just tagged 6.2.0 with this AST format. Oh well, we'll see what comes out of that discussion. |
|
@marijnh I think it'd be a wise idea to in fact publish a 6.2.1 that removes the support in the mean time, seeing that no one is probably depending on that at the moment. If we let it sit for any more time, people could start using the current AST format, and that'd be basically impossible to correct. |
|
That would still violate semver, I think—once a release is out there, releasing a patch version that removes features would be weird. We'll need a major version bump when we make ecmaVersion: 10 the default at some point, so I'm fine with doing the AST change at that point. |
This adds dynamic import support to Acorn. I mostly just copied this out of my existing acorn plugin: https://github.com/kesne/acorn-dynamic-import.
Fixes #833, #832.