Skip to content

perf(build): skip gzip for node_modules files in the size report - #4926

Merged
pi0 merged 3 commits into
nitrojs:mainfrom
onmax:perf/fs-tree-skip-node-modules-gzip
Oct 26, 2026
Merged

pi0 merged 3 commits into
nitrojs:mainfrom
onmax:perf/fs-tree-skip-node-modules-gzip

Conversation

@onmax

@onmax onmax commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

🔗 Linked issue

None. Follow-up to #4567 and #4568, which touched the same report.

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

With logging.compressedSizes on (the default), Nitro gzips every server output file to print the size report. The report never lists node_modules files. It only adds them to the total. They are most of the output, so most of the gzip time goes into one number.

This PR stops gzipping node_modules files. They still count in the total size, and the total gzip now says what it covers:

Σ Total size: 62.1 MB (20.2 MB gzip)                          # before
Σ Total size: 62.1 MB (952 kB gzip, excluding node_modules)   # after

Without node_modules in the output, the log does not change. On main, only the rollup and rolldown builders print this report.

I timed generateFSTree in a Nuxt 4 app with 149 traced packages (57 MB in node_modules):

wall CPU
before, 4 builds 1.6 to 1.9 s 4.7 to 5.4 s
after, 4 builds (same change patched into nitropack 2.13.5) 0.2 to 0.4 s 0.7 to 1.4 s

I also tried gzip level 6 (half the time), level 1 (sizes change by 10 to 15%) and worker threads (no gain, zlib.gzip already uses the thread pool). Skipping node_modules saves the most and keeps the listed sizes exact.

A new unit test checks that only listed files get gzipped.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@onmax
onmax requested a review from pi0 as a code owner October 25, 2026 19:50
@vercel

vercel Bot commented Oct 25, 2026

Copy link
Copy Markdown

@onmax is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4fa3bc06-19b8-4f41-a33c-55f18efa8d5c
Commits

Reviewing files that changed from the base of the PR and between cfce9cb and c7f928f.

Files selected for processing: 2
  • src/utils/fs-tree.ts
  • test/unit/fs-tree.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

generateFSTree enables gzip-size reporting only when requested and no discovered path contains node_modules. Node_modules files still contribute to uncompressed totals. Unit tests cover displayed sizes and skipped gzip calculations.

Changes: Gzip size reporting

Layer / File(s) Summary
Gzip reporting condition and tests
src/utils/fs-tree.ts, test/unit/fs-tree.test.ts
generateFSTree calculates and displays gzip sizes only when requested and no discovered path contains node_modules. Node_modules files still contribute to uncompressed totals. Tests check size output and verify that gzip sizing is skipped when a dependency path is present.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: Minimal · up to c7f92

The change is mergeable with no identified correctness or availability risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title follows Conventional Commits syntax with the perf(build): prefix and accurately describes the gzip optimization for node_modules files.
Description check Passed The description clearly explains the performance change, preserved size-report behavior, timing results, and added unit test.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/nitro@4926

commit: c7f928f

Comment thread src/utils/fs-tree.ts Resolved

@pi0 pi0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thnx!

@pi0
pi0 merged commit 24479b8 into nitrojs:main Oct 26, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants