Skip to content

fix: server side redirects causd by default search param respect rewrite - #5523

Merged
schiller-manuel merged 1 commit into
mainfrom
fix-5477
Oct 18, 2025
Merged

fix: server side redirects causd by default search param respect rewrite#5523
schiller-manuel merged 1 commit into
mainfrom
fix-5477

Conversation

@schiller-manuel

@schiller-manuelschiller-manuel commented Oct 18, 2025

Copy link
Copy Markdown
Collaborator

fixes#5477

Summary by CodeRabbit

  • Bug Fixes
    • Redirect targets now derive from standardized URL data, reducing incorrect or unexpected redirects.
    • Redirect destinations are normalized to remove redundant origin parts so redirects behave consistently across routing scenarios.

@coderabbitai

coderabbitaiBot commented Oct 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

beforeLoad now derives the redirect target from nextLocation.url (not nextLocation.href), strips the origin when present, and calls redirect({ href }) with the normalized value.

Changes

Cohort / File(s)Summary
Before-load redirect normalization
packages/router-core/src/router.ts
Use nextLocation.url as the source for redirect href, remove origin if present, and pass the stripped value to redirect({ href }) instead of using nextLocation.href.

Sequence Diagram

sequenceDiagram
participant Router
participant beforeLoad as beforeLoad
participant Redirect as redirect()
rect rgb(220, 255, 220)
Note over Router,beforeLoad: New redirect resolution
Router->>beforeLoad: determine nextLocation
beforeLoad->>beforeLoad: read nextLocation.url (not .href)
beforeLoad->>beforeLoad: if origin present -> strip origin
beforeLoad->>Redirect: redirect({ href: sanitizedHref })
Redirect->>Router: perform navigation
end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰
I hopped along the route one day,
swapped href for url along the way,
I trimmed the origin, clean and neat,
sent the redirect home — no repeat.
Tiny paws, big fixes — hooray! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Title Check⚠️ WarningThe title contains a typo ("causd" instead of "caused") and uses vague phrasing ("respect rewrite") that makes the intent unclear. While it does reference actual elements of the change—server-side redirects and default search params—the awkward wording and spelling error prevent it from being clear and specific enough for a teammate to quickly understand the primary change from the title alone. The title attempts to address the core issue but falls short of being concise and readable.The title should be revised to correct the typo and clarify the change. A clearer alternative might be "fix: server-side redirects for default search params with base paths" or "fix: default search params redirect with base path configuration". This would maintain the connection to the linked issue while being more concise and grammatically correct.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check✅ PassedIssue #5477 describes a bug where default search params defined via a zod schema fail to redirect correctly when base paths are configured. The expected behavior is that visiting a path like /base/foo should redirect to /base/foo?search=query. The PR modifies the redirect computation in beforeLoad to compute the redirect href from nextLocation.url instead of nextLocation.href, which aligns with URL-based location data and addresses the redirect handling issue described in the linked issue.
Out of Scope Changes Check✅ PassedThe changeset only modifies packages/router-core/src/router.ts with focused changes to the redirect computation logic in the beforeLoad function. This change is directly related to fixing the server-side redirect behavior when default search params are configured with base paths, which is the core objective of linked issue #5477. No unrelated or extraneous changes are present in the PR.
Docstring Coverage✅ PassedNo functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-5477

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d00f2d0c11eba926257603ea8137ac8535d200b8 and d40c54c.

📒 Files selected for processing (1)
  • packages/router-core/src/router.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/router-core/src/router.ts
packages/router-core/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep framework-agnostic core router logic in packages/router-core/

Files:

  • packages/router-core/src/router.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: schiller-manuel
PR: TanStack/router#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.
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
PR: TanStack/router#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:

  • packages/router-core/src/router.ts
🧬 Code graph analysis (1)
packages/router-core/src/router.ts (1)
packages/router-core/src/redirect.ts (1)
  • redirect (57-93)
⏰ 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 (1)
packages/router-core/src/router.ts (1)

1968-1973: LGTM! Consistent origin-stripping pattern for server-side redirects.

The change correctly uses nextLocation.url (the full rewritten URL) instead of nextLocation.href (internal path-only representation), then strips the origin. This ensures server-side redirects respect base path configuration by using the public URL after rewrite processing.

The implementation follows the same pattern as the existing resolveRedirect method (lines 2260-2268), maintaining consistency across the codebase. Based on learnings, this origin-stripping logic is intentional to preserve same-origin redirects as paths while allowing cross-origin redirects to remain full URLs.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud

nx-cloudBot commented Oct 18, 2025

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit d40c54c

CommandStatusDurationResult
nx affected --targets=test:eslint,test:unit,tes...✅ Succeeded5m 24sView ↗
nx run-many --target=build --exclude=examples/*...✅ Succeeded1m 22sView ↗

☁️ Nx Cloud last updated this comment at 2025-10-18 00:30:30 UTC

@pkg-pr-new

pkg-pr-newBot commented Oct 18, 2025

Copy link
Copy Markdown
More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5523

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5523

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5523

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5523

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5523

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5523

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5523

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5523

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5523

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5523

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5523

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5523

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5523

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5523

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5523

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5523

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5523

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5523

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5523

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5523

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5523

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5523

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5523

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5523

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5523

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5523

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5523

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5523

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5523

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5523

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5523

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5523

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5523

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5523

commit: d40c54c

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Start: default search params not working properly with base paths configured

1 participant

@schiller-manuel