-
Notifications
You must be signed in to change notification settings - Fork 787
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
fixes 1000: support ES6 import and export declarations #1149
Conversation
// covering: | ||
// export {default}; // missing fromClause | ||
throwError(lookahead.value ? | ||
Messages.UnexpectedToken : Messages.MissingFromClause, lookahead.value); |
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.
indented weirdly.
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.
that's double tab for a multi-line statement ;)
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.
Sorry, was looking on my phone. Missed it.
@caridy looking great! I had a couple of small comments. Also, can you please sign the jQuery CLA? http://contribute.jquery.org/CLA/ |
7c22c3f
to
52b8969
Compare
declaration = parseStatementListItem(); | ||
return node.finishExportNamedDeclaration(declaration, specifiers, null); | ||
default: | ||
break; |
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.
This default
is unnecessary.
LGTM 👍 |
@mikesherov can you merge this? I'm about to go on vacation for 2 months, and I will like to get this clear before I go this weekend. |
@caridy, will do. |
@caridy Have you signed the CLA? Or perhaps the CLA detection bot did not properly pick that up? |
@ariya I did by email when the transition happened to jquery foundation, and I did it multiple times thru the bot (with n and ñ) it just keep failing on me. Someone will have to do it manually I guess. |
@caridy That's what I expect. We'd just ignore the bot for the time being. Thanks! |
…field This reverts the change landed at benjamn@a4cd3e9 As jquery/esprima#1149 was merged long ago, now the esprima AST is compiliant with the ESTree spec, `id` and `name` fields now longer exist. It's time to unify the specifier definitions. Documentation also available at https://esprima.readthedocs.io/en/4.0/syntax-tree-format.html#import-declaration
…field This reverts the change landed at benjamn@a4cd3e9 As jquery/esprima#1149 was merged long ago, now the esprima AST is compiliant with the ESTree spec, `id` and `name` fields now longer exist. It's time to unify the specifier definitions. Documentation also available at https://esprima.readthedocs.io/en/4.0/syntax-tree-format.html#import-declaration
Bulk of the work porting import and export declaration from harmony (from espree actually) into the master branch.
notes:
sourceType
is a global setting that can bemodule
, which enables import and export declarations. espree is more advanced since it actually turn other features on/off depending on that value, not so sure if esprima wants to have something like that. for now, I make it simple and very restrictive, feel free to refactor it.export default class {}
andexport default function () {}
.tree
case specifying thesourceType
was easy and simple, but having failures to be validated usingsourceType="module"
was a pain, I ended up creating a new type of case calledmodule
to support parsing in module mode but validating the failure. maybe someone can refactor that to make it more generic, maybe adding the type into the failure case when possible.