-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Extracted babel-helper-annotate-as-pure #6267
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/4999/ |
a9a83ea
to
9fd5d36
Compare
if (leadingComments === undefined) { | ||
return false; | ||
} | ||
return leadingComments.some(comment => /[@#]__PURE__/.test(comment.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.
I wonder if there's a good way to prevent false positives here (so it doesn't match, say, "#__PURE___
")
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.
actually this test is taken from the UglifyJS' source code, it tests it exactly like this, foo #__PURE__ bar
are also allowed
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.
I think it would be convenient to have an PureAnnotation node in the Babel AST because It would probably be simpler to detect and manage. I assume babel/minify will also take advantage of it during minification.
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.
@xtuc I dont think thats actually possible, because ast -> code -> ast are not inverse operations (in mathematics sense) regarding comments
i.e. if we annotate each CallExpression with a leading comment in fn()()()
we'll end up with nice AST - 1 comment for each node, when we generate code, we'll end up with
/*foo*//*foo*//*foo*/fn()()()
And ofc if we go back to AST again we'll end up with 3 comments on the outermost CallExpression
if (leadingComments === undefined) { | ||
return false; | ||
} | ||
return leadingComments.some(comment => /[@#]__PURE__/.test(comment.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.
I think it would be convenient to have an PureAnnotation node in the Babel AST because It would probably be simpler to detect and manage. I assume babel/minify will also take advantage of it during minification.
This code is extracted from the #6231 and generally already got reviewed. I've split the mentioned PR into 2 because this part is ready and independent, the other one though still needs some discussion and approval.