-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: various fixes #5593
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
fix: various fixes #5593
Conversation
…ctOptionalPropertyTypes` fixes #5582
WalkthroughThe PR updates the server-routes e2e app with new routes and React Query SSR integration, refactors route tree generation to support nested route groups, renames the catch-all HTTP method from "ALL" to "ANY", improves server function ID normalization, optimizes CSS caching in build plugins, and adjusts SSR type resolution in route type definitions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The PR spans multiple packages with mixed complexity: route tree generation and type system updates require careful validation; server function ID and handler resolution changes are moderate structural edits affecting multiple files; build plugin optimizations are algorithmic refinements; e2e test and route file additions are largely repetitive and straightforward. Possibly related PRs
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)
Comment |
|
View your CI Pipeline Execution ↗ for commit dd85beb
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (10)
e2e/react-start/server-functions/tests/server-functions.spec.ts (1)
453-454: Good defensive assertion to prevent false positives.This guard ensures the test fails if no test cases are found on the page, rather than passing vacuously. Without it, an empty
testCasesarray would skip the loop and incorrectly pass the test.For slightly better readability, consider using
.toBeGreaterThan(0)instead of.not.toBe(0):- expect(testCases.length).not.toBe(0) + expect(testCases.length).toBeGreaterThan(0)packages/start-plugin-core/src/start-manifest-plugin/plugin.ts (3)
11-17: Add explicit return type and include dynamic imports in traversal
- Explicit return type improves strictness and inference.
- Traversing only
chunk.importscan miss CSS fromdynamicImportschunks (common for route-level code splitting). Safest to walk both.- Optional micro-opt: check cache before the visited guard.
Apply:
-const getCSSRecursively = ( +const getCSSRecursively = ( chunk: Rollup.OutputChunk, chunksByFileName: Map<string, Rollup.OutputChunk>, basePath: string, cache: Map<Rollup.OutputChunk, Array<RouterManagedTag>>, visited = new Set<Rollup.OutputChunk>(), -) => { +): Array<RouterManagedTag> => { - if (visited.has(chunk)) { - return [] - } - visited.add(chunk) - const cachedResult = cache.get(chunk) + const cachedResult = cache.get(chunk) if (cachedResult) { return cachedResult } + if (visited.has(chunk)) { + return [] + } + visited.add(chunk) @@ - for (const importedFileName of chunk.imports) { + for (const importedFileName of [ + ...chunk.imports, + ...(chunk.dynamicImports ?? []), + ]) { const importedChunk = chunksByFileName.get(importedFileName) if (importedChunk) { result.push( ...getCSSRecursively( importedChunk, chunksByFileName, basePath, cache, visited, ), ) } }As per coding guidelines (strict TypeScript).
Also applies to: 18-26, 41-54
56-56: Consider freezing cached arrays to prevent accidental mutationYou cache and return the same
resultreference. A downstream mutation would corrupt the cache. Optional safety: freeze before caching or return a shallow copy.Example:
- cache.set(chunk, result) - return result + const frozen = Object.freeze(result.slice()) as Array<RouterManagedTag> + cache.set(chunk, frozen) + return frozen
182-187: Deduplicate CSS assets before assigning to routes/rootMultiple route chunks can contribute the same CSS file. Preloads are deduped later; assets aren’t. Dedup by
hrefto avoid duplicate<link rel="stylesheet">tags.Apply:
+// helper placed above resolvedModuleId +const dedupeByHref = (tags: Array<RouterManagedTag>): Array<RouterManagedTag> => { + const seen = new Set<string>() + const out: Array<RouterManagedTag> = [] + for (const t of tags) { + const href = (t as any).attrs?.href as string | undefined + if (href) { + if (seen.has(href)) continue + seen.add(href) + } + out.push(t) + } + return out +}- routeTreeRoutes[routeId] = { + routeTreeRoutes[routeId] = { ...v, - assets: [...(v.assets || []), ...cssAssetsList], + assets: dedupeByHref([...(v.assets || []), ...cssAssetsList]), preloads: [...(v.preloads || []), ...preloads], }- routeTreeRoutes[rootRouteId]!.assets = [ - ...(routeTreeRoutes[rootRouteId]!.assets || []), - ...entryCssAssetsList, - ] + routeTreeRoutes[rootRouteId]!.assets = dedupeByHref([ + ...(routeTreeRoutes[rootRouteId]!.assets || []), + ...entryCssAssetsList, + ])Also applies to: 210-215
e2e/react-start/server-routes/src/routes/methods/only-any.tsx (2)
10-20: Avoid value/type shadowing for readability.Rename the constant and type to reduce confusion.
-const HttpMethods = [ +const HTTP_METHODS = [ 'GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'OPTIONS', 'HEAD', ] as const -type HttpMethods = (typeof HttpMethods)[number] -function Test({ method }: { method: HttpMethods }) { +type HttpMethod = (typeof HTTP_METHODS)[number] +function Test({ method }: { method: HttpMethod }) { ... - {HttpMethods.map((method) => ( + {HTTP_METHODS.map((method) => ( <Test key={method} method={method} /> ))}Also applies to: 58-60
40-49: Scope the React Query key to this route to prevent cache collisions.Right now it's just
[method]. Prefix it with the route scope.- const query = useQuery({ queryKey: [method], queryFn }) + const query = useQuery({ queryKey: ['only-any', method], queryFn })e2e/react-start/server-routes/src/routes/merge-middleware-context.tsx (1)
8-9: Type the API response instead ofany.Helps TS strict mode and guards tests.
-function MergeMiddlewareContext() { - const [apiResponse, setApiResponse] = React.useState<any>(null) +type MiddlewareContextResponse = { + context?: Record<string, unknown> + error?: string + [k: string]: unknown +} | null + +function MergeMiddlewareContext() { + const [apiResponse, setApiResponse] = + React.useState<MiddlewareContextResponse>(null) ... - const data = await response.json() + const data = (await response.json()) as MiddlewareContextResponseAlso applies to: 11-21, 22-71
e2e/react-start/server-routes/src/router.tsx (1)
9-9: Reduce E2E flakiness: disable retries by defaultTests are more stable if queries/mutations don’t retry. Suggest configuring the QueryClient defaults for this e2e app.
Apply this diff:
- const queryClient = new QueryClient() + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + })e2e/react-start/server-routes/tests/server-routes.spec.ts (1)
21-45: Prefer locator APIs over elementHandles and use visibility assertionThis reduces flakiness and improves readability.
Apply this diff:
- // wait for page to be loaded by waiting for the route component to be rendered - await expect(page.getByTestId('route-component')).toBeInViewport() + // wait for route component to be visible + await expect(page.getByTestId('route-component')).toBeVisible() - const testCases = await page - .locator('[data-testid^="expected-"]') - .elementHandles() - expect(testCases.length).not.toBe(0) - for (const testCase of testCases) { - const testId = await testCase.getAttribute('data-testid') + const expectedLocators = page.locator('[data-testid^="expected-"]') + const count = await expectedLocators.count() + expect(count).toBeGreaterThan(0) + for (let i = 0; i < count; i++) { + const locator = expectedLocators.nth(i) + const testId = await locator.getAttribute('data-testid') if (!testId) { throw new Error('testcase is missing data-testid') } - const suffix = testId.replace('expected-', '') + const suffix = testId.replace('expected-', '') - const expected = - (await page.getByTestId(`expected-${suffix}`).textContent()) || '' + const expected = + (await page.getByTestId(`expected-${suffix}`).textContent())?.trim() ?? '' expect(expected).not.toBe('') await expect(page.getByTestId(`result-${suffix}`)).toContainText(expected) }e2e/react-start/server-routes/src/routes/api/only-any.ts (1)
7-15: Tiny readability nit: inline the returnOptional style tweak.
- ANY: ({ request }) => { - return json( - { - handler: 'ANY', - method: request.method, - }, - { headers: { 'X-HANDLER': 'ANY', 'X-METHOD': request.method } }, - ) - }, + ANY: ({ request }) => + json( + { handler: 'ANY', method: request.method }, + { headers: { 'X-HANDLER': 'ANY', 'X-METHOD': request.method } }, + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
e2e/react-start/server-functions/tests/server-functions.spec.ts(1 hunks)e2e/react-start/server-routes/package.json(1 hunks)e2e/react-start/server-routes/src/routeTree.gen.ts(5 hunks)e2e/react-start/server-routes/src/router.tsx(1 hunks)e2e/react-start/server-routes/src/routes/api/only-any.ts(1 hunks)e2e/react-start/server-routes/src/routes/index.tsx(2 hunks)e2e/react-start/server-routes/src/routes/merge-middleware-context.tsx(2 hunks)e2e/react-start/server-routes/src/routes/methods/index.tsx(1 hunks)e2e/react-start/server-routes/src/routes/methods/only-any.tsx(1 hunks)e2e/react-start/server-routes/src/routes/methods/route.tsx(1 hunks)e2e/react-start/server-routes/tests/server-routes.spec.ts(2 hunks)packages/router-core/src/route.ts(2 hunks)packages/server-functions-plugin/src/index.ts(1 hunks)packages/start-client-core/src/serverRoute.ts(2 hunks)packages/start-plugin-core/src/prerender.ts(2 hunks)packages/start-plugin-core/src/start-manifest-plugin/plugin.ts(5 hunks)packages/start-server-core/src/createStartHandler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/server-routes/src/routes/methods/route.tsxe2e/react-start/server-functions/tests/server-functions.spec.tse2e/react-start/server-routes/src/routes/methods/index.tsxe2e/react-start/server-routes/src/router.tsxpackages/start-server-core/src/createStartHandler.tse2e/react-start/server-routes/src/routes/api/only-any.tse2e/react-start/server-routes/src/routes/index.tsxpackages/start-client-core/src/serverRoute.tse2e/react-start/server-routes/src/routes/merge-middleware-context.tsxe2e/react-start/server-routes/tests/server-routes.spec.tspackages/start-plugin-core/src/prerender.tspackages/server-functions-plugin/src/index.tspackages/start-plugin-core/src/start-manifest-plugin/plugin.tspackages/router-core/src/route.tse2e/react-start/server-routes/src/routeTree.gen.tse2e/react-start/server-routes/src/routes/methods/only-any.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/server-routes/src/routes/methods/route.tsxe2e/react-start/server-routes/src/routes/methods/index.tsxe2e/react-start/server-routes/src/routes/api/only-any.tse2e/react-start/server-routes/src/routes/index.tsxe2e/react-start/server-routes/src/routes/merge-middleware-context.tsxe2e/react-start/server-routes/src/routes/methods/only-any.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/server-routes/src/routes/methods/route.tsxe2e/react-start/server-functions/tests/server-functions.spec.tse2e/react-start/server-routes/src/routes/methods/index.tsxe2e/react-start/server-routes/src/router.tsxe2e/react-start/server-routes/src/routes/api/only-any.tse2e/react-start/server-routes/src/routes/index.tsxe2e/react-start/server-routes/package.jsone2e/react-start/server-routes/src/routes/merge-middleware-context.tsxe2e/react-start/server-routes/tests/server-routes.spec.tse2e/react-start/server-routes/src/routeTree.gen.tse2e/react-start/server-routes/src/routes/methods/only-any.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-server-core/src/createStartHandler.tspackages/start-client-core/src/serverRoute.tspackages/start-plugin-core/src/prerender.tspackages/start-plugin-core/src/start-manifest-plugin/plugin.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/server-routes/package.json
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/route.ts
🧬 Code graph analysis (10)
e2e/react-start/server-routes/src/routes/methods/route.tsx (2)
e2e/react-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)e2e/react-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)
e2e/react-start/server-routes/src/routes/methods/index.tsx (2)
e2e/react-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)e2e/react-start/server-routes/src/routes/methods/route.tsx (1)
Route(3-5)
e2e/react-start/server-routes/src/router.tsx (1)
e2e/solid-start/server-routes/src/router.tsx (1)
getRouter(6-16)
packages/start-server-core/src/createStartHandler.ts (2)
packages/start-client-core/src/serverRoute.ts (1)
RouteMethod(427-435)packages/start-client-core/src/createServerFn.ts (1)
flattenMiddlewares(580-604)
e2e/react-start/server-routes/src/routes/api/only-any.ts (3)
e2e/react-start/server-routes/src/routes/index.tsx (1)
Route(3-5)e2e/react-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)e2e/react-start/server-routes/src/routes/methods/only-any.tsx (1)
Route(5-8)
e2e/react-start/server-routes/src/routes/index.tsx (3)
e2e/react-start/server-routes/src/routes/merge-middleware-context.tsx (1)
Route(4-6)e2e/react-start/server-routes/src/routes/methods/index.tsx (1)
Route(3-5)e2e/react-start/server-routes/src/routes/methods/route.tsx (1)
Route(3-5)
packages/start-plugin-core/src/start-manifest-plugin/plugin.ts (3)
packages/router-core/src/index.ts (1)
RouterManagedTag(73-73)packages/solid-router/src/index.tsx (1)
RouterManagedTag(81-81)packages/react-router/src/index.tsx (1)
RouterManagedTag(82-82)
packages/router-core/src/route.ts (1)
packages/router-core/src/link.ts (1)
ParsePathParams(114-124)
e2e/react-start/server-routes/src/routeTree.gen.ts (1)
e2e/solid-start/server-routes/src/routeTree.gen.ts (5)
FileRoutesByTo(38-42)FileRoutesById(43-48)FileRouteTypes(49-63)FileRoutesByFullPath(33-37)RootRouteChildren(64-68)
e2e/react-start/server-routes/src/routes/methods/only-any.tsx (1)
e2e/react-start/server-routes/src/routes/api/only-any.ts (1)
Route(4-18)
⏰ 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 (17)
packages/server-functions-plugin/src/index.ts (3)
85-85: Improved root path normalization.Normalizing the root path with a trailing slash before comparison ensures consistent prefix detection regardless of the original root format. This is a solid improvement over relying on a potentially inconsistent root length.
87-89: Safer prefix removal logic.The conditional check before slicing ensures the root prefix is only removed when actually present. This prevents incorrect path truncation when
extractedFilenamedoesn't start with the project root.
90-90: Consistent function ID formatting.The fixed
/@id/${file}format ensures a canonical structure for server function IDs. Since the slicedfilepath (line 88) won't have a leading slash, this produces clean, predictable paths like/@id/src/file.ts.packages/start-plugin-core/src/prerender.ts (2)
177-178: Verify the impact of the default behavior change forautoSubfolderIndex.The
autoSubfolderIndexoption now defaults totruewhen undefined. This means routes like/aboutwill generate/about/index.htmlinstead of/about.htmlunlessautoSubfolderIndexis explicitly set tofalse. Ensure that existing users are aware of this behavior change, as it could affect URL structure and SEO.Consider documenting this change in migration notes if this is a breaking change for existing projects.
257-267: LGTM! Efficient directory creation optimization.The introduction of
createdDirsSet to track already-created directories is a solid performance improvement. This eliminates redundantmkdircalls while maintaining the correct behavior withrecursive: truefor parent directory creation.packages/start-server-core/src/createStartHandler.ts (2)
381-384: LGTM! Handler resolution is cleaner and more efficient.The direct uppercase lookup with fallback to 'ANY' simplifies the logic compared to the previous implementation. The approach correctly handles both standard HTTP methods and any non-standard methods by falling back to the 'ANY' handler.
387-401: LGTM! Consolidated handler processing is well-structured.The unified approach to handling both function and object-type handlers improves readability. The code correctly:
- Determines deferral capability based on component presence
- Handles function handlers directly
- Extracts and flattens middleware from object handlers
- Processes nested handlers within object handlers
All branches properly guard against undefined values and maintain consistent middleware wrapping.
packages/start-client-core/src/serverRoute.ts (1)
290-290: Rename fromALLtoANYis complete—no remaining references found.Verification confirmed that all references to the old
ALLmethod have been updated toANYthroughout the codebase. The serverRoute.ts changes at lines 290 and 428 are correct, and handler definitions across examples and e2e tests properly use the newANYmethod. No remaining instances of the old API key were detected.packages/start-plugin-core/src/start-manifest-plugin/plugin.ts (1)
102-106: Per-chunk CSS cache is a solid additionThe
cssPerChunkCacheMap scoped to the manifest build is correct and avoids recomputation across routes.e2e/react-start/server-routes/src/routes/methods/route.tsx (1)
3-5: LGTM for parent route + Outlet.Path and nesting setup look correct and match the index/child routes.
Also applies to: 7-14
e2e/react-start/server-routes/src/routes/methods/index.tsx (1)
3-5: LGTM for index route.Relative link and structure are correct for the nested child.
Also applies to: 7-19
packages/router-core/src/route.ts (2)
163-165: Optional params widened toT | undefined— good.Prevents narrowing pitfalls when constructing partial params.
438-446: Code change verified—no lingeringparent.types.ssrreferences remain.The
ResolveAllSSRtype correctly propagates SSR viaTParentRoute['types']['allSsr']when child SSR is unknown. Comprehensive searches across the codebase confirm no strayparent.types.ssrdependencies exist, validating the change is isolated and complete.e2e/react-start/server-routes/src/router.tsx (1)
2-3: SSR + React Query wiring looks goodCreation order and integration call are correct for SSR. No functional issues spotted.
Also applies to: 9-9, 17-17
e2e/react-start/server-routes/src/routes/index.tsx (1)
13-19: Route.Link + relative targets LGTMLinks correctly use Route.Link with relative paths resolving from “/”.
e2e/react-start/server-routes/src/routes/api/only-any.ts (1)
4-18: ANY handler looks correctEchoes method and sets headers as intended.
e2e/react-start/server-routes/src/routeTree.gen.ts (1)
22-51: Generated route mappings are consistentNew routes (/merge-middleware-context, /methods, /methods/only-any, /api/only-any) are wired with correct parent/child relationships and type augmentations. No action needed.
Also applies to: 68-76, 80-86, 90-100, 102-145, 168-188, 213-226, 238-245, 250-257
| "@tanstack/react-query": "5.66.0", | ||
| "@tanstack/react-router": "workspace:^", | ||
| "@tanstack/react-router-devtools": "workspace:^", | ||
| "@tanstack/react-router-ssr-query": "workspace:^", | ||
| "@tanstack/react-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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Use workspace: for internal dependencies.*
Align internal packages with the repo guideline.
Apply this minimal diff:
"dependencies": {
- "@tanstack/react-query": "5.66.0",
- "@tanstack/react-router": "workspace:^",
- "@tanstack/react-router-devtools": "workspace:^",
- "@tanstack/react-router-ssr-query": "workspace:^",
- "@tanstack/react-start": "workspace:^",
+ "@tanstack/react-query": "5.66.0",
+ "@tanstack/react-router": "workspace:*",
+ "@tanstack/react-router-devtools": "workspace:*",
+ "@tanstack/react-router-ssr-query": "workspace:*",
+ "@tanstack/react-start": "workspace:*",
},
"devDependencies": {
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",
}Please also confirm version compatibility:
Also applies to: 30-31
🌐 Web query:
Do @tanstack/react-query v5.66.0 and React 19 work together without caveats? Check official release notes or docs.
💡 Result:
Short answer: Yes — @tanstack/react-query v5.66.0 is compatible with React 19, but not because that specific release calls out React 19: the official docs state TanStack Query v5 supports React v18+ (i.e., React 19 is within the supported range), and the v5.66.0 release notes show no React-19–specific breaking changes or caveats. [1][2]
Sources:
- TanStack Query installation / compatibility (states "compatible with React v18+"). [1]
- TanStack Query v5.66.0 release notes. [2]
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the actual package.json file to verify current state
cat "e2e/react-start/server-routes/package.json" | head -35 | tail -25
Length of output: 814
🏁 Script executed:
#!/bin/bash
# Also get full context around lines 14-18 and 30-31
jq '.dependencies, .devDependencies' "e2e/react-start/server-routes/package.json"Length of output: 833
Update workspace protocol to workspace:* for all internal dependencies.
The file currently uses workspace:^ for all internal @TanStack packages, which violates the guideline requiring workspace:*. The suggested diff is correct.
Version compatibility is not a concern: @tanstack/react-query v5.66.0 is compatible with React 19 (React Query v5 supports React 18+, with no breaking changes in v5.66.0).
Apply the diff across lines 14–18 and line 30 to align with the workspace protocol guideline.
🤖 Prompt for AI Agents
In e2e/react-start/server-routes/package.json around lines 14–18 and line 30,
update the internal @tanstack dependencies that currently use "workspace:^" to
use the workspace protocol "workspace:*" instead; specifically replace
"@tanstack/react-router": "workspace:^", "@tanstack/react-router-devtools":
"workspace:^", "@tanstack/react-router-ssr-query": "workspace:^", and
"@tanstack/react-start": "workspace:^" with "workspace:*", and make the same
replacement on line 30 so all internal @tanstack packages use "workspace:*".
| function Test({ method }: { method: HttpMethods }) { | ||
| const queryFn = React.useCallback(async () => { | ||
| const response = await fetch(`/api/only-any`, { | ||
| method, | ||
| }) | ||
| try { | ||
| const json = (await response.json()) as Promise<{ | ||
| method: HttpMethods | ||
| handler: HttpMethods & 'ANY' | ||
| }> | ||
| return json | ||
| } catch (e) {} | ||
| // handle HEAD and OPTIONS that have no body | ||
| const result = { | ||
| handler: response.headers.get('x-handler') as HttpMethods & 'ANY', | ||
| method: response.headers.get('x-method') as HttpMethods, | ||
| } | ||
| return result | ||
| }, [method]) |
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.
Fix incorrect types: awaited JSON typed as Promise and invalid 'ANY' intersection.
await response.json()should not be cast toPromise<...>.HttpMethods & 'ANY'isneverunder strict typing. The handler is literally'ANY'.
Apply this diff to correct the types:
- type HttpMethods = (typeof HttpMethods)[number]
- function Test({ method }: { method: HttpMethods }) {
+ type HttpMethod = (typeof HttpMethods)[number]
+ function Test({ method }: { method: HttpMethod }) {
const queryFn = React.useCallback(async () => {
const response = await fetch(`/api/only-any`, {
method,
})
try {
- const json = (await response.json()) as Promise<{
- method: HttpMethods
- handler: HttpMethods & 'ANY'
- }>
+ const json = (await response.json()) as {
+ method: HttpMethod
+ handler: 'ANY'
+ }
return json
} catch (e) {}
// handle HEAD and OPTIONS that have no body
const result = {
- handler: response.headers.get('x-handler') as HttpMethods & 'ANY',
- method: response.headers.get('x-method') as HttpMethods,
+ handler: response.headers.get('x-handler') as 'ANY',
+ method: response.headers.get('x-method') as HttpMethod,
}
return result
}, [method])As per coding guidelines (strict TypeScript).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/react-start/server-routes/src/routes/methods/only-any.tsx around lines
20-38, the awaited JSON is incorrectly cast to Promise<...> and the handler type
uses an invalid intersection HttpMethods & 'ANY'; change the assertion so the
awaited value is typed directly (remove the Promise wrapper) and use a correct
handler type (use the literal 'ANY' or a union like HttpMethods | 'ANY' if both
are possible) — i.e., cast (await response.json()) to an object shape { method:
HttpMethods; handler: 'ANY' } (or { method: HttpMethods; handler: HttpMethods |
'ANY' } if you need both), then return that value.
Summary by CodeRabbit
New Features
Bug Fixes
Tests