fix(nuxt): have vite optimize CommonJS dependencies of module plugins #33851
Conversation
|
|
WalkthroughAdds Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2024-11-05T15:22:54.759ZApplied to files:
⏰ 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). (14)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/src/core/templates.ts(1 hunks)packages/vite/src/plugins/environments.test.ts(1 hunks)packages/vite/src/plugins/environments.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/vite/src/plugins/environments.test.tspackages/nuxt/src/core/templates.tspackages/vite/src/plugins/environments.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,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
Files:
packages/vite/src/plugins/environments.test.tspackages/nuxt/src/core/templates.tspackages/vite/src/plugins/environments.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/vite/src/plugins/environments.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Write unit tests for core functionality using `vitest`
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Write unit tests for core functionality using `vitest`
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/e2e/**/*.{ts,tsx,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 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/core/templates.tspackages/vite/src/plugins/environments.ts
🧬 Code graph analysis (1)
packages/vite/src/plugins/environments.test.ts (1)
packages/vite/src/plugins/environments.ts (1)
EnvironmentsPlugin(10-89)
🪛 GitHub Check: build
packages/vite/src/plugins/environments.test.ts
[failure] 102-102:
This expression is not callable.
[failure] 101-101:
This expression is not callable.
[failure] 83-83:
This expression is not callable.
[failure] 65-65:
This expression is not callable.
[failure] 51-51:
This expression is not callable.
[failure] 34-34:
This expression is not callable.
⏰ 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
🔇 Additional comments (5)
packages/nuxt/src/core/templates.ts (1)
64-65: LGTM! Enables dependency discovery for module plugins.The addition of
write: trueallows Vite's esbuild to naturally discover CommonJS dependencies of module plugins during development, addressing the core issue where these dependencies were not being optimised. This is consistent with the existing pattern used forappConfigTemplate(line 431).packages/vite/src/plugins/environments.ts (1)
34-34: LGTM! Simplified SSR detection logic.The refactor to check
name === 'ssr'directly (instead of relying on storedviteConfig.ssr) simplifies the logic and makes the condition more explicit. The behaviour correctly unsetsoptimizeDeps.includefor SSR environments when the experimental Vite Environment API is disabled.packages/vite/src/plugins/environments.test.ts (3)
1-19: LGTM! Well-structured test setup.The mock configuration properly isolates the plugin from Nitro dependencies whilst preserving other exports from
@nuxt/kit. The minimal mock approach reduces test brittleness.
21-109: Excellent test coverage!The test suite comprehensively validates the
optimizeDeps.includehandling across all scenarios:
- Client/SSR environment isolation
- Experimental flag behaviour
- Missing
optimizeDepsinitialisation- Multiple environment independence
The tests directly address the regression fix mentioned in the PR objectives.
111-127: LGTM! Well-designed test helper.The
createMockNuxthelper effectively reduces duplication and provides a minimal, focused mock with only the properties required by the plugin under test.
deleteme
force-pushed
the
fix-module-plugins-not-being-optimized
branch
from
366e538 to
0a51c91
Compare
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33851 will improve performances by 12.02%Comparing Summary
Benchmarks breakdown
|
deleteme
force-pushed
the
fix-module-plugins-not-being-optimized
branch
from
4839965 to
1f240c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/vite/src/plugins/environments.test.ts (2)
21-115: Test coverage ofoptimizeDeps.includebehaviour across environments is thoroughThe suite exercises the key paths in
configEnvironment: preservingoptimizeDeps.includeon the client, unsetting it for SSR (including whenoptimizeDepsis initially missing), respecting theexperimental.viteEnvironmentApiflag, and handling multiple environments independently on a single plugin instance. Using@ts-expect-erroron theconfigEnvironmentcalls is a reasonable compromise here to avoid overfitting tests to the plugin’s TypeScript signature while still validating the runtime behaviour you care about.
117-133:createMockNuxtis minimal and targeted to plugin needsThe helper provides exactly the Nuxt fields the plugin relies on (
experimental.viteEnvironmentApi,app.buildAssetsDir,buildDir,rootDir) and keeps the mock small. If you ever add tests around dev/prod‑specific behaviour, you might optionally extend this to include an explicitoptions.devflag for clarity, but it’s not required for the current optimise‑deps scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/src/core/templates.ts(1 hunks)packages/vite/src/plugins/environments.test.ts(1 hunks)packages/vite/src/plugins/environments.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vite/src/plugins/environments.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/templates.tspackages/vite/src/plugins/environments.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,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
Files:
packages/nuxt/src/core/templates.tspackages/vite/src/plugins/environments.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/vite/src/plugins/environments.test.ts
🧠 Learnings (6)
📚 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/core/templates.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js} : Write unit tests for core functionality using `vitest`
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Add comments only to explain complex logic or non-obvious implementations
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 Learning: 2024-11-11T12:34:22.648Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 0
File: :0-0
Timestamp: 2024-11-11T12:34:22.648Z
Learning: Ensure that AI-generated summaries accurately reflect the key changes in the PR, focusing on notable changes such as the removal of unused imports and variables starting with underscores.
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Remove code that is not used or needed
Applied to files:
packages/vite/src/plugins/environments.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/e2e/**/*.{ts,tsx,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
packages/vite/src/plugins/environments.test.ts
🧬 Code graph analysis (1)
packages/vite/src/plugins/environments.test.ts (1)
packages/vite/src/plugins/environments.ts (1)
EnvironmentsPlugin(10-89)
⏰ 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
🔇 Additional comments (2)
packages/nuxt/src/core/templates.ts (1)
61-66: Writingplugins.client.mjsto disk is an appropriate fix and integrates cleanlyEmitting the client plugin template (
write: true) is a good way to let Vite/esbuild discover CommonJS dependencies during development, and it lines up with existing conventions (e.g.appConfigTemplate) that also write templates to disk. This change should not disrupt type generation:buildTypeTemplatewill now correctly skip a stub forplugins.client.mjsbecause the file exists, andpluginsDeclaration.existsstill won’t treat this template as a plugin source since itsdstpath doesn’t overlap pluginsrcpaths.packages/vite/src/plugins/environments.test.ts (1)
1-19: Mocking@nuxt/kitwith a partial override ofuseNitrois clean and safeUsing
vi.mockwithvi.importActual<typeof kit>and overriding onlyuseNitrogives the plugin a realistic Nitro shape while preserving the rest of the module behaviour, which is a solid pattern for isolating environment-dependent logic in tests.
…ed by vite & esbuild This writes the plugins.client.mjs file to disk so that esbuild can discover the dependencies and optimize the CommonJS ones.
deleteme
force-pushed
the
fix-module-plugins-not-being-optimized
branch
from
1f240c8 to
a43d3df
Compare
| }, | ||
| configEnvironment (name, config) { | ||
| if (!nuxt.options.experimental.viteEnvironmentApi && viteConfig.ssr) { | ||
| if (!nuxt.options.experimental.viteEnvironmentApi && name === 'ssr') { |
There was a problem hiding this comment.
if the environment api isn't enabled, this will be true for both server + client builds. that's why we trigger on viteConfig.ssr instead.
There was a problem hiding this comment.
The current implementation logic breaks optimizeDeps.include:
- If the vite environment api is not enabled
- and the vite config has
ssrset - (intended to affect the server runtime) - then remove
optimizeDeps.include💥 - (avoids page reloads when vite lazily discovers a new dependency to optimize and transforms CJS deps to ESM for the browser)
There was a problem hiding this comment.
then we should not pass ssr in the client config (in client.ts)...
checking the name here will not work
2 participants
🔗 Linked issue
📚 Description
This fixes two related problems:
vite.optimizeDeps.includeconfig.vite.optimizeDeps.includeworkaround to fail. A regression Commit was unsetting theincludefield.Details
The module plugins would be added to VFS via
plugins.client.mjs, not written to the filesystem, and so esbuild would not naturally discover the dependencies.I'd like to use a prerelease of these changes in my app to verify that this really does fix the problems. The unit test I've added uses mocking and is incapable of verifying the fix. Is there a way for me to do this?