-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
test(vue-start): server-functions #6129
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 e2e Vue 3 "server-functions" example with routes, server-function factories/middleware, Playwright tests, Vite/TS/Tailwind configs, generated route types, many route implementations exercising server-function features, and typing changes in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 5af2ea1
☁️ 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: 11
🧹 Nitpick comments (17)
e2e/vue-start/server-functions/src/routes/raw-response.tsx (2)
12-12: Improve type safety and variable naming.The variable
formDataResultis misleading since it stores a text response, not FormData. Additionally, the typeref<unknown>should beref<string>for better type safety.As per coding guidelines, TypeScript strict mode with extensive type safety should be used.
Apply this diff:
- const formDataResult = ref<unknown>('') + const responseText = ref<string>('')Then update references at lines 30 and 40:
- formDataResult.value = text + responseText.value = text- {JSON.stringify(formDataResult.value)} + {JSON.stringify(responseText.value)}
27-27: Consider removing console.log.The console.log statement could be removed for cleaner code, though it may be useful for debugging in an e2e test environment.
e2e/vue-start/server-functions/src/routes/primitives/index.tsx (3)
80-80: Consider using a union type instead ofanyfor better type safety.The
anytype weakens TypeScript's type checking. Since the test cases use specific types, consider using a union:-] as Array<PrimitiveComponentProps<any>> +] as Array<PrimitiveComponentProps<null | undefined | string>>This maintains type safety while accommodating the heterogeneous test case values.
94-96: Redundant undefined handling.The explicit conversion of
undefinedto'$undefined'is redundant since thestringifyfunction on line 8 already handles this case.Apply this diff to simplify:
queryFn: async () => { const result = await testCase.serverFn[method]({ data: testCase.data.value, }) - if (result === undefined) { - return '$undefined' - } - return result + return result },Then update line 128 to use
stringify:<div data-testid={`result-${testId}`}> - {query.isSuccess.value ? stringify(query.data.value) : ''} + {query.isSuccess.value ? stringify(query.data.value) : ''}Actually, line 128 already calls stringify, so just removing lines 94-96 and returning
resultdirectly would work correctly.
116-132: Consider adding error state handling for better test debugging.The UI only displays results for successful queries. When queries fail during testing, the lack of error information makes debugging more difficult.
Consider adding error state display:
<div key={testId}> <h3>serverFn method={method}</h3> <h4> expected </h4> <div data-testid={`expected-${testId}`}> {stringify(testCase.data.value)} </div> <h4> result</h4> <div data-testid={`result-${testId}`}> - {query.isSuccess.value ? stringify(query.data.value) : ''} + {query.isSuccess.value ? stringify(query.data.value) : ''} + {query.isError.value ? `Error: ${query.error.value}` : ''} </div> <br /> </div>e2e/vue-start/server-functions/src/routes/__root.tsx (1)
27-29: Consider safer error handling for production-like testing.The error component directly renders the full error stack. While acceptable for e2e tests, consider whether this aligns with production-like error handling you want to test.
e2e/vue-start/server-functions/src/routes/return-null.tsx (1)
56-63: Consider adding error handling for the server function calls.The promises from
$allow_return_null_getFn()and$allow_return_null_postFn()don't have.catch()handlers. If a server function fails unexpectedly, the error will be silently swallowed, which could make debugging test failures harder.onClick={() => { $allow_return_null_getFn().then((data) => { getServerResult.value = data + }).catch((e) => { + getServerResult.value = `Error: ${e.message}` }) $allow_return_null_postFn().then((data) => { postServerResult.value = data + }).catch((e) => { + postServerResult.value = `Error: ${e.message}` }) }}e2e/vue-start/server-functions/src/components/DefaultCatchBoundary.tsx (1)
39-48: Theto="/"attribute is misleading when usingwindow.history.back().The "Go Back" link specifies
to="/"but the click handler overrides navigation withwindow.history.back(). This can cause accessibility issues (screen readers will announce "link to home") and confusion if JavaScript is disabled or the handler fails.Consider using a
<button>styled as a link for the "Go Back" action since it doesn't actually navigate to a specific route:- <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 + type="button" + class={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded-sm text-white uppercase font-extrabold`} + onClick={() => window.history.back()} + > + Go Back + </button>e2e/vue-start/server-functions/src/routes/dead-code-preserve.tsx (1)
23-27: Consider clarifying the intentionally unused assignment.The unused
testvariable is intentional to verify that dead code elimination preserves the side effect inupdateCount(). However, usingvoidwould make the intent clearer and avoid needing an ESLint disable.Apply this diff for clarity:
const writeFileServerFn = createServerFn().handler(async () => { - // eslint-disable-next-line unused-imports/no-unused-vars - const test = await updateCount() + // Side effect preserved for DCE test + void (await updateCount()) return getRequestHeader('X-Test') })e2e/vue-start/server-functions/src/routes/headers.tsx (1)
14-17: Clarify the purpose of duplicateheadersandserverHeadersfields.Both
serverHeadersandheadersare assigned the same value (reqHeaders). If they're meant to be identical for testing, consider adding a comment explaining why. If they should differ (e.g., request vs. response headers), update the implementation accordingly.If duplication is intentional, add a comment:
return { + // Both fields return request headers for e2e verification serverHeaders: reqHeaders, headers: reqHeaders, }Otherwise, simplify to a single field:
return { - serverHeaders: reqHeaders, headers: reqHeaders, }e2e/vue-start/server-functions/vite.config.ts (1)
7-10: Consider making the constant ID configuration more maintainable.The hardcoded file paths in
FUNCTIONS_WITH_CONSTANT_IDare brittle and will break silently if files are renamed or moved. While acceptable for e2e test code, a more maintainable approach might reduce maintenance burden.Alternative approaches:
- Use a marker comment or decorator in the source files themselves to indicate constant ID requirement
- Use a naming convention (e.g., functions ending with
_CONSTANT) that the generator can detect- Add a validation step in the build to ensure these paths still exist
e2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.ts (1)
1-5: Consider adding stricter TypeScript types.The
anytype reduces type safety. While this is test code, using generic types would improve type inference and catch potential issues.-export function createFakeFn() { +export function createFakeFn<T = unknown>() { return { - handler: (cb: () => Promise<any>) => cb, + handler: <TResult = T>(cb: () => Promise<TResult>) => cb, } }e2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsx (1)
71-73: Consider clarifying the initial state text.The text
' Loading ...'is shown whenclientDataisnull, which is the initial state before the button is clicked—not an actual loading state. Consider changing to something like'Click button to load'for clarity, though this is a minor concern for an e2e test file.e2e/vue-start/server-functions/src/routes/multipart.tsx (1)
44-55: Missing error handling for server function call.The
handleSubmitfunction doesn't handle errors frommultipartFormDataServerFn. If the server function throws (e.g., validation failure), the error is silently ignored.Consider adding error handling for completeness:
const handleSubmit = (e: Event) => { e.preventDefault() if (!formRef.value) { return } const formData = new FormData(formRef.value) - multipartFormDataServerFn({ data: formData }).then((data) => { - multipartResult.value = data - }) + multipartFormDataServerFn({ data: formData }) + .then((data) => { + multipartResult.value = data + }) + .catch((error) => { + console.error('Server function error:', error) + }) }e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
7-9: Consider usingz.enumfor mode validation.The union of literals can be simplified using
z.enumfor better readability.validateSearch: z.object({ - mode: z.union([z.literal('js'), z.literal('no-js')]).default('js'), + mode: z.enum(['js', 'no-js']).default('js'), }),e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
11-13: Add explicit type annotation for consistency.The server handler has an explicit
stringtype forinput, but the client handler has an implicit type. For consistency and TypeScript strict mode compliance, add the explicit type.const getEcho = createIsomorphicFn() .server((input: string) => 'server received ' + input) - .client((input) => 'client received ' + input) + .client((input: string) => 'client received ' + input)e2e/vue-start/server-functions/src/routes/submit-post-formdata.tsx (1)
4-6: Consider reordering declarations for clarity.The
Routeexport referencesSubmitPostFormDataFnbefore it's defined. While function declarations are hoisted in JavaScript, placing the route definition after the component improves readability and follows the pattern used in other routes (e.g.,isomorphic-fns.tsx).Move the Route export to the end of the file:
import { createFileRoute } from '@tanstack/vue-router' import { createServerFn } from '@tanstack/vue-start' -export const Route = createFileRoute('/submit-post-formdata')({ - component: SubmitPostFormDataFn, -}) - const testValues = { name: 'Sean', } export const greetUser = createServerFn({ method: 'POST' }) // ... server function definition ... function SubmitPostFormDataFn() { // ... component definition ... } + +export const Route = createFileRoute('/submit-post-formdata')({ + component: SubmitPostFormDataFn, +})
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
e2e/vue-start/server-functions/public/favicon.icois excluded by!**/*.icoe2e/vue-start/server-functions/public/favicon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (49)
e2e/vue-start/server-functions/.gitignore(1 hunks)e2e/vue-start/server-functions/.prettierignore(1 hunks)e2e/vue-start/server-functions/package.json(1 hunks)e2e/vue-start/server-functions/playwright.config.ts(1 hunks)e2e/vue-start/server-functions/postcss.config.mjs(1 hunks)e2e/vue-start/server-functions/src/components/DefaultCatchBoundary.tsx(1 hunks)e2e/vue-start/server-functions/src/components/NotFound.tsx(1 hunks)e2e/vue-start/server-functions/src/routeTree.gen.ts(1 hunks)e2e/vue-start/server-functions/src/router.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/__root.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/abort-signal.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/consistent.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/cookies/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/cookies/set.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/dead-code-preserve.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/env-only.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/factory/-functions/createBarServerFn.ts(1 hunks)e2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.ts(1 hunks)e2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.ts(1 hunks)e2e/vue-start/server-functions/src/routes/factory/-functions/functions.ts(1 hunks)e2e/vue-start/server-functions/src/routes/factory/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/headers.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/middleware/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/middleware/send-serverFn.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/multipart.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/primitives/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/raw-response.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/redirect-test-ssr/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/redirect-test/index.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/redirect-test/target.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/return-null.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/serialize-form-data.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/status.tsx(1 hunks)e2e/vue-start/server-functions/src/routes/submit-post-formdata.tsx(1 hunks)e2e/vue-start/server-functions/src/styles/app.css(1 hunks)e2e/vue-start/server-functions/src/vite-env.d.ts(1 hunks)e2e/vue-start/server-functions/tests/server-functions.spec.ts(1 hunks)e2e/vue-start/server-functions/tsconfig.json(1 hunks)e2e/vue-start/server-functions/vite.config.ts(1 hunks)e2e/vue-start/server-routes/package.json(1 hunks)packages/vue-router/README.md(1 hunks)packages/vue-router/src/link.tsx(4 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-functions/src/components/DefaultCatchBoundary.tsxe2e/vue-start/server-functions/src/router.tsxe2e/vue-start/server-functions/src/vite-env.d.tse2e/vue-start/server-functions/src/routes/return-null.tsxe2e/vue-start/server-functions/src/routes/redirect-test/index.tsxe2e/vue-start/server-functions/src/routes/index.tsxe2e/vue-start/server-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/multipart.tsxe2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsxe2e/vue-start/server-functions/src/routes/raw-response.tsxe2e/vue-start/server-functions/src/routes/env-only.tsxe2e/vue-start/server-functions/src/components/NotFound.tsxe2e/vue-start/server-functions/src/routes/submit-post-formdata.tsxe2e/vue-start/server-functions/src/routes/middleware/send-serverFn.tsxe2e/vue-start/server-functions/src/routes/middleware/index.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.tse2e/vue-start/server-functions/src/routes/dead-code-preserve.tsxe2e/vue-start/server-functions/tests/server-functions.spec.tse2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsxe2e/vue-start/server-functions/src/routes/consistent.tsxe2e/vue-start/server-functions/src/routes/serialize-form-data.tsxe2e/vue-start/server-functions/src/routes/redirect-test-ssr/index.tsxe2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsxe2e/vue-start/server-functions/src/routes/headers.tsxe2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsxpackages/vue-router/src/link.tsxe2e/vue-start/server-functions/src/routes/cookies/set.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/createBarServerFn.tse2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/functions.tse2e/vue-start/server-functions/playwright.config.tse2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.tse2e/vue-start/server-functions/src/routeTree.gen.tse2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsxe2e/vue-start/server-functions/src/routes/abort-signal.tsxe2e/vue-start/server-functions/src/routes/factory/index.tsxe2e/vue-start/server-functions/vite.config.tse2e/vue-start/server-functions/src/routes/isomorphic-fns.tsxe2e/vue-start/server-functions/src/routes/status.tsxe2e/vue-start/server-functions/src/routes/primitives/index.tsx
**/*.{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-functions/src/components/DefaultCatchBoundary.tsxe2e/vue-start/server-functions/src/router.tsxe2e/vue-start/server-functions/src/vite-env.d.tse2e/vue-start/server-functions/src/routes/return-null.tsxe2e/vue-start/server-functions/src/routes/redirect-test/index.tsxe2e/vue-start/server-functions/src/routes/index.tsxe2e/vue-start/server-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/multipart.tsxe2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsxe2e/vue-start/server-functions/src/routes/raw-response.tsxe2e/vue-start/server-functions/src/routes/env-only.tsxe2e/vue-start/server-functions/src/components/NotFound.tsxe2e/vue-start/server-functions/src/routes/submit-post-formdata.tsxe2e/vue-start/server-functions/src/routes/middleware/send-serverFn.tsxe2e/vue-start/server-functions/src/routes/middleware/index.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.tse2e/vue-start/server-functions/src/routes/dead-code-preserve.tsxe2e/vue-start/server-functions/tests/server-functions.spec.tse2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsxe2e/vue-start/server-functions/src/routes/consistent.tsxe2e/vue-start/server-functions/src/routes/serialize-form-data.tsxe2e/vue-start/server-functions/src/routes/redirect-test-ssr/index.tsxe2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsxe2e/vue-start/server-functions/src/routes/headers.tsxe2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsxpackages/vue-router/src/link.tsxe2e/vue-start/server-functions/src/routes/cookies/set.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/createBarServerFn.tse2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/src/routes/factory/-functions/functions.tse2e/vue-start/server-functions/playwright.config.tse2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.tse2e/vue-start/server-functions/src/routeTree.gen.tse2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsxe2e/vue-start/server-functions/src/routes/abort-signal.tsxe2e/vue-start/server-functions/src/routes/factory/index.tsxe2e/vue-start/server-functions/vite.config.tse2e/vue-start/server-functions/src/routes/isomorphic-fns.tsxe2e/vue-start/server-functions/src/routes/status.tsxe2e/vue-start/server-functions/src/routes/primitives/index.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/vue-start/server-functions/package.jsone2e/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-functions/.prettierignoree2e/vue-start/server-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/multipart.tsxe2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/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-start/server-functions/.prettierignoree2e/vue-start/server-functions/package.jsone2e/vue-start/server-functions/src/routes/env-only.tsxe2e/vue-start/server-functions/src/routes/dead-code-preserve.tsxe2e/vue-start/server-functions/tsconfig.jsone2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/src/routeTree.gen.tse2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx
📚 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-functions/.prettierignoree2e/vue-start/server-functions/src/routes/return-null.tsxe2e/vue-start/server-functions/src/routes/redirect-test/index.tsxe2e/vue-start/server-functions/src/routes/index.tsxe2e/vue-start/server-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/env-only.tsxe2e/vue-start/server-functions/src/routes/middleware/index.tsxpackages/vue-router/README.mde2e/vue-start/server-functions/src/routes/dead-code-preserve.tsxe2e/vue-start/server-functions/tests/server-functions.spec.tse2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsxe2e/vue-start/server-functions/src/routes/consistent.tsxe2e/vue-start/server-functions/src/routes/headers.tsxe2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/src/routeTree.gen.tse2e/vue-start/server-functions/src/routes/factory/index.tsxe2e/vue-start/server-functions/src/routes/primitives/index.tsx
📚 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-functions/.prettierignoree2e/vue-start/server-functions/src/router.tsxe2e/vue-start/server-functions/src/routes/index.tsxe2e/vue-start/server-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/middleware/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsxe2e/vue-start/server-functions/tsconfig.jsone2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/src/routeTree.gen.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
e2e/vue-start/server-functions/src/routes/redirect-test/index.tsxe2e/vue-start/server-functions/src/routes/redirect-test/target.tsxe2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsxe2e/vue-start/server-functions/src/routes/redirect-test-ssr/index.tsx
📚 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-functions/src/routes/index.tsxe2e/vue-start/server-functions/src/routes/middleware/index.tsxe2e/vue-start/server-functions/src/routeTree.gen.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-functions/src/routes/cookies/index.tsxe2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsxe2e/vue-start/server-functions/src/routes/factory/index.tsxe2e/vue-start/server-functions/src/routes/primitives/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-functions/src/routes/middleware/index.tsxe2e/vue-start/server-functions/src/routes/__root.tsxe2e/vue-start/server-functions/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-start/server-functions/tsconfig.json
🧬 Code graph analysis (32)
e2e/vue-start/server-functions/src/router.tsx (2)
e2e/vue-start/server-functions/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/vue-start/server-functions/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/vue-start/server-functions/src/routes/redirect-test/index.tsx (2)
e2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsx (1)
Route(3-5)e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
Route(5-10)
e2e/vue-start/server-functions/src/routes/index.tsx (9)
e2e/vue-start/server-functions/src/routes/dead-code-preserve.tsx (1)
Route(62-64)e2e/vue-start/server-functions/src/routes/env-only.tsx (1)
Route(85-87)e2e/vue-start/server-functions/src/routes/raw-response.tsx (1)
Route(48-50)e2e/vue-start/server-functions/src/routes/serialize-form-data.tsx (1)
Route(86-88)e2e/vue-start/server-functions/src/routes/abort-signal.tsx (1)
Route(84-86)e2e/vue-start/server-functions/src/routes/consistent.tsx (1)
Route(122-129)e2e/vue-start/server-functions/src/routes/cookies/index.tsx (1)
Route(7-10)e2e/vue-start/server-functions/src/routes/headers.tsx (1)
Route(70-77)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
Route(75-82)
e2e/vue-start/server-functions/src/routes/cookies/index.tsx (1)
e2e/vue-start/server-functions/src/routes/cookies/set.tsx (1)
Route(70-78)
e2e/vue-start/server-functions/src/routes/redirect-test/target.tsx (5)
e2e/vue-start/server-functions/src/routes/env-only.tsx (1)
Route(85-87)e2e/vue-start/server-functions/src/routes/raw-response.tsx (1)
Route(48-50)e2e/vue-start/server-functions/src/routes/__root.tsx (1)
Route(14-32)e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
Route(5-10)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
Route(75-82)
e2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsx (1)
e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
Route(5-10)
e2e/vue-start/server-functions/src/routes/raw-response.tsx (5)
e2e/vue-start/server-functions/src/routes/env-only.tsx (1)
Route(85-87)e2e/vue-start/server-functions/src/routes/primitives/index.tsx (1)
Route(143-146)e2e/vue-start/server-functions/src/routes/factory/index.tsx (1)
Route(221-224)e2e/vue-start/server-functions/src/routes/__root.tsx (1)
Route(14-32)e2e/vue-start/server-functions/src/routes/abort-signal.tsx (1)
Route(84-86)
e2e/vue-start/server-functions/src/routes/env-only.tsx (3)
packages/start-client-core/src/index.tsx (2)
createServerOnlyFn(12-12)createClientOnlyFn(12-12)e2e/vue-start/server-functions/src/routes/raw-response.tsx (1)
Route(48-50)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
Route(75-82)
e2e/vue-start/server-functions/src/components/NotFound.tsx (3)
e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)e2e/solid-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)examples/solid/start-basic/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/vue-start/server-functions/src/routes/submit-post-formdata.tsx (2)
e2e/vue-start/server-functions/src/routes/serialize-form-data.tsx (1)
greetUser(13-34)e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
greetUser(16-33)
e2e/vue-start/server-functions/src/routes/middleware/send-serverFn.tsx (2)
e2e/vue-start/server-functions/src/routes/factory/-functions/functions.ts (2)
barFn(22-27)fooFn(6-11)e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx (1)
Route(80-83)
e2e/vue-start/server-functions/src/routes/middleware/index.tsx (1)
e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx (1)
Route(80-83)
e2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.ts (1)
packages/start-server-core/src/request-response.ts (1)
getRequest(72-75)
e2e/vue-start/server-functions/src/routes/dead-code-preserve.tsx (2)
packages/start-server-core/src/request-response.ts (1)
getRequestHeader(82-84)e2e/vue-start/server-functions/src/routes/env-only.tsx (1)
Route(85-87)
e2e/vue-start/server-functions/tests/server-functions.spec.ts (2)
e2e/vue-start/server-functions/playwright.config.ts (1)
PORT(5-5)packages/start-plugin-core/src/schema.ts (1)
Page(202-202)
e2e/vue-start/server-functions/src/routes/redirect-test-ssr/target.tsx (1)
e2e/solid-start/server-functions/src/routes/redirect-test-ssr/target.tsx (1)
RouteComponent(7-14)
e2e/vue-start/server-functions/src/routes/serialize-form-data.tsx (1)
e2e/vue-start/server-functions/src/routes/submit-post-formdata.tsx (1)
greetUser(12-29)
e2e/vue-start/server-functions/src/routes/redirect-test-ssr/index.tsx (2)
e2e/vue-start/server-functions/src/routes/primitives/index.tsx (1)
Route(143-146)e2e/vue-start/server-functions/src/routes/factory/index.tsx (1)
Route(221-224)
e2e/vue-start/server-functions/src/routes/formdata-redirect/index.tsx (3)
e2e/vue-start/server-functions/src/routes/formdata-redirect/target.$name.tsx (1)
Route(3-5)e2e/vue-start/server-functions/src/routes/serialize-form-data.tsx (2)
Route(86-88)greetUser(13-34)e2e/vue-start/server-functions/src/routes/submit-post-formdata.tsx (1)
greetUser(12-29)
e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx (1)
packages/start-client-core/src/index.tsx (1)
getRouterInstance(99-99)
e2e/vue-start/server-functions/src/routes/factory/-functions/createBarServerFn.ts (1)
e2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.ts (1)
createFooServerFn(14-14)
e2e/vue-start/server-functions/src/routes/__root.tsx (1)
e2e/vue-start/server-functions/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/vue-start/server-functions/src/routes/factory/-functions/functions.ts (3)
e2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.ts (1)
createFooServerFn(14-14)e2e/vue-start/server-functions/src/routes/factory/-functions/createBarServerFn.ts (1)
createBarServerFn(13-13)e2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.ts (1)
createFakeFn(1-5)
e2e/vue-start/server-functions/playwright.config.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/vue-start/server-functions/src/routes/factory/-functions/createFakeFn.ts (1)
e2e/solid-start/server-functions/src/routes/factory/-functions/createFakeFn.ts (1)
createFakeFn(1-5)
e2e/vue-start/server-functions/src/routeTree.gen.ts (1)
e2e/vue-start/server-functions/src/router.tsx (1)
getRouter(8-26)
e2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsx (5)
packages/start-server-core/src/request-response.ts (1)
getRequest(72-75)e2e/vue-start/server-functions/src/routes/env-only.tsx (1)
Route(85-87)e2e/vue-start/server-functions/src/routes/raw-response.tsx (1)
Route(48-50)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
Route(75-82)e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx (1)
Route(80-83)
e2e/vue-start/server-functions/src/routes/abort-signal.tsx (3)
e2e/vue-start/server-functions/src/routes/cookies/index.tsx (1)
Route(7-10)e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
Route(75-82)e2e/vue-start/server-functions/src/routes/middleware/request-middleware.tsx (1)
Route(81-84)
e2e/vue-start/server-functions/src/routes/factory/index.tsx (2)
e2e/vue-start/server-functions/src/routes/factory/-functions/createFooServerFn.ts (1)
fooFnInsideFactoryFile(16-24)e2e/vue-start/server-functions/src/routes/factory/-functions/functions.ts (8)
fooFn(6-11)fooFnPOST(13-20)barFn(22-27)barFnPOST(29-36)localFn(58-65)localFnPOST(67-74)composedFn(86-93)fakeFn(76-81)
e2e/vue-start/server-functions/vite.config.ts (1)
packages/router-core/src/route.ts (1)
id(1549-1551)
e2e/vue-start/server-functions/src/routes/isomorphic-fns.tsx (1)
packages/start-client-core/src/index.tsx (1)
createIsomorphicFn(6-6)
e2e/vue-start/server-functions/src/routes/status.tsx (1)
packages/start-server-core/src/request-response.ts (1)
setResponseStatus(196-204)
⏰ 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)
- GitHub Check: Preview
- GitHub Check: Test
| const abortableServerFn = createServerFn().handler( | ||
| async ({ context, signal }) => { | ||
| console.log('server function started', { context, signal }) | ||
| return new Promise<string>((resolve, reject) => { | ||
| if (signal.aborted) { | ||
| return reject(new Error('Aborted before start')) | ||
| } | ||
| const timerId = setTimeout(() => { | ||
| console.log('server function finished') | ||
| resolve('server function result') | ||
| }, 1000) | ||
| const onAbort = () => { | ||
| clearTimeout(timerId) | ||
| console.log('server function aborted') | ||
| reject(new Error('Aborted')) | ||
| } | ||
| signal.addEventListener('abort', onAbort, { once: true }) | ||
| }) | ||
| }, | ||
| ) |
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.
Potential null reference when signal is undefined.
The handler accesses signal.aborted (line 9) and signal.addEventListener (line 21) without checking if signal exists. When the function is called without an abort signal (line 66), this will cause a runtime error.
Apply this diff to add null checks:
const abortableServerFn = createServerFn().handler(
async ({ context, signal }) => {
console.log('server function started', { context, signal })
return new Promise<string>((resolve, reject) => {
- if (signal.aborted) {
+ if (signal?.aborted) {
return reject(new Error('Aborted before start'))
}
const timerId = setTimeout(() => {
console.log('server function finished')
resolve('server function result')
}, 1000)
const onAbort = () => {
clearTimeout(timerId)
console.log('server function aborted')
reject(new Error('Aborted'))
}
- signal.addEventListener('abort', onAbort, { once: true })
+ signal?.addEventListener('abort', onAbort, { once: true })
})
},
)📝 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 abortableServerFn = createServerFn().handler( | |
| async ({ context, signal }) => { | |
| console.log('server function started', { context, signal }) | |
| return new Promise<string>((resolve, reject) => { | |
| if (signal.aborted) { | |
| return reject(new Error('Aborted before start')) | |
| } | |
| const timerId = setTimeout(() => { | |
| console.log('server function finished') | |
| resolve('server function result') | |
| }, 1000) | |
| const onAbort = () => { | |
| clearTimeout(timerId) | |
| console.log('server function aborted') | |
| reject(new Error('Aborted')) | |
| } | |
| signal.addEventListener('abort', onAbort, { once: true }) | |
| }) | |
| }, | |
| ) | |
| const abortableServerFn = createServerFn().handler( | |
| async ({ context, signal }) => { | |
| console.log('server function started', { context, signal }) | |
| return new Promise<string>((resolve, reject) => { | |
| if (signal?.aborted) { | |
| return reject(new Error('Aborted before start')) | |
| } | |
| const timerId = setTimeout(() => { | |
| console.log('server function finished') | |
| resolve('server function result') | |
| }, 1000) | |
| const onAbort = () => { | |
| clearTimeout(timerId) | |
| console.log('server function aborted') | |
| reject(new Error('Aborted')) | |
| } | |
| signal?.addEventListener('abort', onAbort, { once: true }) | |
| }) | |
| }, | |
| ) |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/src/routes/abort-signal.tsx around lines 5 to
24, the handler uses signal.aborted and signal.addEventListener without checking
for signal, which can be undefined; guard all uses of signal by first testing
if(signal) (or using optional chaining like signal?.aborted and
signal?.addEventListener) so the promise resolves normally when no AbortSignal
is provided, and only attach the timeout cleanup and abort listener when signal
is present.
| cons_postFn1({ data: { username: 'TEST' } }).then((data) => { | ||
| postDirectResult.value = data | ||
| }) |
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 duplicate call to cons_postFn1.
Lines 106-108 already invoke cons_postFn1 and assign the result to postDirectResult.value. The duplicate call at lines 110-112 is redundant and overwrites the same variable.
Apply this diff to remove the duplicate:
cons_postFn1({ data: { username: 'TEST' } }).then((data) => {
postDirectResult.value = data
})
-
- cons_postFn1({ data: { username: 'TEST' } }).then((data) => {
- postDirectResult.value = data
- })
}}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/src/routes/consistent.tsx around lines 110 to
112, there is a duplicate invocation of cons_postFn1 that overwrites
postDirectResult.value already set on lines 106–108; remove the redundant call
(the three lines invoking cons_postFn1 and assigning postDirectResult.value) so
the earlier result is preserved and no duplicate request is made.
| to="./set" | ||
| search={search.value} | ||
| > | ||
| got to route that sets the cookies with {JSON.stringify(search.value)} |
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.
Typo: "got to route" should be "go to route".
- got to route that sets the cookies with {JSON.stringify(search.value)}
+ go to route that sets the cookies with {JSON.stringify(search.value)}📝 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.
| got to route that sets the cookies with {JSON.stringify(search.value)} | |
| go to route that sets the cookies with {JSON.stringify(search.value)} |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/src/routes/cookies/index.tsx around line 21,
the comment/string contains a typo "got to route that sets the cookies with
{JSON.stringify(search.value)}" — change "got" to "go" so it reads "go to route
that sets the cookies with {JSON.stringify(search.value)}"; update the text in
that line and run tests/lint to ensure no other formatting issues.
| const result = ref<null | unknown>(null) | ||
| const comparison = computed(() => { | ||
| if (result.value) { | ||
| const isEqual = deepEqual(result.value, props.expected) | ||
| return isEqual ? 'equal' : 'not equal' | ||
| } | ||
| return 'Loading...' | ||
| }) |
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.
Falsy result values will be incorrectly treated as loading state.
The condition if (result.value) on Line 154 and the ternary on Line 181 will incorrectly treat valid falsy values (like 0, false, '', or null) as still loading. This will cause the comparison logic to fail and display "Loading..." even after a valid result is returned.
Apply this diff to check for null explicitly:
const result = ref<null | unknown>(null)
const comparison = computed(() => {
- if (result.value) {
+ if (result.value !== null) {
const isEqual = deepEqual(result.value, props.expected)
return isEqual ? 'equal' : 'not equal'
}
return 'Loading...'
})And in the render function:
<span data-testid={`fn-result-${props.expected.name}`}>
- {result.value
+ {result.value !== null
? props.type === 'serverFn'
? JSON.stringify(result.value)
: 'localFn'
: 'Loading...'}
</span>{' '}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
e2e/vue-start/server-functions/src/routes/factory/index.tsx lines 152-159: the
code uses a truthy check on result.value which treats valid falsy results (0,
false, '', etc.) as "Loading..."; change the condition to explicitly check for
null (e.g., result.value !== null) in the computed comparison and likewise
update any render ternary to use a null check rather than a truthy check so
valid falsy values are handled and compared correctly.
| <p> | ||
| serverFn when invoked in the loader returns: | ||
| <br /> | ||
| <span data-testid="serverFn-loader-result"> | ||
| {JSON.stringify(serverFnClientResult.value)} | ||
| </span> | ||
| </p> | ||
| <p> | ||
| serverFn when invoked on the client returns: | ||
| <br /> | ||
| <span data-testid="serverFn-client-result"> | ||
| {JSON.stringify(loaderData.value.serverFnLoaderResult)} | ||
| </span> | ||
| </p> |
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.
Test IDs are swapped - loader result displays client data and vice versa.
The data-testid attributes are mismatched with the actual data being displayed:
serverFn-loader-result(line 52) rendersserverFnClientResult.value(the client-invoked result)serverFn-client-result(line 59) rendersloaderData.value.serverFnLoaderResult(the loader result)
This will cause e2e tests to be incorrect or confusing.
<p>
serverFn when invoked in the loader returns:
<br />
- <span data-testid="serverFn-loader-result">
- {JSON.stringify(serverFnClientResult.value)}
+ <span data-testid="serverFn-loader-result">
+ {JSON.stringify(loaderData.value.serverFnLoaderResult)}
</span>
</p>
<p>
serverFn when invoked on the client returns:
<br />
- <span data-testid="serverFn-client-result">
- {JSON.stringify(loaderData.value.serverFnLoaderResult)}
+ <span data-testid="serverFn-client-result">
+ {JSON.stringify(serverFnClientResult.value)}
</span>
</p>📝 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.
| <p> | |
| serverFn when invoked in the loader returns: | |
| <br /> | |
| <span data-testid="serverFn-loader-result"> | |
| {JSON.stringify(serverFnClientResult.value)} | |
| </span> | |
| </p> | |
| <p> | |
| serverFn when invoked on the client returns: | |
| <br /> | |
| <span data-testid="serverFn-client-result"> | |
| {JSON.stringify(loaderData.value.serverFnLoaderResult)} | |
| </span> | |
| </p> | |
| <p> | |
| serverFn when invoked in the loader returns: | |
| <br /> | |
| <span data-testid="serverFn-loader-result"> | |
| {JSON.stringify(loaderData.value.serverFnLoaderResult)} | |
| </span> | |
| </p> | |
| <p> | |
| serverFn when invoked on the client returns: | |
| <br /> | |
| <span data-testid="serverFn-client-result"> | |
| {JSON.stringify(serverFnClientResult.value)} | |
| </span> | |
| </p> |
🤖 Prompt for AI Agents
In
e2e/vue-start/server-functions/src/routes/middleware/client-middleware-router.tsx
around lines 49 to 62, the data-testid attributes are swapped: the span with
data-testid="serverFn-loader-result" renders serverFnClientResult.value and the
span with data-testid="serverFn-client-result" renders
loaderData.value.serverFnLoaderResult. Swap the values rendered (or swap the
data-testid attributes) so that serverFn-loader-result shows
loaderData.value.serverFnLoaderResult and serverFn-client-result shows
serverFnClientResult.value; update only the mismatched attributes/expressions to
keep semantics consistent for e2e tests.
|
|
||
| <div class="overflow-y-auto"> | ||
| <pre data-testid="response"> | ||
| {JSON.stringify(formDataResult.value)} |
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 unnecessary JSON.stringify for direct string display.
Using JSON.stringify on a string value adds extra quotation marks in the display. Since responseText.value (or formDataResult.value) is already a string, it should be displayed directly.
Apply this diff:
- {JSON.stringify(formDataResult.value)}
+ {formDataResult.value}Or with the suggested variable rename:
- {JSON.stringify(responseText.value)}
+ {responseText.value}📝 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.
| {JSON.stringify(formDataResult.value)} | |
| {formDataResult.value} |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/src/routes/raw-response.tsx around line 40,
the JSX uses JSON.stringify(formDataResult.value) which adds extra quotes when
formDataResult.value is already a string; remove the JSON.stringify call and
render formDataResult.value (or responseText.value if you adopt the suggested
rename) directly in the JSX so the raw string displays without extra quotation
marks, and if renaming apply the new variable name consistently where
referenced.
| .using-mouse * { | ||
| outline: none !important; | ||
| } |
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.
Removing outlines with !important is an accessibility anti-pattern.
While the intent appears to be improving UX for mouse users, removing all outlines with !important creates accessibility issues. Users who switch between mouse and keyboard (e.g., tabbing after clicking) will lose focus indicators, and screen reader users may be affected.
Consider one of these alternatives:
- Use
:focus-visibleto show outlines only for keyboard navigation - Apply Tailwind's
outline-hiddenutility selectively instead of a blanket removal - Use a more granular selector that doesn't affect all elements
- .using-mouse * {
- outline: none !important;
- }
+ .using-mouse *:focus:not(:focus-visible) {
+ outline: none;
+ }📝 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.
| .using-mouse * { | |
| outline: none !important; | |
| } | |
| .using-mouse *:focus:not(:focus-visible) { | |
| outline: none; | |
| } |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/src/styles/app.css around lines 27-29, the
rule ".using-mouse * { outline: none !important; }" removes focus outlines
globally which breaks accessibility; replace this blanket rule with a
focus-visible approach by removing the universal selector and !important and
instead target non-keyboard focus states (e.g., use :focus:not(:focus-visible)
or the :focus-visible pseudo-class) so outlines are suppressed only for mouse
interactions, or apply Tailwind's outline-hidden selectively to specific
components rather than disabling outlines for all elements.
| test("server function's dead code is preserved if already there", async ({ | ||
| page, | ||
| }) => { | ||
| await page.goto('/dead-code-preserve') | ||
|
|
||
| await page.waitForLoadState('networkidle') | ||
| await page.getByTestId('test-dead-code-fn-call-btn').click() | ||
| await page.waitForLoadState('networkidle') | ||
|
|
||
| await expect(page.getByTestId('dead-code-fn-call-response')).toContainText( | ||
| '1', | ||
| ) | ||
|
|
||
| await fs.promises.rm('count-effect.txt') | ||
| }) |
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.
Add force: true to prevent test failure if file doesn't exist.
fs.promises.rm will throw an ENOENT error if count-effect.txt doesn't exist (e.g., if the server function failed or test ran in isolation). Use { force: true } for robustness.
- await fs.promises.rm('count-effect.txt')
+ await fs.promises.rm('count-effect.txt', { force: true })📝 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.
| test("server function's dead code is preserved if already there", async ({ | |
| page, | |
| }) => { | |
| await page.goto('/dead-code-preserve') | |
| await page.waitForLoadState('networkidle') | |
| await page.getByTestId('test-dead-code-fn-call-btn').click() | |
| await page.waitForLoadState('networkidle') | |
| await expect(page.getByTestId('dead-code-fn-call-response')).toContainText( | |
| '1', | |
| ) | |
| await fs.promises.rm('count-effect.txt') | |
| }) | |
| test("server function's dead code is preserved if already there", async ({ | |
| page, | |
| }) => { | |
| await page.goto('/dead-code-preserve') | |
| await page.waitForLoadState('networkidle') | |
| await page.getByTestId('test-dead-code-fn-call-btn').click() | |
| await page.waitForLoadState('networkidle') | |
| await expect(page.getByTestId('dead-code-fn-call-response')).toContainText( | |
| '1', | |
| ) | |
| await fs.promises.rm('count-effect.txt', { force: true }) | |
| }) |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/tests/server-functions.spec.ts around lines
231 to 245, the cleanup call uses fs.promises.rm('count-effect.txt') which
throws if the file does not exist; update the call to pass the options object
with force: true (i.e., fs.promises.rm('count-effect.txt', { force: true })) so
the test won't fail when the file is absent.
| page.getByTestId('formdata-redirect-target-name'), | ||
| ).toContainText(expected) | ||
|
|
||
| expect(page.url().endsWith(`/formdata-redirect/target/${expected}`)) |
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.
Assertion is incomplete — expect() result is not evaluated.
The expect() call returns a matcher object but no assertion method (.toBe(true), .toBeTruthy()) is chained. This line does nothing.
- expect(page.url().endsWith(`/formdata-redirect/target/${expected}`))
+ expect(page.url().endsWith(`/formdata-redirect/target/${expected}`)).toBe(true)📝 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.
| expect(page.url().endsWith(`/formdata-redirect/target/${expected}`)) | |
| expect(page.url().endsWith(`/formdata-redirect/target/${expected}`)).toBe(true) |
🤖 Prompt for AI Agents
In e2e/vue-start/server-functions/tests/server-functions.spec.ts around line
350, the expect(...) call is incomplete and doesn't assert anything; chain an
assertion method to evaluate the boolean returned by page.url().endsWith(...).
For example, change the line to call .toBe(true) or .toBeTruthy() on the expect
expression (or use Playwright's expect(page).toHaveURL(...) alternative) so the
test actually checks the URL.
| "combinate": "^1.1.11", | ||
| "postcss": "^8.5.1", | ||
| "srvx": "^0.8.6", | ||
| "srvx": "^0.9.8", |
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.
Version 0.9.8 of srvx does not exist; latest available is 0.9.1.
Update the version specifier from ^0.9.8 to ^0.9.1 to use the latest published release, or pin a specific earlier version if needed. The dependency cannot be resolved with the current version constraint.
🤖 Prompt for AI Agents
In e2e/vue-start/server-routes/package.json around line 35, the dependency
"srvx": "^0.9.8" references a non-existent version; change the version specifier
to a valid published release (e.g., "srvx": "^0.9.1") or pin to a specific
existing version you require, then run yarn/npm install to verify the dependency
resolves successfully and update lockfile if applicable.
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: 1
🧹 Nitpick comments (1)
packages/vue-router/src/link.tsx (1)
666-666: Unavoidable type assertion due to Vue 3 limitationsThe
as anycast at line 666 bypasses TypeScript's type checking to work around Vue 3's known typing limitations with generic components passed toh(). While this violates strict type safety guidelines, it's a documented pattern for higher-order components in Vue 3 wheredefineComponent-created components don't reliably infer types when used dynamically withh(). Consider documenting this as a necessary workaround with a code comment explaining the Vue 3 typing constraint.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue-router/src/link.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/vue-router/src/link.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/vue-router/src/link.tsx
⏰ 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)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/vue-router/src/link.tsx (2)
594-598: LGTM! Improved type flexibility.The change from
Vue.VNodetoVue.VNodeChildbroadens the type to include strings, numbers, and other primitive types that Vue 3 accepts as children. This aligns better with Vue 3's type system and provides more flexibility for component consumers.
626-637: LGTM! Enhanced generic type inference.The addition of
in outvariance modifiers and theTDefaultFromgeneric parameter improves type inference capabilities. This change allows for strongerdefaultFrominference while maintaining backwards compatibility through sensible defaults.
| export const Link = LinkImpl as unknown as Vue.Component<unknown> & | ||
| LinkComponent<'a'> |
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
# Read the link.tsx file to understand the context around line 752-753
wc -l packages/vue-router/src/link.tsx
echo "---"
sed -n '740,760p' packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 546
🏁 Script executed:
#!/bin/bash
# Find LinkImpl definition
echo "=== Searching for LinkImpl definition ==="
grep -n "const LinkImpl\|LinkImpl =" packages/vue-router/src/link.tsx | head -20
echo ""
echo "=== Searching for LinkComponent interface ==="
grep -n "interface LinkComponent\|type LinkComponent" packages/vue-router/src/link.tsx | head -20
echo ""
echo "=== Reading lines around LinkImpl definition ==="
sed -n '680,750p' packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 2472
🏁 Script executed:
#!/bin/bash
# Get the complete LinkComponent type definition
sed -n '601,650p' packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 1439
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation around this export
sed -n '745,760p' packages/vue-router/src/link.tsx
echo ""
echo "=== Check for similar patterns elsewhere in the file ==="
grep -n "as unknown as" packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 620
🏁 Script executed:
#!/bin/bash
# Check if there are any TypeScript errors or compiler settings that might explain this
echo "=== Check tsconfig for strict mode ==="
find packages/vue-router -name "tsconfig*.json" -exec cat {} \; 2>/dev/null | head -50
echo ""
echo "=== Verify LinkImpl return type from Vue.defineComponent ==="
sed -n '671,680p' packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 886
🏁 Script executed:
#!/bin/bash
# Check the root tsconfig to see if strict mode is enabled
cat tsconfig.json | head -30
echo ""
echo "=== Check if there's any way to avoid the double assertion ==="
# Look for similar component exports in the codebase
find packages -name "*.tsx" -o -name "*.ts" | xargs grep -l "defineComponent" | head -5 | while read f; do
echo "File: $f"
grep -A 2 "export const.*defineComponent" "$f" | head -10
doneRepository: TanStack/router
Length of output: 1430
🏁 Script executed:
#!/bin/bash
# Check if removing the assertion causes TypeScript errors
echo "=== Attempt to understand type incompatibility ==="
# Let's see what Vue.defineComponent returns
grep -n "Vue.defineComponent" packages/vue-router/src/link.tsx | head -3
echo ""
echo "=== Check if LinkComponent can be satisfied without the assertion ==="
# Look at how LinkComponent is used elsewhere
grep -n "LinkComponent" packages/vue-router/src/link.tsx | head -15Repository: TanStack/router
Length of output: 658
🏁 Script executed:
#!/bin/bash
# Check if there are any GitHub issues or comments about this pattern
echo "=== Search for comments explaining the assertion ==="
sed -n '745,765p' packages/vue-router/src/link.tsx
echo ""
echo "=== Check how other similar libraries handle this ==="
grep -B 5 -A 1 "as unknown as Vue.Component" packages/vue-router-devtools/src/*.tsx 2>/dev/null | head -20
echo ""
echo "=== Verify the LinkImpl props array matches what's expected ==="
sed -n '671,730p' packages/vue-router/src/link.tsx | grep -E "props:|defineComponent|setup"Repository: TanStack/router
Length of output: 873
🏁 Script executed:
#!/bin/bash
# Check the git history to see if this pattern was intentionally added
echo "=== Check git log for this file ==="
cd packages/vue-router && git log --oneline -n 20 -- src/link.tsx 2>/dev/null || echo "Git history not available"
echo ""
echo "=== Check if there's any PR or issue reference ==="
cd - >/dev/null
grep -r "LinkComponent.*LinkImpl" packages/vue-router/src/ 2>/dev/null
echo ""
echo "=== Check if LinkComponent is used in tests to see expected behavior ==="
find packages/vue-router -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "Link" 2>/dev/null | head -3Repository: TanStack/router
Length of output: 387
🏁 Script executed:
#!/bin/bash
# Check the recent commit that mentions "narrow type on link"
cd packages/vue-router && git show 89e6f6d38 --stat 2>/dev/null | head -30
echo ""
echo "=== Check the diff to understand the change ==="
git show 89e6f6d38 -- src/link.tsx 2>/dev/null | grep -A 10 -B 10 "as unknown as" | head -40Repository: TanStack/router
Length of output: 3105
🏁 Script executed:
#!/bin/bash
# Let's verify if direct casting works without the double assertion
echo "=== Check what happens without 'as unknown as' ==="
# Try to understand the actual type incompatibility by checking if there's documentation
grep -B 5 -A 5 "LinkComponent<'a'>" packages/vue-router/src/link.tsx
echo ""
echo "=== Verify the pattern is consistent with project standards ==="
# Check if other exports in the same file use similar patterns
grep -n "export const.*as" packages/vue-router/src/link.tsxRepository: TanStack/router
Length of output: 506
🏁 Script executed:
#!/bin/bash
# Final verification: check if there are any type errors without the assertion
echo "=== Check createLinkComponent for comparison ==="
sed -n '650,665p' packages/vue-router/src/link.tsx
echo ""
echo "=== Verify Vue.defineComponent return type ==="
# Check what Vue version is being used
grep -r "vue.*version\|@vue/runtime-core" packages/vue-router/package.json 2>/dev/null | head -5Repository: TanStack/router
Length of output: 525
Add documentation explaining the type assertion necessity.
The double type assertion as unknown as Vue.Component<unknown> & LinkComponent<'a'> is intentional but requires clarification. Add a comment explaining:
- Why
Vue.defineComponent()return type cannot be directly cast to theLinkComponentintersection type - That this assertion is necessary to expose the correct public API combining both Vue's component protocol and custom
LinkComponentinterface - That this pattern is used consistently in the codebase (see lines 539, 705) for similar type bridging
Consider whether the existing inline comment could be expanded to justify this approach in the context of TypeScript strict mode requirements.
🤖 Prompt for AI Agents
In packages/vue-router/src/link.tsx around lines 752-753, add a concise inline
comment above the double type assertion explaining that the double cast `as
unknown as Vue.Component<unknown> & LinkComponent<'a'>` is required because
Vue.defineComponent()'s inferred return type cannot be directly intersected with
our custom LinkComponent interface, that this assertion intentionally exposes
the combined public API (Vue component shape plus LinkComponent methods/props)
for consumers, and that this pattern is used elsewhere in the file (see lines
539 and 705) to bridge TypeScript's strict-mode inference limitations; keep the
comment brief and mention strict-mode compatibility so future readers understand
why the non-ideal double-cast is necessary.
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
♻️ Duplicate comments (1)
packages/vue-router/src/link.tsx (1)
762-764: Address the past review comment about type assertion documentation.This double type assertion has been previously flagged for lacking documentation. Please see the existing review comment that requests adding a concise inline comment explaining why this assertion pattern is necessary for bridging Vue's component types with the custom
LinkComponentinterface under strict TypeScript mode.
🧹 Nitpick comments (1)
packages/vue-router/src/link.tsx (1)
536-542: Remove unnecessaryanycasts.The
activePandinactivePvariables are of typeStyledProps, which includes an index signature[key: string]: unknown. You can access properties directly without casting toany.Apply this diff:
for (const key of Object.keys(activeP)) { if (key !== 'class' && key !== 'style') { - result[key] = (activeP as any)[key] + result[key] = activeP[key] } } for (const key of Object.keys(inactiveP)) { if (key !== 'class' && key !== 'style') { - result[key] = (inactiveP as any)[key] + result[key] = inactiveP[key] } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/vue-router/package.json(1 hunks)packages/vue-router/src/link.tsx(17 hunks)packages/vue-router/src/ssr/renderRouterToStream.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
packages/vue-router/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/vue-router/src/ssr/renderRouterToStream.tsxpackages/vue-router/src/link.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/vue-router/src/ssr/renderRouterToStream.tsxpackages/vue-router/src/link.tsx
🧠 Learnings (1)
📚 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:
packages/vue-router/src/link.tsx
⏰ 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)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (10)
packages/vue-router/src/ssr/renderRouterToStream.tsx (1)
17-17: LGTM! Good cleanup of unnecessary async.Removing the
asynckeyword from thestartcallback is correct since the function doesn't contain anyawaitexpressions. The asynchronous work is properly delegated to thepump()function with appropriate error handling on line 35.packages/vue-router/src/link.tsx (9)
22-47: LGTM! Well-structured type definitions.The new
DataAttributesandLinkHTMLAttributestypes properly extend Vue's runtime-dom types while adding external API compatibility for camelCase events andasChildpattern support. The inline comments clearly explain the design decisions.
49-62: LGTM! Proper component props inference.The
PropsOfComponentutility correctly handles both functional components and Vue components with appropriate fallback typing.
207-246: LGTM! Necessary cast for dynamic property access.The
optionRecordcast enables safe indexed access while maintaining type safety in the filtering logic.
254-278: LGTM! Appropriate typing for external link props.The flexible
Record<string, unknown>type during construction followed by the final cast toLinkHTMLAttributesproperly handles external link scenarios.
282-282: LGTM! Modernized to PointerEvent.Upgrading from
MouseEventtoPointerEventimproves cross-device compatibility for mouse, touch, and pen inputs while maintaining backward compatibility.
560-615: LGTM! Consistent type system updates.The type aliases are properly updated to use the new
LinkHTMLAttributes,PropsOfComponent, andVue.VNodeChildtypes, maintaining consistency across the API surface.
636-667: LGTM! Proper variance annotations.The
in outvariance modifiers on generic type parameters correctly enable bivariant behavior for component types, which is appropriate for the flexible component API. The default type parameters are well-designed.
676-676: Acceptable cast for internal component wiring.Using
LinkImpl as anyis reasonable here sinceLinkImplis the implementation component with complex generic requirements. The cast allows flexible vnode creation in thecreateLinkfactory.
736-741: LGTM! Correct SVG handling.Removing the
classproperty from SVG link props is appropriate since SVG elements handle classes differently. The structure wraps an<a>element inside the SVG correctly.
| "@tanstack/history": "workspace:*", | ||
| "@tanstack/router-core": "workspace:*", | ||
| "@tanstack/vue-store": "^0.8.0", | ||
| "@vue/runtime-dom": "^3.5.25", |
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
# Inspect the actual package.json file to see dependency structure
echo "=== packages/vue-router/package.json content ==="
cat packages/vue-router/package.json
echo -e "\n=== Search for @vue/runtime-dom usage in packages/vue-router ==="
rg -n "@vue/runtime-dom" packages/vue-router --type ts --type tsx --type js --type jsx -A 2Repository: TanStack/router
Length of output: 2669
🏁 Script executed:
#!/bin/bash
# Search for @vue/runtime-dom imports in source files
echo "=== Searching for @vue/runtime-dom imports in packages/vue-router/src ==="
rg -n "@vue/runtime-dom" packages/vue-router/src -A 2 -B 1
# Also search for runtime-dom in general
echo -e "\n=== Searching for runtime-dom references ==="
rg -n "runtime-dom" packages/vue-router/src -A 1 -B 1
# Check if there are any imports from @vue packages
echo -e "\n=== Searching for @vue imports ==="
rg -n "from ['\"]@vue/" packages/vue-router/src --max-count 20Repository: TanStack/router
Length of output: 1103
Version constraint misalignment: @vue/runtime-dom at ^3.5.25 exceeds the Vue peer dependency minimum.
@vue/runtime-dom is imported for type definitions (AnchorHTMLAttributes, ReservedProps) in the public API, making it a valid runtime dependency. However, the hard constraint of ^3.5.25 conflicts with the Vue peer dependency of ^3.3.0. Users installing Vue 3.3.x or 3.4.x will be forced to upgrade @vue/runtime-dom to 3.5.25+, creating potential version lock situations.
Consider aligning the version constraint to ^3.3.0 to match the peer dependency minimum, or document why the newer version is required.
🤖 Prompt for AI Agents
packages/vue-router/package.json around line 75: the dependency @vue/runtime-dom
is pinned to ^3.5.25 which exceeds the Vue peerDependency minimum ^3.3.0 and can
force users on Vue 3.3/3.4 to upgrade runtime-dom; change the dependency
constraint to align with the peer (for example ^3.3.0 or >=3.3.0 <4.0) so it
does not require 3.5.25+, or if 3.5.25 is actually required, document in the
package.json or package readme why the newer runtime-dom is necessary; update
the version string accordingly and add a short note in package metadata if
keeping the higher minimum.
|
|
||
| // Return the computed ref itself - callers should access .value | ||
| return computedProps as unknown as HTMLAttributes | ||
| return computedProps as unknown as LinkHTMLAttributes |
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.
Verify the return type pattern.
The function signature declares useLinkProps(...): LinkHTMLAttributes, but line 549 returns computedProps (a ComputedRef) cast as LinkHTMLAttributes. The comment on line 548 acknowledges "callers should access .value", and line 715 shows callers indeed cast it back to ComputedRef. This pattern bypasses TypeScript's type safety.
Consider updating the function signature to return Vue.ComputedRef<LinkHTMLAttributes> to eliminate the unnecessary double assertion and improve type safety.
Apply this diff:
export function useLinkProps<
TRouter extends AnyRouter = RegisteredRouter,
TFrom extends RoutePaths<TRouter['routeTree']> | string = string,
TTo extends string = '',
TMaskFrom extends RoutePaths<TRouter['routeTree']> | string = TFrom,
TMaskTo extends string = '',
>(
options: UseLinkPropsOptions<TRouter, TFrom, TTo, TMaskFrom, TMaskTo>,
-): LinkHTMLAttributes {
+): Vue.ComputedRef<LinkHTMLAttributes> {
// ... implementation ...
- // Return the computed ref itself - callers should access .value
- return computedProps as unknown as LinkHTMLAttributes
+ return computedProps
}Then update line 715:
- ) as unknown as Vue.ComputedRef<LinkHTMLAttributes>
+ )Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/vue-router/src/link.tsx around lines 549 and 715, change the
function signature of useLinkProps to return Vue.ComputedRef<LinkHTMLAttributes>
instead of LinkHTMLAttributes; update the return at line 549 to return
computedProps without forcing it to LinkHTMLAttributes, and update the caller
usage at line 715 to treat the result as a ComputedRef (remove the cast back to
ComputedRef and access .value where needed). Ensure imports and typing reference
Vue.ComputedRef (or import ComputedRef) so TypeScript type-safety is preserved.
Summary by CodeRabbit
New Features
Chores
Style
New API surface
✏️ Tip: You can customize this high-level summary in your review settings.