Skip to content

Conversation

@pi0
Copy link
Member

@pi0 pi0 commented Dec 9, 2025

With introduction of bundling by default in next release, libs (node_module dependnecies) are bundled by default which is more efficient thant tracing as libs are tree-shaken but making debugging harder with chunk groups containing both user code and libraries.

One solution is to simply use better chunk names but it still has issue of mixed code. This PR uses chunk groups to explicitly seperate each bundled dependency by name.

This change will also be useful in the future for low-effort bundle analysis.

@vercel
Copy link

vercel bot commented Dec 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
nitro.build Ready Ready Preview Comment Dec 10, 2025 4:49pm

@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Warning

Rate limit exceeded

@pi0 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 55 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c15f7bd and 1fdb9d1.

📒 Files selected for processing (2)
  • src/build/rolldown/config.ts (4 hunks)
  • src/build/rollup/config.ts (3 hunks)
📝 Walkthrough

Walkthrough

Refactors chunk naming: adds NODE_MODULES_RE and libChunkName, changes getChunkName to accept a chunk object and expands routing for libs, runtime, WASM, SSR, build/runtime/routes/tasks, and updates build configs to group node_modules via advancedChunks/manualChunks.

Changes

Cohort / File(s) Change Summary
Core chunk naming and routing
src/build/chunks.ts
Added exported NODE_MODULES_RE and libChunkName. Reworked getChunkName signature to getChunkName(chunk: { name: string; moduleIds: string[] }, nitro) and expanded routing (handles _libs/, rolldown-runtime, WASM, SSR, _build, runtime/presets, routes, tasks, virtual/raw chunks). Removed parseNodeModulePath usage and simplified default chunk path to _chunks/[name].mjs.
Rolldown build config
src/build/rolldown/config.ts
Switched chunkFileNames usage to getChunkName(chunk, nitro). Added advancedChunks grouping for node_modules using NODE_MODULES_RE and libChunkName. Removed sanitizeFilePath/sanitize option. Adjusted types and conditional removal of advancedChunks for iife output.
Rollup build config
src/build/rollup/config.ts
Updated imports to use libChunkName and NODE_MODULES_RE. Changed chunk filename generation to getChunkName(chunk, nitro). Added manualChunks hook to group node_modules via libChunkName. Removed sanitizeFilePath. Added explicit typing and conditional removal of manualChunks for iife.
Vite/Rollup integration
src/build/vite/rollup.ts
Replaced getChunkName(nitro, chunk.moduleIds) with getChunkName(chunk, nitro). When using Rolldown, adds advancedChunks with NODE_MODULES_RE/libChunkName; otherwise sets manualChunks for node_modules. Removed sanitizeFilePath import and adjusted chunking strategy based on build mode.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay extra attention to getChunkName decision branches handling virtual vs raw moduleIds and the newly added chunk-type checks (WASM, SSR, runtime, routes, tasks).
  • Verify libChunkName produces deterministic names for mixed/monorepo package paths and that NODE_MODULES_RE pattern covers targeted modules.
  • Confirm advancedChunks vs manualChunks behavior is correctly toggled/removed for iife outputs and that removing sanitizeFilePath doesn't introduce unsafe filenames.

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: lib chunks' follows conventional commits format with 'feat:' prefix and clearly describes the main change: introducing library chunk separation.
Description check ✅ Passed The description explains the problem (mixed code in chunks when bundling libs) and the solution (explicit separation by name), directly related to the changeset.

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 and usage tips.

@pi0 pi0 marked this pull request as ready for review December 10, 2025 16:22
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/build/vite/rollup.ts (1)

70-70: Consider removing the commented-out code.

The commented sanitizeFileName: sanitizeFilePath line should either be removed entirely or include a brief comment explaining why it's retained (e.g., for future reference or pending upstream support).

-      // sanitizeFileName: sanitizeFilePath,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d2e6b8 and c929557.

📒 Files selected for processing (4)
  • src/build/chunks.ts (2 hunks)
  • src/build/rolldown/config.ts (2 hunks)
  • src/build/rollup/config.ts (2 hunks)
  • src/build/vite/rollup.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/build/rolldown/config.ts (1)
src/build/chunks.ts (3)
  • getChunkName (15-83)
  • NODE_MODULES_RE (6-6)
  • libChunkName (8-13)
src/build/chunks.ts (1)
src/types/nitro.ts (1)
  • Nitro (16-37)
src/build/rollup/config.ts (1)
src/build/chunks.ts (3)
  • getChunkName (15-83)
  • NODE_MODULES_RE (6-6)
  • libChunkName (8-13)
src/build/vite/rollup.ts (1)
src/build/chunks.ts (3)
  • getChunkName (15-83)
  • NODE_MODULES_RE (6-6)
  • libChunkName (8-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: tests-rollup (ubuntu-latest)
  • GitHub Check: tests-rollup (windows-latest)
🔇 Additional comments (7)
src/build/rolldown/config.ts (1)

56-59: LGTM! Clean implementation of lib chunking for Rolldown.

The advancedChunks configuration correctly uses NODE_MODULES_RE to identify node_modules and libChunkName to generate consistent chunk names. This aligns well with the PR objective of separating bundled dependencies into distinct chunks.

src/build/rollup/config.ts (1)

74-85: LGTM! Consistent lib chunking implementation for Rollup.

The manualChunks hook correctly mirrors the Rolldown implementation using Rollup's native API. The pattern of testing with NODE_MODULES_RE and returning libChunkName(id) ensures consistent chunk naming across both bundlers.

src/build/vite/rollup.ts (1)

45-63: LGTM! Correct conditional chunking for Vite's dual bundler support.

The implementation properly handles both Rolldown (using advancedChunks) and standard Rollup (using manualChunks) paths. The logic is consistent with the standalone config files.

src/build/chunks.ts (4)

6-13: LGTM! Well-designed regex patterns for node_modules handling.

The NODE_MODULES_RE correctly excludes hidden directories (like .bin, .cache) by requiring a non-dot character after the separator. The libChunkName regex properly handles both scoped (@scope/package) and unscoped packages with a sensible fallback to "common".


15-30: Clear chunk categorization with early returns.

The known-group handling for _libs/ prefix and rolldown-runtime provides clean early exits before the more complex classification logic. The empty moduleIds guard is a sensible defensive check.


85-96: Route path sanitization looks reasonable.

The routeToFsPath function handles dynamic route segments (:param, *wildcard) by converting them to $ prefixes, and sanitizes other special characters. The fallback to "index" for root routes is appropriate.


64-80: Verify bundler ordering guarantees for handler matching.

The logic using ids.at(-1) assumes the last non-virtual module ID in a chunk corresponds to the handler entry point. This assumption is unvalidated since the handler matching logic was newly introduced in the recent chunk naming refactor. Confirm whether Rollup/Rolldown guarantee module ordering such that entry-point handlers consistently appear as the last module in their chunks, or add defensive checks (e.g., match against any module ID in the chunk, not just the last).

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 10, 2025

Open in StackBlitz

npm i https://pkg.pr.new/nitrojs/nitro@3849

commit: 1fdb9d1

@pi0 pi0 merged commit f661802 into main Dec 10, 2025
12 checks passed
@pi0 pi0 deleted the feat/lib-chunks branch December 10, 2025 16:54
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