fix(e2e): unblock the dev e2e suite after the nuxt-nightly 5x upgrade - #1054
Merged
Merged
Conversation
Three independent issues were failing `pnpm test:e2e:dev` on every push since nuxt#1048 (nuxt-nightly 5x): - `@pinia/nuxt@1.0.1`'s own compatibility gate (`^3.15.0 || ^4.0.0`) doesn't know about Nuxt 5 yet, so Nuxt silently disabled the module in `tab-pinia` (NUXT_B8013) — breaking `defineStore` auto-imports and crashing the whole playground. Wrap it in a small local module (`pinia-nuxt5-compat.ts`) that loosens the gate via the module's own `getMeta()` contract before installing it for real; nothing in its `setup()` actually depends on Nuxt-4-only internals. - `tab-pinia`/`tab-server-route`'s `custom-module` server handler called the global `defineEventHandler` without importing it — unlike every other server file in this repo, which imports it from `h3` explicitly. Nitro's dev auto-import doesn't reliably cover locally-added module routes outside the conventional `server/` dir, so the handler occasionally failed to resolve, wedging the whole Nitro dev worker (`defineEventHandler is not defined`, 503s on every route). Import it explicitly like the rest of the codebase does. - The built-in-components e2e test asserted on component names (`NuxtPage`/`NuxtLink`/...) rendered inside a `<VDropdown>` trigger, which floating-vue fails to render under this stack — the same 'known remaining issue' nuxt#1048 flagged but didn't fix. Assert on the section heading/count instead, which render outside the broken dropdown. 🤖 Generated with the help of an agent.
Contributor
📝 WalkthroughWalkthroughThe change adds a Nuxt 5 compatibility wrapper for Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@tests/e2e/specs/tabs.spec.ts`:
- Around line 50-53: Update the component-count assertion in the devtoolsFrame
body check so it rejects zero and validates the expected stable component count,
rather than accepting any digit sequence. Keep the existing “Built-in
components” assertion and timeout unchanged.
🪄 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 Plus
Run ID: 20aa3e25-e186-4968-b907-04d7d72daf85
📒 Files selected for processing (5)
playgrounds/tab-pinia/modules/custom-module/runtime/server/api/hello.tsplaygrounds/tab-pinia/modules/pinia-nuxt5-compat.tsplaygrounds/tab-pinia/nuxt.config.tsplaygrounds/tab-server-route/modules/custom-module/runtime/server/api/hello.tstests/e2e/specs/tabs.spec.ts
Comment on lines
50
to
+53
| await expect(devtoolsFrame().locator('body')) | ||
| .toContainText(/NuxtPage|NuxtLink|NuxtLayout/, { timeout: 15_000 }) | ||
| .toContainText(/Built-in components/i, { timeout: 15_000 }) | ||
| await expect(devtoolsFrame().locator('body')) | ||
| .toContainText(/Total components: \d+/i, { timeout: 15_000 }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a non-zero or exact component count.
Line 53 accepts Total components: 0 and any incorrect positive count. A component discovery failure can therefore pass this test. Assert a non-zero count or the expected stable count.
Proposed fix
- .toContainText(/Total components: \d+/i, { timeout: 15_000 })
+ .toContainText(/Total components: [1-9]\d*/i, { timeout: 15_000 })📝 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.
Suggested change
| await expect(devtoolsFrame().locator('body')) | |
| .toContainText(/NuxtPage|NuxtLink|NuxtLayout/, { timeout: 15_000 }) | |
| .toContainText(/Built-in components/i, { timeout: 15_000 }) | |
| await expect(devtoolsFrame().locator('body')) | |
| .toContainText(/Total components: \d+/i, { timeout: 15_000 }) | |
| await expect(devtoolsFrame().locator('body')) | |
| .toContainText(/Built-in components/i, { timeout: 15_000 }) | |
| await expect(devtoolsFrame().locator('body')) | |
| .toContainText(/Total components: [1-9]\d*/i, { timeout: 15_000 }) |
🤖 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 `@tests/e2e/specs/tabs.spec.ts` around lines 50 - 53, Update the
component-count assertion in the devtoolsFrame body check so it rejects zero and
validates the expected stable component count, rather than accepting any digit
sequence. Keep the existing “Built-in components” assertion and timeout
unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
e2ehas been failing on every push tomainsince #1048 (nuxt-nightly5x). Three independent issues were behind it:
@pinia/nuxtdisabled on Nuxt 5.@pinia/nuxt@1.0.1's owncompatibility gate (
^3.15.0 || ^4.0.0) doesn't know about Nuxt 5 yet,so Nuxt silently disables the module (
NUXT_B8013) in thetab-piniaplayground — breaking
defineStoreauto-imports and crashing the wholepage.
playgrounds/tab-pinia/modules/pinia-nuxt5-compat.tswraps themodule and loosens the gate via its own
getMeta()contract (a stable@nuxt/kitAPI) before installing it for real, rather than patching thepackage or forking the compatibility check. Nothing in its
setup()actually depends on Nuxt-4-only internals — it just registers a plugin,
wires up SSR hydration, and adds auto-imports.
tab-pinia's andtab-server-route'scustom-moduleserver handler called the globaldefineEventHandlerwithout importing it, unlike every other serverfile in this repo (which imports it from
h3explicitly). Nitro's devauto-import doesn't reliably cover locally-added module routes outside
the conventional
server/dir, so the handler intermittently failed toresolve at bundle time, crashing the whole dev worker
(
defineEventHandler is not defined) and 503ing every route — not justthe custom one. Importing it explicitly, like the rest of the codebase
already does, removes the dependency on that auto-import entirely.
e2e test asserted on component names (
NuxtPage/NuxtLink/...)rendered inside a
<VDropdown>trigger. floating-vue fails to renderthat trigger's default slot under this stack (confirmed live: the
row's
<button>/<ComponentName>never mounts;PopperthrowsCannot destructure property 'popperId' of 'undefined'invoking itsown scoped slot) — the same 'known remaining issue' deps: upgrade to nuxt-nightly (5x) and vite ^8.1 #1048 already
flagged but didn't fix. It affects every component/composable/route
name rendered inside a dropdown trigger, not just this list. The test
now asserts on the section heading and count, which render directly
from
ComponentsList.vuerather than through the broken dropdown.Verification
pnpm test:e2e:dev— 20 passed, 0 failed, 52 skipped (was 3 failed).pnpm test:e2e:built— unaffected, still green.pnpm build,pnpm lint,pnpm test:unit,pnpm typecheckall pass.🤖 Generated with the help of an agent.