fix(config): add patch ssr.resolve.conditions#1732
Conversation
commit: |
📝 WalkthroughWalkthroughThis change updates the Estimated code review effort: 1 (Trivial) | ~5 minutes Changes
Related issues: Suggested labels: bug, vitest Suggested reviewers: danielroe 🚥 Pre-merge checks | ✅ 5✅ 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.
🧹 Nitpick comments (1)
src/config.ts (1)
219-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider returning a partial config instead of mutating
configdirectly.Vite's plugin API docs recommend returning a partial config object (deep-merged automatically) and reserve direct mutation for cases where merging can't achieve the desired result: It can return a partial config object that will be deeply merged into existing config, or directly mutate the config (if the default merging cannot achieve the desired result). The official
configEnvironmentexample for this exact use case also returns a partial config: configEnvironment(name: string, options: EnvironmentOptions) { // add "workerd" condition to the rsc environment if (name === 'rsc') { return { resolve: { conditions: ['workerd'], }, } } }♻️ Align with documented pattern
{ // https://github.com/nuxt/test-utils/issues/1635 name: 'nuxt:test-utils:patch-ssr-conditions', enforce: 'post', configEnvironment(name, config) { - if (name === 'ssr' && config.resolve?.conditions) { - config.resolve.conditions = config.resolve.conditions.filter(x => x !== 'import') - } + if (name === 'ssr' && config.resolve?.conditions) { + return { + resolve: { + conditions: config.resolve.conditions.filter(x => x !== 'import'), + }, + } + } }, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config.ts` around lines 219 - 228, The `nuxt:test-utils:patch-ssr-conditions` plugin in `configEnvironment` is mutating `config.resolve.conditions` directly, but Vite recommends returning a partial config instead. Update `configEnvironment(name, config)` so that when `name === 'ssr'` and conditions include `'import'`, it returns a partial `{ resolve: { conditions: [...] } }` object with `'import'` removed, instead of editing `config` in place. Keep the existing behavior and guard around `config.resolve?.conditions`, but express the change through the returned config object to match the documented `configEnvironment` pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/config.ts`:
- Around line 219-228: The `nuxt:test-utils:patch-ssr-conditions` plugin in
`configEnvironment` is mutating `config.resolve.conditions` directly, but Vite
recommends returning a partial config instead. Update `configEnvironment(name,
config)` so that when `name === 'ssr'` and conditions include `'import'`, it
returns a partial `{ resolve: { conditions: [...] } }` object with `'import'`
removed, instead of editing `config` in place. Keep the existing behavior and
guard around `config.resolve?.conditions`, but express the change through the
returned config object to match the documented `configEnvironment` pattern.
| enforce: 'post', | ||
| configEnvironment(name, config) { | ||
| if (name === 'ssr' && config.resolve?.conditions) { | ||
| config.resolve.conditions = config.resolve.conditions.filter(x => x !== 'import') |
There was a problem hiding this comment.
should this live in the plugin above?
There was a problem hiding this comment.
Thank you.
config.ssr.resolve.conditions was causing the issue.
updated it to adjust config.ssr via the configResolved hook and unified it into the plugin above.
🔗 Linked issue
resolves #1635
📚 Description
This was an issue in
test-utils.config.environments.ssr.resolve.conditionsseems to be applied toconfig.ssr.resolve.conditionsfor backward compatibility. If'import'is included inconfig.ssr.resolve.conditions, Vitest fails to resolve thepgmodule in test run.Reproduction
https://stackblitz.com/edit/nuxt-test-utils-pull-1732-after?file=package.json