fix: redirect respects rewrites #5315
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughCentralizes server-side origin resolution with a new getOrigin(request) utility, uses it across SSR/start handlers, normalizes redirect hrefs by stripping matching origins to paths, and updates example apps to use http://localhost:3000 (protocol change only). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Server
participant SSRHandler as SSR Handler
participant Router
participant Util as getOrigin
Client->>Server: HTTP request
Server->>SSRHandler: handle(request)
SSRHandler->>Util: getOrigin(request)
Util-->>SSRHandler: origin
SSRHandler->>Router: update({ origin: router.options.origin ?? origin })
alt route triggers redirect
Router->>Router: resolveRedirect() (build URL via location builder)
Router->>Router: strip origin if matching -> path (fallback '/')
Router-->>SSRHandler: redirect with href/path and Location header
SSRHandler-->>Server: 3xx response
Server-->>Client: Redirect
else render
SSRHandler-->>Server: SSR response (200)
Server-->>Client: HTML/stream
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Comment |
|
View your CI Pipeline Execution ↗ for commit 86a5961
☁️ 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-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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/react/basic-ssr-file-based/src/entry-server.tsx(1 hunks)examples/react/basic-ssr-streaming-file-based/src/entry-server.tsx(2 hunks)packages/router-core/src/router.ts(1 hunks)packages/router-core/src/ssr/createRequestHandler.ts(3 hunks)packages/router-core/src/ssr/server.ts(1 hunks)packages/router-core/src/ssr/ssr-server.ts(1 hunks)packages/start-server-core/src/createStartHandler.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
examples/react/basic-ssr-file-based/src/entry-server.tsxpackages/router-core/src/ssr/ssr-server.tspackages/router-core/src/ssr/server.tspackages/router-core/src/router.tspackages/start-server-core/src/createStartHandler.tspackages/router-core/src/ssr/createRequestHandler.tsexamples/react/basic-ssr-streaming-file-based/src/entry-server.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/react/basic-ssr-file-based/src/entry-server.tsxexamples/react/basic-ssr-streaming-file-based/src/entry-server.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/ssr/ssr-server.tspackages/router-core/src/ssr/server.tspackages/router-core/src/router.tspackages/router-core/src/ssr/createRequestHandler.ts
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.ts
🧬 Code graph analysis (4)
packages/router-core/src/ssr/ssr-server.ts (1)
packages/router-core/src/ssr/server.ts (1)
getOrigin(10-10)
packages/router-core/src/router.ts (1)
packages/router-core/src/redirect.ts (1)
redirect(57-93)
packages/start-server-core/src/createStartHandler.ts (1)
packages/router-core/src/ssr/ssr-server.ts (1)
getOrigin(145-157)
packages/router-core/src/ssr/createRequestHandler.ts (1)
packages/router-core/src/ssr/ssr-server.ts (1)
getOrigin(145-157)
⏰ 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
🔇 Additional comments (10)
examples/react/basic-ssr-streaming-file-based/src/entry-server.tsx (1)
21-21: Protocol change looks intentional but verify consistency.The base URL protocol changed from
httpstohttp. This aligns with the newgetOriginutility's fallback default of'http://localhost'(as seen in the related changes). Ensure this matches your local development environment configuration.examples/react/basic-ssr-file-based/src/entry-server.tsx (1)
21-21: LGTM! Consistent protocol change.The base URL protocol change from
httpstohttpis consistent with the streaming variant and aligns with the centralized origin resolution strategy introduced in this PR.packages/router-core/src/ssr/server.ts (1)
10-10: LGTM! Exports new origin resolution utility.The addition of
getOriginto the public exports enables consumers to use the centralized origin resolution logic. This is part of the broader refactoring to standardize origin handling across SSR flows.packages/router-core/src/ssr/createRequestHandler.ts (1)
3-3: LGTM! Centralized origin resolution for SSR.The changes properly:
- Import the new
getOriginutility- Compute the origin from the request using
getOrigin(request)- Respect explicit
router.options.originconfiguration while falling back to the computed originThis ensures consistent origin handling across SSR flows and integrates cleanly with the redirect resolution fix.
Also applies to: 30-42
packages/router-core/src/router.ts (1)
2231-2246: Ensure path-only hrefs are supported byrouter.navigatein all contexts.
Test the redirect flow in both SSR (start-server, router-ssr-query) and CSR (solid-start, react-start, load-matches) to confirmrouter.navigate(resolveRedirect(err).options)correctly handles relativeoptions.href. Validate reproduction of issue #5300: navigating from/my-app/posts/2should redirect to/my-app/errorrather than/error.packages/router-core/src/ssr/ssr-server.ts (1)
145-157: LGTM! Clean origin resolution with sensible fallbacks.The implementation correctly prioritizes the Origin header, falls back to parsing request.url, and provides a safe default. The try/catch blocks appropriately handle invalid URLs without exposing error details.
packages/start-server-core/src/createStartHandler.ts (4)
14-17: Good refactor: centralizing origin resolution.Moving to a shared
getOriginutility from@tanstack/router-core/ssr/serverimproves consistency across SSR handlers.
100-100: LGTM! Consistent origin computation.Computing the origin once at the start of the request handler ensures consistent origin resolution across fetch patching and router initialization.
115-115: LGTM! Fetch patching uses consistent origin.Both URL construction paths (string input and Request object) correctly use the shared
originvalue, ensuring relative URLs resolve consistently.Also applies to: 124-124
161-161: LGTM! Correct origin precedence for redirect resolution.The fallback logic
router.options.origin ?? origincorrectly prioritizes explicit configuration while using the computed origin from the request when not specified. This should ensure redirects respect the rewrite base path as intended in the PR objectives.
examples/react/basic-ssr-streaming-file-based/src/entry-server.tsx
Outdated
Show resolved
Hide resolved
schiller-manuel
force-pushed
the
fix-5300
branch
from
c6e1e48 to
ed72004
Compare
1 participant
fixes #5300
Summary by CodeRabbit