Repository navigation
fix(javascript): keep what the minifier's fuzzing found it changed - #22570
Merged
Merged
Conversation
- Space a regular expression ending an operand of `in` or `instanceof`. - Keep a sloppy block's function unset outside its block until its statement runs (Annex B), in reduce_vars, sequences and dead code. - Back `compress.arguments` off a parameter a destructuring assigns. - Keep an unused compound write that throws on a BigInt or a parameter. - Keep a computed read converting its key where `pure_conversions` is off. - Parenthesize a destructuring assignment after the first argument, which V8 refuses bare.
|
Contributor
|
This PR is packaged and the instant preview is available (36df072). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@36df072
yarn add -D webpack@https://pkg.pr.new/webpack@36df072
pnpm add -D webpack@https://pkg.pr.new/webpack@36df072 |
github-actions
Bot
added
the
area: parser
Contributor
Types CoverageCoverage after merging fix/minifier-fuzz-findings into main will be
Coverage Report |
Contributor
|
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; 6 remain after this review. 📝 WalkthroughWalkthroughThe compressor preserves selected writes and reads, accounts for Annex B block functions, and avoids rewriting parameters assigned through destructuring. The printer adjusts parentheses and spacing for specific expression forms. ChangesJavaScript syntax and compression
Suggested reviewers: Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The changes address targeted JavaScript compression and printing cases, with no actionable merge-blocking issue established. The PR appears ready for normal checks. 🚥 Pre-merge checks | ✅ 3 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Full details: Title checkExplanation The title uses valid Conventional Commit syntax with the allowed type
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22570 +/- ##
===========================================
+ Coverage 75.17% 96.18% +21.00%
===========================================
Files 594 773 +179
Lines 119952 128159 +8207
Branches 40216 42524 +2308
===========================================
+ Hits 90179 123269 +33090
+ Misses 29773 4890 -24883
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:
|
Contributor
Generated code sizeComparing base (
Read 560 asset(s) changed size, test untouched, biggest 20 by raw or gzip change
14 asset(s) changed size, test edited
109 asset(s) this pull request adds, biggest 20 by raw size
1 runtime(s) changed which runtime modules they carry
21 runtime(s) this pull request adds or no longer builds
Built |
…n engine Bun reads terser's bare `f(a+=1,[b]=c)`, so the case cannot require the reference output to fail where it runs.
…es smaller The Annex B correction keeps the call of a block's empty function whose name is also read outside the block.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A seeded fuzzer (separate PR) found programs whose minified output prints something else or does not parse. terser writes the same output in every case, so each fix is a correction: with
corrections.enabledoff, the output is terser's bytes again.!/a/ instanceof y!/a/instanceof y(SyntaxError)!/a/ instanceof y!function(){ try { throw 0; { function g() {} } } catch (e) { console.log(typeof g) } }()functionundefinedfunction f(c) { if (c) { function g() {} } return typeof g },f(0)"function"undefinedfunction f(p) { [p] = [2]; return arguments[0] }withcompress.arguments,f()2undefinedfunction f(...r) { r *= 1n }f(0, 0)f((a += 1), ([b] = c))f(a+=1,[b]=c)(V8 refuses it)f(a+=1,([b]=c))undefinedoutside the block until its statement runs (Annex B).reduce_vars,sequencesand dead-code extraction no longer assume it is set from the start of the scope. Strict code and modules keep terser's output.compress.argumentsno longer rewritesarguments[i]to a parameter that a destructuring assignment writes.pure_conversions: false, a computed read whose key may be an object now stays, matching what the option already does for writes. The default is unchanged.a += 1, although the spec allows it.One entry in the "mangle exactly as terser does" list now runs with
reduce_vars: false. terser foldstypeofof a block's function there, which the Annex B fix keeps.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes, in
test/unitCases/syntax-printer.unittest.js: fiveCORRECTED_CASESentries, thepure_conversionsinput, and a test that strict and module code still fold a block's function as terser does. Every changed line inlib/javascript/syntax-printer.jsis covered.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. The pending changeset
025-javascript-minifier-corrections.mdalready covers these fixes.Use of AI
Written with Claude Code: it reduced each fuzzer finding to a repro, compared it with terser, wrote the failing tests, then the corrections, and checked coverage. The requester reviewed the results.
🤖 Generated with Claude Code
https://claude.ai/code/session_011CuRTB8fAv8PE7di49qDL4
Generated by Claude Code
Summary by CodeRabbit
inorinstanceof.