fix: forward __NO_SIDE_EFFECTS__ annotations to function expressions in variable declarations#6272
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#no-side-effects-function-expressionNotice: Ensure you have installed the latest nightly Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
There was a problem hiding this comment.
Pull request overview
Fixes Rollup’s Rust AST conversion so /*#__NO_SIDE_EFFECTS__*/ (and JSDoc equivalents) placed before a variable declaration are forwarded to function expression initializers (not just arrow functions), enabling correct tree-shaking.
Changes:
- Forward
NoSideEffectsannotations fromVariableDeclaratortoExpr::Fninitializers in the Rust AST converter. - Extend the existing form test sample to cover function expressions (including async) across the different annotation styles.
- Update the sample’s imports/usages to exercise the additional exported functions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
rust/parse_ast/src/ast_nodes/variable_declarator.rs |
Forwards collected NoSideEffects annotations for Expr::Fn initializers, matching existing behavior for Expr::Arrow. |
test/form/samples/no-side-effects-function-declaration/functions.js |
Adds function-expression variants for the existing annotation styles to validate the forwarding behavior. |
test/form/samples/no-side-effects-function-declaration/main.js |
Imports/calls the new functions so the form test asserts they are tree-shaken when annotated. |
Performance report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6272 +/- ##
=======================================
Coverage 98.76% 98.77%
=======================================
Files 273 273
Lines 10712 10712
Branches 2855 2855
=======================================
+ Hits 10580 10581 +1
Misses 89 89
+ Partials 43 42 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…le declarations (#6272) Handle __NO_SIDE_EFFECTS__ for function expressions in declarations.
|
This PR has been released as part of rollup@4.58.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
When a
/*#__NO_SIDE_EFFECTS__*/annotation (or its JSDoc equivalents@__NO_SIDE_EFFECTS__/#__NO_SIDE_EFFECTS__) precedes aconst/let/vardeclaration, the Rust AST converter needs to forward the annotation from theVariableDeclarationnode to the actual function value in the initializer so thatannotationNoSideEffectsgets set on the function node.Previously, this forwarding only handled
Expr::Arrow(arrow functions). As a result, function expressions (function() {}) assigned to a variable with a leading#__NO_SIDE_EFFECTS__annotation were silently ignored—the annotation was consumed but never attached to theFunctionExpressionnode—so those variables were never tree-shaken.The fix adds
Expr::Fnto the match arm invariable_declarator.rs, so leading#__NO_SIDE_EFFECTS__annotations are also forwarded to function expression initializers.The existing test
no-side-effects-function-declarationhas been extended to cover function expressions alongside arrow functions for all annotation styles (inline, leading block comment, JSDoc@__NO_SIDE_EFFECTS__, and JSDoc#__NO_SIDE_EFFECTS__).Note: inline annotations placed directly before the
functionkeyword (e.g.const fn = /*#__NO_SIDE_EFFECTS__*/ function() {}) already worked correctly and continue to do so, as those comments are attached to theFunctionExpressionnode directly by the parser.