-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
test(vue-router) basepath file based #6110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new end-to-end test example project for Vue Router with basepath support is added under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
View your CI Pipeline Execution ↗ for commit f6abd59
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
e2e/vue-router/basepath-file-based/src/main.tsx (1)
23-30: Unusual conditional mounting pattern.Checking
!rootElement.innerHTMLbefore mounting is non-standard. This pattern might be for hot module replacement (HMR) support, but it could cause issues if the element has any content.If this is for HMR support, consider documenting the reason with a comment:
+// Only mount if the app hasn't been mounted yet (useful for HMR) if (!rootElement.innerHTML) { const app = createApp({Alternatively, if HMR support isn't needed for this e2e test, simplify to always mount:
-if (!rootElement.innerHTML) { - const app = createApp({ - setup() { - return () => h(RouterProvider, { router }) - }, - }) - app.mount('#app') -} +const app = createApp({ + setup() { + return () => h(RouterProvider, { router }) + }, +}) +app.mount('#app')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
e2e/vue-router/basepath-file-based/.gitignore(1 hunks)e2e/vue-router/basepath-file-based/index.html(1 hunks)e2e/vue-router/basepath-file-based/package.json(1 hunks)e2e/vue-router/basepath-file-based/playwright.config.ts(1 hunks)e2e/vue-router/basepath-file-based/src/main.tsx(1 hunks)e2e/vue-router/basepath-file-based/src/routeTree.gen.ts(1 hunks)e2e/vue-router/basepath-file-based/src/routes/__root.tsx(1 hunks)e2e/vue-router/basepath-file-based/src/routes/about.tsx(1 hunks)e2e/vue-router/basepath-file-based/src/routes/index.tsx(1 hunks)e2e/vue-router/basepath-file-based/tests/reload-document.test.ts(1 hunks)e2e/vue-router/basepath-file-based/tests/setup/global.setup.ts(1 hunks)e2e/vue-router/basepath-file-based/tests/setup/global.teardown.ts(1 hunks)e2e/vue-router/basepath-file-based/tsconfig.json(1 hunks)e2e/vue-router/basepath-file-based/vite.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/vue-router/basepath-file-based/src/routes/__root.tsxe2e/vue-router/basepath-file-based/playwright.config.tse2e/vue-router/basepath-file-based/src/routes/about.tsxe2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/tests/reload-document.test.tse2e/vue-router/basepath-file-based/tests/setup/global.setup.tse2e/vue-router/basepath-file-based/src/main.tsxe2e/vue-router/basepath-file-based/tests/setup/global.teardown.tse2e/vue-router/basepath-file-based/src/routeTree.gen.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/vue-router/basepath-file-based/src/routes/__root.tsxe2e/vue-router/basepath-file-based/playwright.config.tse2e/vue-router/basepath-file-based/src/routes/about.tsxe2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/tests/reload-document.test.tse2e/vue-router/basepath-file-based/tests/setup/global.setup.tse2e/vue-router/basepath-file-based/vite.config.jse2e/vue-router/basepath-file-based/src/main.tsxe2e/vue-router/basepath-file-based/tests/setup/global.teardown.tse2e/vue-router/basepath-file-based/src/routeTree.gen.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/vue-router/basepath-file-based/package.json
🧠 Learnings (9)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/vue-router/basepath-file-based/.gitignoree2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/src/main.tsxe2e/vue-router/basepath-file-based/tsconfig.jsone2e/vue-router/basepath-file-based/package.jsone2e/vue-router/basepath-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/vue-router/basepath-file-based/.gitignoree2e/vue-router/basepath-file-based/playwright.config.tse2e/vue-router/basepath-file-based/src/routes/about.tsxe2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/tests/setup/global.setup.tse2e/vue-router/basepath-file-based/src/main.tsxe2e/vue-router/basepath-file-based/tests/setup/global.teardown.tse2e/vue-router/basepath-file-based/tsconfig.jsone2e/vue-router/basepath-file-based/package.jsone2e/vue-router/basepath-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-09T12:59:14.842Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/public/site.webmanifest:2-3
Timestamp: 2025-10-09T12:59:14.842Z
Learning: In e2e test fixtures (files under e2e directories), empty or placeholder values in configuration files like site.webmanifest are acceptable and should not be flagged unless the test specifically validates those fields.
Applied to files:
e2e/vue-router/basepath-file-based/playwright.config.tse2e/vue-router/basepath-file-based/tests/setup/global.setup.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/vue-router/basepath-file-based/src/routes/about.tsxe2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/src/routeTree.gen.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/vue-router/basepath-file-based/src/routes/about.tsxe2e/vue-router/basepath-file-based/src/routes/index.tsxe2e/vue-router/basepath-file-based/vite.config.jse2e/vue-router/basepath-file-based/src/main.tsxe2e/vue-router/basepath-file-based/tsconfig.jsone2e/vue-router/basepath-file-based/package.jsone2e/vue-router/basepath-file-based/src/routeTree.gen.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
e2e/vue-router/basepath-file-based/tests/setup/global.setup.tse2e/vue-router/basepath-file-based/package.jsone2e/vue-router/basepath-file-based/src/routeTree.gen.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript strict mode with extensive type safety for all code
Applied to files:
e2e/vue-router/basepath-file-based/tsconfig.json
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
e2e/vue-router/basepath-file-based/package.json
🧬 Code graph analysis (7)
e2e/vue-router/basepath-file-based/src/routes/__root.tsx (2)
e2e/vue-router/basepath-file-based/src/routes/about.tsx (1)
Route(3-5)e2e/vue-router/basepath-file-based/src/routes/index.tsx (1)
Route(3-5)
e2e/vue-router/basepath-file-based/playwright.config.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/vue-router/basepath-file-based/src/routes/about.tsx (2)
e2e/vue-router/basepath-file-based/src/routes/__root.tsx (1)
Route(3-3)e2e/vue-router/basepath-file-based/src/routes/index.tsx (1)
Route(3-5)
e2e/vue-router/basepath-file-based/src/routes/index.tsx (2)
e2e/vue-router/basepath-file-based/src/routes/__root.tsx (1)
Route(3-3)e2e/vue-router/basepath-file-based/src/routes/about.tsx (1)
Route(3-5)
e2e/vue-router/basepath-file-based/tests/setup/global.setup.ts (2)
e2e/vue-router/basepath-file-based/src/main.tsx (1)
setup(25-27)scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/vue-router/basepath-file-based/src/main.tsx (1)
e2e/solid-router/js-only-file-based/src/main.jsx (1)
rootElement(14-14)
e2e/vue-router/basepath-file-based/tests/setup/global.teardown.ts (2)
scripts/set-ts-version.js (1)
packageJson(33-33)e2e/solid-router/basic-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Preview
🔇 Additional comments (14)
e2e/vue-router/basepath-file-based/.gitignore (1)
1-11: ✓ Solid gitignore setup.The patterns appropriately cover build artifacts, test outputs, and dependencies for a Vite + Playwright e2e project. No issues detected.
e2e/vue-router/basepath-file-based/tsconfig.json (1)
1-16: LGTM! TypeScript configuration is appropriate.The configuration correctly enables strict mode as per guidelines and sets up proper Vue JSX integration with Vite tooling.
e2e/vue-router/basepath-file-based/index.html (1)
1-11: LGTM! Standard Vite/Vue HTML entry point.The minimal structure correctly provides the app mount point and loads the main entry module.
e2e/vue-router/basepath-file-based/package.json (2)
15-19: Good use of workspace protocol for internal dependencies.The workspace protocol is correctly applied to internal TanStack packages as per coding guidelines.
20-20: PostCSS 8.5.1 is compatible with Tailwind CSS 4.1.17No breaking changes or incompatibilities exist between PostCSS 8.5.1 and Tailwind CSS 4.1.17. This version meets Tailwind v4's PostCSS 8.x requirement. The caret syntax (
^8.5.1) allows automatic upgrades to newer 8.5.x versions if needed.e2e/vue-router/basepath-file-based/vite.config.js (1)
1-17: LGTM! Vite configuration correctly sets up basepath.The configuration properly enables the
/app/basepath and integrates TanStack Router with Vue target and automatic code splitting.e2e/vue-router/basepath-file-based/tests/reload-document.test.ts (1)
1-18: LGTM! E2E test comprehensively validates basepath navigation.The test correctly verifies navigation behavior with
reloadDocument: trueunder the/app/basepath, including component visibility checks.e2e/vue-router/basepath-file-based/tests/setup/global.teardown.ts (1)
1-6: LGTM! Standard test teardown implementation.The teardown correctly stops the e2e test server using the package name, following the established pattern.
e2e/vue-router/basepath-file-based/src/routes/about.tsx (1)
1-20: LGTM! Route implementation correctly uses TanStack Vue Router APIs.The component properly uses
Route.useNavigate()and implements navigation withreloadDocument: true, which is correctly tested in the e2e suite.e2e/vue-router/basepath-file-based/tests/setup/global.setup.ts (1)
1-6: LGTM!The global setup correctly starts the dummy server using the package name. The JSON import attributes syntax is appropriate for this use case.
e2e/vue-router/basepath-file-based/src/main.tsx (1)
1-19: LGTM - Router configuration is correct.The router setup with basepath '/app/' aligns with the test objectives. The module augmentation for type safety follows TanStack Router best practices.
e2e/vue-router/basepath-file-based/playwright.config.ts (1)
1-41: LGTM - Standard Playwright configuration for e2e testing.The configuration properly sets up:
- Port management for test and dummy servers
- Global setup/teardown lifecycle
- Vite build and preview server
- Single worker to avoid port conflicts
The top-level await for port resolution is appropriate given the async nature of port allocation.
e2e/vue-router/basepath-file-based/src/routeTree.gen.ts (1)
1-77: Skipping review - autogenerated file.This file is autogenerated by TanStack Router and should not be manually modified. Based on learnings, these files are not reviewed.
e2e/vue-router/basepath-file-based/src/routes/index.tsx (1)
7-25: Remove verification request — this pattern is correct for TanStack Vue Router.Vue 3 functional components with JSX are a fully supported pattern for route components in TanStack Vue Router, and
Route.useNavigate()works correctly when called directly in the function body. The vite.config.js correctly enables thevueJsxplugin, and the identical pattern is used in both index.tsx and about.tsx, with e2e tests confirming the implementation works as intended.Likely an incorrect or invalid review comment.
| } | ||
| } | ||
|
|
||
| const rootElement = document.getElementById('app')! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-null assertion could fail if element is missing.
The non-null assertion (!) assumes the #app element always exists. If the element is missing from the HTML, this will cause a runtime error.
Consider adding a null check:
-const rootElement = document.getElementById('app')!
+const rootElement = document.getElementById('app')
+if (!rootElement) {
+ throw new Error('Failed to find #app element')
+}📝 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.
| const rootElement = document.getElementById('app')! | |
| const rootElement = document.getElementById('app') | |
| if (!rootElement) { | |
| throw new Error('Failed to find #app element') | |
| } |
🤖 Prompt for AI Agents
In e2e/vue-router/basepath-file-based/src/main.tsx around line 21, remove the
non-null assertion on document.getElementById('app') as it can throw at runtime
if the element is absent; instead get the element into a nullable variable,
check for null and handle it (for example throw a clear error like "Root element
#app not found" or create/append a fallback DIV with id "app" before mounting),
then use the verified element for mounting.
| import { createRootRoute } from '@tanstack/vue-router' | ||
|
|
||
| export const Route = createRootRoute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Outlet component in root route.
The root route lacks a component with an Outlet, which is required to render child routes (index and about). Without it, navigation will fail and the e2e tests will not work correctly.
Apply this diff to add the required component with Outlet:
-import { createRootRoute } from '@tanstack/vue-router'
+import { createRootRoute, Outlet } from '@tanstack/vue-router'
-export const Route = createRootRoute()
+export const Route = createRootRoute({
+ component: () => <Outlet />,
+})📝 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.
| import { createRootRoute } from '@tanstack/vue-router' | |
| export const Route = createRootRoute() | |
| import { createRootRoute, Outlet } from '@tanstack/vue-router' | |
| export const Route = createRootRoute({ | |
| component: () => <Outlet />, | |
| }) |
🤖 Prompt for AI Agents
In e2e/vue-router/basepath-file-based/src/routes/__root.tsx lines 1-3, the root
route is created without a component that renders an Outlet, so child routes
can't render; import the Outlet component from '@tanstack/vue-router', define a
small root component that returns the Outlet (e.g., a functional component or
setup component that renders Outlet), and pass that component into
createRootRoute({ component: YourRootComponent }) before exporting Route so
child routes can mount.
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.