Skip to content

fix: revert rewrite fix #6211

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.

Sign up for GitHub

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

Merged
schiller-manuel merged 2 commits into from
Dec 24, 2025
Merged

fix: revert rewrite fix #6211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6331b1
fix: revert rewrite fix
schiller-manuel Dec 24, 2025
83df511
revert tests
schiller-manuel Dec 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions e2e/react-start/custom-basepath/tests/navigation.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ test('server-side redirect', async ({ page, baseURL }) => {
expect(page.url()).toBe(`${baseURL}/posts/1`)

// do not follow redirects since we want to test the Location header
// Both requests (with or without basepath) should redirect directly to the final destination.
// The router is smart enough to skip the intermediate "add basepath" redirect and go
// straight to where the route's beforeLoad redirect points to.
// first go to the route WITHOUT the base path, this will just add the base path
await page.request
.get('/redirect/throw-it', { maxRedirects: 0 })
.then((res) => {
const headers = new Headers(res.headers())
expect(headers.get('location')).toBe('/custom/basepath/posts/1')
expect(headers.get('location')).toBe('/custom/basepath/redirect/throw-it')
})
await page.request
.get('/custom/basepath/redirect/throw-it', { maxRedirects: 0 })
Expand Down
2 changes: 1 addition & 1 deletion e2e/react-start/i18n-paraglide/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ import handler from '@tanstack/react-start/server-entry'

export default {
fetch(req: Request): Promise<Response> {
return paraglideMiddleware(req, ({ request }) => handler.fetch(request))
return paraglideMiddleware(req, () => handler.fetch(req))
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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\(' -A2

Repository: 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=js

Repository: 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.json

Repository: 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.

Copy link
Contributor Author

@schiller-manuel schiller-manuel Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link

@mustafa-barakat mustafa-barakat Dec 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schiller-manuel Does this mean, the router had a bug prior to 1.142.1, that caused the router not to redirect?

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

},
}
6 changes: 2 additions & 4 deletions e2e/solid-start/custom-basepath/tests/navigation.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ test('server-side redirect', async ({ page, baseURL }) => {
expect(page.url()).toBe(`${baseURL}/posts/1`)

// do not follow redirects since we want to test the Location header
// Both requests (with or without basepath) should redirect directly to the final destination.
// The router is smart enough to skip the intermediate "add basepath" redirect and go
// straight to where the route's beforeLoad redirect points to.
// first go to the route WITHOUT the base path, this will just add the base path
await page.request
.get('/redirect/throw-it', { maxRedirects: 0 })
.then((res) => {
const headers = new Headers(res.headers())
expect(headers.get('location')).toBe('/custom/basepath/posts/1')
expect(headers.get('location')).toBe('/custom/basepath/redirect/throw-it')
})
await page.request
.get('/custom/basepath/redirect/throw-it', { maxRedirects: 0 })
Expand Down
6 changes: 2 additions & 4 deletions e2e/vue-start/custom-basepath/tests/navigation.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ test('server-side redirect', async ({ page, baseURL }) => {
expect(page.url()).toBe(`${baseURL}/posts/1`)

// do not follow redirects since we want to test the Location header
// Both requests (with or without basepath) should redirect directly to the final destination.
// The router is smart enough to skip the intermediate "add basepath" redirect and go
// straight to where the route's beforeLoad redirect points to.
// first go to the route WITHOUT the base path, this will just add the base path
await page.request
.get('/redirect/throw-it', { maxRedirects: 0 })
.then((res) => {
const headers = new Headers(res.headers())
expect(headers.get('location')).toBe('/custom/basepath/posts/1')
expect(headers.get('location')).toBe('/custom/basepath/redirect/throw-it')
})
await page.request
.get('/custom/basepath/redirect/throw-it', { maxRedirects: 0 })
Expand Down
2 changes: 1 addition & 1 deletion examples/react/i18n-paraglide/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# TanStack Router - i18n with Paraglide Example

This example shows how to use Paraglide with TanStack Router. The source code can be found [in the Paraglide monorepo](https://github.com/opral/monorepo/tree/main/inlang/packages/paraglide/paraglide-js/examples/tanstack-router).
This example shows how to use Paraglide with TanStack Router.

- [TanStack Router Docs](https://tanstack.com/router)
- [Paraglide Documentation](https://inlang.com/m/gerre34r/library-inlang-paraglideJs)
Expand Down
4 changes: 2 additions & 2 deletions examples/react/start-i18n-paraglide/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# TanStack Start example with Paraglide

This example shows how to use Paraglide with TanStack Start. The source code can be found [in the Paraglide monorepo](https://github.com/opral/monorepo/tree/main/inlang/packages/paraglide/paraglide-js/examples/tanstack-start).
This example shows how to use Paraglide with TanStack Start.

- [TanStack Router Docs](https://tanstack.com/router)
- [Paraglide Documentation](https://inlang.com/m/gerre34r/library-inlang-paraglideJs)
Expand Down Expand Up @@ -81,7 +81,7 @@ import { paraglideMiddleware } from './paraglide/server.js'
import handler from '@tanstack/react-start/server-entry'
export default {
fetch(req: Request): Promise<Response> {
return paraglideMiddleware(req, ({ request }) => handler.fetch(request))
return paraglideMiddleware(req, () => handler.fetch(req))
},
}
```
Expand Down
2 changes: 1 addition & 1 deletion examples/solid/i18n-paraglide/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# TanStack Router example

This example shows how to use Paraglide with TanStack Router. The source code can be found [here](https://github.com/opral/monorepo/tree/main/inlang/packages/paraglide/paraglide-js/examples/tanstack-router).
This example shows how to use Paraglide with TanStack Router.

## Getting started

Expand Down
4 changes: 2 additions & 2 deletions examples/solid/start-i18n-paraglide/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# TanStack Start example

This example shows how to use Paraglide with TanStack Start. The source code can be found [here](https://github.com/opral/monorepo/tree/main/inlang/packages/paraglide/paraglide-js/examples/tanstack-start).
This example shows how to use Paraglide with TanStack Start.

## Getting started

Expand Down Expand Up @@ -71,7 +71,7 @@ import handler from '@tanstack/solid-start/server-entry'

export default {
fetch(req: Request): Promise<Response> {
return paraglideMiddleware(req, ({ request }) => handler.fetch(request))
return paraglideMiddleware(req, () => handler.fetch(req))
},
}
```
Expand Down
49 changes: 0 additions & 49 deletions packages/react-router/tests/router.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3125,55 +3125,6 @@ describe('Router rewrite functionality', () => {
expect(history.location.pathname).toBe('/user')
})

it('should not cause redirect loops with i18n locale prefix rewriting', async () => {
// This test simulates an i18n middleware that:
// - Input: strips locale prefix (e.g., /en/home -> /home)
// - Output: adds locale prefix back (e.g., /home -> /en/home)

const rootRoute = createRootRoute({
component: () => <Outlet />,
})

const homeRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/home',
component: () => <div data-testid="home">Home</div>,
})

const routeTree = rootRoute.addChildren([homeRoute])

// The history starts at the public-facing locale-prefixed URL.
// The input rewrite strips the locale prefix for internal routing.
const history = createMemoryHistory({ initialEntries: ['/en/home'] })

const router = createRouter({
routeTree,
history,
rewrite: {
input: ({ url }) => {
// Strip locale prefix: /en/home -> /home
if (url.pathname.startsWith('/en')) {
url.pathname = url.pathname.replace(/^\/en/, '')
}
return url
},
},
})

render(<RouterProvider router={router} />)

await waitFor(() => {
expect(screen.getByTestId('home')).toBeInTheDocument()
})

// The internal pathname should be /home (after input rewrite strips /en)
expect(router.state.location.pathname).toBe('/home')

// The publicHref should include the locale prefix (via output rewrite)
// Since we only have input rewrite here, publicHref equals the internal href
expect(router.state.location.publicHref).toBe('/home')
})

it('should handle i18n rewriting with navigation between localized routes', async () => {
// Tests navigation between routes with i18n locale prefix rewriting

Expand Down
26 changes: 2 additions & 24 deletions packages/router-core/src/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1188,33 +1188,11 @@ export class RouterCore<

const fullPath = url.href.replace(url.origin, '')

// Save the internal pathname for route matching (before output rewrite)
const internalPathname = url.pathname

// Compute publicHref by applying the output rewrite.
//
// The publicHref represents the URL as it should appear in the browser.
// This must match what buildLocation computes for the same logical route,
// otherwise the server-side redirect check will see a mismatch and trigger
// an infinite redirect loop.
//
// We always apply the output rewrite (not conditionally) because the
// incoming URL may have already been transformed by external middleware
// before reaching the router. In that case, the input rewrite has nothing
// to do, but we still need the output rewrite to reconstruct the correct
// public-facing URL.
//
// Clone the URL to avoid mutating the one used for route matching.
const urlForOutput = new URL(url.href)
const rewrittenUrl = executeRewriteOutput(this.rewrite, urlForOutput)
const publicHref =
rewrittenUrl.pathname + rewrittenUrl.search + rewrittenUrl.hash

return {
href: fullPath,
publicHref,
publicHref: href,
url: url,
pathname: decodePath(internalPathname),
pathname: decodePath(url.pathname),
searchStr,
search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any,
hash: url.hash.split('#').reverse()[0] ?? '',
Expand Down
Loading