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
added basic support for module attributes and tests updated #10962
Changes from 4 commits
7e1a783
b810e31
c35b06e
4243ec1
202af5a
14aae84
8c4ecf9
bbb6020
ba5482e
a038601
cde4f92
4f16179
a264662
c7c2c78
2b5b437
d36d314
0a0958d
f28f6ad
7806809
a3c6a55
65f9f5c
1dacc99
187d0d6
5cb50ed
40ddfab
41bb994
4030426
792d096
a3c56ff
cbcdb3d
e392984
256510b
1ee26a7
2a708a5
f795d36
35a85e9
e3976f2
efacdaf
4202fa9
e1016ad
f89c15f
18dd064
31f6a38
b98824f
1c2be01
180b757
ca2608f
3581a82
b9d2939
24f7830
b1270c6
b7280e2
6e4e730
7a2f4f8
69f153e
522bdc6
2a3e3d0
b1e7ddc
2e243af
f250937
dbf36f6
0ce6e62
9f80885
aa22f1a
967a9be
d06d08b
f051fa1
01933e4
dde6294
907aacd
63b7c44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2031,13 +2031,32 @@ export default class StatementParser extends ExpressionParser { | |||||||||||||||||||||||||
// import '...' | ||||||||||||||||||||||||||
node.specifiers = []; | ||||||||||||||||||||||||||
if (!this.match(tt.string)) { | ||||||||||||||||||||||||||
// check if we have a default import like | ||||||||||||||||||||||||||
// import React from "react"; | ||||||||||||||||||||||||||
const hasDefault = this.maybeParseDefaultImportSpecifier(node); | ||||||||||||||||||||||||||
/* we are checking if we do not have a default import, then it is obvious that we need named imports | ||||||||||||||||||||||||||
* import { get } from "axios"; | ||||||||||||||||||||||||||
* but if we do have a default import | ||||||||||||||||||||||||||
* we need to check if we have a comma after that and | ||||||||||||||||||||||||||
* that is where this `|| this.eat` condition comes into play | ||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||
const parseNext = !hasDefault || this.eat(tt.comma); | ||||||||||||||||||||||||||
// if we do have to parse the next set of specifiers, we first check for star imports | ||||||||||||||||||||||||||
// import React, * from "react"; | ||||||||||||||||||||||||||
const hasStar = parseNext && this.maybeParseStarImportSpecifier(node); | ||||||||||||||||||||||||||
// now we check if we need to parse the next imports | ||||||||||||||||||||||||||
// but only if they are not importing * (everything) | ||||||||||||||||||||||||||
if (parseNext && !hasStar) this.parseNamedImportSpecifiers(node); | ||||||||||||||||||||||||||
this.expectContextual("from"); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
node.source = this.parseImportSource(); | ||||||||||||||||||||||||||
// new proposal | ||||||||||||||||||||||||||
// https://github.com/tc39/proposal-module-attributes | ||||||||||||||||||||||||||
// parse module attributes if the next token is with or ignore and finish the ImportDeclaration node. | ||||||||||||||||||||||||||
const attributes = this.maybeParseModuleAttributes(); | ||||||||||||||||||||||||||
vivek12345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
if (attributes) { | ||||||||||||||||||||||||||
node.attributes = attributes; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
xtuc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
this.semicolon(); | ||||||||||||||||||||||||||
return this.finishNode(node, "ImportDeclaration"); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
@@ -2068,6 +2087,33 @@ export default class StatementParser extends ExpressionParser { | |||||||||||||||||||||||||
node.specifiers.push(this.finishNode(specifier, type)); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
maybeParseModuleAttributes() { | ||||||||||||||||||||||||||
if (!this.eat(tt._with)) { | ||||||||||||||||||||||||||
if (this.hasPlugin("moduleAttributes")) return []; | ||||||||||||||||||||||||||
xtuc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
this.expectPlugin("moduleAttributes"); | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: can we do something like this?
Suggested change
By doing so, we throw the "missing plugin" error pointing at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense. I will update the code 👍 |
||||||||||||||||||||||||||
const attrs = []; | ||||||||||||||||||||||||||
do { | ||||||||||||||||||||||||||
// we are trying to parse a node which has the following syntax | ||||||||||||||||||||||||||
// with type: "json" | ||||||||||||||||||||||||||
// [with -> keyword], [type -> Identifier], [":" -> token for colon], ["json" -> StringLiteral] | ||||||||||||||||||||||||||
const node = this.startNode(); | ||||||||||||||||||||||||||
node.key = this.parseIdentifier(); | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
To align with Property Name, the identifier name should be liberal, that is, import foo from "bar" with for: "test" should be valid. |
||||||||||||||||||||||||||
this.expect(tt.colon); | ||||||||||||||||||||||||||
xtuc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
if (!this.match(tt.string)) { | ||||||||||||||||||||||||||
return this.unexpected( | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
this.state.start, | ||||||||||||||||||||||||||
"Only string literal values are allowed", | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this to the Also, let's give context to the error message:
Suggested change
|
||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
node.value = this.parseLiteral(this.state.value, "StringLiteral"); | ||||||||||||||||||||||||||
this.finishNode(node, "ImportAttribute"); | ||||||||||||||||||||||||||
attrs.push(node); | ||||||||||||||||||||||||||
} while (this.eat(tt.comma)); | ||||||||||||||||||||||||||
return attrs; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
maybeParseDefaultImportSpecifier(node: N.ImportDeclaration): boolean { | ||||||||||||||||||||||||||
if (this.shouldParseDefaultImport(node)) { | ||||||||||||||||||||||||||
// import defaultObj, { x, y as z } from '...' | ||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
import foo from "foo.json" with type: "json"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"throws": "This experimental syntax requires enabling the parser plugin: 'moduleAttributes' (1:32)", | ||
"sourceType": "module", | ||
"plugins": [] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
import "x" | ||
with ({}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @xtuc Although The current implementation allow the following to be parsed. import "x"
with type: "jpeg" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"plugins": [ | ||
"moduleAttributes", | ||
"estree" | ||
], | ||
"sourceType": "module", | ||
"throws": "Unexpected token (2:5)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for reviewers: this is because ASI doesn't apply, since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unexpected token error comes from |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
import "x" with type: "json" | ||
[0] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"plugins": ["moduleAttributes"], | ||
"sourceType": "module" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
{ | ||
"type": "File", | ||
"start": 0, | ||
"end": 32, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 0 | ||
}, | ||
"end": { | ||
"line": 2, | ||
"column": 3 | ||
} | ||
}, | ||
"program": { | ||
"type": "Program", | ||
"start": 0, | ||
"end": 32, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 0 | ||
}, | ||
"end": { | ||
"line": 2, | ||
"column": 3 | ||
} | ||
}, | ||
"sourceType": "module", | ||
"interpreter": null, | ||
"body": [ | ||
{ | ||
"type": "ImportDeclaration", | ||
"start": 0, | ||
"end": 28, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 0 | ||
}, | ||
"end": { | ||
"line": 1, | ||
"column": 28 | ||
} | ||
}, | ||
"specifiers": [], | ||
"source": { | ||
"type": "StringLiteral", | ||
"start": 7, | ||
"end": 10, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 7 | ||
}, | ||
"end": { | ||
"line": 1, | ||
"column": 10 | ||
} | ||
}, | ||
"extra": { | ||
"rawValue": "x", | ||
"raw": "\"x\"" | ||
}, | ||
"value": "x" | ||
}, | ||
"attributes": [ | ||
{ | ||
"type": "ImportAttribute", | ||
"start": 16, | ||
"end": 28, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 16 | ||
}, | ||
"end": { | ||
"line": 1, | ||
"column": 28 | ||
} | ||
}, | ||
"key": { | ||
"type": "Identifier", | ||
"start": 16, | ||
"end": 20, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 16 | ||
}, | ||
"end": { | ||
"line": 1, | ||
"column": 20 | ||
}, | ||
"identifierName": "type" | ||
}, | ||
"name": "type" | ||
}, | ||
"value": { | ||
"type": "StringLiteral", | ||
"start": 22, | ||
"end": 28, | ||
"loc": { | ||
"start": { | ||
"line": 1, | ||
"column": 22 | ||
}, | ||
"end": { | ||
"line": 1, | ||
"column": 28 | ||
} | ||
}, | ||
"extra": { | ||
"rawValue": "json", | ||
"raw": "\"json\"" | ||
}, | ||
"value": "json" | ||
} | ||
} | ||
] | ||
}, | ||
{ | ||
"type": "ExpressionStatement", | ||
"start": 29, | ||
"end": 32, | ||
"loc": { | ||
"start": { | ||
"line": 2, | ||
"column": 0 | ||
}, | ||
"end": { | ||
"line": 2, | ||
"column": 3 | ||
} | ||
}, | ||
"expression": { | ||
"type": "ArrayExpression", | ||
"start": 29, | ||
"end": 32, | ||
"loc": { | ||
"start": { | ||
"line": 2, | ||
"column": 0 | ||
}, | ||
"end": { | ||
"line": 2, | ||
"column": 3 | ||
} | ||
}, | ||
"elements": [ | ||
{ | ||
"type": "NumericLiteral", | ||
"start": 30, | ||
"end": 31, | ||
"loc": { | ||
"start": { | ||
"line": 2, | ||
"column": 1 | ||
}, | ||
"end": { | ||
"line": 2, | ||
"column": 2 | ||
} | ||
}, | ||
"extra": { | ||
"rawValue": 0, | ||
"raw": "0" | ||
}, | ||
"value": 0 | ||
} | ||
] | ||
} | ||
} | ||
], | ||
"directives": [] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import "x" | ||
with | ||
type: "json" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"plugins": ["moduleAttributes"], | ||
"sourceType": "module" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vivek12345 I don't know if I already thanked you, but I love these comments!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @nicolo-ribaudo . Means a lot 🙏