fix(nuxt): recompile templates on change events#29954
Conversation
|
|
WalkthroughThe pull request introduces significant modifications to the Nuxt framework's handling of file system events and import management. In Changes
Assessment against linked issues
Possibly related PRs
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 (4)
packages/nuxt/src/core/builder.ts (1)
23-35: Enhance component detection patternsThe component detection logic could be improved in the following areas:
- The regex patterns
^app\.and^error\.might match unrelated files (e.g.,app.config.ts).- Early breaks in the loops could miss multiple component changes in different layers.
Consider this more specific implementation:
- if (relativePath.match(/^app\./i)) { + if (relativePath.match(/^app\.(vue|jsx?|tsx?)$/i)) { app.mainComponent = undefined - break } - if (relativePath.match(/^error\./i)) { + if (relativePath.match(/^error\.(vue|jsx?|tsx?)$/i)) { app.errorComponent = undefined - break }packages/nuxt/src/imports/module.ts (3)
Line range hint
114-139: Add error handling for directory scanningWhile the regeneration logic is solid, consider adding try-catch blocks around the scanning operations to gracefully handle potential file system errors.
const regenerateImports = async () => { await ctx.modifyDynamicImports(async (imports) => { // Clear old imports imports.length = 0 // Scan for `composables/` and `utils/` directories if (options.scan) { + try { const scannedImports = await scanDirExports(composablesDirs, { fileFilter: file => !isIgnored(file), }) for (const i of scannedImports) { i.priority = i.priority || priorities.find(([dir]) => i.from.startsWith(dir))?.[1] } imports.push(...scannedImports) + } catch (error) { + logger.warn(`Error scanning composables directories: ${error.message}`) + } } // Modules extending await nuxt.callHook('imports:extend', imports) return imports })
Line range hint
141-156: Consider debouncing rapid file changesThe current implementation might trigger multiple regenerations if several files change in quick succession. Consider implementing debouncing to optimize performance.
+import { debounce } from 'perfect-debounce' + +const debouncedRegenerate = debounce(async () => { + await regenerateImports() +}, 100) + // Watch composables/ directory nuxt.hook('builder:watch', async (_, relativePath) => { const path = resolve(nuxt.options.srcDir, relativePath) if (options.scan && composablesDirs.some(dir => dir === path || path.startsWith(dir + '/'))) { - await regenerateImports() + await debouncedRegenerate() } })
Line range hint
1-201: Consider implementing on-demand recompilationAs mentioned in the PR objectives, future improvements could include implementing on-demand recompilation through a virtual plugin. This would be more efficient than the current watch-based approach.
Some considerations for the implementation:
- Use virtual file system events instead of physical file watching
- Implement selective recompilation based on dependency graph
- Add cache invalidation hooks for better control
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/nuxt/src/core/builder.ts(1 hunks)packages/nuxt/src/imports/module.ts(1 hunks)
🔇 Additional comments (4)
packages/nuxt/src/core/builder.ts (2)
23-40: Overall implementation looks solid
The changes effectively address the PR objectives by:
- Properly handling component changes through layer-aware detection
- Ensuring template recompilation when needed
- Using debouncing for performance optimization
The implementation is clean, maintainable, and follows Nuxt's architectural patterns.
38-40: 🛠️ Refactor suggestion
Consider conditional template recompilation
The current implementation recompiles templates for all file changes. This might be inefficient for changes that don't affect templates.
Consider adding a condition to check if the changed file actually affects templates:
- // Recompile app templates
- await generateApp()
+ // Recompile app templates only when necessary
+ const isTemplateChange = relativePath.match(/\.(vue|jsx?|tsx?)$/i)
+ if (isTemplateChange) {
+ await generateApp()
+ }Let's verify the file extensions that require template recompilation:
packages/nuxt/src/imports/module.ts (2)
110-112: Good refactor: Improved template detection logic
Nice improvement to centralize the imports template detection pattern into a regex constant. This makes the code more maintainable and follows the DRY principle.
Line range hint 163-201: Consider cache invalidation strategy
The import path cache is effective for performance, but consider implementing a cache invalidation strategy when dependencies change (e.g., when package.json is modified).
Let's verify if there are any package.json watchers already in place:
🔗 Linked issue
resolves #29891
📚 Description
This allows templates to be updated when files change. Also shipped two small perf changes which speed up the compilation.
We might consider on-demand recompilation in future, for example, doing so within the virtual plugin rather than relying on our own watcher.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor