-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: support #__NO_SIDE_EFFECTS__
annotation for function declaration
#5024
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## master #5024 +/- ##
=======================================
Coverage 98.97% 98.97%
=======================================
Files 222 222
Lines 8180 8205 +25
Branches 2253 2256 +3
=======================================
+ Hits 8096 8121 +25
Misses 30 30
Partials 54 54
|
As mentioned here by @skyrpex it seems that there is an existing |
Exactly. What we care about is about side effects, not whether the function is pure. Eg, a pure function wouldn't read global state, but if we don't use the data it returns and didn't cause any side effect, we can safely remove it. BTW, I love where this is heading ❤️ |
I'd be happy to adapt to |
While I appreciate the notion, I hope you will understand that I would not want to release this with the existing annotation. If we release this, then we do this to allow people to use it, and then we created precedence and basically changed the meaning of PURE even if we did not want to.
I would actually be super happy to support existing annotations that are well-defined, regardless of style. If there are existing code-bases that use these annotations, they would immediately profit from it, and we can always extend annotation styles later. In this case, however, To my limited knowledge of Closure Compiler, this annotation only has an effect in Here is my suggestion how to move forward without being blocked: Taking a leaf out of Closure Compiler's book, So how about
For brevity, I would be leaning towards the latter, but you may have even better ideas. |
#__NO_SIDE_EFFECTS__
annotation for function declaration
I have updated the implementation to use
The reason I chose it over |
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install antfu/rollup#pure-declaration or load it into the REPL: |
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.
Looks great, just some minor 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.
Looks great, just some minor 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.
Awesome, looks great! Will release this by tomorrow if there are no further comments.
If this gets implemented in Terser, I'll make sure to use the same syntax <3 |
This PR has been released as part of rollup@3.24.0. You can test it via |
A side note, if you are using A workaround to that is to add Also, if you'd like to see this supported by esbuild natively, please vote or provide insights in this issue: evanw/esbuild#3149 |
This PR contains:
Are tests included?
Breaking Changes?
Description
Currently, the
#__PURE__
notation only supported on the function callee site, where it's usually functions' nature to be side-effect free. While Rollup already auto check if there would be side-effects inside a function to mark it pure, sometime the detection may not be perfect (#2962), especially for very complex functions.This PR brings the capability of manually marking a function as pure on declarations, by introducing a new annotation syntax
#__NO_SIDE_EFFECTS__
along side#__PURE__
. When using with a function declaration, it automatically makes all calls of the function side-effect free.All the following examples work:
References
Existing Discussions:
Existing workaround:
/*#__PURE__*/
before calleeSolutions from other tools:
Potentially solves:
References:
TODO