refactor(vite): migrate plugins internally to vite environments #32461
Conversation
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
WalkthroughThis update refactors the Vite integration within the Nuxt ecosystem to consistently use the Nuxt instance directly, replacing previous reliance on context objects. Numerous plugin APIs are updated: function signatures now accept a Nuxt instance, and plugin configuration is derived from resolved Vite configs rather than external parameters. Several plugins are renamed for clarity, and new plugins are introduced or refactored, including those for Vue feature flags and sourcemap preservation. The build and server entry resolution is centralised in utility functions. Middleware, hooks, and plugin applications are streamlined, and experimental hooks for asset URL resolution are added. No logic changes to error handling or manifest processing are made. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.ts`: Follow standard TypeScript conventions and best practices
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (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 (7)
packages/vite/src/plugins/vue-feature-flags.ts (1)
13-13: Consider more robust access to Nitro instance.The type casting
(nuxt as any)._nitrosuggests accessing a private API, which could be fragile. Consider adding error handling or using a more stable API if available.- ((nuxt as any)._nitro as Nitro).options.replace[key] = config.define[key] + const nitro = (nuxt as any)._nitro as Nitro + if (nitro?.options?.replace) { + nitro.options.replace[key] = config.define[key] + }packages/vite/src/plugins/type-check.ts (1)
18-18: Consider error handling for entry resolution.The
resolveClientEntry(config)call could throw an error if no entry is found. Consider wrapping this in a try-catch block or ensure that the client config always has a valid entry before this plugin runs.- entry = resolveClientEntry(config) + try { + entry = resolveClientEntry(config) + } catch (error) { + throw new Error(`TypeCheckPlugin: ${error.message}`) + }packages/vite/src/utils/config.ts (2)
9-9: Improve error message specificity.The error message could be more specific to help with debugging.
- throw new Error('No entry found in rollupOptions.input') + throw new Error('No client entry found in rollupOptions.input')
18-18: Improve error message specificity.The error message could be more specific to help with debugging.
- throw new Error('No entry found in rollupOptions.input') + throw new Error('No server entry found in rollupOptions.input')packages/vite/src/plugins/module-preload-polyfill.ts (1)
17-17: Consider error handling for entry resolution.Similar to the TypeCheckPlugin, the
resolveClientEntry(config)call could throw an error. Consider adding error handling for better debugging experience.- entry = resolveClientEntry(config) + try { + entry = resolveClientEntry(config) + } catch (error) { + throw new Error(`ModulePreloadPolyfillPlugin: ${error.message}`) + }packages/vite/src/server.ts (1)
115-117: Consider using undefined assignment instead of delete operator.While the performance impact is minimal in build-time code, it's still good practice to avoid the
deleteoperator.if (serverConfig.build?.rollupOptions?.output && !Array.isArray(serverConfig.build.rollupOptions.output)) { - delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefined }packages/vite/src/vite-node.ts (1)
124-159: Well-structured SSR handler extraction.Extracting the SSR handlers into a separate function improves code organization and makes the responsibilities clearer. The error handling with proper status codes is appropriate.
Consider adding more descriptive error messages to help with debugging:
if (!id || !ssrServer) { - throw createError({ statusCode: 400 }) + throw createError({ statusCode: 400, statusMessage: 'Invalid module ID or SSR server not available' }) }if (moduleId === '/' || !ssrServer) { - throw createError({ statusCode: 400 }) + throw createError({ statusCode: 400, statusMessage: 'Invalid module ID or SSR server not available' }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/vite/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (15)
packages/vite/src/client.ts(8 hunks)packages/vite/src/manifest.ts(4 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)packages/vite/src/plugins/dev-style-ssr.ts(1 hunks)packages/vite/src/plugins/module-preload-polyfill.ts(1 hunks)packages/vite/src/plugins/public-dirs.ts(3 hunks)packages/vite/src/plugins/runtime-paths.ts(2 hunks)packages/vite/src/plugins/sourcemap-preserver.ts(2 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/plugins/type-check.ts(2 hunks)packages/vite/src/plugins/vue-feature-flags.ts(1 hunks)packages/vite/src/server.ts(5 hunks)packages/vite/src/utils/config.ts(1 hunks)packages/vite/src/vite-node.ts(7 hunks)packages/vite/src/vite.ts(10 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 116-116: 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 (1)
- GitHub Check: codeql (javascript-typescript)
🔇 Additional comments (30)
packages/vite/src/manifest.ts (1)
13-13: Clean refactoring pattern applied consistently.The destructuring of
nuxtfrom the context parameter improves code readability and aligns with the broader refactoring to use Nuxt instances directly throughout the codebase.packages/vite/src/plugins/dev-style-ssr.ts (1)
15-15: Appropriate environment targeting added.The
applyToEnvironmenthook correctly restricts this plugin to the client environment, which is appropriate for development style handling that should not run on the server side.packages/vite/src/plugins/runtime-paths.ts (1)
9-17: Excellent plugin interface simplification.The refactoring removes external options dependency and instead captures the
sourcemapsetting internally from the resolved Vite configuration. The addition ofapplyToEnvironmenthook properly restricts the plugin to the client environment. This creates a cleaner, more self-contained plugin interface.packages/vite/src/plugins/analyze.ts (2)
5-5: Import path updated appropriately.The import change from
nuxt/schemato@nuxt/schemaappears to reflect a schema reorganisation and is consistent with importing theNuxttype directly.
8-9: Function signature improvement follows refactoring pattern.The renaming to
AnalyzePlugin(capitalised) follows plugin constructor conventions, and the parameter change from context object to directNuxtinstance aligns with the broader refactoring goals. The direct access tonuxt.options.build.analyzeis cleaner than the previousctx.nuxtpattern.packages/vite/src/plugins/vue-feature-flags.ts (2)
8-8: Appropriate environment targeting for feature flag optimisation.The
applyToEnvironmenthook correctly restricts this plugin to SSR environment in production mode, which is appropriate for Vue feature flag tree-shaking optimisation.
10-15: ```shell
#!/bin/bashSearch for Vue feature flag patterns in .js, .ts and .vue files
rg -n "_VUE[A-Z]+" -g '.js' -g '.ts' -g '*.vue'
</details> <details> <summary>packages/vite/src/plugins/ssr-styles.ts (2)</summary> `13-13`: **LGTM! Interface renamed for consistency.** The interface rename to `SSRStylesPluginOptions` aligns with the consistent naming conventions being adopted across the codebase. --- `26-26`: **LGTM! Function renamed for consistency.** The function rename to `SSRStylesPlugin` follows PascalCase convention and aligns with similar naming updates across other Vite plugins in this refactor. </details> <details> <summary>packages/vite/src/plugins/type-check.ts (1)</summary> `8-16`: **Well-structured plugin refactor.** The plugin correctly transitions from external options to direct Nuxt instance usage. The environment restriction to client-only and conditional application based on type checking settings are appropriate. </details> <details> <summary>packages/vite/src/utils/config.ts (2)</summary> `3-10`: **Solid utility function with proper fallback logic.** The function correctly prioritises environment-specific configuration over general configuration. The type checking ensures safe property access. --- `12-19`: **Solid utility function with consistent implementation.** The server entry resolution follows the same pattern as the client version, which is good for consistency. </details> <details> <summary>packages/vite/src/plugins/module-preload-polyfill.ts (1)</summary> `7-13`: **Clean refactor to internal config resolution.** The plugin correctly moves from external options to internal configuration management. The environment restriction to client-only is appropriate for module preload polyfill functionality. </details> <details> <summary>packages/vite/src/plugins/public-dirs.ts (3)</summary> `18-18`: **Good naming convention update.** The function rename to `PublicDirsPlugin` aligns with the consistent naming conventions being adopted across the Vite plugin ecosystem. --- `22-108`: **Well-executed refactor from unplugin to native Vite plugin array.** The structural change from unplugin to native Vite plugin array is implemented correctly. The hook signatures have been properly updated (using `order` instead of `enforce`), and the sourcemap handling follows the consistent pattern established in other plugins. The plugin logic appears to be preserved during the refactoring. --- `48-50`: **Consistent sourcemap handling pattern.** The sourcemap configuration is now derived from the resolved Vite config, which is consistent with the approach taken in other plugins like `TypeCheckPlugin` and `ModulePreloadPolyfillPlugin`. </details> <details> <summary>packages/vite/src/plugins/sourcemap-preserver.ts (3)</summary> `13-15`: **Good defensive programming with early return.** The early return when sourcemaps are disabled or in development mode prevents unnecessary plugin registration and improves performance. --- `48-53`: **Environment-aware plugin application is well implemented.** The combination of `applyToEnvironment` and `apply` hooks ensures the plugin only runs in SSR production builds with sourcemaps enabled, which is exactly when it's needed. --- `40-44`: **Verify that the Nitro hook integration doesn't conflict with existing plugins.** The use of `defu` to merge the Rollup config is appropriate for preserving existing plugins. However, ensure that this plugin doesn't conflict with other plugins that might also be modifying the Rollup configuration. ```shell #!/bin/bash # Description: Check for other places where nitro:build:before hook is used with rollupConfig modifications # Search for other nitro:build:before hooks that modify rollupConfig rg -A 5 "nitro:build:before.*rollupConfig" --type tspackages/vite/src/server.ts (2)
18-19: Good refactoring to use Nuxt instance directly.The function signature change and renaming of
entrytoserverEntryimproves clarity and aligns with the overall refactoring pattern.
29-32: New plugin integration looks correct.The addition of
VueFeatureFlagsPluginandSourcemapPreserverPluginis appropriate for the server build. The comment clearly explains the purpose of the sourcemap preserver plugin.packages/vite/src/vite.ts (3)
60-61: Good conditional helper prefix based on Nitro imports configuration.The conditional assignment of the helper prefix ensures compatibility when Nitro imports are disabled.
69-89: Well-structured experimentalrenderBuiltUrlimplementation.The implementation provides granular control over URL generation for different asset types and contexts (SSR vs client-side). The logic correctly handles:
- CSS files with relative paths
- Client-side assets with runtime URL resolution
- Public and build assets with appropriate helper functions
This will be valuable for the Vite Environment API adoption.
214-225: SSRStylesPlugin configuration is comprehensive.The plugin receives all necessary configuration including the client CSS map, chunks with inlined CSS, and global CSS. The mode-based configuration (server/client) ensures proper behaviour in different environments.
packages/vite/src/client.ts (3)
24-24: Consistent refactoring to accept Nuxt instance directly.The function signature change aligns with the pattern established in
buildServerand improves consistency across the codebase.
130-134: Clean plugin instantiation with appropriate parameters.The plugins are instantiated with the necessary parameters:
RuntimePathsPluginno longer needs explicit options (derives from config)ViteNodePlugincorrectly passes the context getterTypeCheckPluginandModulePreloadPolyfillPluginare added unconditionallyThis simplification improves maintainability.
189-189: Dynamic import for analyze plugin is appropriate.The conditional dynamic import of the analyze plugin only when needed helps reduce the initial bundle size and improves startup performance.
packages/vite/src/vite-node.ts (3)
38-38: Good use of environment targeting.The
applyToEnvironmenthook ensures the plugin only runs in the client environment, preventing unnecessary execution in SSR builds.
39-59: Clean middleware setup with h3 app.The use of h3's
createApp()and route handlers provides a cleaner API than direct middleware manipulation. The deferred SSR handler registration via the hook is a good separation of concerns.
139-142: Important security check for file serving.The
isFileServingAllowedcheck prevents unauthorised access to files outside the allowed directories. This is crucial for security.
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32461 will not alter performanceComparing Summary
|
8eff387 to
d4412b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/vite/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (16)
packages/nuxt/src/components/plugins/islands-transform.ts(1 hunks)packages/vite/src/client.ts(8 hunks)packages/vite/src/manifest.ts(4 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)packages/vite/src/plugins/dev-style-ssr.ts(1 hunks)packages/vite/src/plugins/module-preload-polyfill.ts(1 hunks)packages/vite/src/plugins/public-dirs.ts(3 hunks)packages/vite/src/plugins/runtime-paths.ts(2 hunks)packages/vite/src/plugins/sourcemap-preserver.ts(2 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/plugins/type-check.ts(2 hunks)packages/vite/src/plugins/vue-feature-flags.ts(1 hunks)packages/vite/src/server.ts(5 hunks)packages/vite/src/utils/config.ts(1 hunks)packages/vite/src/vite-node.ts(7 hunks)packages/vite/src/vite.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/vite/src/plugins/runtime-paths.ts
- packages/vite/src/plugins/vue-feature-flags.ts
- packages/vite/src/manifest.ts
- packages/vite/src/plugins/ssr-styles.ts
- packages/vite/src/plugins/analyze.ts
- packages/vite/src/utils/config.ts
- packages/vite/src/plugins/dev-style-ssr.ts
- packages/vite/src/plugins/module-preload-polyfill.ts
- packages/vite/src/plugins/public-dirs.ts
- packages/vite/src/plugins/type-check.ts
- packages/vite/src/vite.ts
- packages/vite/src/client.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Follow standard TypeScript conventions and best practices
**/*.ts: Follow standard TypeScript conventions and best practices
packages/nuxt/src/components/plugins/islands-transform.tspackages/vite/src/plugins/sourcemap-preserver.tspackages/vite/src/vite-node.tspackages/vite/src/server.ts
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 116-116: 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 (1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, 20)
🔇 Additional comments (10)
packages/nuxt/src/components/plugins/islands-transform.ts (1)
192-193: LGTM! Environment targeting improvementThe changes properly implement the new plugin environment targeting pattern. Removing the
configparameter fromapplyand adding explicitapplyToEnvironmentmakes the plugin's scope clearer and aligns with the broader refactoring goals.packages/vite/src/plugins/sourcemap-preserver.ts (3)
9-15: Well-structured plugin refactor with proper early exitThe function signature change to accept a
Nuxtinstance directly aligns with the broader refactoring pattern. The early return logic efficiently prevents plugin creation when sourcemaps are disabled or in development mode, improving performance.
40-44: Proper Nitro integration using defuThe hook registration for Nitro build configuration is well-implemented. Using
defuto merge the rollup config ensures proper configuration precedence without overriding existing plugins.
46-75: Excellent environment targeting implementationThe plugin now properly restricts itself to SSR production builds with sourcemaps enabled using both
applyToEnvironmentandapplyhooks. The sourcemap export logic remains intact whilst being better integrated into the new plugin structure.packages/vite/src/server.ts (2)
18-19: Excellent refactor to direct Nuxt instance usageThe function signature change and consistent usage of the
nuxtparameter instead ofctx.nuxtthroughout the function significantly improves clarity and aligns with the broader refactoring goals. The variable rename toserverEntryalso enhances readability.
28-32: Proper integration of new pluginsThe addition of
VueFeatureFlagsPluginandSourcemapPreserverPlugindirectly to the plugins array is well-placed and follows the new plugin architecture where plugins receive the Nuxt instance directly.packages/vite/src/vite-node.ts (4)
17-17: Consistent function signature refactorThe change from
ViteBuildContexttoNuxtinstance follows the established refactoring pattern and improves consistency across the codebase.
38-59: Well-structured server configuration with proper environment targetingThe addition of
applyToEnvironmentfor client-only application and the restructured server configuration using h3 app middleware is well-implemented. The separation of concerns between client server setup and SSR handler registration improves modularity.
124-159: Excellent modularisation of SSR handlersThe new
registerSSRHandlersfunction properly encapsulates the resolve and module endpoint logic. The error handling is comprehensive, and the use of SSR server's environment API (ssrServer.environments.ssr.fetchModule) aligns with the Vite Environment API preparation goals.
168-192: Proper server initialisation with updated entry resolutionThe
initViteNodeServerfunction correctly uses the new utility functions for entry resolution and maintains proper file structure generation. The use ofresolveServerEntryand direct Nuxt instance access is consistent with the refactoring goals.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Vite builder to better integrate with Vite environments in preparation for adopting the Vite Environment API in Nuxt v5. The changes include updating plugin imports and names, refactoring the usage of context from ctx.nuxt to nuxt, and ensuring configuration hooks and experimental options use the updated Vite environment APIs.
- Updated plugin names and import methods (e.g. SSRStylesPlugin and PublicDirsPlugin)
- Replaced ctx.nuxt references with nuxt to streamline configuration access
- Refactored various build and server options in multiple files to align with the new Vite environment API
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/vite/src/vite.ts | Refactored experimental options and plugin usage, updated hooks |
| packages/vite/src/vite-node.ts | Updated plugin configuration to use nuxt and ssrServer environments |
| packages/vite/src/utils/config.ts | New utility functions for resolving client and server entries |
| packages/vite/src/server.ts | Refactored buildServer to use nuxt directly for configuration |
| Other plugin and client files | Consistent migration from ctx.nuxt to nuxt throughout |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
🔗 Linked issue
resolves #32342
📚 Description
this is a set of changes to the vite builder to prepare for adopting the Vite Environment API in Nuxt v5