-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix(nuxt): invalidate module cache on composable export type changes #33671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
WalkthroughThis change introduces HMR handling for composables directory files in Nuxt. The implementation tracks Vite servers (client and SSR) and registers a new Vite plugin (nuxt:composables-hmr) that detects when an export type changes (default versus named export) within updated composables files. When an export type change is detected, the plugin recursively invalidates affected modules across all tracked Vite servers and triggers a full page reload. If no export type change occurs, the standard HMR flow continues. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/imports/module.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/nuxt/src/imports/module.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/imports/module.ts
🧬 Code graph analysis (1)
packages/nuxt/src/imports/module.ts (1)
packages/kit/src/ignore.ts (1)
isIgnored(14-34)
⏰ 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). (1)
- GitHub Check: code
| // Check if export type changed (default ↔ named) | ||
| let exportTypeChanged = false | ||
| for (const newImport of newImports) { | ||
| const oldImport = oldImports.find(i => (i.as || i.name) === (newImport.as || newImport.name)) | ||
| if (oldImport && oldImport.name !== newImport.name) { | ||
| exportTypeChanged = true | ||
| break | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detection misses default↔named switches when the alias changes
Line 120: if the export flips between default and named while you also rename the symbol (e.g. export default function useKey() → export function useKeyRenamed()), oldImport is never found so exportTypeChanged stays false. The module cache is left untouched and the original (…).default is not a function crash still reproduces. Please derive exportTypeChanged from the presence of a name === 'default' entry before versus after (and only fall back to the alias comparison when that check is inconclusive) so we catch these alias-renaming cases as well. Otherwise the fix remains incomplete.
CodSpeed Performance ReportMerging #33671 will not alter performanceComparing Summary
|
|
|
||
| // Vite plugin to detect and handle export type changes BEFORE HMR | ||
| nuxt.hook('vite:configResolved', (viteConfig) => { | ||
| if (!viteConfig.plugins) { return } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt we initialise the list here and push the plugin in anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viteConfig.plugins? I guess no. Because it is readonly and the vite:configResolved hook is called after Vite's config resolution, which always includes a plugins array. The early return is defensive programming - if plugins doesn't exist, something is fundamentally wrong with the Vite setup and we shouldn't proceed.
|
this may also be an approach which also works for some linked issues: |
🔗 Linked issue
Fixes: #33468
Description
When changing a composable's export type during development (e.g., switching between export default and export function), the dev server crashes with
(0, __vite_ssr_import_2__.default) is not a function.This happens because Vite's HMR doesn't invalidate the module cache when export types change. The old import statement tries to use the new export type, causing a runtime error.
Solution
Added a Vite plugin in the imports module that:
composablesusingscanDirExportsclientandSSRVite serversThe plugin runs in the
handleHotUpdatehook before Vite's HMR logic executes, ensuring proper cache invalidation. For normal code changes without export type modifications, HMR continues to work as usual.Works with both Vite's new Environment API and legacy mode by tracking all Vite server instances via the
vite:serverCreatedhook.Testing
Tested by switching between:
export default function() { return 'default' }and
export function useName() { return 'named' }