router icon indicating copy to clipboard operation
router copied to clipboard

fix: compare rewritten url to decide if server needs to redirect

Open schiller-manuel opened this issue 1 week ago • 3 comments

Summary by CodeRabbit

  • Refactor

    • Restructured internal URL handling to use URL objects consistently throughout router packages, improving href generation and origin normalization during navigation and redirects.
    • Added helper method for reliable href string extraction from URL objects.
  • Bug Fixes

    • Enhanced href extraction in React, Solid, and Vue link components to correctly derive string values from URL objects.

✏️ Tip: You can customize this high-level summary in your review settings.

schiller-manuel avatar Dec 11 '25 20:12 schiller-manuel

Walkthrough

The PR changes ParsedLocation.url from a string to a URL object in router-core, updates router logic to propagate and consume URL objects, and updates framework link components and tests to read .href from those URL objects.

Changes

Cohort / File(s) Summary
Router core type definition
packages/router-core/src/location.ts
ParsedLocation.url changed from string to URL; JSDoc for publicHref adjusted.
Router core location handling
packages/router-core/src/router.ts
parse/build/redirect/commit flows now store and compare URL objects; getParsedLocationHref(location) helper added to produce safe href strings (origin stripping/normalization) and used throughout.
Framework link components
packages/react-router/src/link.tsx, packages/solid-router/src/link.tsx, packages/vue-router/src/link.tsx
Link hooks/components now extract href via .href from ParsedLocation.url (e.g., maskedLocation.url.href, nextLocation.url.href) instead of using the url field directly.
Tests
packages/react-router/tests/*.tsx, packages/solid-router/tests/*.tsx, e2e/*/custom-basepath/tests/navigation.spec.ts
Tests updated to expect URL objects or to access .href (e.g., assert on location.url.href); e2e navigation specs add explicit Location header checks for redirects.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to router.ts changes (parsing, redirects, rewrites) and correctness of getParsedLocationHref.
  • Verify all call sites and tests correctly treat ParsedLocation.url as URL (not string).
  • Confirm no remaining assumptions about location.url being a string (serialization, comparisons, logging).

Possibly related PRs

  • TanStack/router#5523 — similar change switching redirect target computation to use ParsedLocation.url as a URL object and adjusting origin handling.
  • TanStack/router#5315 — touches router-core href/origin normalization and URL vs string transitions in redirect/path logic.
  • TanStack/router#5330 — refactors redirect href construction and ParsedLocation handling from string hrefs to URL-based usage.

Suggested reviewers

  • birkskyum
  • nlynzaad
  • SeanCassiere

Poem

🐰 A hop, a sniff, a tiny cheer,

URLs now wear jackets clear,
.href I fetch with gentle paw,
Router paths align—oh law!
Hops of joy across the code so dear 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: converting from string href comparisons to URL object comparisons for determining server redirect logic.
✨ 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-rewrite-redirect

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

coderabbitai[bot] avatar Dec 11 '25 20:12 coderabbitai[bot]

View your CI Pipeline Execution ↗ for commit 6a00d7bb2537afb6dcc4057a71c17a7955a7f284

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 12m 14s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 42s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-12 19:20:19 UTC

nx-cloud[bot] avatar Dec 11 '25 20:12 nx-cloud[bot]

More templates

@tanstack/arktype-adapter

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router-devtools@6072
@tanstack/vue-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router-ssr-query@6072
@tanstack/vue-start

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

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start-client@6072
@tanstack/vue-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start-server@6072
@tanstack/zod-adapter

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

commit: 6a00d7b

pkg-pr-new[bot] avatar Dec 11 '25 20:12 pkg-pr-new[bot]