Skip to content

Update dynamic imports AST #844

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.

Sign up for GitHub

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 4 commits into from
Aug 12, 2019
Merged

Update dynamic imports AST #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14246be
Update dynamic imports AST
mysticatea Jul 12, 2019
3714d48
remove tt.ellipsis handling
mysticatea Aug 12, 2019
18ace9d
remove new argument
mysticatea Aug 12, 2019
e3bf151
improve "verify ending" logic
mysticatea Aug 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions acorn-loose/src/expression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ lp.parseExprAtom = function() {
return this.parseTemplate()

case tt._import:
if (this.options.ecmaVersion > 10) {
return this.parseDynamicImport()
if (this.options.ecmaVersion >= 11) {
return this.parseExprImport()
} else {
return this.dummyIdent()
}
Expand All @@ -308,10 +308,21 @@ lp.parseExprAtom = function() {
}
}

lp.parseDynamicImport = function() {
lp.parseExprImport = function() {
const node = this.startNode()
this.next()
return this.finishNode(node, "Import")
this.next() // skip `import`
switch (this.tok.type) {
case tt.parenL:
return this.parseDynamicImport(node)
default:
node.name = "import"
return this.finishNode(node, "Identifier")
}
}

lp.parseDynamicImport = function(node) {
node.source = this.parseExprList(tt.parenR)[0] || this.dummyString()
return this.finishNode(node, "ImportExpression")
}

lp.parseNew = function() {
Expand Down
53 changes: 32 additions & 21 deletions acorn/src/expression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ pp.parseSubscript = function(base, startPos, startLoc, noCalls, maybeAsyncArrow)
this.yieldPos = 0
this.awaitPos = 0
this.awaitIdentPos = 0
let exprList = this.parseExprList(tt.parenR, this.options.ecmaVersion >= 8 && base.type !== "Import", false, refDestructuringErrors)
let exprList = this.parseExprList(tt.parenR, this.options.ecmaVersion >= 8, false, refDestructuringErrors)
if (maybeAsyncArrow && !this.canInsertSemicolon() && this.eat(tt.arrow)) {
this.checkPatternErrors(refDestructuringErrors, false)
this.checkYieldAwaitInDefaultParams()
Expand All @@ -299,16 +299,6 @@ pp.parseSubscript = function(base, startPos, startLoc, noCalls, maybeAsyncArrow)
let node = this.startNodeAt(startPos, startLoc)
node.callee = base
node.arguments = exprList
if (node.callee.type === "Import") {
if (node.arguments.length !== 1) {
this.raise(node.start, "import() requires exactly one argument")
}

const importArg = node.arguments[0]
if (importArg && importArg.type === "SpreadElement") {
this.raise(importArg.start, "... is not allowed in import()")
}
}
base = this.finishNode(node, "CallExpression")
} else if (this.type === tt.backQuote) {
let node = this.startNodeAt(startPos, startLoc)
Expand Down Expand Up @@ -420,8 +410,8 @@ pp.parseExprAtom = function(refDestructuringErrors) {
return this.parseTemplate()

case tt._import:
if (this.options.ecmaVersion > 10) {
return this.parseDynamicImport()
if (this.options.ecmaVersion >= 11) {
return this.parseExprImport()
} else {
return this.unexpected()
}
Expand All @@ -431,13 +421,34 @@ pp.parseExprAtom = function(refDestructuringErrors) {
}
}

pp.parseDynamicImport = function() {
pp.parseExprImport = function() {
const node = this.startNode()
this.next()
if (this.type !== tt.parenL) {
this.next() // skip `import`
switch (this.type) {
case tt.parenL:
return this.parseDynamicImport(node)
default:
this.unexpected()
}
return this.finishNode(node, "Import")
}

pp.parseDynamicImport = function(node) {
this.next() // skip `(`

// Parse node.source.
node.source = this.parseMaybeAssign()

// Verify ending.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adjusting the ellipses thing. Any reason the 8 lines below can't simply be this.expect(tt.parenR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to show a detailed message for trailing commas because import(s) is similar to call expressions, but doesn't support trailing commas. I.e., it handles import(s,) as special, but not import(a,b) and import(s!.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved the logic to the errored code path.

if (!this.eat(tt.parenR)) {
const errorPos = this.start
if (this.eat(tt.comma) && this.eat(tt.parenR)) {
this.raiseRecoverable(errorPos, "Trailing comma is not allowed in import()")
} else {
this.unexpected(errorPos)
}
}

return this.finishNode(node, "ImportExpression")
}

pp.parseLiteral = function(value) {
Expand Down Expand Up @@ -547,12 +558,12 @@ pp.parseNew = function() {
this.raiseRecoverable(node.start, "new.target can only be used in functions")
return this.finishNode(node, "MetaProperty")
}
let startPos = this.start, startLoc = this.startLoc
let startPos = this.start, startLoc = this.startLoc, isImport = this.type === tt._import
node.callee = this.parseSubscripts(this.parseExprAtom(), startPos, startLoc, true)
if (this.options.ecmaVersion > 10 && node.callee.type === "Import") {
this.raise(node.callee.start, "Cannot use new with import(...)")
if (isImport && node.callee.type === "ImportExpression") {
this.raise(startPos, "Cannot use new with import()")
}
if (this.eat(tt.parenL)) node.arguments = this.parseExprList(tt.parenR, this.options.ecmaVersion >= 8 && node.callee.type !== "Import", false)
if (this.eat(tt.parenL)) node.arguments = this.parseExprList(tt.parenR, this.options.ecmaVersion >= 8, false)
else node.arguments = empty
return this.finishNode(node, "NewExpression")
}
Expand Down
161 changes: 142 additions & 19 deletions test/tests-dynamic-import.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@ test(
start: 0,
end: 26,
expression: {
type: 'CallExpression',
type: 'ImportExpression',
start: 0,
end: 26,
callee: { type: 'Import', start: 0, end: 6 },
arguments: [
{
type: 'Literal',
start: 7,
end: 25,
value: 'dynamicImport.js',
raw: "'dynamicImport.js'"
}
]
source: {
type: 'Literal',
start: 7,
end: 25,
value: 'dynamicImport.js',
raw: "'dynamicImport.js'"
}
}
}
],
Expand All @@ -38,6 +35,49 @@ test(
{ ecmaVersion: 11 }
);

// Assignment is OK.
test(
"import(a = 'dynamicImport.js')",
{
"type": "Program",
"start": 0,
"end": 30,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 30,
"expression": {
"type": "ImportExpression",
"start": 0,
"end": 30,
"source": {
"type": "AssignmentExpression",
"start": 7,
"end": 29,
"operator": "=",
"left": {
"type": "Identifier",
"start": 7,
"end": 8,
"name": "a"
},
"right": {
"type": "Literal",
"start": 11,
"end": 29,
"value": "dynamicImport.js",
"raw": "'dynamicImport.js'"
}
}
}
}
],
"sourceType": "script"
},
{ ecmaVersion: 11 }
);

test(
"function* a() { yield import('http'); }",
{
Expand Down Expand Up @@ -69,11 +109,10 @@ test(
end: 36,
delegate: false,
argument: {
type: 'CallExpression',
type: 'ImportExpression',
start: 22,
end: 36,
callee: { type: 'Import', start: 22, end: 28 },
arguments: [{ type: 'Literal', start: 29, end: 35, value: 'http', raw: "'http'" }]
source: { type: 'Literal', start: 29, end: 35, value: 'http', raw: "'http'" }
}
}
}
Expand All @@ -86,6 +125,85 @@ test(
{ ecmaVersion: 11 }
);

// `new import(s)` is syntax error, but `new (import(s))` is not.
test(
"new (import(s))",
{
"type": "Program",
"start": 0,
"end": 15,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 15,
"expression": {
"type": "NewExpression",
"start": 0,
"end": 15,
"callee": {
"type": "ImportExpression",
"start": 5,
"end": 14,
"source": {
"type": "Identifier",
"start": 12,
"end": 13,
"name": "s"
}
},
"arguments": []
}
}
],
"sourceType": "script"
},
{ ecmaVersion: 11 }
);

// `import(s,t)` is syntax error, but `import((s,t))` is not.
test(
"import((s,t))",
{
"type": "Program",
"start": 0,
"end": 13,
"body": [
{
"type": "ExpressionStatement",
"start": 0,
"end": 13,
"expression": {
"type": "ImportExpression",
"start": 0,
"end": 13,
"source": {
"type": "SequenceExpression",
"start": 8,
"end": 11,
"expressions": [
{
"type": "Identifier",
"start": 8,
"end": 9,
"name": "s"
},
{
"type": "Identifier",
"start": 10,
"end": 11,
"name": "t"
}
]
}
}
}
],
"sourceType": "script"
},
{ ecmaVersion: 11 }
);

testFail('function failsParse() { return import.then(); }', 'Unexpected token (1:37)', {
ecmaVersion: 11,
loose: false
Expand All @@ -102,27 +220,32 @@ testFail("import('test.js')", 'Unexpected token (1:6)', {
sourceType: 'module'
});

testFail("import()", 'import() requires exactly one argument (1:0)', {
testFail("import()", 'Unexpected token (1:7)', {
ecmaVersion: 11,
loose: false
});

testFail("import(a, b)", 'Unexpected token (1:8)', {
ecmaVersion: 11,
loose: false
});

testFail("import(a, b)", 'import() requires exactly one argument (1:0)', {
testFail("import(...[a])", 'Unexpected token (1:7)', {
ecmaVersion: 11,
loose: false
});

testFail("import(...[a])", '... is not allowed in import() (1:7)', {
testFail("import(source,)", 'Trailing comma is not allowed in import() (1:13)', {
ecmaVersion: 11,
loose: false
});

testFail("import(source,)", 'Unexpected token (1:14)', {
testFail("new import(source)", 'Cannot use new with import() (1:4)', {
ecmaVersion: 11,
loose: false
});

testFail("new import(source)", 'Cannot use new with import(...) (1:4)', {
testFail("(import)(s)", 'Unexpected token (1:7)', {
ecmaVersion: 11,
loose: false
});