feat(vitest-environment): support @vitest-environment-options#1673
feat(vitest-environment): support @vitest-environment-options#1673yamachi4416 wants to merge 1 commit intonuxt:mainfrom
@vitest-environment-options#1673Conversation
commit: |
yamachi4416
force-pushed
the
support-vitest-environment-options
branch
4 times, most recently
from
d80da26 to
acf91a9
Compare
📝 WalkthroughWalkthroughMerges per-test Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🧹 Nitpick comments (4)
src/runtime/shared/environment.ts (1)
79-83: Redundant optional chaining on required parameter.
environmentOptionsis now required (non-optional) per the newSetupWindowNuxtEnvironmentOptionssignature, so theenvironmentOptions?.optional chain on Line 80–81 is dead. The inner?.nuxtRuntimeConfig?.app?.baseURLchain is still needed sincenuxtRuntimeConfigis optional.♻️ Proposed tweak
const manifestOutputPath = joinURL( - environmentOptions?.nuxtRuntimeConfig?.app?.baseURL || '/', - environmentOptions?.nuxtRuntimeConfig?.app?.buildAssetsDir || '_nuxt', + environmentOptions.nuxtRuntimeConfig?.app?.baseURL || '/', + environmentOptions.nuxtRuntimeConfig?.app?.buildAssetsDir || '_nuxt', 'builds', )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/shared/environment.ts` around lines 79 - 83, The manifestOutputPath computation uses redundant optional chaining on the now-required parameter environmentOptions; change the references from environmentOptions?.nuxtRuntimeConfig to environmentOptions.nuxtRuntimeConfig (keep the remaining ?.nuxtRuntimeConfig?.app?.baseURL and ?.app?.buildAssetsDir chains intact) so that only the unnecessary leading ?. is removed; update the expression that constructs manifestOutputPath (the joinURL call where manifestOutputPath is declared) to reference environmentOptions directly while preserving the inner optional chains.src/module/plugins/options.ts (1)
22-28: DoubleJSON.stringifyis correct here, but add a brief comment.
definevalues must be JavaScript source expressions, so the payload needs to be a string literal whose runtime value is the JSON text.JSON.stringify(serializedOptions)achieves exactly that (turning{"a":1}into"\"{\\\"a\\\":1}\""), which is thenJSON.parse'd insrc/environments/vitest/index.ts. This looks odd at first glance and is easy to "simplify" incorrectly — a one-line comment would prevent future regressions.♻️ Suggested comment
config() { return { define: { + // Double-stringify: `define` values must be source expressions, so we need a JS string literal + // whose runtime value is the JSON text (consumed via JSON.parse in the Vitest env setup). 'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__': JSON.stringify(serializedOptions), }, } },Also worth confirming the
defineactually reaches the Vitest environment module (src/environments/vitest/index.ts) at runtime — this depends on that file being transformed by Vite.@nuxt/test-utilsis inserver.deps.inlineso this should hold, but it's worth a manual spot-check (e.g., log the raw env value from inside the environment) given a silent failure here would degrade to the exact bug#1541describes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/plugins/options.ts` around lines 22 - 28, Add a one-line inline comment above the define entry in the config() method (referencing define and serializedOptions) that explains why we call JSON.stringify(serializedOptions) twice so future readers don't remove the extra stringification (i.e., define expects a JS source expression string which will be parsed at runtime in src/environments/vitest/index.ts). After adding the comment, perform a quick runtime spot-check by logging the raw process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ value from src/environments/vitest/index.ts to confirm the define is being transformed and the JSON text is present at runtime.examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.ts (1)
1-17: Coverage looks good, but consider assertingjsdom-specific global to avoid UA-string false positives.
navigator.userAgentcontaining"jsdom"is a reasonable signal today, but a stronger assertion is the presence of a jsdom-only global/marker (e.g.,window._virtualConsoleor checking that happy-dom's signature is absent) to protect against happy-dom one day including the substring in its UA. Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.ts` around lines 1 - 17, Update the test "can override domEnvironment via `@vitest-environment-options`" to assert a jsdom-specific global in addition to the navigator check: keep the existing navigator.userAgent assertion and add a second assertion that a jsdom-only marker (for example window._virtualConsole is defined or another jsdom-specific property) exists so the test doesn't rely solely on UA-string content; modify the test body that references navigator.userAgent and add the additional expectation against window._virtualConsole (or another jsdom-only global) to strengthen the environment check.src/config.ts (1)
373-399: Docs accurately reflect the new scope.Calling out the
@vitest-environment-optionsoverride on exactly the two supported fields (url,domEnvironment) is appropriate. Consider also noting in the interface doc (or README) that othernuxt.*options inside an annotation are currently ignored, so users don't get surprised when setting e.g.rootIdvia annotation has no effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.ts` around lines 373 - 399, Update the interface/doc comment around the Nuxt test environment options (the properties url and domEnvironment) to explicitly state that only these two fields are read from the `@vitest-environment-options` annotation and that other nuxt.* options (for example rootId or values under overrides/dotenv) present in the annotation will be ignored; mention this behavior in the comment block near the url, domEnvironment and rootId declarations so users aren’t surprised when setting other options via the annotation has no effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/environments/vitest/index.ts`:
- Around line 70-90: mergeSetupWindowEnvironmentOptions currently only keeps
nuxt.url and nuxt.domEnvironment and silently drops any other keys under nuxt,
and also blindly spreads all top-level environmentOptions which lets unintended
top-level keys override base config; update mergeSetupWindowEnvironmentOptions
to (1) detect any keys present on the incoming environmentOptions.nuxt other
than "url" and "domEnvironment" and emit a single console.warn per unknown key
(include the key name and that it's ignored) to aid debugging, and (2) replace
the broad "...environmentOptions" spread with a narrow allowlist of safe
top-level keys (e.g., happyDom, jsdom, nuxtRuntimeConfig, nuxtRouteRules —
whatever the project deems safe) so only those top-level annotation keys can
override base config; reference the mergeSetupWindowEnvironmentOptions function
and the nuxt property when making these changes.
---
Nitpick comments:
In `@examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.ts`:
- Around line 1-17: Update the test "can override domEnvironment via
`@vitest-environment-options`" to assert a jsdom-specific global in addition to
the navigator check: keep the existing navigator.userAgent assertion and add a
second assertion that a jsdom-only marker (for example window._virtualConsole is
defined or another jsdom-specific property) exists so the test doesn't rely
solely on UA-string content; modify the test body that references
navigator.userAgent and add the additional expectation against
window._virtualConsole (or another jsdom-only global) to strengthen the
environment check.
In `@src/config.ts`:
- Around line 373-399: Update the interface/doc comment around the Nuxt test
environment options (the properties url and domEnvironment) to explicitly state
that only these two fields are read from the `@vitest-environment-options`
annotation and that other nuxt.* options (for example rootId or values under
overrides/dotenv) present in the annotation will be ignored; mention this
behavior in the comment block near the url, domEnvironment and rootId
declarations so users aren’t surprised when setting other options via the
annotation has no effect.
In `@src/module/plugins/options.ts`:
- Around line 22-28: Add a one-line inline comment above the define entry in the
config() method (referencing define and serializedOptions) that explains why we
call JSON.stringify(serializedOptions) twice so future readers don't remove the
extra stringification (i.e., define expects a JS source expression string which
will be parsed at runtime in src/environments/vitest/index.ts). After adding the
comment, perform a quick runtime spot-check by logging the raw
process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ value from
src/environments/vitest/index.ts to confirm the define is being transformed and
the JSON text is present at runtime.
In `@src/runtime/shared/environment.ts`:
- Around line 79-83: The manifestOutputPath computation uses redundant optional
chaining on the now-required parameter environmentOptions; change the references
from environmentOptions?.nuxtRuntimeConfig to
environmentOptions.nuxtRuntimeConfig (keep the remaining
?.nuxtRuntimeConfig?.app?.baseURL and ?.app?.buildAssetsDir chains intact) so
that only the unnecessary leading ?. is removed; update the expression that
constructs manifestOutputPath (the joinURL call where manifestOutputPath is
declared) to reference environmentOptions directly while preserving the inner
optional chains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30713eed-6c4a-4211-9f04-49d66ebcbf86
📒 Files selected for processing (7)
examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.tsexamples/app-vitest-full/tests/nuxt/vitest-environment-options-2.spec.tsexamples/app-vitest-full/tests/resolved-files.spec.tssrc/config.tssrc/environments/vitest/index.tssrc/module/plugins/options.tssrc/runtime/shared/environment.ts
yamachi4416
force-pushed
the
support-vitest-environment-options
branch
from
acf91a9 to
773442d
Compare
yamachi4416
force-pushed
the
support-vitest-environment-options
branch
from
773442d to
f00a8e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/environments/vitest/index.ts`:
- Around line 70-87: The mergeSetupWindowEnvironmentOptions function currently
falls back to an empty object when
process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ is missing, which loses
incoming options; change the fallback to use the already-parsed
nuxtEnvironmentOptions (from the local variable nuxtEnvironmentOptions) as the
base for defu instead of '{}', ensuring you JSON.parse the env var when present
and pass nuxt: { url, domEnvironment, ... } merged with
JSON.parse(process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ ||
JSON.stringify({ nuxt: nuxtEnvironmentOptions })) so properties like
nuxt.rootId, nuxt.mock, nuxt.h3Version, nuxtRuntimeConfig, and nuxtRouteRules
are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ad164f9-5977-44e7-993c-de3ba0c329ad
📒 Files selected for processing (7)
examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.tsexamples/app-vitest-full/tests/nuxt/vitest-environment-options-2.spec.tsexamples/app-vitest-full/tests/resolved-files.spec.tssrc/config.tssrc/environments/vitest/index.tssrc/module/plugins/options.tssrc/runtime/shared/environment.ts
✅ Files skipped from review due to trivial changes (3)
- examples/app-vitest-full/tests/resolved-files.spec.ts
- examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.ts
- src/config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/module/plugins/options.ts
- examples/app-vitest-full/tests/nuxt/vitest-environment-options-2.spec.ts
- src/runtime/shared/environment.ts
yamachi4416
force-pushed
the
support-vitest-environment-options
branch
from
f00a8e1 to
4dc68f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/module/plugins/options.ts`:
- Around line 22-26: The Vite plugin's config() currently only adds a Vite
define replacement for
'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__', which doesn't set
the real runtime env used by Vitest environments; modify the plugin's config()
implementation to also assign
process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ = serializedOptions (or
equivalent stringified value) at runtime before workers start so
src/environments/vitest/index.ts can read it during custom environment setup,
and keep the existing define entry only for transformed code that also needs
compile-time replacement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 755d2479-745d-4743-9c3c-022986dd2252
📒 Files selected for processing (7)
examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.tsexamples/app-vitest-full/tests/nuxt/vitest-environment-options-2.spec.tsexamples/app-vitest-full/tests/resolved-files.spec.tssrc/config.tssrc/environments/vitest/index.tssrc/module/plugins/options.tssrc/runtime/shared/environment.ts
✅ Files skipped from review due to trivial changes (4)
- examples/app-vitest-full/tests/resolved-files.spec.ts
- src/config.ts
- examples/app-vitest-full/tests/nuxt/vitest-environment-options-1.spec.ts
- examples/app-vitest-full/tests/nuxt/vitest-environment-options-2.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/environments/vitest/index.ts
| config() { | ||
| return { | ||
| define: { | ||
| 'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__': JSON.stringify(serializedOptions), | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify how the resolved Nuxt Vitest environment options are transported.
# Expectation: at least one assignment to the real process env before environment setup runs.
rg -n -C3 \
'__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__|process\.env\[[^]]*NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS|process\.env\.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__\s*='Repository: nuxt/test-utils
Length of output: 1057
🏁 Script executed:
#!/bin/bash
# Check for all references to the RESOLVED_OPTIONS env var to ensure we understand full context
rg -n '__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__' --type-list | head -20Repository: nuxt/test-utils
Length of output: 448
🏁 Script executed:
#!/bin/bash
# Look at the full context of the plugin to understand when config() is called
cat -n src/module/plugins/options.ts | head -40Repository: nuxt/test-utils
Length of output: 1085
🏁 Script executed:
#!/bin/bash
# Examine the environment setup to understand when it reads the env var
cat -n src/environments/vitest/index.ts | head -90 | tail -40Repository: nuxt/test-utils
Length of output: 1607
🏁 Script executed:
#!/bin/bash
# Check how this plugin is registered/used in the setup
rg -n 'NuxtVitestEnvironmentOptionsPlugin' --type ts -B 2 -A 2Repository: nuxt/test-utils
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Check how the environment is initialized and when it reads the env var
rg -n 'mergeSetupWindowEnvironmentOptions|setupWindow|environment.*setup' src/environments/vitest/index.ts -B 3 -A 3Repository: nuxt/test-utils
Length of output: 1262
🏁 Script executed:
#!/bin/bash
# Look at the full setup function to understand the execution order
sed -n '1,80p' src/environments/vitest/index.tsRepository: nuxt/test-utils
Length of output: 2828
🏁 Script executed:
#!/bin/bash
# Check if there are any test files that demonstrate the issue
find . -type f -name '*.test.*' -o -name '*.spec.*' | xargs grep -l '__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__' 2>/dev/null | head -5Repository: nuxt/test-utils
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the config to see when the plugin runs and how environment is set up
rg -n 'environmentOptions|test\.environment' src/config.ts | head -20Repository: nuxt/test-utils
Length of output: 685
🏁 Script executed:
#!/bin/bash
# Check Vitest documentation or code comments about when custom environments load
rg -n 'custom.*environment|environment.*load' src/ -i | head -20Repository: nuxt/test-utils
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check if the environment module is directly imported anywhere (would bypass Vite transforms)
rg -n "from.*vitest.*index|import.*environments/vitest" --type tsRepository: nuxt/test-utils
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Verify that process.env assignment would work in Vite plugin config()
rg -n 'plugin.*config.*function|export function.*Plugin' src/module/plugins/ -A 5 | head -30Repository: nuxt/test-utils
Length of output: 496
Set the resolved options to the actual runtime environment before tests load.
The Vite define replacement at line 25 only applies to code transformed by Vite, but src/environments/vitest/index.ts reads process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ at runtime during custom environment setup (line 78). Custom Vitest environments may load outside Vite's transform pipeline, leaving the env var undefined and causing annotated options to discard config defaults.
Set the real env var in the plugin's config() method before Vitest workers initialize the environment, and keep the define replacement only if transformed code also needs it.
Refs: Vite define, Vitest custom environments.
Proposed fix
const PLUGIN_NAME = 'nuxt:vitest:nuxt-environment-options'
const STUB_ID = 'nuxt-vitest-environment-options'
+const RESOLVED_OPTIONS_ENV = '__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__'
export function NuxtVitestEnvironmentOptionsPlugin(environmentOptions: EnvironmentOptions = {}): Plugin {
const serializedOptions = JSON.stringify(environmentOptions)
return {
@@
},
config() {
+ process.env[RESOLVED_OPTIONS_ENV] = serializedOptions
+
return {
define: {
- 'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__': JSON.stringify(serializedOptions),
+ [`process.env.${RESOLVED_OPTIONS_ENV}`]: JSON.stringify(serializedOptions),
},
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config() { | |
| return { | |
| define: { | |
| 'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__': JSON.stringify(serializedOptions), | |
| }, | |
| config() { | |
| process.env[RESOLVED_OPTIONS_ENV] = serializedOptions | |
| return { | |
| define: { | |
| [`process.env.${RESOLVED_OPTIONS_ENV}`]: JSON.stringify(serializedOptions), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/module/plugins/options.ts` around lines 22 - 26, The Vite plugin's
config() currently only adds a Vite define replacement for
'process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__', which doesn't set
the real runtime env used by Vitest environments; modify the plugin's config()
implementation to also assign
process.env.__NUXT_VITEST_ENVIRONMENT_RESOLVED_OPTIONS__ = serializedOptions (or
equivalent stringified value) at runtime before workers start so
src/environments/vitest/index.ts can read it during custom environment setup,
and keep the existing define entry only for transformed code that also needs
compile-time replacement.
1 participant
🔗 Linked issue
resolves #1541
📚 Description
Added support for
@vitest-environment-options, initially limited tourlanddomEnvironmentas they are safe to change per test file.Reproduction
Before:
https://stackblitz.com/edit/nuxt-test-utils-pull-1673-before?file=package.json
After:
https://stackblitz.com/edit/nuxt-test-utils-pull-1673-after?file=package.json