feat(browser): add render helper for vitest browser mode#1725
Open
yamachi4416 wants to merge 2 commits into
Open
feat(browser): add render helper for vitest browser mode#1725yamachi4416 wants to merge 2 commits into
yamachi4416 wants to merge 2 commits into
Conversation
commit: |
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
from
2059d23 to
a041a16
Compare
yamachi4416
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR introduces a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 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: 6
🤖 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-browser/app/pages/components/hello.vue`:
- Line 3: The `name` computed property is read-only (getter-only) but is used
with `v-model` on line 11, which requires a writable binding. Modify the `name`
computed property to include a setter function that handles updates to the
value, or alternatively refactor to use a separate `ref` for tracking the name
state while initializing its value from `route.query.name` to maintain the
initial synchronization. The computed property with a setter approach would
allow `v-model` updates to be properly handled while maintaining the route query
synchronization logic.
In `@examples/app-vitest-browser/test/nuxt/components/render-with-fetch.spec.ts`:
- Around line 7-10: The registerEndpoint() call that registers the `/api/hello`
endpoint lacks a corresponding teardown mechanism, leaving the endpoint
registered in the shared registry after the test completes. This causes
cross-test state leakage where the endpoint remains registered for subsequent
tests. Add an afterEach() cleanup hook that unregisters or clears the
`/api/hello` endpoint registration after each test runs to ensure a clean state
between tests and prevent order-dependent test failures.
In `@examples/app-vitest-browser/test/nuxt/components/render.spec.ts`:
- Line 27: The assertion in the render spec test is too strict by matching the
exact JSON string with specific quote marks and spacing in getByText(). Replace
the exact string match with a regex pattern or flexible matcher that tolerates
various whitespace and formatting variations while still validating that the
buildAssetsDir key and /_nuxt/ value are present in the rendered output. This
will prevent false test failures from non-functional formatting changes to the
JSON serialization.
In `@examples/app-vitest-workspace/app3/test/browser/render.spec.ts`:
- Around line 25-27: The 'can rerender' test is currently using `page.render()`
to set up the initial render, which duplicates the page-specific render path
already tested elsewhere and prevents the actual testing of the
`render().rerender()` pattern. Replace the `page.render()` call in this test
with the standard `render()` function to properly initialize the component,
allowing the subsequent `rerender()` call to test the non-page rerender
functionality as intended.
In `@examples/app-vitest-workspace/package.json`:
- Around line 25-27: The package.json is missing an explicit declaration of the
`playwright` dependency which is required as a peer dependency by
`@vitest/browser-playwright@4.1.8`. Although `playwright-core` is present, it
does not satisfy the peer dependency requirement. Add `"playwright": "1.60.0"`
to the devDependencies object in package.json, placing it alongside the existing
entries like `@vitest/browser-playwright`, `happy-dom`, and `playwright-core` to
ensure the required peer dependency is explicitly declared.
In `@package.json`:
- Around line 54-58: The typesVersions entries for "vitest-browser-nuxt" and
"vitest-browser-nuxt/pure" in package.json are pointing to root-level
declaration files (./dist/index.d.mts and ./dist/pure.d.mts), but these files
are actually generated in nested locations under the vitest-browser-nuxt
directory structure. Update the paths for both the "vitest-browser-nuxt" and
"vitest-browser-nuxt/pure" entries in the typesVersions object to point to the
correct nested locations where the declaration files are actually generated
during the build process.
🪄 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: 50490d1a-d8d8-4898-8910-8b8851b22b5c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
examples/app-vitest-browser/app.vueexamples/app-vitest-browser/app/app.vueexamples/app-vitest-browser/app/components/Header.vueexamples/app-vitest-browser/app/components/MyCounter.vueexamples/app-vitest-browser/app/components/MyHello.vueexamples/app-vitest-browser/app/composables/useCounter.tsexamples/app-vitest-browser/app/composables/useHelloApi.tsexamples/app-vitest-browser/app/layouts/default.vueexamples/app-vitest-browser/app/pages/components.vueexamples/app-vitest-browser/app/pages/components/counter.vueexamples/app-vitest-browser/app/pages/components/hello.vueexamples/app-vitest-browser/app/pages/index.vueexamples/app-vitest-browser/nuxt.config.tsexamples/app-vitest-browser/server/api/hello.get.tsexamples/app-vitest-browser/test/nuxt/components/mock-nuxt-import.spec.tsexamples/app-vitest-browser/test/nuxt/components/mount-suspended.spec.tsexamples/app-vitest-browser/test/nuxt/components/render-with-fetch.spec.tsexamples/app-vitest-browser/test/nuxt/components/render.spec.tsexamples/app-vitest-browser/test/nuxt/components/render.vue.spec.tsexamples/app-vitest-browser/test/nuxt/pages/render-components-hello.spec.tsexamples/app-vitest-browser/test/nuxt/pages/render-index.spec.tsexamples/app-vitest-browser/tsconfig.jsonexamples/app-vitest-browser/vitest.config.tsexamples/app-vitest-workspace/app3/nuxt.config.tsexamples/app-vitest-workspace/app3/test/browser/render.spec.tsexamples/app-vitest-workspace/app3/vitest.browser.config.tsexamples/app-vitest-workspace/app3/vitest.config.tsexamples/app-vitest-workspace/package.jsonpackage.jsonsrc/runtime-utils/utils/suspended.tssrc/vitest-browser-nuxt/index.tssrc/vitest-browser-nuxt/pure.tstsdown.config.ts
💤 Files with no reviewable changes (1)
- examples/app-vitest-browser/app.vue
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
2 times, most recently
from
1242c5f to
5ebdb4a
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
from
5ebdb4a to
fb97146
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-browser/server/api/hello.get.ts`:
- Around line 4-5: The getQuery function does not enforce runtime type
validation, so the name parameter can be undefined, a string, or an array of
strings at runtime, but the code assumes it is always a string. Add a
normalization step after extracting name from the query object to handle these
cases: check if name is undefined or an array, and either provide a default
value (like an empty string or "Guest"), extract the first element if it is an
array, or trim/validate the string value. This ensures the return statement in
the hello.get.ts endpoint always receives a properly handled name value.
In `@package.json`:
- Around line 21-22: The export paths for the "./vitest-browser-nuxt" and
"./vitest-browser-nuxt/pure" subpaths are pointing to dist directories instead
of source directories, which breaks workspace and local development imports
before build artifacts are generated. Change both export paths from
"./dist/vitest-browser-nuxt/..." to "./src/vitest-browser-nuxt/..." to match the
pattern of other top-level exports, allowing the publishConfig.exports to handle
the dist paths for published versions.
🪄 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: a58936ad-9640-45f3-ac90-9a68f7d81022
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
examples/app-vitest-browser/app.vueexamples/app-vitest-browser/app/app.vueexamples/app-vitest-browser/app/components/Header.vueexamples/app-vitest-browser/app/components/MyCounter.vueexamples/app-vitest-browser/app/components/MyHello.vueexamples/app-vitest-browser/app/composables/useCounter.tsexamples/app-vitest-browser/app/composables/useHelloApi.tsexamples/app-vitest-browser/app/layouts/default.vueexamples/app-vitest-browser/app/pages/components.vueexamples/app-vitest-browser/app/pages/components/counter.vueexamples/app-vitest-browser/app/pages/components/hello.vueexamples/app-vitest-browser/app/pages/index.vueexamples/app-vitest-browser/nuxt.config.tsexamples/app-vitest-browser/server/api/hello.get.tsexamples/app-vitest-browser/test/nuxt/components/mock-nuxt-import.spec.tsexamples/app-vitest-browser/test/nuxt/components/mount-suspended.spec.tsexamples/app-vitest-browser/test/nuxt/components/render-with-fetch.spec.tsexamples/app-vitest-browser/test/nuxt/components/render.spec.tsexamples/app-vitest-browser/test/nuxt/components/render.vue.spec.tsexamples/app-vitest-browser/test/nuxt/pages/render-components-hello.spec.tsexamples/app-vitest-browser/test/nuxt/pages/render-index.spec.tsexamples/app-vitest-browser/tsconfig.jsonexamples/app-vitest-browser/vitest.config.tsexamples/app-vitest-workspace/app3/nuxt.config.tsexamples/app-vitest-workspace/app3/test/browser/render.spec.tsexamples/app-vitest-workspace/app3/vitest.browser.config.tsexamples/app-vitest-workspace/app3/vitest.config.tsexamples/app-vitest-workspace/package.jsonpackage.jsonsrc/runtime-utils/utils/suspended.tssrc/vitest-browser-nuxt/index.tssrc/vitest-browser-nuxt/pure.tstsdown.config.ts
💤 Files with no reviewable changes (1)
- examples/app-vitest-browser/app.vue
✅ Files skipped from review due to trivial changes (1)
- examples/app-vitest-browser/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (29)
- examples/app-vitest-browser/test/nuxt/components/mount-suspended.spec.ts
- examples/app-vitest-browser/app/composables/useCounter.ts
- examples/app-vitest-workspace/app3/vitest.browser.config.ts
- examples/app-vitest-browser/nuxt.config.ts
- examples/app-vitest-browser/app/components/Header.vue
- examples/app-vitest-browser/tsconfig.json
- examples/app-vitest-browser/app/layouts/default.vue
- examples/app-vitest-browser/test/nuxt/pages/render-index.spec.ts
- src/runtime-utils/utils/suspended.ts
- examples/app-vitest-browser/test/nuxt/components/render.spec.ts
- examples/app-vitest-browser/app/pages/index.vue
- examples/app-vitest-browser/app/components/MyCounter.vue
- examples/app-vitest-workspace/app3/vitest.config.ts
- examples/app-vitest-workspace/app3/nuxt.config.ts
- examples/app-vitest-browser/app/app.vue
- examples/app-vitest-browser/app/components/MyHello.vue
- tsdown.config.ts
- examples/app-vitest-browser/app/composables/useHelloApi.ts
- examples/app-vitest-workspace/app3/test/browser/render.spec.ts
- examples/app-vitest-browser/app/pages/components.vue
- examples/app-vitest-browser/app/pages/components/hello.vue
- examples/app-vitest-browser/test/nuxt/components/render.vue.spec.ts
- examples/app-vitest-browser/test/nuxt/components/render-with-fetch.spec.ts
- examples/app-vitest-browser/test/nuxt/pages/render-components-hello.spec.ts
- examples/app-vitest-browser/app/pages/components/counter.vue
- examples/app-vitest-workspace/package.json
- src/vitest-browser-nuxt/pure.ts
- src/vitest-browser-nuxt/index.ts
- examples/app-vitest-browser/test/nuxt/components/mock-nuxt-import.spec.ts
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
from
fb97146 to
fa70fad
Compare
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
2 times, most recently
from
38053be to
a608509
Compare
yamachi4416
force-pushed
the
feat-improve-vitest-browser-mode
branch
from
a608509 to
86ac63d
Compare
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
1 participant
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.
🔗 Linked issue
📚 Description
Added a render helper wrapping
vitest-browser-vuewith a suspense helper, to be used in Vitest Browser Mode.