fix(es/minifier): Preserve pure IIFE annotations - #12167
fix(es/minifier): Preserve pure IIFE annotations#12167mrlonely (mameikagou) wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: d7b1652 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85c6f73867
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f068ed17b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| let dont_invoke_pure_iife = | ||
| !self.mode.preserve_vars() && self.can_drop_unused_pat(&var.name); |
There was a problem hiding this comment.
Limit IIFE protection to removable patterns
For an unused binding such as let {...rest} = /*#__PURE__*/ function (v) { /* large body */ return v }({}), can_drop_unused_pat returns true because the rest identifier has no usages, so this disables IIFE invocation and adds the persistent no-inline mark. However, take_pat_if_unused immediately preserves every object pattern containing a rest property, meaning the declaration cannot be removed; the condition is recomputed on every pass, so the previously eligible IIFE remains uninlined and retains its full body in minified output. Only enable this protection when unused-pattern cleanup can actually remove the pattern.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
| let dont_invoke_pure_iife = | ||
| !self.mode.preserve_vars() && self.can_drop_unused_pat(&var.name); |
There was a problem hiding this comment.
Check the initializer before scanning every binding
This computes can_drop_unused_pat for every variable declarator, including declarations without an initializer and initializers that are not annotated IIFEs. That helper calls find_pat_ids once through can_drop_unused_vars and then again itself, allocating and walking every binding pattern twice on each optimizer pass even though the result is only needed for the rare pure-IIFE case. Check that var.init is an annotated function/arrow call first, then perform the usage scan only for that case to avoid adding repeated hot-path work to ordinary minification.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
| if self.ctx.bit_ctx.contains(BitCtx::DontInvokePureIife) | ||
| && call.ctxt.has_mark(self.marks.pure) | ||
| { | ||
| log_abort!("iife: Has pure annotation"); | ||
| return false; |
There was a problem hiding this comment.
Preserve throws from unused destructuring
With an unused binding such as let {x} = /*#__PURE__*/ function () { return null }(), this guard preserves the pure call until take_pat_if_unused, which treats the annotation as permission to remove the whole pattern and call. The annotation only says invoking the function has no side effects; destructuring its null result still throws a TypeError, so the optimized program incorrectly continues instead of throwing. Preserve the destructuring operation unless its returned value is known to be safely destructurable.
AGENTS.md reference: crates/swc_ecma_minifier/AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b16528a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| if !matches!( | ||
| expr, | ||
| Expr::Call(..) | Expr::New(..) | Expr::TaggedTpl(..) | Expr::Seq(..) |
There was a problem hiding this comment.
Preserve non-PURE comments when replacing annotated IIFEs
When comments are retained and an annotation is accompanied by another leading comment, such as let dead = /*! license */ /*#__PURE__*/ (() => {})(console.log("effect")), this branch keeps the surviving call's argument span instead of the removed IIFE span. The code generator only takes leading comments at emitted node spans, so both comments remain attached to the now-unreferenced IIFE position and the license/preserve comment disappears. Move or selectively remove the PURE annotation while transferring other comments to the replacement expression.
Useful? React with 👍 / 👎.
|
It seems fine if we don't invoke any IIFE with pure annotation at all. It's very rare to have pure annotations and Terser behaves the same. |
Description:
Pure annotations on unused IIFEs could be lost when the IIFE was inlined before unused-variable cleanup, leaving assignments that dead-code elimination could not remove.
This preserves annotated IIFEs while optimizing initializers whose bindings are already known to be removable. Later cleanup can then remove the call while retaining argument and sequence side effects. Live bindings remain eligible for the existing optimizations.
Related issue (if exists):
Closes #12165
Tests:
cargo test -p swc_ecma_minifier./scripts/exec.sh terser_reduce_vars_iifecargo clippy -p swc_ecma_minifier --features concurrent --tests -- -D warningscargo fmt --all -- --check