Conversation
sokra
left a comment
There was a problem hiding this comment.
Also add a test for asynchronous iteration
| for(let propIndex = 0, len = expression.properties.length; propIndex < len; propIndex++) { | ||
| const prop = expression.properties[propIndex]; | ||
| if(prop.type === "SpreadElement") { | ||
| this.walkExpression(prop.argument); |
There was a problem hiding this comment.
I see the test, but for some reason code coverage seem to say it's not covered...
It's would also be great to add a test with ModuleConcatenation to see if the renaming would correctly rename SpreadElements
There was a problem hiding this comment.
It's not that simple. Here we are in walkObjectExpression so it only matches for object spread which is not supported in node 6 and node 8. If I add a test, it would break the CI.
There was a problem hiding this comment.
There is a CI/test filter you can add. Have you seen how we do it with uglifyjs-webpack-plugin?
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
The minimum test ratio has been reached. Thanks! |
|
Thanks |
What kind of change does this PR introduce?
feature
Did you add tests for your changes?
yes
If relevant, link to documentation update:
n/a
Summary
Update the
Parserto suport ES2018 new features.Does this PR introduce a breaking change?
no