Repository navigation
fix(javascript): keep a sloppy block's function in its block where Annex B does not hoist it in the minifier - #22598
Conversation
…nex B does not hoist it in the minifier Annex B hoists a block's function only where a `var` of its name is no early error, but every one was hoisted, so the minifier threw `"f" is redeclared` on a later `let f`, dropped a read that throws and mangled a block's own call apart. A `let`, `const`, class, `for` head, `catch` pattern or parameter of its name now keeps it in its block.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk was identified in the changed sloppy block-function handling. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title uses the required Conventional Commit form, fix(javascript): subject, and accurately describes the sloppy block-function minifier fix. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
coderabbitai
Bot
added
the
area: parser
…`let` of its name in a `catch` body A `catch` body's statements sit in the catch scope with no block of their own, so a `let` there was missed and the function still hoisted. Also covers labeled, `while`, `do` and `with` bodies, arrow, generator and async functions, renamed, defaulted, nested and `for-of` patterns, an inlined function, `toplevel`, and a `let` in an outer function, where the function still hoists.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22598 +/- ##
==========================================
+ Coverage 95.76% 96.23% +0.46%
==========================================
Files 774 773 -1
Lines 128803 128616 -187
Branches 42745 42589 -156
==========================================
+ Hits 123351 123770 +419
+ Misses 5452 4846 -606
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:
|
|
Reported by Types Coverage #1180 for 03032f7, computed by this pull request's own code. Types CoverageCoverage after merging fix/minify-annex-b-skipped-block-function into main will be
Coverage Report |
|
Reported by Code Size #4393 for 03032f7, computed by this pull request's own code. Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
…n after its block in the minifier
Annex B sets a sloppy block's function's `var` where its block runs, but the minifier fixed the name to another function or `var` of it, so `{ function f() { return 1 } } f(); function f() { return 2 }` returned 2 and collapsing moved `var f = 2` past the block. 8 test262 Annex B files now pass.
|
This PR is packaged and the instant preview is available (86da462). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@86da462
yarn add -D webpack@https://pkg.pr.new/webpack@86da462
pnpm add -D webpack@https://pkg.pr.new/webpack@86da462 |
Summary
Summary:
Annex B hoists a sloppy block's function only where a var of its name is no early error, and sets that var only when its block runs. Every one was hoisted and the name read as fixed, so a let, const, class, for head, catch pattern, parameter or other function of that name made the minifier throw or misprint.
Shapes swept: 60/60 right (uglify-js 39, oxc 10, terser 8); test262 annexB: 777/784 from 673, no regression; 21 bundles: 0 bytes. V8's reading of an outer same-name block function stays.
What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes, test/unitCases/syntax-printer.unittest.js: each declaration kind and option set, the code main threw on, and where Annex B still hoists.
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
Use of AI
I used Claude Code to find the failing test262 cases, compare minifiers, write the fix and tests. I reviewed every change myself.
Summary by CodeRabbit