fix: #29894 addTemplate should consider the entire path#29895
fix: #29894 addTemplate should consider the entire path#29895
Conversation
|
|
WalkthroughThe changes in this pull request modify the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/kit/src/template.ts (1)
25-27: Consider adding documentation and warning for this behavior change.While this change restores the expected Nuxt 2 behavior, it modifies how template deduplication works. Consider:
- Adding documentation about this behavior change
- Adding a warning when multiple templates with the same filename are detected, to help users understand why their templates are now being processed differently
Here's a suggested enhancement to make the change more transparent:
// Remove any existing template with the same destination path +const existingTemplate = nuxt.options.build.templates + .find(p => normalizeTemplate(p).filename === template.filename && normalizeTemplate(p).dst !== template.dst) +if (existingTemplate) { + console.warn(`Multiple templates with filename "${template.filename}" detected. ` + + `Both templates will be processed as they have different destination paths. ` + + `This is a change from previous behavior where only the last template would be kept.`) +} nuxt.options.build.templates = nuxt.options.build.templates .filter(p => normalizeTemplate(p).dst !== template.dst)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/kit/src/template.ts(1 hunks)
🔇 Additional comments (1)
packages/kit/src/template.ts (1)
25-27: LGTM! This change fixes the template filtering issue.
The modification to filter templates based on destination path (dst) instead of filename correctly addresses issue #29894, allowing templates with the same filename to coexist in different subfolders.
Let's verify the impact of this change:
✅ Verification successful
Verified: The change to filter templates by destination path is correct and safe.
The modification aligns with the template system's design where each template should be uniquely identified by its full destination path. Analysis of template usage across the codebase shows no patterns that would be negatively impacted by this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any existing templates that might be affected by this change
# Search for addTemplate usage to understand current patterns
rg "addTemplate\(" -A 5 -B 5
# Look for potential template definitions with same filenames
fd -e ts -e js -x sh -c '
filename=$(basename {})
count=$(find . -name "$filename" | wc -l)
if [ "$count" -gt 1 ]; then
echo "Found $count occurrences of $filename:"
find . -name "$filename"
fi
'
Length of output: 350486
I think the code suggestion from coderabbit is irrelevant because the dst variable already includes the filename. The part about the warning i don't know how you usually do |
danielroe
left a comment
There was a problem hiding this comment.
This seems good to me - thank you ❤️
I think there's some more optimisation we could do here for a future PR. (Do we need to normalise each template at every iteration? 🤔)
🔗 Linked issue
resolves #29894
📚 Description
When using addTemplate with the same filename but different subfolders only the last template is considered, the ones before it are silently ignored.
It is a breaking change in nuxt 3 from nuxt 2 because on the previous version all files are created as expected.
Feel free to edit or request changes if needed.
Summary by CodeRabbit