feat: align tree shaking behavior with webpack - #15397
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea717e5bef
ℹ️ 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 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 address that feedback".
📦 Binary Size-limit
❌ Size increased by 108.13KB from 68.30MB to 68.40MB (⬆️0.15%) |
Rsdoctor Bundle Diff Analysis
Found 5 projects in monorepo, 5 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 popular-libsPath:
📦 Download Diff Report: popular-libs Bundle Diff 📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff 📁 ui-componentsPath:
📦 Download Diff Report: ui-components Bundle Diff 🤖 AI Degradation Analysis (Click to expand)📁 popular-libs📊 Size Changes
🔍 Root Cause Analysis
|
Merging this PR will degrade performance by 3.04%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | rust@create_chunk_hashes |
1.6 ms | 1.7 ms | -4.7% |
| ❌ | Simulation | rust@scan_dependencies@three_module |
12.9 ms | 13.3 ms | -3.39% |
| ❌ | Simulation | bundle@misc-production-sourcemap |
3.6 s | 3.7 s | -2.53% |
| ❌ | Simulation | bundle@misc-development |
2.8 s | 2.8 s | -2.5% |
| ❌ | Simulation | rust@create_full_hash |
2.3 ms | 2.3 ms | -2.07% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing seal/tree-shaking-bundler-audit (7a941e4) with main (f281cc9)2
Footnotes
-
47 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(e434f1f) during the generation of this report, so f281cc9 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5be836198b
ℹ️ 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 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 address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Exclude reassigned declarations from automatic purity
In production configurations where automatic pure-function analysis is enabled, this new path marks a function declaration as side-effect-free without consulting the written set used by the annotation pass. Thus function f() {}; f = () => effect(); export const unused = f(); still classifies the final call as pure and can remove effect(). Apply the same write invalidation to automatically inferred function declarations, including exported declarations.
ℹ️ 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 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 address that feedback".
|
Addressed the additional review-summary finding in 7a941e4: automatic purity inference now excludes top-level function bindings recorded as written, including exported function declarations. I also added regression coverage for ordinary and exported declarations reassigned to effectful functions. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a941e4095
ℹ️ 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 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 address that feedback".
| } else if analyze_side_effects_free | ||
| && let Some(callee_expr) = callee.as_expr() | ||
| && let Some(imported_callee) = imported_pure_callee(parser, callee_expr) |
There was a problem hiding this comment.
When a namespace member call participates in a cycle, treating it as removable based only on the target function's purity can suppress a required TDZ error. For example, if entry module B imports A before initializing export const f = () => 1, while A evaluates import * as ns from "./b"; export const unused = ns.f();, ns.f must throw because f is still uninitialized; this new branch defers only the function-purity check and can remove the entire call when unused is unused. Keep namespace/member accesses observable unless the import is known to have completed initialization.
Useful? React with 👍 / 👎.
1 participant
Summary
Object.definePropertyreexports, unused side-effect-freerequire()calls, and exported-functionthishandlingNO_SIDE_EFFECTSannotationssideEffectsresolution, glob rebasing, symlink handling, and star-reexport graph reconnect behavioroutput.environment.spreadtarget/default/runtime handling and port the corresponding regression fixturesRelated links
Validation
pnpm run test:unit(Rspack: 9280 passed, 4 skipped; CLI: 90 passed, 1 skipped; binding type tests passed)cargo clippy --workspace --all-targets --tests --locked -- -D warningspnpm run format:rspnpm run format:jscargo fmt --all --checkgit diff --checkChecklist