feat(kit,nuxt)!: defer installing modules with installModule
#32431
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32431 will not alter performanceComparing Summary
|
WalkthroughA mechanism for deferring module installation was introduced. The 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous 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). (5)
✨ 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. 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
🧹 Nitpick comments (1)
packages/nuxt/src/core/nuxt.ts (1)
584-584: Use undefined assignment instead of delete operator.The delete operator can impact performance by changing object shape. Consider using undefined assignment as suggested by static analysis.
- delete nuxt._modulesToDefer + nuxt._modulesToDefer = undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/kit/src/module/install.ts(1 hunks)packages/nuxt/src/core/nuxt.ts(1 hunks)packages/schema/src/types/nuxt.ts(1 hunks)test/basic.test.ts(1 hunks)test/fixtures/basic/modules/example.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{ts,tsx}`: Follow standard TypeScript conventions and best practices
**/*.{ts,tsx}: Follow standard TypeScript conventions and best practices
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
packages/schema/src/types/nuxt.tspackages/kit/src/module/install.tstest/fixtures/basic/modules/example.tstest/basic.test.tspackages/nuxt/src/core/nuxt.ts
`**/*.{ts,tsx,vue}`: Use clear, descriptive variable and function names Add comm...
**/*.{ts,tsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
packages/schema/src/types/nuxt.tspackages/kit/src/module/install.tstest/fixtures/basic/modules/example.tstest/basic.test.tspackages/nuxt/src/core/nuxt.ts
`**/*.{test,spec}.{ts,tsx}`: Write unit tests for core functionality using vitest
**/*.{test,spec}.{ts,tsx}: Write unit tests for core functionality using vitest
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
test/basic.test.ts
🧠 Learnings (5)
packages/schema/src/types/nuxt.ts (2)
Learnt from: GalacticHypernova
PR: nuxt/nuxt#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.
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
packages/kit/src/module/install.ts (1)
Learnt from: huang-julien
PR: nuxt/nuxt#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.
test/fixtures/basic/modules/example.ts (5)
Learnt from: GalacticHypernova
PR: nuxt/nuxt#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.
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:48:28.134Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Write unit tests for core functionality using vitest
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:48:28.134Z
Learning: Applies to **/*.e2e.{ts,js} : Write end-to-end tests using Playwright and @nuxt/test-utils
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:48:28.134Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Learnt from: huang-julien
PR: nuxt/nuxt#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.
test/basic.test.ts (3)
undefined
<retrieved_learning>
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:48:28.134Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Write unit tests for core functionality using vitest
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T13:48:28.134Z
Learning: Applies to **/*.e2e.{ts,js} : Write end-to-end tests using Playwright and @nuxt/test-utils
</retrieved_learning>
<retrieved_learning>
Learnt from: huang-julien
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:
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : nullinstead of wrapping the import in a computed property or importing the component unconditionally.
</retrieved_learning>
packages/nuxt/src/core/nuxt.ts (2)
Learnt from: GalacticHypernova
PR: nuxt/nuxt#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.
Learnt from: huang-julien
PR: nuxt/nuxt#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.
🧬 Code Graph Analysis (1)
test/fixtures/basic/modules/example.ts (1)
packages/kit/src/module/install.ts (1)
installModule(26-80)
🪛 Biome (1.9.4)
packages/nuxt/src/core/nuxt.ts
[error] 584-584: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, 20)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, 20)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, 20)
- GitHub Check: release-pr
- GitHub Check: test-benchmark
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: code
- GitHub Check: lint-docs
🔇 Additional comments (13)
packages/schema/src/types/nuxt.ts (1)
95-95: LGTM! Well-structured type definition for deferred module installation.The type definition correctly supports the deferred module installation mechanism:
- Optional property ensures backward compatibility
- Map structure allows flexible module identification
- Array value type supports multiple option sets per module as described in the PR objectives
- Follows established private field naming conventions
test/basic.test.ts (1)
112-116: LGTM! Well-structured test for module registration functionality.This test properly verifies the new deferred module installation feature by:
- Using the appropriate test context to access the Nuxt instance
- Reading from the virtual file system to check generated module logs
- Asserting the correct sequence of module registration (parent → child → grandchild)
- Following established vitest patterns with clear naming and structure
The test aligns well with the PR objectives and provides good coverage for the nested module registration functionality.
packages/nuxt/src/core/nuxt.ts (3)
576-576: LGTM!Initialising the deferred modules map correctly sets up the two-phase installation mechanism.
578-578: Correct use of immediate installation flag.Passing
{ defer: false }ensures modules are installed immediately in the first phase, maintaining expected behaviour.
580-583: Well-implemented deferred module installation logic.The implementation correctly handles multiple registrations by reversing the options array and merging them with
defu, ensuring first-registered options take precedence whilst preventing recursive deferral.packages/kit/src/module/install.ts (3)
17-24: Well-documented interface for module installation options.The interface clearly defines the deferral behaviour with appropriate documentation explaining the purpose and default behaviour.
29-29: Backward-compatible signature update.Adding the options parameter with a default empty object maintains compatibility with existing code whilst enabling the new deferral feature.
30-35: Correct implementation of module deferral logic.The implementation properly checks for deferral conditions, accumulates multiple registrations, and returns early to defer installation. The comment clearly indicates the deferral behaviour.
test/fixtures/basic/modules/example.ts (5)
1-1: Correct import for nested module installation.Adding
installModuleimport enables the demonstration of nested module registration.
8-8: Required changes for async module operations.Making setup async and accepting the nuxt parameter enables the use of await for nested module installation.
13-14: Acceptable use of @ts-expect-error for testing.Using @ts-expect-error is appropriate here as this is test code verifying that modules can extend nuxt.options with custom properties.
16-29: Excellent demonstration of nested module installation.The implementation effectively showcases:
- Inline module definition with configKey and defaults
- Nested module installation using await
- Option passing and logging at each level
- The deferred installation mechanism in action
41-44: Smart approach for exposing test logs.Using
addTemplateto export the collected logs as a module enables effective testing of the module installation order and option merging behaviour.
|
@danielroe
Here is a repo with a variant of this setup using layers instead
The setups can get more complicated when locales have file arrays instead of a single file path, it's hard to demonstrate but you can imagine that the merging/overwrite order is important in these setups. |
|
But to be clear, the changes in this PR are probably good for the majority of use cases |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI on ``: Open
✅ |
🔗 Linked issue
resolves #31674
📚 Description
this is a draft PR to look at the effect of deferring modules registered with
installModuleuntil the end of the module install cycle. The kit utility is backwards compatible with previous versions of Nuxt (which won't set_modulesToDefer).If
installedModuleis called more than once, it will merge all sets of options into resolved configuration - and, of course, respect the user's configuration.(This will not solve the issue where a layer adds a module but another module in the user's project modifies the config or calls
installModuleitself.)