fix: revert rewrite fix #6211
fix: revert rewrite fix #6211
Conversation
📝 WalkthroughWalkthroughRemoved output-rewrite reconstruction from RouterCore.parseLocation so publicHref now mirrors the incoming href; simplified paraglide middleware callback usage in examples and READMEs; removed one i18n redirect-loop test; updated several example READMEs with paraglideVitePlugin and URL rewrite helpers. Changes
Sequence Diagram(s)(omitted — changes are not a new multi-component control-flow feature requiring a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-12-21T12:52:35.231ZApplied 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 (3)
Comment |
|
View your CI Pipeline Execution ↗ for commit 83df511
☁️ Nx Cloud last updated this comment at |
![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: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
e2e/react-start/i18n-paraglide/src/server.tsexamples/react/i18n-paraglide/README.mdexamples/react/start-i18n-paraglide/README.mdexamples/solid/i18n-paraglide/README.mdexamples/solid/start-i18n-paraglide/README.mdpackages/react-router/tests/router.test.tsxpackages/router-core/src/router.ts
💤 Files with no reviewable changes (1)
- packages/react-router/tests/router.test.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/router-core/src/router.tse2e/react-start/i18n-paraglide/src/server.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/router-core/src/router.tse2e/react-start/i18n-paraglide/src/server.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: schiller-manuel
Repo: TanStack/router PR: 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.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
examples/react/i18n-paraglide/README.mdexamples/solid/i18n-paraglide/README.md
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 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
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
packages/router-core/src/router.ts
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Applied to files:
packages/router-core/src/router.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 (13)
examples/react/i18n-paraglide/README.md (1)
3-3: LGTM! Documentation simplified.The introduction text has been streamlined while maintaining clarity.
examples/solid/i18n-paraglide/README.md (3)
3-3: LGTM! Documentation simplified.Introduction text streamlined consistently with other examples.
19-28: LGTM! Paraglide plugin configuration documented.The Vite plugin setup is clearly documented with the required configuration options.
44-51: LGTM! URL rewriting pattern documented.The rewrite configuration using
deLocalizeUrlandlocalizeUrlis consistent with the router changes and provides a clear example of the input/output rewrite pattern.examples/solid/start-i18n-paraglide/README.md (4)
3-3: LGTM! Documentation simplified.Introduction text streamlined.
19-39: LGTM! Comprehensive plugin configuration documented.The Vite plugin configuration is well-documented with all necessary options including outputStructure, cookieName, strategy, and urlPatterns.
55-63: LGTM! URL rewriting configuration documented.The rewrite setup is consistent with other examples and clearly shows the input/output pattern.
74-74: Server callback pattern updated - consistent with other examples.The simplified callback pattern matches the change in
e2e/react-start/i18n-paraglide/src/server.tsand is consistent across all documentation.examples/react/start-i18n-paraglide/README.md (4)
3-3: LGTM! Documentation simplified.Introduction text streamlined consistently.
30-50: LGTM! Comprehensive plugin configuration documented.The Vite plugin configuration includes all necessary options for Paraglide integration with React Start.
66-74: LGTM! URL rewriting configuration documented.The rewrite pattern is correctly documented and consistent with the router changes.
84-84: Server callback pattern updated - consistent with other examples.The simplified callback pattern is consistent across all framework examples.
packages/router-core/src/router.ts (1)
1193-1195: The implementation is correct and aligns with the stated goal of reverting PR #6201 with the fix from PR #6172:
i18n/locale routing patterns: ✓ Confirmed working. The test "should maintain consistent publicHref between parseLocation and buildLocation" explicitly validates this with input rewrite (
/fr→/) and output rewrite (/→/fr) as inverses, resulting in consistentpublicHref: '/fr'from both methods.publicHref as user-facing URL: ✓ Confirmed intentional. In
parseLocation,publicHrefpreserves the raw browser URL without output rewrite application, whilepathnameuses the input-rewritten value for routing. InbuildLocation,publicHrefapplies output rewrite to match what the browser will display after navigation. Both methods compute to the same user-facing URL when input/output rewrites are inverses.Downstream consumers: ✓ Confirmed working. The
Transitionercomponents rely onpublicHreffor canonical URL comparison to detect and apply rewrites—the pattern works correctly as designed.
| export default { | ||
| fetch(req: Request): Promise<Response> { | ||
| return paraglideMiddleware(req, ({ request }) => handler.fetch(request)) | ||
| return paraglideMiddleware(req, () => handler.fetch(req)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for paraglideMiddleware implementation to verify callback signature
echo "=== Searching for paraglideMiddleware definition ==="
rg -n --type=ts -A10 'function paraglideMiddleware|const paraglideMiddleware|export.*paraglideMiddleware'
echo ""
echo "=== Searching for other paraglideMiddleware usage patterns ==="
rg -n --type=ts 'paraglideMiddleware\(' -A2Repository: TanStack/router
Length of output: 871
🏁 Script executed:
#!/bin/bash
# Check package.json files for paraglideMiddleware dependency
echo "=== Checking for paraglideMiddleware in package.json files ==="
find . -name "package.json" -type f | head -10 | xargs grep -l "paraglide" 2>/dev/null
echo ""
echo "=== Checking e2e and examples directory package.json ==="
cat e2e/react-start/i18n-paraglide/package.json 2>/dev/null | grep -A2 -B2 "paraglide"
echo ""
echo "=== Comparing full context of both usage patterns ==="
echo "--- File under review (e2e) ---"
head -15 e2e/react-start/i18n-paraglide/src/server.ts
echo ""
echo "--- Examples file for comparison ---"
head -15 examples/react/start-i18n-paraglide/src/server.ts 2>/dev/null || echo "File not found"Repository: TanStack/router
Length of output: 1051
🏁 Script executed:
#!/bin/bash
# Search for local paraglide/server.js files
echo "=== Finding paraglide/server.js files ==="
find . -path "*/paraglide/server.js" -o -path "*/paraglide/server.ts"
echo ""
echo "=== Content of e2e paraglide/server.js ==="
cat e2e/react-start/i18n-paraglide/src/paraglide/server.js
echo ""
echo "=== Content of examples paraglide/server.js ==="
cat examples/react/start-i18n-paraglide/src/paraglide/server.js 2>/dev/null || echo "File not found"Repository: TanStack/router
Length of output: 312
🏁 Script executed:
#!/bin/bash
# Explore paraglide directory structure
echo "=== Listing e2e paraglide directory ==="
find e2e/react-start/i18n-paraglide/src/paraglide -type f 2>/dev/null || echo "Directory not found"
echo ""
echo "=== Listing examples paraglide directory ==="
find examples/react/start-i18n-paraglide/src/paraglide -type f 2>/dev/null || echo "Directory not found"
echo ""
echo "=== Searching for all files in directories containing 'paraglide' ==="
find . -type d -name "*paraglide*" | head -5
echo ""
echo "=== Check if server.js is actually referenced or if it's generated ==="
rg -n "paraglide/server" --type=ts --type=jsRepository: TanStack/router
Length of output: 827
🏁 Script executed:
#!/bin/bash
# List all files in the paraglide directories
echo "=== Files in e2e/react-start/i18n-paraglide ==="
find e2e/react-start/i18n-paraglide -type f | head -20
echo ""
echo "=== Files in examples/react/start-i18n-paraglide ==="
find examples/react/start-i18n-paraglide -type f | head -20
echo ""
echo "=== Check for README or documentation ==="
find e2e/react-start/i18n-paraglide -name "README*" -o -name "*.md"
echo ""
echo "=== Check for build/generation scripts ==="
rg -n "paraglide" e2e/react-start/i18n-paraglide/package.json examples/react/start-i18n-paraglide/package.jsonRepository: TanStack/router
Length of output: 2732
🌐 Web query:
@inlang/paraglide-js paraglideMiddleware callback signature documentation
💡 Result:
Signature (from docs):
paraglideMiddleware(request, resolve, callbacks?): Promise
- resolve: (args) => T | Promise — called with an args object (examples show { request, locale }). [1]
- callbacks?: { onRedirect?: (response) => void } — optional callback invoked on redirects. [1]
Usage examples show resolve being used as: serverMiddleware(event.request, ({ request, locale }) => resolve(request)). [2]
Sources:
[1] Paraglide JS — server (paraglideMiddleware) documentation.
[2] Paraglide JS — server-side rendering / usage examples.
Callback pattern is incorrect and bypasses middleware request processing.
The callback ignores the middleware's processed request object. According to @inlang/paraglide-js documentation, paraglideMiddleware calls the callback with { request, locale } where request is the processed request. The code currently uses () => handler.fetch(req), ignoring this parameter entirely.
Other similar files in the repository correctly destructure the callback: ({ request }) => handler.fetch(request). Change to match this pattern to ensure the middleware's request processing is applied.
🤖 Prompt for AI Agents
In e2e/react-start/i18n-paraglide/src/server.ts around line 6, the callback
passed to paraglideMiddleware ignores the processed request and always forwards
the original req; change the callback to accept the middleware result and
forward its request (i.e., destructure { request } and call
handler.fetch(request)) so the middleware-processed request and locale are used.
There was a problem hiding this comment.
we specifically don't want the processed request as this will modify the url (delocalizes it). we need the untouched url so that router can correctly perform rewrites itself
There was a problem hiding this comment.
@schiller-manuel Thank you for the clarification! That makes perfect sense. The router needs the original, unmodified URL to correctly apply its own rewrite logic (using deLocalizeUrl/localizeUrl), so bypassing paraglideMiddleware's URL processing is intentional. I apologize for the incorrect flag.
✏️ Learnings added
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6211
File: e2e/react-start/i18n-paraglide/src/server.ts:6-6
Timestamp: 2025-12-24T22:47:40.621Z
Learning: In TanStack Router projects using `inlang/paraglide-js`, the callback passed to `paraglideMiddleware` should use `() => handler.fetch(req)` (referencing the outer `req`) instead of `({ request }) => handler.fetch(request)`. This is intentional because the router needs the untouched URL to perform its own rewrite logic with `deLocalizeUrl`/`localizeUrl`. The middleware's processed request would delocalize the URL and interfere with the router's rewrite handling.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: schiller-manuel
Repo: TanStack/router PR: 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.
There was a problem hiding this comment.
@schiller-manuel Does this mean, the router had a bug prior to 1.142.1, that caused the router not to redirect?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
this reverts #6201
the correct fix from #6172 was applied instead.
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.