-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor(router-core): deprecate json function in favor of standard Response.json function
#6181
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
refactor(router-core): deprecate json function in favor of standard Response.json function
#6181
Conversation
WalkthroughReplaced framework-specific Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (49)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (30)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/**/*.{md,mdx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-06T15:03:07.223ZApplied to files:
📚 Learning: 2025-11-02T16:16:24.898ZApplied to files:
⏰ 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)
🔇 Additional comments (28)
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 5ed42c0
☁️ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/vue-start/basic-vue-query/src/routes/api/users.$id.ts (1)
14-27: TypeScript strict mode requires explicit parameter types for the handler function.The Response.json() migration is correct. However, the handler function violates TypeScript strict mode requirements specified in the coding guidelines. The destructured parameters
requestandparamslack explicit type annotations, and the async handler function has no return type. With the root tsconfig.json enforcingstrict: true, all code must have comprehensive type safety. Add explicit types to the handler parameters and return type to comply with strict mode and the coding guideline requirement for "extensive type safety for all code."
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
docs/start/framework/react/guide/isr.mddocs/start/framework/react/guide/observability.mddocs/start/framework/react/guide/server-routes.mddocs/start/framework/solid/guide/observability.mddocs/start/framework/solid/guide/server-routes.mde2e/react-start/basic-react-query/src/routes/api.users.tse2e/react-start/basic-react-query/src/routes/api/users.$id.tse2e/react-start/basic/src/routes/api.users.tse2e/react-start/basic/src/routes/api/users.$id.tse2e/react-start/custom-basepath/src/routes/api.users.tse2e/react-start/custom-basepath/src/routes/api/users.$id.tse2e/react-start/server-routes/src/routes/api/middleware-context.tse2e/react-start/server-routes/src/routes/api/only-any.tse2e/solid-start/basic-solid-query/src/routes/api.users.tse2e/solid-start/basic-solid-query/src/routes/api/users.$id.tse2e/solid-start/basic/src/routes/api/users.$userId.tse2e/solid-start/basic/src/routes/api/users.tse2e/solid-start/custom-basepath/src/routes/api/users.$userId.tse2e/solid-start/custom-basepath/src/routes/api/users.tse2e/solid-start/server-routes/src/routes/api/middleware-context.tse2e/solid-start/server-routes/src/routes/api/only-any.tse2e/vue-start/basic-vue-query/src/routes/api.users.tse2e/vue-start/basic-vue-query/src/routes/api/users.$id.tse2e/vue-start/basic/src/routes/api/users.$userId.tse2e/vue-start/basic/src/routes/api/users.tse2e/vue-start/custom-basepath/src/routes/api/users.$userId.tse2e/vue-start/custom-basepath/src/routes/api/users.tse2e/vue-start/server-routes/src/routes/api/middleware-context.tse2e/vue-start/server-routes/src/routes/api/only-any.tsexamples/react/start-basic-cloudflare/src/routes/api/users.$userId.tsexamples/react/start-basic-cloudflare/src/routes/api/users.tsexamples/react/start-basic-react-query/src/routes/api/users.$id.tsexamples/react/start-basic-react-query/src/routes/api/users.tsexamples/react/start-basic/src/routes/api/users.$userId.tsexamples/react/start-basic/src/routes/api/users.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.$userId.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.tsexamples/solid/start-basic-netlify/src/routes/api/users.$userId.tsexamples/solid/start-basic-netlify/src/routes/api/users.tsexamples/solid/start-basic-nitro/src/routes/api/users.$userId.tsexamples/solid/start-basic-nitro/src/routes/api/users.tsexamples/solid/start-basic-solid-query/src/routes/api/users.$id.tsexamples/solid/start-basic-solid-query/src/routes/api/users.tsexamples/solid/start-basic/src/routes/api/users.$userId.tsexamples/solid/start-basic/src/routes/api/users.tspackages/router-core/src/ssr/json.tspackages/router-core/tests/json.test.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/tests/serverRoute.test-d.ts
💤 Files with no reviewable changes (1)
- packages/router-core/tests/json.test.ts
🧰 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/custom-basepath/src/routes/api/users.tse2e/solid-start/custom-basepath/src/routes/api/users.$userId.tse2e/react-start/basic/src/routes/api.users.tsexamples/react/start-basic-react-query/src/routes/api/users.$id.tse2e/solid-start/server-routes/src/routes/api/middleware-context.tsexamples/solid/start-basic/src/routes/api/users.$userId.tse2e/react-start/server-routes/src/routes/api/middleware-context.tse2e/solid-start/basic-solid-query/src/routes/api/users.$id.tsexamples/react/start-basic-cloudflare/src/routes/api/users.$userId.tse2e/solid-start/basic/src/routes/api/users.tse2e/vue-start/basic-vue-query/src/routes/api/users.$id.tsexamples/solid/start-basic-nitro/src/routes/api/users.$userId.tse2e/vue-start/basic/src/routes/api/users.$userId.tsexamples/react/start-basic-react-query/src/routes/api/users.tse2e/react-start/basic/src/routes/api/users.$id.tse2e/vue-start/server-routes/src/routes/api/only-any.tsexamples/solid/start-basic-netlify/src/routes/api/users.tse2e/react-start/custom-basepath/src/routes/api/users.$id.tse2e/solid-start/basic-solid-query/src/routes/api.users.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.$userId.tsexamples/solid/start-basic-solid-query/src/routes/api/users.$id.tsexamples/solid/start-basic-nitro/src/routes/api/users.tsexamples/react/start-basic-cloudflare/src/routes/api/users.tsexamples/react/start-basic/src/routes/api/users.$userId.tse2e/vue-start/server-routes/src/routes/api/middleware-context.tse2e/vue-start/custom-basepath/src/routes/api/users.$userId.tse2e/solid-start/custom-basepath/src/routes/api/users.tse2e/solid-start/basic/src/routes/api/users.$userId.tsexamples/solid/start-basic/src/routes/api/users.tse2e/solid-start/server-routes/src/routes/api/only-any.tsexamples/solid/start-basic-netlify/src/routes/api/users.$userId.tsexamples/react/start-basic/src/routes/api/users.tse2e/react-start/custom-basepath/src/routes/api.users.tse2e/vue-start/basic-vue-query/src/routes/api.users.tse2e/react-start/server-routes/src/routes/api/only-any.tspackages/start-server-core/tests/serverRoute.test-d.tse2e/react-start/basic-react-query/src/routes/api.users.tsexamples/solid/start-basic-solid-query/src/routes/api/users.tse2e/react-start/basic-react-query/src/routes/api/users.$id.tse2e/vue-start/basic/src/routes/api/users.tspackages/start-server-core/src/createStartHandler.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.tspackages/router-core/src/ssr/json.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/vue-start/custom-basepath/src/routes/api/users.tse2e/solid-start/custom-basepath/src/routes/api/users.$userId.tse2e/react-start/basic/src/routes/api.users.tsexamples/react/start-basic-react-query/src/routes/api/users.$id.tse2e/solid-start/server-routes/src/routes/api/middleware-context.tsexamples/solid/start-basic/src/routes/api/users.$userId.tse2e/react-start/server-routes/src/routes/api/middleware-context.tse2e/solid-start/basic-solid-query/src/routes/api/users.$id.tsexamples/react/start-basic-cloudflare/src/routes/api/users.$userId.tse2e/solid-start/basic/src/routes/api/users.tse2e/vue-start/basic-vue-query/src/routes/api/users.$id.tsexamples/solid/start-basic-nitro/src/routes/api/users.$userId.tse2e/vue-start/basic/src/routes/api/users.$userId.tsexamples/react/start-basic-react-query/src/routes/api/users.tse2e/react-start/basic/src/routes/api/users.$id.tse2e/vue-start/server-routes/src/routes/api/only-any.tsexamples/solid/start-basic-netlify/src/routes/api/users.tse2e/react-start/custom-basepath/src/routes/api/users.$id.tse2e/solid-start/basic-solid-query/src/routes/api.users.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.$userId.tsexamples/solid/start-basic-solid-query/src/routes/api/users.$id.tsexamples/solid/start-basic-nitro/src/routes/api/users.tsexamples/react/start-basic-cloudflare/src/routes/api/users.tsexamples/react/start-basic/src/routes/api/users.$userId.tse2e/vue-start/server-routes/src/routes/api/middleware-context.tse2e/vue-start/custom-basepath/src/routes/api/users.$userId.tse2e/solid-start/custom-basepath/src/routes/api/users.tse2e/solid-start/basic/src/routes/api/users.$userId.tsexamples/solid/start-basic/src/routes/api/users.tse2e/solid-start/server-routes/src/routes/api/only-any.tsexamples/solid/start-basic-netlify/src/routes/api/users.$userId.tsexamples/react/start-basic/src/routes/api/users.tse2e/react-start/custom-basepath/src/routes/api.users.tse2e/vue-start/basic-vue-query/src/routes/api.users.tse2e/react-start/server-routes/src/routes/api/only-any.tspackages/start-server-core/tests/serverRoute.test-d.tse2e/react-start/basic-react-query/src/routes/api.users.tsexamples/solid/start-basic-solid-query/src/routes/api/users.tse2e/react-start/basic-react-query/src/routes/api/users.$id.tse2e/vue-start/basic/src/routes/api/users.tspackages/start-server-core/src/createStartHandler.tsexamples/solid/start-basic-cloudflare/src/routes/api/users.tspackages/router-core/src/ssr/json.ts
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use relative links to
docs/folder format (e.g.,./guide/data-loading) for internal documentation references
Files:
docs/start/framework/react/guide/isr.mddocs/start/framework/react/guide/server-routes.mddocs/start/framework/react/guide/observability.mddocs/start/framework/solid/guide/server-routes.mddocs/start/framework/solid/guide/observability.md
🧠 Learnings (4)
📚 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:
docs/start/framework/react/guide/isr.mddocs/start/framework/react/guide/server-routes.mddocs/start/framework/react/guide/observability.mddocs/start/framework/solid/guide/server-routes.mddocs/start/framework/solid/guide/observability.md
📚 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:
docs/start/framework/react/guide/server-routes.mde2e/react-start/server-routes/src/routes/api/middleware-context.tsdocs/start/framework/react/guide/observability.mde2e/vue-start/server-routes/src/routes/api/middleware-context.tsexamples/react/start-basic/src/routes/api/users.tspackages/start-server-core/src/createStartHandler.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/solid-start/basic-solid-query/src/routes/api/users.$id.tse2e/solid-start/basic-solid-query/src/routes/api.users.tsexamples/solid/start-basic-solid-query/src/routes/api/users.$id.tsexamples/solid/start-basic-solid-query/src/routes/api/users.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
docs/start/framework/solid/guide/observability.mdpackages/start-server-core/tests/serverRoute.test-d.ts
⏰ 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 (65)
e2e/react-start/server-routes/src/routes/api/middleware-context.ts (1)
2-2: LGTM! Clean migration to standard Web API.The removal of the
jsonimport and replacement withResponse.json()is correct and aligns with the PR objective to use the standard Web API. The payload structure and handler logic remain unchanged, ensuring functional equivalence.Also applies to: 21-21
examples/react/start-basic/src/routes/api/users.ts (2)
3-3: LGTM - Import cleanup is correct.The removal of the deprecated
jsonimport aligns with the PR objective to migrate to the standardResponse.jsonAPI.
57-59: LGTM - Correct migration to standard Web API.The migration from the deprecated
json()helper toResponse.json()is correct and preserves the same behavior. The payload structure (mapped user objects withid,name,examples/solid/start-basic-cloudflare/src/routes/api/users.ts (1)
3-3: LGTM! Clean migration to Web standard API.The removal of the
jsonimport and the switch toResponse.json()correctly implements the PR's goal of adopting the standard Web API. The payload structure and route behavior remain unchanged, ensuring backward compatibility.Also applies to: 58-60
examples/solid/start-basic-solid-query/src/routes/api/users.ts (1)
14-16: LGTM – Clean migration to standard Web API.The transition to
Response.json()is correct and follows modern best practices by using the standard Web API instead of a framework-specific helper.examples/solid/start-basic-solid-query/src/routes/api/users.$id.ts (2)
14-18: LGTM – Correct usage of standard Response.json().The success response correctly uses
Response.json()with the user data payload.
21-21: LGTM – Proper error response with status code.The error response correctly uses
Response.json()with the{ status: 404 }option, maintaining the same HTTP semantics as the previous implementation.examples/solid/start-basic-netlify/src/routes/api/users.ts (1)
3-3: LGTM! Clean migration to standard Web API.The refactoring correctly:
- Removes the unused
jsonimport after replacing its usage- Uses the standard
Response.json()method to construct the JSON response- Preserves the response payload structure and middleware behavior
This aligns with the PR objective to deprecate the custom helper in favor of the Fetch API standard.
Also applies to: 58-60
examples/react/start-basic/src/routes/api/users.$userId.ts (1)
19-26: LGTM! Correct usage of standardResponse.json().Both the success response (line 19) and error response (line 26) correctly use the standard Web API
Response.json()method as replacements for the deprecated customjsonhelper. The syntax and options (including thestatus: 404) are properly applied.examples/react/start-basic-cloudflare/src/routes/api/users.ts (2)
3-3: LGTM: Import migration completed.The removal of the
jsonimport from@tanstack/react-startaligns with the PR objective to migrate to the standard Web API.
57-59: LGTM: Correct migration toResponse.json().The replacement of the deprecated
json()helper with the standardResponse.json()is correct. The static Response.json() method is available in Cloudflare Workers runtime and can be used to initialize a Response object with a JSON-serialized payload. The payload structure and behavior remain unchanged.examples/react/start-basic-cloudflare/src/routes/api/users.$userId.ts (2)
19-23: LGTM: Success response migration correct.The replacement of
json()withResponse.json()for the success case is correct and maintains the same payload structure.
26-26: LGTM: Error response migration correct.The error response correctly uses
Response.json()with the status code passed via the init options object{ status: 404 }, which matches the standard Web API signature.e2e/solid-start/custom-basepath/src/routes/api/users.$userId.ts (3)
21-25: LGTM! Correct usage of standardResponse.json().The success response correctly uses the Web API
Response.json()method with the data object as the first parameter. The API automatically sets theContent-Typeheader toapplication/jsonand defaults to a 200 status code.
28-28: LGTM! Error response correctly configured.The error response properly uses
Response.json()with the status code passed in the options object (second parameter). This correctly returns a 404 status with JSON error payload.
21-28: No action required. The project's Node.js version (24.8.0 via .nvmrc) satisfies the Response.json() API requirement of Node 18.0.0+. The code complies with TypeScript strict mode as configured in tsconfig.json.examples/react/start-basic-react-query/src/routes/api/users.$id.ts (2)
14-18: LGTM! Correct migration to standard Web API.The replacement of the custom
json()helper withResponse.json()is correct and follows the standard Web API. The payload structure and default 200 status are preserved.
21-21: Response.json() correctly implements error response with status code.The Response.json() static method correctly creates a Response object with JSON body and an appropriate Content-Type header, with status and other options also configurable. Response.json() is available in Node.js 18 LTS and later, and this project enforces Node.js 24.8.0 via .nvmrc, ensuring compatibility. The implementation properly maintains behavior with the status code preserved in the options parameter.
e2e/vue-start/custom-basepath/src/routes/api/users.ts (2)
2-2: LGTM! Import cleanup is correct.The removal of the
jsonhelper import is appropriate since the code now uses the standardResponse.json()API.
59-61: Migration toResponse.json()correctly uses standard Web API.The json() static method of the Response interface returns a Response that contains the provided JSON data as body, and a Content-Type header which is set to application/json. The Fetch API has been part of the standard Node.js library since version 18.0.0, released in April 2022. The code meets TypeScript strict mode requirements (enabled in tsconfig.json) and inherits ESLint router best practices from @tanstack/config/eslint.
examples/solid/start-basic-nitro/src/routes/api/users.$userId.ts (2)
20-24: Migration to standardResponse.json()looks correct.The success response construction correctly uses the standard Web API
Response.json()method, which defaults to status 200. The payload structure is preserved from the previous implementation.
27-27: Error response migration is correct; Node.js compatibility confirmed.The Response API is available as a global in Node.js 18, and this example indirectly requires Node 22.12.0+ through its
solid-startdependency, which provides full support forResponse.json(). The migration preserves the existing behavior without issues.examples/solid/start-basic-nitro/src/routes/api/users.ts (1)
3-3: LGTM: Import cleanup aligns with deprecation.The removal of the
jsonhelper import is correct and consistent with the PR objective to migrate to the standardResponse.jsonAPI.e2e/solid-start/basic/src/routes/api/users.$userId.ts (1)
21-28: LGTM! Clean migration to standard Response.json.Both success and error response paths correctly use the Web API's
Response.json()with appropriate status codes and payload structures preserved.e2e/solid-start/server-routes/src/routes/api/middleware-context.ts (1)
21-25: LGTM! Correct migration to Response.json.The middleware context handler properly uses
Response.json()with the payload structure preserved.e2e/solid-start/custom-basepath/src/routes/api/users.ts (1)
59-61: LGTM! Proper use of Response.json with array payload.The response correctly uses
Response.json()with the mapped user list, maintaining the same data transformation logic.e2e/vue-start/custom-basepath/src/routes/api/users.$userId.ts (1)
21-28: LGTM! Consistent migration in Vue Start route.Both response paths correctly use
Response.json()with proper status codes, matching the pattern used across other framework examples.e2e/solid-start/server-routes/src/routes/api/only-any.ts (1)
7-13: LGTM! Correct usage of Response.json with custom headers.The response properly uses
Response.json()with the init options object to set custom headers, demonstrating full compatibility with the previousjson()helper's capabilities.e2e/react-start/basic/src/routes/api/users.$id.ts (1)
19-26: LGTM! Clean migration in React Start route.Both success and error response paths correctly use
Response.json()with appropriate status handling, consistent with the broader migration effort.e2e/react-start/basic/src/routes/api.users.ts (1)
21-23: LGTM! Proper array response with Response.json.The users list endpoint correctly uses
Response.json()with the mapped array payload.e2e/solid-start/basic-solid-query/src/routes/api.users.ts (1)
18-20: LGTM! Correct migration to Response.json.The endpoint properly uses
Response.json()with the mapped user list, maintaining consistency with the broader refactoring effort. Response.json() has been implemented in Node.js v18 LTS, so the migration is safe for deployment targets running Node.js 18 or later.e2e/vue-start/basic/src/routes/api/users.ts (1)
2-2: LGTM: Clean migration to standard Web API.The removal of the
jsonimport and replacement withResponse.json()is correct and maintains the same payload structure and response semantics.Also applies to: 59-61
e2e/react-start/server-routes/src/routes/api/only-any.ts (1)
7-13: LGTM: Correct usage of Response.json with headers.The migration properly preserves both the payload and custom headers using the standard
Response.json(data, { headers })signature.e2e/vue-start/basic-vue-query/src/routes/api.users.ts (1)
18-20: LGTM: Standard Web API adoption.The switch to
Response.json()maintains the same response payload while reducing dependency on framework-specific helpers.examples/solid/start-basic-netlify/src/routes/api/users.$userId.ts (2)
20-24: LGTM: Success path correctly migrated.The response construction uses the standard Web API while preserving the user data payload.
27-27: LGTM: Error response properly migrated with status code.The error path correctly uses
Response.json(data, { status: 404 })to maintain the same 404 status while returning the error payload.docs/start/framework/solid/guide/observability.md (1)
131-131: LGTM: Documentation examples updated consistently.All observability examples (user routes, health checks, metrics, and error reporting) have been updated to use the standard
Response.json()API, providing developers with current best practices.Also applies to: 202-202, 316-316, 461-461
e2e/vue-start/server-routes/src/routes/api/middleware-context.ts (1)
2-2: LGTM: Middleware test route correctly updated.The import cleanup and response migration maintain the middleware context testing functionality while adopting the standard Web API.
Also applies to: 21-25
e2e/react-start/basic-react-query/src/routes/api.users.ts (1)
18-20: LGTM: React Query example updated correctly.The migration to
Response.json()maintains the same user data transformation and response structure.e2e/react-start/basic-react-query/src/routes/api/users.$id.ts (1)
18-25: LGTM! Clean migration to standard Web API.The replacement of the custom
jsonhelper withResponse.json()is correct and maintains the same behavior. Both success and error responses properly use the standard API with appropriate status codes.e2e/react-start/custom-basepath/src/routes/api.users.ts (1)
21-23: LGTM! Standard Web API correctly applied.The migration to
Response.json()is clean and maintains the same response structure for the user list.examples/solid/start-basic/src/routes/api/users.ts (2)
2-2: LGTM! Import cleanup is correct.Properly removed the
jsonhelper import while retainingcreateMiddleware.
53-55: LGTM! Standard API usage is correct.The replacement with
Response.json()maintains the same response structure and works correctly with the middleware chain.e2e/solid-start/basic-solid-query/src/routes/api/users.$id.ts (1)
18-25: LGTM! Consistent migration to standard Web API.The migration maintains proper error handling and status codes. Both success and error paths correctly use
Response.json().e2e/vue-start/server-routes/src/routes/api/only-any.ts (1)
7-13: LGTM! Proper usage of Response.json with headers.The migration correctly uses
Response.json()with custom headers in the second argument, maintaining the same behavior for the ANY handler.e2e/vue-start/basic/src/routes/api/users.$userId.ts (1)
21-28: LGTM! Clean standard API migration.The replacement maintains proper error handling and response structure. Both success and error responses correctly use
Response.json()with appropriate status codes.examples/solid/start-basic-cloudflare/src/routes/api/users.$userId.ts (1)
20-27: LGTM! Standard API works correctly in Cloudflare context.The migration to
Response.json()is appropriate for Cloudflare Workers, which fully support the standard Web API. Error handling and response structure are preserved.docs/start/framework/solid/guide/server-routes.md (1)
370-410: LGTM! Documentation accurately reflects the migration.The documentation has been properly updated to:
- Reference the standard
Response.jsonAPI instead of the custom helper- Include an MDN link for authoritative reference
- Update all code examples to use
Response.json()This aligns well with the codebase changes and provides clear guidance for users migrating to the standard Web API.
e2e/react-start/custom-basepath/src/routes/api/users.$id.ts (1)
19-27: LGTM! Clean migration to standard Response.json.The replacement of the custom
jsonhelper with the standard Web APIResponse.json()is correct. Both the success and error responses maintain their original payload structure and status codes.docs/start/framework/react/guide/server-routes.md (2)
370-390: LGTM! Documentation accurately reflects the API migration.The documentation correctly updates references from the custom
jsonhelper to the standard Web APIResponse.json(). The MDN link provides appropriate external documentation, and the code examples demonstrate proper usage.
410-410: LGTM! Consistent usage of Response.json.The example correctly demonstrates using
Response.json()to return user data.e2e/solid-start/basic/src/routes/api/users.ts (2)
2-2: LGTM! Import correctly updated.The import statement properly removes the deprecated
jsonhelper while retainingcreateMiddleware.
59-61: LGTM! Correct migration to Response.json.The response construction correctly uses the standard
Response.json()API while preserving the original payload mapping logic.docs/start/framework/react/guide/observability.md (4)
131-131: LGTM! Consistent use of Response.json in observability examples.The middleware example correctly uses
Response.json()to return user data.
202-202: LGTM! Health check endpoint uses standard Response.json.The health check route correctly returns JSON using the standard API.
316-323: LGTM! Metrics endpoint properly structured.The metrics collection example correctly uses
Response.json()with properly structured system and application data.
461-461: LGTM! Error reporting endpoint updated correctly.The error reporting example appropriately uses
Response.json()to return error data.docs/start/framework/react/guide/isr.md (4)
91-100: LGTM! Correct usage of Response.json with cache headers.The ISR example properly demonstrates
Response.json()with cache control headers in the init parameter, maintaining the original caching behavior.
134-134: LGTM! Middleware example updated correctly.The middleware pattern correctly uses
Response.json()for the product response.
174-174: LGTM! Error response with status code.The authentication error response correctly passes the status code as an option to
Response.json().
192-192: LGTM! Revalidation response.The on-demand revalidation success response correctly uses
Response.json().packages/start-server-core/src/createStartHandler.ts (3)
2-2: LGTM! Import correctly updated.The import statement properly removes the deprecated
jsonhelper while retaining the required utilities.
177-184: LGTM! Error response for unsupported Accept headers.The error response correctly uses
Response.json()with an appropriate 500 status code for requests that don't accept HTML.
258-266: LGTM! Manual redirect serialization.Both manual redirect handling paths correctly serialize redirect options as JSON when the
x-tsr-redirect: manualheader is present. The logic appropriately includes theisSerializedRedirectflag and preserves response headers.Note: The two similar blocks at lines 258-266 and 299-307 appear to handle different code paths (before and after redirect resolution), which is appropriate.
Also applies to: 299-307
packages/start-server-core/tests/serverRoute.test-d.ts (1)
3-288: Verify why type tests are commented out.The test file contains a dummy test while all actual type-checking tests are commented out. The Response.json() usage within the commented tests is correct, but the wholesale commenting out of tests may indicate:
- Temporary disabling during migration
- Known type issues being investigated
- Tests awaiting other changes
Please confirm whether these tests should be re-enabled as part of this PR or tracked separately.
#!/bin/bash # Check git history to see when these tests were commented out echo "=== Checking git log for this test file ===" git log --oneline --follow -n 20 -- packages/start-server-core/tests/serverRoute.test-d.ts echo "" echo "=== Checking for related issues or TODOs ===" rg -n "serverRoute.test-d|TODO.*type.*test" --type=ts --type=md -C2
| return Response.json( | ||
| list.map((u) => ({ id: u.id, name: u.name, email: u.email })), | ||
| ) |
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
# Check for Node version requirements in package configuration
# Check root package.json for engines field
echo "=== Root package.json engines ==="
jq '.engines' package.json 2>/dev/null || echo "No engines field found in root package.json"
# Check for .nvmrc or .node-version files
echo -e "\n=== Node version files ==="
fd -t f '^\.nvmrc$|^\.node-version$' -x cat {} \; 2>/dev/null || echo "No Node version files found"
# Check for Node version in CI configs
echo -e "\n=== CI Node version references ==="
rg "node-version|node_version|node:" .github/workflows/ -A 2 -B 2 2>/dev/null || echo "No CI configs found"Repository: TanStack/router
Length of output: 181
🏁 Script executed:
#!/bin/bash
# Verify the file exists and examine the code context
# Check if file exists
echo "=== File existence check ==="
if [ -f "examples/react/start-basic-react-query/src/routes/api/users.ts" ]; then
echo "File found"
echo -e "\n=== Lines 10-20 context ==="
sed -n '10,20p' examples/react/start-basic-react-query/src/routes/api/users.ts | cat -n
else
echo "File not found"
fiRepository: TanStack/router
Length of output: 500
🏁 Script executed:
#!/bin/bash
# Check TypeScript configuration for strict mode
echo "=== tsconfig.json strict setting ==="
jq '.compilerOptions.strict' tsconfig.json 2>/dev/null || echo "No tsconfig.json at root"
# Check for tsconfig in subdirectories
echo -e "\n=== Searching for tsconfig files ==="
fd -t f 'tsconfig\.json$' | head -20Repository: TanStack/router
Length of output: 1208
🏁 Script executed:
#!/bin/bash
# Check ESLint configuration for router plugin
echo "=== ESLint configuration files ==="
fd -t f 'eslint\.config\.\(js\|mjs\)$|\.eslintrc\.\(json\|js\|yml\)$' | head -20
# Check for router plugin references
echo -e "\n=== Router plugin references ==="
rg "@tanstack/eslint-plugin-router|eslint-plugin-router" -i 2>/dev/null || echo "No router plugin references found"Repository: TanStack/router
Length of output: 2222
🏁 Script executed:
#!/bin/bash
# Check for Node version documentation elsewhere
echo "=== package.json engines field (detailed) ==="
cat package.json | jq '.engines // "No engines field"'
echo -e "\n=== Node version in README ==="
rg "node.*version|minimum.*node|requires.*node" -i README.md docs/README.md 2>/dev/null | head -10 || echo "No Node version mentions in main READMEs"
echo -e "\n=== Check example project structure ==="
ls -la examples/react/start-basic-react-query/ | head -20Repository: TanStack/router
Length of output: 858
🏁 Script executed:
#!/bin/bash
# Check if example has eslint configuration
echo "=== ESLint config in example directory ==="
fd -t f 'eslint\|\.eslintrc' examples/react/start-basic-react-query/ 2>/dev/null || echo "No ESLint config in example"
echo -e "\n=== package.json of example ==="
cat examples/react/start-basic-react-query/package.json | jq '.devDependencies, .engines' 2>/dev/null || echo "Could not read example package.json"Repository: TanStack/router
Length of output: 411
🏁 Script executed:
#!/bin/bash
# Verify exact line numbers in the file
echo "=== Full file content with line numbers ==="
cat -n examples/react/start-basic-react-query/src/routes/api/users.tsRepository: TanStack/router
Length of output: 845
Add ESLint configuration for router best practices to comply with coding guidelines.
The migration to Response.json() is appropriate, but this .ts example file violates the coding guideline requiring ESLint rules for router best practices using the @tanstack/eslint-plugin-router plugin. The example project currently has no ESLint configuration or router plugin installed.
Additionally, note that the project does not document a minimum Node version requirement for Response.json() (available since Node 18) in package.json engines field, CI configuration, or documentation—this should be added to prevent usage on incompatible Node versions.
| return Response.json( | ||
| list.map((u) => ({ id: u.id, name: u.name, email: u.email })), | ||
| ) |
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:
# First, let's check the file location and context
fd -t f 'package\.json' | head -5Repository: TanStack/router
Length of output: 232
🏁 Script executed:
# Check the specific file content
cat -n examples/solid/start-basic-nitro/src/routes/api/users.ts 2>/dev/null || echo "File not found at specified path"Repository: TanStack/router
Length of output: 2527
🏁 Script executed:
# Look for Node version requirements in package.json
if [ -f package.json ]; then
jq '.engines.node // .packageManager // "Not specified"' package.json
else
echo "Root package.json not found"
fiRepository: TanStack/router
Length of output: 73
🏁 Script executed:
# Check for .nvmrc or .node-version files
fd -t f '(\.nvmrc|\.node-version)$'Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check CI workflows for Node version
fd -t f '\.(yml|yaml)$' .github/workflows 2>/dev/null | head -5Repository: TanStack/router
Length of output: 173
🏁 Script executed:
# Check TypeScript configuration
fd -t f 'tsconfig\.json$' | xargs -I {} sh -c 'echo "File: {}" && jq ".compilerOptions.strict // .compilerOptions" {} 2>/dev/null || cat {}'Repository: TanStack/router
Length of output: 25008
🏁 Script executed:
# Check root package.json for Node version requirements
cat package.json | jq '.engines // "No engines specified"'Repository: TanStack/router
Length of output: 81
🏁 Script executed:
# Check GitHub Actions workflow for Node version matrix
grep -A 10 "node-version" .github/workflows/pr.yml | head -20Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Find and check ESLint configuration
fd -t f 'eslint\.config\.(js|mjs|cjs)$|\.eslintrc\.(json|js|cjs)$' | head -5Repository: TanStack/router
Length of output: 269
🏁 Script executed:
# Check for ESLint router plugin in package.json
grep -i "eslint.*router\|router.*eslint" package.json || echo "Not found in root package.json"Repository: TanStack/router
Length of output: 113
🏁 Script executed:
# Check the specific example's package.json for Node requirements
if [ -f examples/solid/start-basic-nitro/package.json ]; then
cat examples/solid/start-basic-nitro/package.json | jq '.engines // .packageManager // "No engines specified"'
else
echo "No package.json in example directory"
fiRepository: TanStack/router
Length of output: 81
🏁 Script executed:
# Check TypeScript strict mode for the specific file's tsconfig
cat examples/solid/start-basic-nitro/tsconfig.json 2>/dev/null | jq '.compilerOptions.strict // "Not specified"'Repository: TanStack/router
Length of output: 63
🏁 Script executed:
# Check root ESLint config for router plugin rules
cat eslint.config.js | head -50Repository: TanStack/router
Length of output: 770
🏁 Script executed:
# Check if there are specific ESLint router rules that might apply
grep -r "Response\.json\|router.*rules" packages/eslint-plugin-router/ 2>/dev/null | head -10Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Verify the GitHub Actions workflow actually runs Node tests
cat .github/workflows/pr.yml | grep -A 5 "node\|Node"Repository: TanStack/router
Length of output: 144
🏁 Script executed:
# Search for any ESLint violations or router-specific rules that might affect Response.json usage
fd -t f '\.ts$' examples/solid/start-basic-nitro/src/routes/api --exec grep -l "eslint-disable" {} \;Repository: TanStack/router
Length of output: 180
🏁 Script executed:
# Check if there's documentation or comments about Response.json requirements
grep -r "Response\.json\|Node.*18" examples/solid/start-basic-nitro/ 2>/dev/null || echo "No explicit version requirements found"Repository: TanStack/router
Length of output: 385
🏁 Script executed:
# Check for Node version info in README or documentation
fd -t f 'README\.(md|txt)$' | xargs grep -i "node\|node.*version" 2>/dev/null | head -10Repository: TanStack/router
Length of output: 577
🏁 Script executed:
# Check package.json packageManager field which sometimes specifies runtime
cat package.json | jq '.packageManager'Repository: TanStack/router
Length of output: 73
🏁 Script executed:
# Verify the actual GitHub Actions setup-node action to see what Node version is used
grep -A 10 "setup.*node\|actions/setup-node" .github/workflows/pr.yml | head -20Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if .tool-versions or other files specify Node
fd -t f '\.tool-versions$|\.npmrc$|\.pnpmfile\.cjs$' | xargs cat 2>/dev/nullRepository: TanStack/router
Length of output: 41
Response.json() usage is correct, but Node 18+ version support is not explicitly enforced.
The Response.json() implementation on lines 58-60 correctly follows Web API standards and maintains type safety. However, Response.json requires Node 18+, yet the codebase has no explicit engines.node constraint in package.json or configuration files. Consider adding "engines": { "node": ">=18" } to enforce this runtime requirement.
🤖 Prompt for AI Agents
In examples/solid/start-basic-nitro/src/routes/api/users.ts around lines 58-60,
Response.json() requires Node 18+ but the repository does not enforce a Node
engine; add an "engines": { "node": ">=18" } entry to the project's package.json
(and update any workspace root package.json if monorepo) to ensure runtime
compatibility, and update CI/tooling configs or README if they validate or
document Node versions.
| return Response.json({ | ||
| id: user.id, | ||
| name: user.name, | ||
| email: user.email, | ||
| }) |
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
# Check Node.js version requirements and file content
# 1. Check package.json for Node.js version requirement
echo "=== Checking Node.js version requirement ==="
fd -t f '^package\.json$' --max-results 1 --exec cat {} \; | jq -r '.engines.node // "not specified"'
# 2. Check for .nvmrc or .node-version files
echo -e "\n=== Checking .nvmrc/.node-version files ==="
fd -e nvmrc -e node-version --max-results 5 -x cat {}
# 3. Check the actual file exists
echo -e "\n=== Checking file existence ==="
ls -l "examples/solid/start-basic/src/routes/api/users.\$userId.ts" 2>&1 || echo "File not found at expected path"
# 4. Check TypeScript config for strict mode
echo -e "\n=== Checking tsconfig.json ==="
find . -name "tsconfig.json" -type f | head -2 | xargs -I {} cat {} | jq '.compilerOptions.strict // "not specified"'
# 5. Check ESLint config for router plugin
echo -e "\n=== Checking ESLint config ==="
fd -e eslintrc.json -e eslintrc.js -e eslintrc.cjs --max-results 3 -x cat {}Repository: TanStack/router
Length of output: 1080
🏁 Script executed:
#!/bin/bash
# Read the actual file to verify code content
echo "=== File content (lines 1-30) ==="
head -30 "examples/solid/start-basic/src/routes/api/users.\$userId.ts"Repository: TanStack/router
Length of output: 971
🏁 Script executed:
#!/bin/bash
# Get full package.json to check engines and dependencies
echo "=== Full package.json ==="
cat package.json | head -50
echo -e "\n=== Looking for ESLint configuration ==="
find . -name ".eslintrc*" -o -name "eslint.config.*" 2>/dev/null | head -5Repository: TanStack/router
Length of output: 2513
🏁 Script executed:
#!/bin/bash
# Check ESLint configuration content
echo "=== ESLint config files ==="
for file in $(find . -maxdepth 3 \( -name ".eslintrc.json" -o -name ".eslintrc.js" -o -name "eslint.config.js" -o -name ".eslintrc.cjs" \) 2>/dev/null); do
echo "File: $file"
cat "$file" | head -100
echo "---"
done
# Also check if ESLint router plugin is mentioned in package.json
echo -e "\n=== Checking for router ESLint plugin in package.json ==="
grep -i "eslint.*router\|router.*eslint" package.json || echo "No explicit router ESLint plugin found in package.json"Repository: TanStack/router
Length of output: 13458
Document Node.js 18+ requirement for Response.json compatibility.
The migration to Response.json() is implemented correctly in both the success and error paths. However, the project does not currently document the Node.js 18+ requirement needed for this API. Add the engines field to package.json:
"engines": {
"node": ">=18"
}This ensures developers and CI systems enforce the minimum Node.js version required by Response.json.
🤖 Prompt for AI Agents
In examples/solid/start-basic/src/routes/api/users.$userId.ts around lines 17 to
21, Response.json() requires Node.js 18+ but the project package.json does not
declare this; update the repository root package.json to include an "engines"
field with "node": ">=18" so local development and CI enforce the minimum Node
version (add or merge the engines entry under the top-level package.json
object).
| /** | ||
| * @deprecated Use [`Response.json`](https://developer.mozilla.org/en-US/docs/Web/API/Response/json_static) from the standard Web API directly. | ||
| */ | ||
| export interface JsonResponse<TData> extends Response { | ||
| json: () => Promise<TData> | ||
| } | ||
|
|
||
| /** | ||
| * @deprecated Use [`Response.json`](https://developer.mozilla.org/en-US/docs/Web/API/Response/json_static) from the standard Web API directly. | ||
| */ | ||
| export function json<TData>( | ||
| payload: TData, | ||
| init?: ResponseInit, | ||
| ): JsonResponse<TData> { | ||
| return new Response(JSON.stringify(payload), { | ||
| ...init, | ||
| headers: mergeHeaders( | ||
| { 'content-type': 'application/json' }, | ||
| init?.headers, | ||
| ), | ||
| }) | ||
| return Response.json(payload, init) | ||
| } |
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
# Check if package.json specifies minimum Node.js version
echo "=== Checking root package.json ==="
if [ -f "package.json" ]; then
jq '.engines.node // "not specified"' package.json
else
echo "No root package.json found"
fi
echo ""
echo "=== Checking packages/router-core/package.json ==="
if [ -f "packages/router-core/package.json" ]; then
jq '.engines.node // "not specified"' packages/router-core/package.json
else
echo "File not found"
fi
echo ""
echo "=== Checking for version requirement in package.json files ==="
fd -t f "package.json" | head -5 | xargs -I {} bash -c 'echo "File: {}"; jq ".engines.node // empty" {} 2>/dev/null'Repository: TanStack/router
Length of output: 484
Update package.json to require Node.js 18+ and add deprecation notice about minimum version.
The code delegates to Response which is available as a global in Node.js 18, but packages/router-core/package.json currently specifies ">=12". This creates a runtime compatibility issue—the code will fail on Node.js 12-17. Update the engines field from ">=12" to ">=18" to match the actual API requirement, and consider adding a note to the deprecation comment about the minimum Node.js version dependency for users of this deprecated helper.
🤖 Prompt for AI Agents
In packages/router-core/src/ssr/json.ts lines 1-16 and
packages/router-core/package.json, the deprecated helper now relies on the
global Response API introduced in Node.js 18; update
packages/router-core/package.json engines field from ">=12" to ">=18" to reflect
the runtime requirement, and add a short note to the deprecation comment at the
top of src/ssr/json.ts stating that this helper requires Node.js 18+
(encouraging users to use Response.json directly and noting the minimum Node
version).
… `Response.json` function
f390d55 to
5ed42c0
Compare
I'm not sure if this change makes sense for this project, but I wanted to suggest it anyway.
I deprecated the
jsonfunction of TanStack, replaced its implementation with callingResponse.jsonand updated relevant docs pages and example project to useResponse.jsondirectly.We are already successfully using the standard Web Api function
Response.jsoninstead of thejsonhelper function of TanStack Start. Both functions do the same, all tests pass, removing/deprecating thejsonfunction of TanStack would reduce the API surface and relying on standards is probably preferred. According to MDN,Response.jsonis available since Node 18.Summary by CodeRabbit
Documentation
Deprecations
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.