Skip to content

fix(config): browser setup in defineVitestProject#1724

Open
yamachi4416 wants to merge 1 commit into
nuxt:mainfrom
yamachi4416:fix-vitest-projects-browser-mode
Open

fix(config): browser setup in defineVitestProject#1724
yamachi4416 wants to merge 1 commit into
nuxt:mainfrom
yamachi4416:fix-vitest-projects-browser-mode

Conversation

@yamachi4416

@yamachi4416 yamachi4416 commented Jun 16, 2026

Copy link
Copy Markdown
Member

🔗 Linked issue

📚 Description

Fixed an issue where the browser mode setup was not executed if the environment was not specified as 'nuxt' in defineVitestProject

@socket-security

socket-security Bot commented Jun 16, 2026

Copy link
Copy Markdown

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

@pkg-pr-new

pkg-pr-new Bot commented Jun 16, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@nuxt/test-utils@1724
npm i https://pkg.pr.new/vitest-environment-nuxt@1724

commit: 4b90de9

@yamachi4416 yamachi4416 force-pushed the fix-vitest-projects-browser-mode branch from 1bbac88 to 4b90de9 Compare June 16, 2026 15:18
@yamachi4416 yamachi4416 marked this pull request as ready for review June 16, 2026 15:31
@yamachi4416 yamachi4416 requested a review from danielroe as a code owner June 16, 2026 15:31
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

defineVitestProject in src/config.ts is changed to inject test.environment: 'nuxt' into the project config before calling resolveConfig, using defu, rather than assigning to resolvedConfig.test.environment after resolution. This ensures environment-gated logic inside resolveConfig (such as nuxt-browser setup injection) observes the environment value. A new vitest.browser.config.ts is added under examples/app-vitest-workspace/app1/ defining a browser-app1 Vitest project scoped to *.browser.spec.ts files with Playwright/Chromium. A corresponding app.browser.spec.ts tests useAppConfig() key shape and NuxtLink slot rendering via mountSuspended. @vitest/browser and @vitest/browser-playwright at 4.1.8 are added to devDependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing browser setup initialization in defineVitestProject when the environment is not explicitly set to 'nuxt'.
Description check ✅ Passed The description clearly explains the issue that was fixed: browser mode setup not being executed without explicit 'nuxt' environment specification in defineVitestProject.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@examples/app-vitest-workspace/app1/test/app.browser.spec.ts`:
- Around line 7-9: The assertion in the useAppConfig test block is checking for
exact key equality with toEqual(['nuxt']), which is brittle because it will fail
if any additional unrelated config keys are added. Instead of checking for exact
equality of all keys, change the expectation to use toContain('nuxt') to assert
only that the 'nuxt' key is present in the returned config object, making the
smoke test more resilient to future config additions.
🪄 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: 35d3e297-b4dd-4497-992c-2dd4d8f1484e

📥 Commits

Reviewing files that changed from the base of the PR and between e1a0f63 and 4b90de9.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • examples/app-vitest-workspace/app1/test/app.browser.spec.ts
  • examples/app-vitest-workspace/app1/vitest.browser.config.ts
  • examples/app-vitest-workspace/package.json
  • src/config.ts

Comment on lines +7 to +9
it('useAppConfig', () => {
expect(Object.keys(useAppConfig())).toEqual(['nuxt'])
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid strict full-key equality for useAppConfig() in this smoke test.

This assertion is brittle to unrelated config growth and can fail even when browser+Nuxt setup works. Assert the presence of the nuxt key instead.

Suggested patch
-    expect(Object.keys(useAppConfig())).toEqual(['nuxt'])
+    expect(useAppConfig()).toHaveProperty('nuxt')
🤖 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 `@examples/app-vitest-workspace/app1/test/app.browser.spec.ts` around lines 7 -
9, The assertion in the useAppConfig test block is checking for exact key
equality with toEqual(['nuxt']), which is brittle because it will fail if any
additional unrelated config keys are added. Instead of checking for exact
equality of all keys, change the expectation to use toContain('nuxt') to assert
only that the 'nuxt' key is present in the returned config object, making the
smoke test more resilient to future config additions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants