Repository navigation
fix(javascript): keep the comments of an empty statement the SyntaxTree printer drops - #22649
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The printer now preserves retained comments when dropping empty statements. No actionable merge-blocking risk is established for this change. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
The title follows the required Conventional Commit form: fix(javascript): subject. The branch prefix is not provided, so the required type match cannot be verified. |
Provide the branch name or confirm that its prefix is fix. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. 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.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 ast-grep (0.45.3)
-
lib/javascript/syntax-printer.js
-
ast-grep timed out on this file
-
🔧 ESLint
-
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
lib/javascript/syntax-printer.js
-
ESLint failed to execute (timeout).
-
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
area: parser
Types CoverageCoverage after merging fix/javascript-tree-print-empty-statement-comments into main will be
Coverage Report |
|
This PR is packaged and the instant preview is available (703b88d). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@703b88d
yarn add -D webpack@https://pkg.pr.new/webpack@703b88d
pnpm add -D webpack@https://pkg.pr.new/webpack@703b88d |
…ee printer drops Ported from #22649 so this branch's unit job passes on the current main.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22649 +/- ##
==========================================
+ Coverage 96.28% 96.31% +0.03%
==========================================
Files 774 774
Lines 130216 131988 +1772
Branches 43417 44090 +673
==========================================
+ Hits 125376 127126 +1750
- Misses 4840 4862 +22
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 |
…eal types (#22642) * refactor(types): replace EXPECTED_OBJECT and EXPECTED_FUNCTION with real types Type loader functions, HMR module factories and self-accept handlers, guard frames, syntax-printer walker callbacks, source maps and parse options, and the CSS/HTML print options instead of object/Function. * refactor(types): use real types instead of unknown and Record<string, unknown> Type the minimizer options, transform switches, profiler parameters, CLI configuration values, HMR handlers and the deprecated externals function precisely. * refactor(types): let transform callbacks return a splice or the skip marker * refactor(types): let transform callbacks return null * refactor(types): type the self-accept error as unknown, as catch reads it * refactor(types): type parseTree's options as terser's parse options * fix(javascript): keep the comments of an empty statement the SyntaxTree printer drops Ported from #22649 so this branch's unit job passes on the current main.
…e minifier's source map (#22654) * fix(javascript): leave the enclose wrapper's unnamed source out of the minifier's source map terser adds a null entry to sources for the wrapper enclose parses from no file, which no mapping references and which Rollup rejects (terser#1333). * fix(javascript): keep the comments of an empty statement the SyntaxTree printer drops Same change as #22649, which main needs for the syntax-printer suite to pass.
Summary
mainfailssyntax-printer › correct phase › should keep the comments of an empty statement it drops. #22628 taught the old printer to keep a dropped;'s comments, and #22643 (tested on an older base) added the SyntaxTree printer without it. The tree printer now does the same.minify(input, { compress: false, mangle: false, format: { comments: "all" } }):Code and source map match the old path byte for byte on 4 inputs × 3
commentssettings.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
The existing test from #22628 covers it; it fails on
mainand passes here.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
Claude Code found the failure, wrote the fix and compared it with the old path under the author's direction. The author reviewed the result.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Tgxgyb4N35ndP6cC4KcrXC
Generated by Claude Code
Summary by CodeRabbit