Repository navigation
refactor(javascript): analyze the mangler's scopes with ScopeAnalyzer - #22428
Conversation
The JS minifier's mangler had its own port of terser's `figure_out_scope`. It now runs on `ScopeAnalyzer`: a `MinifierScopeAnalyzer` subclass reuses the shared walk, `Scope`/`Variable` model and binding index, and adds only terser's scoping rules. Output is byte-identical. Claude-Session: https://claude.ai/code/session_01Tgxgyb4N35ndP6cC4KcrXC
|
|
This PR is packaged and the instant preview is available (63f2330). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@63f2330
yarn add -D webpack@https://pkg.pr.new/webpack@63f2330
pnpm add -D webpack@https://pkg.pr.new/webpack@63f2330 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds minifier-specific scope analysis to ChangesMinifier scope analysis
Priority: ⬇️ Low Change: Refactor Merge Risk: ⚪ Minimal · up to The change integrates minification with the new scope analyzer, and the available evidence identifies no concrete user-facing failure or merge-blocking risk. Normal test checks remain appropriate. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected path preserves dynamic-name protections and stops processing on analysis errors. No introduced security issue was established, but equivalence with the previous version could not be fully confirmed. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Warning Some tools did not complete. Review the errors below. 🔧 ast-grep (0.45.3)lib/javascript/syntax-printer.jsast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Types CoverageCoverage after merging refactor/minifier-shared-scope-analyzer into main will be
Coverage Report…truncated |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22428 +/- ##
========================================
Coverage 96.01% 96.01%
========================================
Files 772 772
Lines 121639 121757 +118
Branches 39412 39443 +31
========================================
+ Hits 116791 116906 +115
- Misses 4848 4851 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
Merging this PR will improve performance by 34.3%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "css-modules", scenario '{"name":"mode-development","mode":"development"}' |
12.5 MB | 8.9 MB | +40.39% |
| ⚡ | Memory | benchmark "future-defaults", scenario '{"name":"mode-production","mode":"production"}' |
20.3 MB | 15.8 MB | +28.48% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing refactor/minifier-shared-scope-analyzer (9f8bfca) with main (62f7a27)
Footnotes
-
6 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. ↩
Summary
webpack has two scope analyzers:
lib/javascript/ScopeAnalyzer.js, used by module concatenation, and a port of terser's scope analysis inside the JavaScript minifier. This is the first step toward one: the mangler's scope analysis now runs onScopeAnalyzer.ScopeAnalyzer.jsgains opt-in minifier subclasses (MinifierScopeAnalyzer,MinifierScope,MinifierVariable,MinifierLabel) that apply terser's scoping rules and record what the mangler reads. The existing classes and walk are unchanged, and the subclasses reuse the sharedScope/Referencemodel and the shared visit for node kinds both scope the same way.analyzeManglingScopesinsyntax-printer.jsbecomes a thin adapter; the mangler's own scope, definition and label model (about 1,000 lines) is deleted.Map, and a definition's kinds are a bitmask instead of an array.The compressor's scope analysis (
figureOutScope,addChildScope) still uses the terser port; moving it is the next step.Before / after against
main:--max-old-space-size, typescript.js mangleScopeAnalyzeron lodash for concatenation (callgrind)Output is byte-identical to
main: lodash, acorn and typescript under 10 option sets (code, source maps, nameCache,format.ast, spidermonkey output, errors), and 276,961 corpus comparisons (terser, swc, oxc) with 0 mismatches.What kind of change does this PR introduce?
refactor
Did you add tests for your changes?
Yes: the scope parity tests now run against the new model, and one test covers it directly. The existing minifier suites and the concatenation config cases cover the rest.
Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a. Internal refactor with no user-facing change, so no changeset.
Use of AI
Written with Claude Code: it made the changes, ran the differential checks against
mainand measured CPU and memory. The author reviews every commit.🤖 Generated with Claude Code
https://claude.ai/code/session_01Tgxgyb4N35ndP6cC4KcrXC
Generated by Claude Code
Summary by CodeRabbit