Skip to content

perf!: remove nodeModulesDirs#3798

Merged
pi0 merged 2 commits into
mainfrom
perf/extra-module-dirs
Dec 10, 2025
Merged

perf!: remove nodeModulesDirs#3798
pi0 merged 2 commits into
mainfrom
perf/extra-module-dirs

Conversation

@pi0
Copy link
Copy Markdown
Member

@pi0 pi0 commented Nov 17, 2025 •
edited
Loading

We were adding several node_module dirs (including workspace) to all resolutions, a requirement inherited from Nuxt to allow modules with non-hoisted dependencies to work.

Removing extra node_modules can significantly help to reduce resolution times and also avoid wrong behaviors like picking dependencies from directories outside.

The config is removed altogether. Implicit resolutions only happen from rootDir

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 17, 2025 •
edited
Loading

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 8:55pm

@pi0 pi0 force-pushed the perf/extra-module-dirs branch from 6625aad to ebd447e Compare November 17, 2025 12:18
@pi0 pi0 force-pushed the perf/extra-module-dirs branch from f1b95df to 63ee2d0 Compare December 10, 2025 20:52
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 10, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The PR removes the nodeModulesDirs configuration option entirely from Nitro. This involves deleting the type definition, configuration defaults, and all logic that previously populated or consumed this option. Module resolution now uses rootDir instead where applicable.

Changes

Cohort / File(s) Summary
Configuration interface and defaults removal
src/types/config.ts, src/config/defaults.ts		 Removed the nodeModulesDirs: string[] field from the NitroOptions interface and the nodeModulesDirs: [] default from NitroDefaults.
Build resolution refactoring
src/build/rollup/config.ts, src/build/types.ts		 Removed modulePaths option from the nodeResolve plugin configuration; updated resolveModulePath in writeTypes to use rootDir instead of nodeModulesDirs as the resolution base.
Path resolver cleanup
src/config/resolvers/paths.ts		 Removed pkgDir import and deleted the entire block that previously augmented options.nodeModulesDirs with dist/node_modules, node_modules, and pnpm-related paths. Retained plugin path resolution logic unchanged.
Cloudflare preset update
src/presets/cloudflare/dev.ts		 Changed module resolution scope for the "wrangler" dependency lookup from nodeModulesDirs to rootDir.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The changes follow a consistent removal pattern across files, reducing cognitive load
  • No new logic introduced; only feature removal and straightforward substitutions
  • All changes directly trace back to the nodeModulesDirs deprecation decision
  • Verify that rootDir is a suitable replacement in all contexts where nodeModulesDirs was previously used

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 'perf!: remove nodeModulesDirs' follows conventional commits format with a breaking change indicator (!) and clearly describes the main change in the changeset.
Description check ✅ Passed The description explains the rationale for removing nodeModulesDirs (performance and correctness) and relates directly to the changes made across all modified files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch perf/extra-module-dirs
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4754f28 and 8d33a00.

📒 Files selected for processing (6)
  • src/build/rollup/config.ts (0 hunks)
  • src/build/types.ts (1 hunks)
  • src/config/defaults.ts (0 hunks)
  • src/config/resolvers/paths.ts (1 hunks)
  • src/presets/cloudflare/dev.ts (1 hunks)
  • src/types/config.ts (0 hunks)
💤 Files with no reviewable changes (3)
  • src/build/rollup/config.ts
  • src/types/config.ts
  • src/config/defaults.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-03T19:09:13.905Z 
Learnt from: medz
Repo: nitrojs/nitro PR: 3834
File: src/presets/cloudflare/server-entry.ts:63-63
Timestamp: 2025-12-03T19:09:13.905Z
Learning: In the Nitro Cloudflare preset (src/presets/cloudflare/server-entry.ts), native errors from oxc-parser and Node.js readFile operations should be allowed to bubble up naturally without wrapping, as their native error messages are more user-friendly and provide better diagnostic information.

Applied to files:

  • src/presets/cloudflare/dev.ts
⏰ 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-rolldown (windows-latest)
  • GitHub Check: tests-rollup (windows-latest)
🔇 Additional comments (3)
src/build/types.ts (1)

63-76: Root-based type import resolution aligns with new strategy

Switching the resolveModulePath origin to nitro.options.rootDir keeps auto-import type resolution consistent with the new, simplified root-only resolution model and avoids leaking dependencies from external workspaces. No further changes needed here.

src/config/resolvers/paths.ts (1)

1-7: Import cleanup matches runtimeDir-only usage

Dropping unused imports so this file now only pulls in runtimeDir from nitro/meta is correct and keeps the import list aligned with actual usage.

src/presets/cloudflare/dev.ts (1)

25-30: Wrangler resolution scoped to rootDir is consistent with nodeModulesDirs removal

Using nitro.options.rootDir as the origin when resolving wrangler matches the new, simplified resolution model and avoids accidentally picking up workspace-level installs outside the project root. Behavior looks correct as long as rootDir has been normalized before this preset runs.

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 changed the title perf!: remove extra default nodeModulesDirs perf!: remove nodeModulesDirs Dec 10, 2025
@pi0 pi0 merged commit 79340a5 into main Dec 10, 2025
10 of 11 checks passed
@pi0 pi0 deleted the perf/extra-module-dirs branch December 10, 2025 20:57
@coderabbitai coderabbitai Bot mentioned this pull request Dec 12, 2025
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Mar 29, 2026
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pi0