-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
test(vue-start): server-routes #6127
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
WalkthroughAdds a new Vue end-to-end test project under Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
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 488e336
☁️ 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: 5
🧹 Nitpick comments (6)
e2e/vue-start/server-routes/tsconfig.json (1)
10-10: Consider adding Vite client types for better type support.For Vite-based projects, it's recommended to include
"types": ["vite/client"]in compilerOptions to get proper typing for Vite-specific features likeimport.meta.env.Add after line 10:
"lib": ["DOM", "DOM.Iterable", "ES2022"], + "types": ["vite/client"], "isolatedModules": true,e2e/vue-start/server-routes/src/routes/methods/route.tsx (1)
7-14: Consider adding explicit return type for strict TypeScript.The
RouteComponentfunction lacks an explicit return type annotation. While TypeScript can infer the type, explicit annotations improve type safety and align with strict TypeScript practices.Apply this diff to add explicit typing:
-function RouteComponent() { +function RouteComponent(): JSX.Element { return ( <div class="p-8"> <h1 class="font-bold text-lg">Server Routes Methods E2E tests</h1> <Outlet /> </div> ) }As per coding guidelines requiring TypeScript strict mode with extensive type safety.
e2e/vue-start/server-routes/src/routes/api/middleware-context.ts (1)
20-26: Consider explicit type annotations for handler parameters.The GET handler parameters lack explicit type annotations. While TypeScript can infer types from the createFileRoute context, explicit annotations improve code clarity and type safety.
This is a minor improvement suggestion for strict TypeScript practices, though type inference should work correctly here. As per coding guidelines requiring extensive type safety.
e2e/vue-start/server-routes/src/router.tsx (1)
13-14: Minor inconsistency in component references.
DefaultCatchBoundaryis passed directly whileNotFoundis wrapped in an arrow function. For consistency, both could be passed directly:defaultErrorComponent: DefaultCatchBoundary, - defaultNotFoundComponent: () => <NotFound />, + defaultNotFoundComponent: NotFound,However, the current implementation is functionally correct.
e2e/vue-start/server-routes/src/routes/merge-middleware-context.tsx (1)
7-7: Consider adding a proper type instead ofany.Using
ref<any>bypasses TypeScript's type checking. Define an interface for the expected API response shape to improve type safety.+interface MiddlewareContextResponse { + url?: string + context?: { testParent?: boolean; test?: boolean } + error?: string +} + const MergeMiddlewareContext = defineComponent({ name: 'MergeMiddlewareContext', setup() { - const apiResponse = ref<any>(null) + const apiResponse = ref<MiddlewareContextResponse | null>(null)As per coding guidelines, TypeScript strict mode with extensive type safety is expected.
e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (1)
75-77: Addkeyprop to mapped elements.When mapping over arrays in JSX, each element should have a unique
keyprop to help Vue efficiently update the DOM.{HttpMethods.map((method) => ( - <Test method={method} /> + <Test key={method} method={method} /> ))}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
e2e/vue-start/server-routes/public/favicon.icois excluded by!**/*.icoe2e/vue-start/server-routes/public/favicon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
e2e/vue-start/server-routes/.gitignore(1 hunks)e2e/vue-start/server-routes/.prettierignore(1 hunks)e2e/vue-start/server-routes/package.json(1 hunks)e2e/vue-start/server-routes/playwright.config.ts(1 hunks)e2e/vue-start/server-routes/postcss.config.mjs(1 hunks)e2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsx(1 hunks)e2e/vue-start/server-routes/src/components/NotFound.tsx(1 hunks)e2e/vue-start/server-routes/src/routeTree.gen.ts(1 hunks)e2e/vue-start/server-routes/src/router.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/__root.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/api/middleware-context.ts(1 hunks)e2e/vue-start/server-routes/src/routes/api/only-any.ts(1 hunks)e2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.ts(1 hunks)e2e/vue-start/server-routes/src/routes/api/params/$foo/route.ts(1 hunks)e2e/vue-start/server-routes/src/routes/index.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/merge-middleware-context.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/methods/index.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/methods/only-any.tsx(1 hunks)e2e/vue-start/server-routes/src/routes/methods/route.tsx(1 hunks)e2e/vue-start/server-routes/src/styles/app.css(1 hunks)e2e/vue-start/server-routes/src/vite-env.d.ts(1 hunks)e2e/vue-start/server-routes/tests/server-routes.spec.ts(1 hunks)e2e/vue-start/server-routes/tsconfig.json(1 hunks)e2e/vue-start/server-routes/vite.config.ts(1 hunks)packages/vue-router/src/index.tsx(1 hunks)packages/vue-router/src/link.tsx(1 hunks)packages/vue-router/src/route.ts(6 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-start/server-routes/src/vite-env.d.tspackages/vue-router/src/link.tsxpackages/vue-router/src/index.tsxe2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsxe2e/vue-start/server-routes/vite.config.tse2e/vue-start/server-routes/src/routes/methods/index.tsxe2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/playwright.config.tse2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.tse2e/vue-start/server-routes/src/router.tsxe2e/vue-start/server-routes/src/routes/merge-middleware-context.tsxe2e/vue-start/server-routes/src/routes/methods/only-any.tsxe2e/vue-start/server-routes/src/routes/__root.tsxpackages/vue-router/src/route.tse2e/vue-start/server-routes/src/routes/api/params/$foo/route.tse2e/vue-start/server-routes/src/components/NotFound.tsxe2e/vue-start/server-routes/tests/server-routes.spec.tse2e/vue-start/server-routes/src/routes/api/only-any.tse2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsxe2e/vue-start/server-routes/src/routes/api/middleware-context.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/vue-start/server-routes/src/vite-env.d.tspackages/vue-router/src/link.tsxpackages/vue-router/src/index.tsxe2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsxe2e/vue-start/server-routes/vite.config.tse2e/vue-start/server-routes/src/routes/methods/index.tsxe2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/playwright.config.tse2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.tse2e/vue-start/server-routes/src/router.tsxe2e/vue-start/server-routes/src/routes/merge-middleware-context.tsxe2e/vue-start/server-routes/src/routes/methods/only-any.tsxe2e/vue-start/server-routes/src/routes/__root.tsxpackages/vue-router/src/route.tse2e/vue-start/server-routes/src/routes/api/params/$foo/route.tse2e/vue-start/server-routes/src/components/NotFound.tsxe2e/vue-start/server-routes/tests/server-routes.spec.tse2e/vue-start/server-routes/src/routes/api/only-any.tse2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsxe2e/vue-start/server-routes/src/routes/api/middleware-context.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/vue-start/server-routes/package.json
🧠 Learnings (9)
📚 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-start/server-routes/.prettierignoree2e/vue-start/server-routes/.gitignoree2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/src/routes/__root.tsxe2e/vue-start/server-routes/src/routes/api/params/$foo/route.tse2e/vue-start/server-routes/tsconfig.jsone2e/vue-start/server-routes/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-start/server-routes/.prettierignoree2e/vue-start/server-routes/.gitignoree2e/vue-start/server-routes/src/routes/methods/index.tsxe2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/src/routes/methods/only-any.tsxe2e/vue-start/server-routes/src/routes/api/params/$foo/route.tse2e/vue-start/server-routes/tests/server-routes.spec.tse2e/vue-start/server-routes/src/routes/api/only-any.tse2e/vue-start/server-routes/tsconfig.jsone2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsxe2e/vue-start/server-routes/src/routes/api/middleware-context.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-start/server-routes/.prettierignoree2e/vue-start/server-routes/src/routes/methods/index.tsxe2e/vue-start/server-routes/src/routes/methods/only-any.tsxe2e/vue-start/server-routes/src/routes/__root.tsxe2e/vue-start/server-routes/tests/server-routes.spec.tse2e/vue-start/server-routes/tsconfig.jsone2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsx
📚 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-start/server-routes/.prettierignoree2e/vue-start/server-routes/package.jsone2e/vue-start/server-routes/tsconfig.jsone2e/vue-start/server-routes/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: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/vue-start/server-routes/src/routes/methods/index.tsxe2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsx
📚 Learning: 2025-12-17T02:17:47.423Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:47.423Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
e2e/vue-start/server-routes/src/routes/methods/route.tsxe2e/vue-start/server-routes/src/routes/__root.tsxe2e/vue-start/server-routes/src/routeTree.gen.tse2e/vue-start/server-routes/src/routes/index.tsxe2e/vue-start/server-routes/src/routes/api/middleware-context.ts
📚 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-start/server-routes/src/routes/methods/only-any.tsx
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/vue-start/server-routes/tests/server-routes.spec.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-start/server-routes/tsconfig.json
🧬 Code graph analysis (12)
e2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
packages/vue-router/src/index.tsx (6)
ErrorComponentProps(59-59)useRouter(300-300)useMatch(241-241)rootRouteId(12-12)ErrorComponent(128-128)Link(143-143)
e2e/vue-start/server-routes/src/routes/methods/index.tsx (3)
e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)e2e/vue-start/server-routes/src/routes/index.tsx (1)
Route(3-5)e2e/vue-start/server-routes/src/routes/methods/route.tsx (1)
Route(3-5)
e2e/vue-start/server-routes/src/routes/methods/route.tsx (2)
e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)e2e/vue-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)
e2e/vue-start/server-routes/playwright.config.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.ts (2)
e2e/vue-start/server-routes/src/routes/api/params/$foo/route.ts (1)
Route(3-11)e2e/vue-start/server-routes/src/routes/index.tsx (1)
Route(3-5)
e2e/vue-start/server-routes/src/router.tsx (2)
e2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/vue-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (3)
e2e/vue-start/server-routes/src/routes/api/only-any.ts (1)
Route(4-18)e2e/vue-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)e2e/vue-start/server-routes/src/routes/methods/route.tsx (1)
Route(3-5)
e2e/vue-start/server-routes/src/routes/__root.tsx (2)
e2e/vue-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)e2e/vue-start/query-integration/src/routes/__root.tsx (1)
RootComponent(30-81)
packages/vue-router/src/route.ts (2)
packages/vue-router/src/index.tsx (3)
LinkComponentRoute(222-222)useRouter(300-300)Link(143-143)packages/router-core/src/route.ts (1)
fullPath(1557-1559)
e2e/vue-start/server-routes/src/components/NotFound.tsx (2)
e2e/solid-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/vue-start/server-routes/src/routes/api/only-any.ts (9)
e2e/vue-start/server-routes/src/routes/merge-middleware-context.tsx (1)
Route(73-75)e2e/vue-start/server-routes/src/routes/__root.tsx (1)
Route(14-32)e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)e2e/vue-start/server-routes/src/routes/api/middleware-context.ts (1)
Route(16-29)e2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.ts (1)
Route(3-11)e2e/vue-start/server-routes/src/routes/api/params/$foo/route.ts (1)
Route(3-11)e2e/vue-start/server-routes/src/routes/index.tsx (1)
Route(3-5)e2e/vue-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)e2e/vue-start/server-routes/src/routes/methods/route.tsx (1)
Route(3-5)
e2e/vue-start/server-routes/src/routeTree.gen.ts (1)
e2e/vue-start/server-routes/src/router.tsx (1)
getRouter(8-20)
⏰ 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: Test
🔇 Additional comments (24)
e2e/vue-start/server-routes/tsconfig.json (2)
3-22: LGTM! Solid TypeScript configuration for Vue e2e tests.The compiler options are well-configured with strict mode, modern module resolution (Bundler), Vue JSX support, and appropriate ES2022 target settings. The path alias and noEmit settings are also appropriate for a Vite-bundled Vue project.
2-2: Remove the non-existentpublic/script*.jspattern from the include array.The include pattern in tsconfig.json contains
"public/script*.js", but no such files exist in the public directory. This is unnecessary configuration that should be removed: change["**/*.ts", "**/*.tsx", "public/script*.js"]to["**/*.ts", "**/*.tsx"].⛔ Skipped due to learnings
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.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 codeLearnt 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 routere2e/vue-start/server-routes/src/routes/api/middleware-context.ts (1)
4-14: Middleware context merging behavior is correct and as expected.The middleware chain properly merges context properties across parent and child middleware. When
testParentMiddlewarepasses{ testParent: true }andtestMiddlewarepasses{ test: true }, both properties are merged into the final context, as confirmed by the test assertionexpectedContext: { testParent: true, test: true }at line 24. This pattern is consistent across all implementation variants (React, Solid, Vue) and follows TanStack Start's designed behavior where properties passed to context are merged.e2e/vue-start/server-routes/src/routes/index.tsx (1)
1-23: LGTM! Route implementation follows TanStack Router conventions.The root route is correctly defined with file-based routing, and the Home component provides clear navigation to server route test pages.
e2e/vue-start/server-routes/src/styles/app.css (1)
1-30: LGTM! Tailwind configuration is well-structured.The CSS properly configures Tailwind with dark mode support and appropriate base styles. The outline suppression for mouse users (line 28) is correctly scoped to the
.using-mouseclass.e2e/vue-start/server-routes/vite.config.ts (1)
1-19: LGTM! Vite configuration is correct.The plugin order and configuration are appropriate for a Vue + TanStack Start project with JSX support.
e2e/vue-start/server-routes/src/router.tsx (1)
8-20: LGTM! Router setup is correct.The router configuration properly integrates QueryClient with SSR support and sets up appropriate defaults for error handling and not-found scenarios.
e2e/vue-start/server-routes/src/components/NotFound.tsx (1)
1-25: LGTM! NotFound component is correctly implemented.The component properly uses Vue's
classattribute and provides appropriate fallback UI with navigation options.e2e/vue-start/server-routes/src/routes/api/params/$foo/route.ts (1)
1-11: LGTM! Server route handler is appropriate for E2E testing.The GET handler correctly demonstrates dynamic parameter routing. The simple implementation is suitable for testing purposes.
e2e/vue-start/server-routes/src/routes/methods/index.tsx (1)
12-14: Good use of typed Route.Link for type safety.Using
Route.Linkprovides route-specific type safety. Note that other files in this PR use the importedLinkcomponent directly, creating a minor inconsistency in link usage patterns across the codebase.e2e/vue-start/server-routes/src/routes/api/params/$foo/$bar.ts (1)
1-11: LGTM! Nested parameter route is correctly implemented.The GET handler properly demonstrates two-level dynamic parameter routing, consistent with the single-parameter route pattern.
e2e/vue-start/server-routes/src/routes/merge-middleware-context.tsx (1)
1-75: Well-structured e2e test component.The component correctly implements the middleware context testing functionality with proper error handling and data-testid attributes for Playwright tests.
e2e/vue-start/server-routes/playwright.config.ts (1)
1-35: LGTM!The Playwright configuration follows the established patterns in the repository. The top-level await for port assignment, web server configuration with build command, and CI-aware server reuse are all correctly implemented.
e2e/vue-start/server-routes/tests/server-routes.spec.ts (2)
4-18: LGTM!The middleware context test correctly navigates, triggers the action, and validates both the presence of context values and their content. The use of
networkidlefor synchronization is appropriate for this fetch-based interaction.
20-46: LGTM!The dynamic test case discovery pattern is well-implemented. It collects all expected elements, validates each has a corresponding result, and provides clear error messages when test IDs are missing.
e2e/vue-start/server-routes/src/routes/api/only-any.ts (1)
1-18: LGTM!The API route correctly implements an
ANYhandler that responds to all HTTP methods. The response includes both JSON body and custom headers (X-HANDLER,X-METHOD), which aligns with the client-side test inmethods/only-any.tsxthat falls back to headers for HEAD/OPTIONS requests.e2e/vue-start/server-routes/src/routes/__root.tsx (1)
1-47: LGTM!The root route configuration is well-structured with proper head management (charset, viewport, stylesheet), error/not-found components, and a clean document structure. The pattern aligns with similar setups in other Vue e2e test projects.
e2e/vue-start/server-routes/src/routes/methods/only-any.tsx (1)
26-69: Well-implemented test component.The
Testcomponent correctly handles all HTTP methods, including the edge case where HEAD/OPTIONS responses have no body. The fallback to readingx-handlerandx-methodheaders aligns with the server implementation inapi/only-any.ts.e2e/vue-start/server-routes/src/routeTree.gen.ts (1)
1-257: Skipping review of auto-generated file.This file is auto-generated by TanStack Router and should not be manually modified. Based on learnings, no review comments are generated for
routeTree.gen.tsfiles.packages/vue-router/src/index.tsx (1)
222-222: LGTM!Clean type export addition that properly exposes the new
LinkComponentRouteinterface to the public API.packages/vue-router/src/route.ts (3)
7-7: LGTM!Runtime import of
Linkcomponent is necessary for the newLinkproperty implementations.
46-48: LGTM!Importing Vue as a runtime dependency (rather than type-only) is required for using
Vue.h()in the Link implementations, and adding theLinkComponentRoutetype import is appropriate.
78-78: LGTM!Adding the
Linkproperty toRouteExtensionsproperly extends the route API surface with the typed link component.packages/vue-router/src/link.tsx (1)
633-651: ThedefaultFromproperty is type-only and the implementations are correct.The
LinkComponentRouteinterface declaresdefaultFromas a type-level property within outvariance to capture the generic parameter for TypeScript inference. Inside the callable signature, it accesses this viathis['defaultFrom']purely at the type level. The implementations in route.ts correctly cast functions toLinkComponentRoute<...>without assigning a runtime.defaultFromproperty because none is needed—this is the intended design pattern for type-safe generic routing.Likely an incorrect or invalid review comment.
| "@tanstack/vue-router": "workspace:^", | ||
| "@tanstack/vue-router-devtools": "workspace:^", | ||
| "@tanstack/vue-router-ssr-query": "workspace:^", | ||
| "@tanstack/vue-start": "workspace:^", |
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.
Use workspace:* protocol for internal dependencies.
Internal TanStack packages currently use workspace:^ but the coding guidelines specify using workspace:* for workspace dependencies. This ensures exact version matching across the monorepo.
Apply this diff to align with project conventions:
"dependencies": {
"@tanstack/vue-query": "^5.90.9",
- "@tanstack/vue-router": "workspace:^",
- "@tanstack/vue-router-devtools": "workspace:^",
- "@tanstack/vue-router-ssr-query": "workspace:^",
- "@tanstack/vue-start": "workspace:^",
+ "@tanstack/vue-router": "workspace:*",
+ "@tanstack/vue-router-devtools": "workspace:*",
+ "@tanstack/vue-router-ssr-query": "workspace:*",
+ "@tanstack/vue-start": "workspace:*",
"js-cookie": "^3.0.5",
"redaxios": "^0.5.1", "devDependencies": {
"@playwright/test": "^1.50.1",
"@tailwindcss/postcss": "^4.1.15",
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",
"@types/js-cookie": "^3.0.6",As per coding guidelines for workspace dependencies.
Also applies to: 30-30
🤖 Prompt for AI Agents
In e2e/vue-start/server-routes/package.json around lines 16 to 19 (and also line
30), the internal TanStack dependencies use the "workspace:^" protocol; change
them to use "workspace:*" to follow project conventions. Edit each workspace
dependency entry (e.g., "@tanstack/vue-router", "@tanstack/vue-router-devtools",
"@tanstack/vue-router-ssr-query", "@tanstack/vue-start") and replace the version
specifier "workspace:^" with "workspace:*" so internal packages resolve to the
exact workspace versions.
| <Link | ||
| to="/" | ||
| class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`} | ||
| onClick={(e: MouseEvent) => { | ||
| e.preventDefault() | ||
| window.history.back() | ||
| }} | ||
| > | ||
| Go Back | ||
| </Link> |
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.
Misleading to prop on "Go Back" link.
The Go Back link specifies to="/" but prevents default navigation and calls window.history.back() instead. The to prop is misleading since the link doesn't actually navigate to the root route.
Consider refactoring to use a button element instead:
- <Link
- to="/"
- class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`}
- onClick={(e: MouseEvent) => {
- e.preventDefault()
- window.history.back()
- }}
- >
- Go Back
- </Link>
+ <button
+ onClick={() => {
+ window.history.back()
+ }}
+ class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`}
+ >
+ Go Back
+ </button>This makes the intent clearer and avoids the misleading to prop.
📝 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.
| <Link | |
| to="/" | |
| class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`} | |
| onClick={(e: MouseEvent) => { | |
| e.preventDefault() | |
| window.history.back() | |
| }} | |
| > | |
| Go Back | |
| </Link> | |
| <button | |
| onClick={() => { | |
| window.history.back() | |
| }} | |
| class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`} | |
| > | |
| Go Back | |
| </button> |
🤖 Prompt for AI Agents
In e2e/vue-start/server-routes/src/components/DefaultCatchBoundary.tsx around
lines 39 to 48, the Link uses a misleading to="/" prop while preventing default
and calling window.history.back(); replace the Link with a semantic <button>
element (type="button") using the same className/styles and keep the onClick
handler that calls window.history.back() so no navigation prop is present and
intent is clear; remove the to prop and any Link-specific imports if no longer
used.
| Link: LinkComponentRoute<RouteTypesById<TRouter, TId>['fullPath']> = (( | ||
| props, | ||
| ctx?: Vue.SetupContext, | ||
| ) => { | ||
| const router = useRouter() | ||
| const fullPath = router.routesById[this.id as string].fullPath | ||
| return Vue.h( | ||
| Link as any, | ||
| { from: fullPath as never, ...(props as any) }, | ||
| ctx?.slots, | ||
| ) | ||
| }) as LinkComponentRoute<RouteTypesById<TRouter, TId>['fullPath']> |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check if the file exists and examine its structure
find . -path "*/packages/vue-router/src/route.ts" -type f 2>/dev/null | head -5Repository: TanStack/router
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Also list the packages directory to understand the structure
ls -la packages/ 2>/dev/null | head -20Repository: TanStack/router
Length of output: 1294
🏁 Script executed:
#!/bin/bash
# Check file size and read the file
wc -l ./packages/vue-router/src/route.tsRepository: TanStack/router
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Read the file with context around lines 147-158
cat -n ./packages/vue-router/src/route.ts | sed -n '130,165p'Repository: TanStack/router
Length of output: 1438
🏁 Script executed:
#!/bin/bash
# Check the tsconfig.json for TypeScript strict mode
find . -name "tsconfig.json" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Repository: TanStack/router
Length of output: 1744
🏁 Script executed:
#!/bin/bash
# Read the beginning of the file to see imports and understand the context
cat -n ./packages/vue-router/src/route.ts | head -50Repository: TanStack/router
Length of output: 1757
🏁 Script executed:
#!/bin/bash
# Look for LinkComponentRoute type definition and related types
cat -n ./packages/vue-router/src/route.ts | grep -A 5 "LinkComponentRoute\|useRouter\|Link:"Repository: TanStack/router
Length of output: 2397
🏁 Script executed:
#!/bin/bash
# Check the class context around the Link property
cat -n ./packages/vue-router/src/route.ts | sed -n '100,160p'Repository: TanStack/router
Length of output: 2112
🏁 Script executed:
#!/bin/bash
# Check the root tsconfig.json
cat ./tsconfig.jsonRepository: TanStack/router
Length of output: 846
🏁 Script executed:
#!/bin/bash
# Look at the link.ts file to understand LinkComponentRoute type
head -100 ./packages/vue-router/src/link.tsRepository: TanStack/router
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for how LinkComponentRoute is defined
grep -n "LinkComponentRoute\|export type\|interface" ./packages/vue-router/src/link.ts | head -30Repository: TanStack/router
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Search for link files in packages/vue-router
find ./packages/vue-router -name "*link*" -type fRepository: TanStack/router
Length of output: 222
🏁 Script executed:
#!/bin/bash
# Search for LinkComponentRoute definition across the codebase
rg "LinkComponentRoute" --type ts -B 3 -A 3 | head -100Repository: TanStack/router
Length of output: 5763
🏁 Script executed:
#!/bin/bash
# List all files in packages/vue-router/src
ls -la ./packages/vue-router/src/Repository: TanStack/router
Length of output: 2890
🏁 Script executed:
#!/bin/bash
# Read the link.tsx file to understand LinkComponentRoute interface
head -150 ./packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 4343
🏁 Script executed:
#!/bin/bash
# Look at the LinkComponentRoute interface definition
rg "export interface LinkComponentRoute" -A 20 ./packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 528
🏁 Script executed:
#!/bin/bash
# Check how the Link component is implemented
grep -n "^export.*Link\|^function\|^const.*Link" ./packages/vue-router/src/link.tsx | head -20Repository: TanStack/router
Length of output: 776
🏁 Script executed:
#!/bin/bash
# Look at the full LinkComponentRoute interface and surrounding code
cat -n ./packages/vue-router/src/link.tsx | sed -n '633,700p'Repository: TanStack/router
Length of output: 2139
🏁 Script executed:
#!/bin/bash
# Check how Link is defined and exported
cat -n ./packages/vue-router/src/link.tsx | sed -n '746,760p'Repository: TanStack/router
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Look for any comments or context explaining why these casts are used
cat -n ./packages/vue-router/src/route.ts | sed -n '140,165p'Repository: TanStack/router
Length of output: 1104
🏁 Script executed:
#!/bin/bash
# Check if similar patterns exist in react-router or solid-router for comparison
rg "Link as any.*as never" --type ts --type tsx -B 2 -A 2 | head -50Repository: TanStack/router
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Look for any TODOs or FIXMEs related to Link or type assertions
rg "TODO|FIXME|XXX" ./packages/vue-router/src/route.ts -B 2 -A 2Repository: TanStack/router
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the useRouter implementation to understand its signature
cat ./packages/vue-router/src/useRouter.tsxRepository: TanStack/router
Length of output: 562
🏁 Script executed:
#!/bin/bash
# Search for similar patterns in other framework implementations to understand if this is intentional
rg "Link as any" --type ts -B 3 -A 3 packages/Repository: TanStack/router
Length of output: 1362
🏁 Script executed:
#!/bin/bash
# Check git history for this file to see if there are comments about these type assertions
git log --oneline -n 20 packages/vue-router/src/route.tsRepository: TanStack/router
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Look at the RouteApi class structure to understand the full context
cat -n ./packages/vue-router/src/route.ts | sed -n '85,110p'Repository: TanStack/router
Length of output: 921
Remove excessive type assertions to comply with TypeScript strict mode.
The implementation uses multiple type assertions (as any, as never, and the final cast) that circumvent TypeScript's type checking. This violates the coding guideline requiring "TypeScript strict mode with extensive type safety for all code."
The assertions suggest the LinkComponentRoute interface signature doesn't properly align with what can be implemented as a simple function component. Consider:
- Properly typing the props spreading to avoid
...(props as any) - Fixing the
fromtype assertion instead of usingas never - Ensuring the
Linkcomponent type matches without needingas any - Reviewing the
LinkComponentRouteinterface design to better accommodate Vue function components
| Link: LinkComponentRoute<TFullPath> = ((props, ctx?: Vue.SetupContext) => { | ||
| return Vue.h( | ||
| Link as any, | ||
| { from: this.fullPath as never, ...(props as any) }, | ||
| ctx?.slots, | ||
| ) | ||
| }) as LinkComponentRoute<TFullPath> |
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.
Remove excessive type assertions to comply with TypeScript strict mode.
Similar to the RouteApi.Link implementation, this code uses multiple type assertions that suppress TypeScript's type checking:
Link as anyfrom: this.fullPath as never...(props as any)- Final cast to
LinkComponentRoute<TFullPath>
This violates the coding guideline requiring strict TypeScript with extensive type safety.
As per coding guidelines, TypeScript files should use strict mode with extensive type safety. Please properly type these expressions instead of using assertions.
🤖 Prompt for AI Agents
In packages/vue-router/src/route.ts around lines 297 to 303, the inline
component factory uses multiple unsafe casts (Link as any, this.fullPath as
never, props as any, and the final cast to LinkComponentRoute<TFullPath>);
replace these casts by typing the factory and its parameters correctly:
import/declare the Link component's proper Vue component type and its prop type,
type the factory's props parameter as that prop type (or a subset mapped to
Link's props), type ctx as Vue.SetupContext properly, and call Vue.h with the
typed component and a typed props object that includes from: this.fullPath (use
the correct prop type for from so no never cast is needed); finally set the
function's return type to LinkComponentRoute<TFullPath> without asserting.
Ensure all intermediate values conform to those declared types so no as
any/never casts are required.
| Link: LinkComponentRoute<'/'> = ((props, ctx?: Vue.SetupContext) => { | ||
| return Vue.h( | ||
| Link as any, | ||
| { from: this.fullPath as never, ...(props as any) }, | ||
| ctx?.slots, | ||
| ) | ||
| }) as LinkComponentRoute<'/'> |
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.
Remove excessive type assertions to comply with TypeScript strict mode.
This implementation follows the same problematic pattern as the other Link implementations, using multiple type assertions that bypass TypeScript's type checking.
As per coding guidelines, TypeScript files should use strict mode with extensive type safety. Consider refactoring to properly type the Link component integration without relying on assertions.
🤖 Prompt for AI Agents
packages/vue-router/src/route.ts lines 543-549: the inline Link factory uses
multiple "as any"/"as never" assertions which defeat strict-mode type checking;
remove those assertions and properly type the function parameters and the props
object so you can pass a correctly-typed props object to Vue.h without casts.
Concretely, change the params to accept the correct Link props type and
Vue.SetupContext type, build a new props object that sets from: this.fullPath
(typed as the expected string/location type) merged with the incoming props
using the proper props type, and call Vue.h(Link, propsObject, ctx?.slots)
returning a Vue.VNode; ensure the function signature remains
LinkComponentRoute<'/'> but uses explicit, correct types instead of any/never.
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.