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 1 commit
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 |
---|---|---|
|
@@ -1460,6 +1460,26 @@ export default class ExpressionParser extends LValParser { | |
return this.finishNode(node, "TemplateLiteral"); | ||
} | ||
|
||
parseKeyValuePairs() { | ||
const propHash: any = Object.create(null); | ||
const node = this.startNode(); | ||
node.properties = []; | ||
|
||
do { | ||
const prop = this.parseObjectMember(); | ||
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. It maybe overkill to use import foo from "foo" with loaders: ["bar", "qux"] are not allowed because array is not a primitive type. As a stage-1 proposal, I suggest we implement the minimal superset of proposal. Should the proposal accepts more types, we can always drops restrictions later. 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. Understood. I will write a simple parseKeyValue function. Thank you 👍 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. Refactored with the suggested changes 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. We aren't sure if we want to allow any value there, let's keep it simple for now I think; only stringliterals 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. Made the changes and now we are only allowing string literals. |
||
// $FlowIgnore | ||
if (prop.shorthand) { | ||
this.addExtra(prop, "shorthand", true); | ||
} | ||
|
||
node.properties.push(prop); | ||
} while (this.eat(tt.comma)); | ||
if (!this.match(tt.eq) && propHash.start !== undefined) { | ||
this.raise(propHash.start, "Redefinition of __proto__ property"); | ||
} | ||
return this.finishNode(node, "ObjectExpression"); | ||
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. I don't think that module attributes allow arbitrary values (I might be wrong). I think that this should be simplified to (pseudo-JavaScript) const attrs = [];
do {
const node = startNode();
parseIdentifier();
expect(":");
parseLiteral();
} while (eat(","));
return attrs; // Return the array of attributes, not a wrapper node. It could also be inlined in 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 looks way better and simpler. I will make the changes. Thank you for the inputs 👍 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. few questions here, should the above implementation look something like this const attrs = [];
do {
const node = this.startNode();
node.key = this.parseIdentifier();
node.computed = false;
node.method = false;
node.shorthand = false;
this.expect(tt.colon);
node.value = this.parseLiteral(this.state.value);
this.finishNode(node, "ObjectProperty");
attrs.push(node);
} while (this.eat(tt.comma)); One thing that I am able to notice here is that parseLiteral would also need a type value which I need to deduce somehow? I don't think I should be using a 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. or should we just do node.value = this.parseLiteral(this.state.value, "Literal"); 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 second
A primitive value is a member of one of the following built-in types: Undefined, Null, Boolean, Number, String, and Symbol. So you can guard against
Note that 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. These changed have been updated. Thank you 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. I'd recommend starting with just supporting string literals at this point. 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. Made the changes and now we are only supporting String Literals. |
||
} | ||
|
||
// Parse an object literal or binding pattern. | ||
|
||
parseObj<T: N.ObjectPattern | N.ObjectExpression>( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2005,13 +2005,26 @@ 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 comes into play | ||
*/ | ||
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. @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 commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @nicolo-ribaudo . Means a lot 🙏 |
||
const parseNext = !hasDefault || this.eat(tt.comma); | ||
// if we do have to parse the next set of specifiers, we first check for start imports | ||
// import React, * from "react"; | ||
const hasStar = parseNext && this.maybeParseStarImportSpecifier(node); | ||
// now we check that if we need to parse the next imports | ||
// but only if he/she is not importing * which means all | ||
vivek12345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (parseNext && !hasStar) this.parseNamedImportSpecifiers(node); | ||
this.expectContextual("from"); | ||
} | ||
node.source = this.parseImportSource(); | ||
node.attributes = this.maybeParseModuleAttributes(); | ||
this.semicolon(); | ||
return this.finishNode(node, "ImportDeclaration"); | ||
} | ||
|
@@ -2042,6 +2055,15 @@ export default class StatementParser extends ExpressionParser { | |
node.specifiers.push(this.finishNode(specifier, type)); | ||
} | ||
|
||
maybeParseModuleAttributes() { | ||
if (this.match(tt._with) && !this.hasPlugin("moduleAttributes")) { | ||
this.expectPlugin("moduleAttributes"); | ||
} else if (this.eat(tt._with)) { | ||
return this.parseKeyValuePairs(); | ||
} | ||
return []; | ||
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. I think that we should do something like this:
By doing so, we don't return an empty array if the plugin is disabled (since disabled plugins shouldn't have any effect on the AST). 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. Understood. That makes sense. I will make the changes accordingly. 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. updated the code with these changes. |
||
} | ||
|
||
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 |
---|---|---|
|
@@ -128,7 +128,8 @@ | |
"raw": "'foobar'" | ||
}, | ||
"value": "foobar" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,6 +114,7 @@ | |
}, | ||
"value": "bar" | ||
}, | ||
"attributes": [], | ||
"trailingComments": [ | ||
{ | ||
"type": "CommentBlock", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,7 +115,8 @@ | |
"raw": "\"bar\"" | ||
}, | ||
"value": "bar" | ||
} | ||
}, | ||
"attributes": [] | ||
}, | ||
{ | ||
"type": "ExportNamedDeclaration", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,7 +138,8 @@ | |
"raw": "\"x\"" | ||
}, | ||
"value": "x" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,7 +181,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,7 +118,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,7 +118,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,7 +116,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,7 +116,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,7 +101,8 @@ | |
"raw": "\"acorn\"" | ||
}, | ||
"value": "acorn" | ||
} | ||
}, | ||
"attributes": [] | ||
}, | ||
"alternate": null | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,8 @@ | |
"raw": "'baz'" | ||
}, | ||
"value": "baz" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"\"" | ||
}, | ||
"value": "" | ||
} | ||
}, | ||
"attributes": [] | ||
}, | ||
{ | ||
"type": "ExportNamedDeclaration", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,8 @@ | |
"raw": "\"jquery\"" | ||
}, | ||
"value": "jquery" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"jquery\"" | ||
}, | ||
"value": "jquery" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,8 @@ | |
"raw": "\"crypto\"" | ||
}, | ||
"value": "crypto" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,8 @@ | |
"raw": "\"crypto\"" | ||
}, | ||
"value": "crypto" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -194,7 +194,8 @@ | |
"raw": "\"crypto\"" | ||
}, | ||
"value": "crypto" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,8 @@ | |
"raw": "\"bar\"" | ||
}, | ||
"value": "bar" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"crypto\"" | ||
}, | ||
"value": "crypto" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,7 +145,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,7 +128,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"jquery\"" | ||
}, | ||
"value": "jquery" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,8 @@ | |
"raw": "\"foo\"" | ||
}, | ||
"value": "foo" | ||
} | ||
}, | ||
"attributes": [] | ||
} | ||
], | ||
"directives": [] | ||
|
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.
Nit: undo