fix(kit): only exclude node_modules/ if no custom srcDir
#32987
Merged
Conversation
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
|
|
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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). (17)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/kit/src/template.ts (1)
221-221: Normalise srcDir paths before prefix checks to avoid edge-case mismatchesIf any layer’s srcDir were ever relative (or differently normalised), prefix checks may fail. Minor hardening: ensure absolute, normalised paths before adding a trailing slash.
Apply this diff:
- const sourceDirs = nuxt.options._layers.map(layer => withTrailingSlash(layer.config.srcDir ?? layer.cwd)) + const sourceDirs = nuxt.options._layers.map((layer) => { + const src = layer.config.srcDir ?? layer.cwd + return withTrailingSlash(isAbsolute(src) ? src : resolve(layer.cwd, src)) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/kit/src/template.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/kit/src/template.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: build
- GitHub Check: code
Comment on lines
+225
to
+229
| // we only need to exclude node_modules directories if they are | ||
| // being included automatically by being inside the source directory | ||
| if (!sourceDirs.some(srcDir => dir.startsWith(srcDir))) { | ||
| exclude.add(relativeWithDot(nuxt.options.buildDir, dir)) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inverted condition: node_modules exclusion logic contradicts comment and PR intent
The comment says “we only need to exclude node_modules directories if they are being included automatically by being inside the source directory”, but the code excludes when they are NOT inside a source directory. This inverts the intended behaviour:
- With a custom srcDir (node_modules outside srcDir), this still excludes node_modules — which defeats the purpose of the PR.
- With default srcDir=rootDir (node_modules inside srcDir), this will not exclude node_modules — likely causing a large performance regression.
Flip the condition and normalise the compared paths to avoid prefix false negatives.
Apply this diff:
// node_modules folders
for (const dir of nuxt.options.modulesDir) {
- // we only need to exclude node_modules directories if they are
- // being included automatically by being inside the source directory
- if (!sourceDirs.some(srcDir => dir.startsWith(srcDir))) {
- exclude.add(relativeWithDot(nuxt.options.buildDir, dir))
- }
+ const dirWithSlash = withTrailingSlash(dir)
+ // Exclude node_modules only when they are included automatically
+ // by being inside any source directory (e.g. srcDir === rootDir).
+ if (sourceDirs.some(srcDir => dirWithSlash.startsWith(srcDir))) {
+ exclude.add(relativeWithDot(nuxt.options.buildDir, dir))
+ }
nodeExclude.add(relativeWithDot(nuxt.options.buildDir, dir))
legacyExclude.add(relativeWithDot(nuxt.options.buildDir, dir))
}Follow-up:
- Please double-check with both scenarios:
- Default srcDir (rootDir): node_modules should be excluded.
- Custom srcDir (e.g. /src): node_modules should not be excluded.
🏁 Script executed:
#!/bin/bash
# Show the exclusion logic around the reported lines
sed -n '220,240p' packages/kit/src/template.ts
# Locate where sourceDirs is defined
rg -n 'const sourceDirs' -t ts -C3 packages/kit/src/template.tsLength of output: 1517
Reverse node_modules exclusion to align with comment and PR intent
The current logic in packages/kit/src/template.ts (around lines 223–228) uses if (!sourceDirs.some(...)), which excludes modules outside your source directories and leaves those inside—exactly the opposite of what the comment and intended behaviour describe. We need to:
• Flip the condition so that we only exclude node_modules when they are within a source directory.
• Normalise both paths with withTrailingSlash to avoid prefix collisions.
Apply this diff:
// node_modules folders
for (const dir of nuxt.options.modulesDir) {
- // we only need to exclude node_modules directories if they are
- // being included automatically by being inside the source directory
- if (!sourceDirs.some(srcDir => dir.startsWith(srcDir))) {
- exclude.add(relativeWithDot(nuxt.options.buildDir, dir))
- }
+ const dirWithSlash = withTrailingSlash(dir)
+ // Exclude node_modules only when they are included automatically
+ // by being inside any source directory (e.g. srcDir === rootDir).
+ if (sourceDirs.some(srcDir => dirWithSlash.startsWith(srcDir))) {
+ exclude.add(relativeWithDot(nuxt.options.buildDir, dir))
+ }
nodeExclude.add(relativeWithDot(nuxt.options.buildDir, dir))
legacyExclude.add(relativeWithDot(nuxt.options.buildDir, dir))
}Please verify both scenarios after this change:
- Default srcDir (rootDir):
node_modulesshould now be excluded. - Custom srcDir (e.g.
<root>/src):node_modulesshould not be excluded.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // we only need to exclude node_modules directories if they are | |
| // being included automatically by being inside the source directory | |
| if (!sourceDirs.some(srcDir => dir.startsWith(srcDir))) { | |
| exclude.add(relativeWithDot(nuxt.options.buildDir, dir)) | |
| } | |
| // node_modules folders | |
| for (const dir of nuxt.options.modulesDir) { | |
| const dirWithSlash = withTrailingSlash(dir) | |
| // Exclude node_modules only when they are included automatically | |
| // by being inside any source directory (e.g. srcDir === rootDir). | |
| if (sourceDirs.some(srcDir => dirWithSlash.startsWith(srcDir))) { | |
| exclude.add(relativeWithDot(nuxt.options.buildDir, dir)) | |
| } | |
| nodeExclude.add(relativeWithDot(nuxt.options.buildDir, dir)) | |
| legacyExclude.add(relativeWithDot(nuxt.options.buildDir, dir)) | |
| } |
🤖 Prompt for AI Agents
In packages/kit/src/template.ts around lines 225 to 229, the condition for
excluding node_modules is inverted relative to the comment and intent; change
the predicate to exclude node_modules only when the node_modules path is inside
one of the source directories (i.e. flip the ! to remove the negation), and
normalize both srcDir and dir with withTrailingSlash (or equivalent) before
comparing to avoid prefix collisions; update the exclude.add call to use
relativeWithDot as before and verify that with default srcDir (rootDir)
node_modules is excluded and with a custom srcDir (e.g. <root>/src) node_modules
inside root is not excluded.
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32987 will not alter performanceComparing Summary
Footnotes |
Merged
danielroe
added a commit
that referenced
this pull request
Sep 2, 2025
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Merged
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.
🔗 Linked issue
resolves #31298
📚 Description
we can safely not exclude
node_modulesfrom the typescript project references if there's a custom source directory (e.g. no perf cost)