fix: rewrite causes infinite redirects #6201
Conversation
WalkthroughAdds two Paraglide i18n example apps (React Router and React Start) and router-core/transitioner changes that introduce a browser-visible Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Router
participant LocationEngine as Location Engine
participant Rewrite as Rewrite Pipeline
participant Transitioner
Browser->>Router: Navigate to /de/about
Router->>LocationEngine: parseLocation(url=/de/about)
LocationEngine->>Rewrite: input rewrite (deLocalize)
Rewrite-->>LocationEngine: /about (internal)
LocationEngine->>LocationEngine: match internal route (/about)
LocationEngine->>Rewrite: output rewrite (localize)
Rewrite-->>LocationEngine: /de/about (public)
LocationEngine-->>Router: return { href: "/about", publicHref: "/de/about", pathname: "/about" }
Router->>Transitioner: commitLocation(nextLocation)
Note over Transitioner: Compare publicHref values
Transitioner->>Transitioner: latestLocation.publicHref == nextLocation.publicHref ?
alt match
Transitioner-->>Router: no replace/redirect
else
Transitioner->>Browser: update URL (replace=true)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 (5)📓 Common learnings📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-12-17T02:17:55.086ZApplied 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 231c544
☁️ 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: 2
🧹 Nitpick comments (14)
e2e/react-start/i18n-paraglide/src/utils/seo.ts (2)
12-30: Consider filtering tags with undefined content values.When optional parameters (
description,keywords) are not provided, the corresponding meta tags will havecontent: undefined. Depending on how these tag descriptors are rendered, this could result in invalid HTML (e.g.,<meta name="description" content="undefined">) or rendering errors.🔎 Proposed fix
Filter out tags with undefined content before returning:
const tags = [ { title }, { name: 'description', content: description }, { name: 'keywords', content: keywords }, { name: 'twitter:title', content: title }, { name: 'twitter:description', content: description }, { name: 'twitter:creator', content: '@tannerlinsley' }, { name: 'twitter:site', content: '@tannerlinsley' }, { name: 'og:type', content: 'website' }, { name: 'og:title', content: title }, { name: 'og:description', content: description }, ...(image ? [ { name: 'twitter:image', content: image }, { name: 'twitter:card', content: 'summary_large_image' }, { name: 'og:image', content: image }, ] : []), - ] + ].filter((tag) => { + if ('title' in tag) return true + return tag.content !== undefined + }) return tags
18-19: Consider parameterizing the Twitter handle.The Twitter creator and site are hardcoded to
@tannerlinsley. While this may be acceptable for an example/test project, consider making this configurable if this utility is intended to be reusable across different projects or contexts.🔎 Optional enhancement
export const seo = ({ title, description, keywords, image, + twitterHandle = '@tannerlinsley', }: { title: string description?: string image?: string keywords?: string + twitterHandle?: string }) => { const tags = [ { title }, { name: 'description', content: description }, { name: 'keywords', content: keywords }, { name: 'twitter:title', content: title }, { name: 'twitter:description', content: description }, - { name: 'twitter:creator', content: '@tannerlinsley' }, - { name: 'twitter:site', content: '@tannerlinsley' }, + { name: 'twitter:creator', content: twitterHandle }, + { name: 'twitter:site', content: twitterHandle },e2e/react-start/i18n-paraglide/tests/navigation.spec.ts (1)
82-101: Good test coverage for the redirect loop fix.The test correctly verifies that a single redirect doesn't cause a loop. One minor observation: Line 96 asserts
followUp.status() < 400, which would allow a 3xx redirect status. If the goal is to ensure no further redirects occur, consider asserting< 300or explicitly checking for 200.🔎 Optional: Stricter assertion for no further redirects
// Following the redirect should not cause another redirect loop const followUp = await page.request.get(location!, { maxRedirects: 0 }) // The follow-up should be a 200 or another valid response, not another redirect to the same location - expect(followUp.status()).toBeLessThan(400) + expect(followUp.status()).toBe(200)e2e/react-start/i18n-paraglide/src/routes/index.tsx (1)
21-29: Component correctly demonstrates loader data usage.The third
<h2>at Line 27 re-invokesm.example_message()on the client side. If this duplication is intentional to demonstrate client-side i18n, consider adding a brief comment to clarify the purpose for future readers.e2e/react-router/i18n-paraglide/tests/navigation.spec.ts (1)
197-218: Consider strengthening the back navigation assertion.The comment on line 216 acknowledges uncertainty about the expected state after
goBack(). While the current loose assertion is acceptable for verifying no redirect loops occur, consider capturing the URL before the locale switch to make the expected back navigation target explicit.Suggested improvement
// Navigate to about await page.getByTestId('about-link').click() await page.waitForLoadState('networkidle') + const aboutUrlBeforeLocaleSwitch = page.url() // Switch to German await page.getByTestId('locale-de').click() await page.waitForLoadState('networkidle') expect(page.url()).toContain('/de/ueber') // Go back await page.goBack() await page.waitForLoadState('networkidle') - // Should be on about page (English or previous state) + // Should return to about page before locale switch await expect(page.getByTestId('about-content')).toBeVisible() + expect(page.url()).toBe(aboutUrlBeforeLocaleSwitch)packages/solid-router/src/Transitioner.tsx (1)
58-63: Aligns Solid Transitioner with public-facing canonical URL semanticsSwitching the mount-time equality check to compare
trimPathRight(latestLocation.publicHref)vstrimPathRight(nextLocation.publicHref)is correct and matches the server-sidebeforeLoadredirect check. This keeps internalpathname/hrefdecoupled from the browser-visible URL, preventing rewrite-based redirect loops while still normalizing trailing slashes viatrimPathRight.packages/vue-router/src/Transitioner.tsx (1)
126-131: Vue adapter correctly switched topublicHreffor canonical URL checkUsing
trimPathRight(router.latestLocation.publicHref)vstrimPathRight(nextLocation.publicHref)here matches the new router-core contract and the React/Solid adapters. This ensures the Vue transitioner respects the browser-facing canonical URL while avoiding unnecessary commits when only internalhrefdiffers.packages/router-core/src/router.ts (5)
1191-1218: Clean separation between internal matching path and public-facing URL inparseLocationCapturing
internalPathnamebefore applying the output rewrite and then:
- Using
decodePath(internalPathname)forpathname(routing/matching), and- Deriving
publicHreffromexecuteRewriteOutputon a cloned URL,is the right split. It guarantees:
- Route matching only depends on input rewrites and basepath handling.
publicHrefalways reflects the final output-rewritten browser URL, in sync withbuildLocation.This is exactly what’s needed to make
parseLocationandbuildLocationagree onpublicHrefand avoid redirect loops under i18n-style rewrites.
1793-1807:buildLocationnow returns a stablepublicHrefwhile keeping internalhrefThe new return shape:
- Applies
executeRewriteOutputto the URL.- Sets:
publicHreffrom the rewritten URL (what should go into history / be compared on the client & server).hreffrom the pre-outputfullPath(internal router href).urlto the rewritten URL (used downstream e.g. in redirects).This matches
parseLocation’spublicHrefsemantics and gives a single source of truth for the browser-visible URL, which is what the Transitioners and serverbeforeLoadnow rely on.
1876-1963: History writes correctly switched topublicHref, while dedup still uses internalhrefWithin
commitLocation:
- URL dedup still checks
isSameUrlvia trimmed internalhref, which keeps the “don’t push duplicates” behavior tied to router-internal paths.- The actual history write now uses
nextHistory.publicHrefinstead ofnextHistory.href, so the browser URL always reflects the output rewrite (and any basepath) rather than the internal routing path.This combination is sensible: it avoids spurious history entries while ensuring the user sees the canonical URL. Just be aware of the deliberate distinction: internal equality uses
href, external navigation usespublicHref.
2084-2105: ServerbeforeLoadredirect check now aligned withpublicHrefand originThe updated server-side check:
if ( this.latestLocation.publicHref !== nextLocation.publicHref || nextLocation.url.origin !== this.origin ) { const href = this.getParsedLocationHref(nextLocation) throw redirect({ href }) }is a good match for the new model:
- It compares
publicHref(not the internalhref), so input/output rewrites that only affect the browser URL no longer cause false mismatches.- The separate origin comparison ensures cross-origin rewrites still produce a proper HTTP redirect with a full absolute Location.
This is the critical fix that prevents infinite redirect loops under i18n output rewrites.
2425-2443:getParsedLocationHref/resolveRedirectpreserve same-origin path-only Location headersUsing
getParsedLocationHref(location)to derive the HTTPLocationheader insideresolveRedirect:
- Strips
this.originfromlocation.url.hrefwhen origins match, yielding/path?...for same-origin redirects.- Leaves absolute URLs intact when
location.url.origin !== this.origin, so cross-origin redirects remain fully qualified.This keeps the intentional behavior described in the learnings (same-origin → path-only, cross-origin → absolute) while integrating with the new
publicHref/rewrite pipeline. The change to avoid usingpublicHrefdirectly here is correct.packages/react-router/src/Transitioner.tsx (1)
55-61: React Transitioner correctly moved canonical check topublicHrefComparing
trimPathRight(router.latestLocation.publicHref)againsttrimPathRight(nextLocation.publicHref)during mount ensures:
- The SPA normalizes the browser URL to the same canonical public URL the server uses.
- Internal rewrites that only affect routing no longer trigger unnecessary client-side replaces or loops.
This is consistent with router-core’s new
publicHrefcontract and the other framework adapters.packages/react-router/tests/router.test.tsx (1)
3128-3367: New i18n/publicHref tests exercise the right invariants for rewrite behaviorThe added tests around locale-prefix rewrites:
- Cover input-only i18n rewrites, navigation between localized routes, and direct entry on localized URLs.
- Assert both internal
pathname(de-localized) andpublicHref/history pathname (localized), matching the newpublicHrefcontract.- Explicitly verify that
parseLocationandbuildLocationcompute the samepublicHreffor the same logical route, which is the key invariant to prevent redirect loops.This is a solid regression suite for the reported issue; any future change that reintroduces a mismatch between internal and public URLs should now be caught here.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
e2e/react-start/i18n-paraglide/public/favicon.icois excluded by!**/*.icoe2e/react-start/i18n-paraglide/public/logo192.pngis excluded by!**/*.pnge2e/react-start/i18n-paraglide/public/logo512.pngis excluded by!**/*.pnge2e/react-start/i18n-paraglide/src/logo.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
e2e/react-router/i18n-paraglide/.gitignoree2e/react-router/i18n-paraglide/index.htmle2e/react-router/i18n-paraglide/messages/de.jsone2e/react-router/i18n-paraglide/messages/en.jsone2e/react-router/i18n-paraglide/package.jsone2e/react-router/i18n-paraglide/playwright.config.tse2e/react-router/i18n-paraglide/project.inlang/.gitignoree2e/react-router/i18n-paraglide/project.inlang/project_ide2e/react-router/i18n-paraglide/project.inlang/settings.jsone2e/react-router/i18n-paraglide/src/main.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/routes/about.tsxe2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-router/i18n-paraglide/src/styles.csse2e/react-router/i18n-paraglide/tests/navigation.spec.tse2e/react-router/i18n-paraglide/tsconfig.jsone2e/react-router/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/.gitignoree2e/react-start/i18n-paraglide/.prettierignoree2e/react-start/i18n-paraglide/.vscode/extensions.jsone2e/react-start/i18n-paraglide/.vscode/settings.jsone2e/react-start/i18n-paraglide/messages/de.jsone2e/react-start/i18n-paraglide/messages/en.jsone2e/react-start/i18n-paraglide/package.jsone2e/react-start/i18n-paraglide/playwright.config.tse2e/react-start/i18n-paraglide/project.inlang/.gitignoree2e/react-start/i18n-paraglide/project.inlang/project_ide2e/react-start/i18n-paraglide/project.inlang/settings.jsone2e/react-start/i18n-paraglide/public/manifest.jsone2e/react-start/i18n-paraglide/public/robots.txte2e/react-start/i18n-paraglide/src/routeTree.gen.tse2e/react-start/i18n-paraglide/src/router.tsxe2e/react-start/i18n-paraglide/src/routes/__root.tsxe2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/src/server.tse2e/react-start/i18n-paraglide/src/styles.csse2e/react-start/i18n-paraglide/src/utils/prerender.tse2e/react-start/i18n-paraglide/src/utils/seo.tse2e/react-start/i18n-paraglide/src/utils/translated-pathnames.tse2e/react-start/i18n-paraglide/tests/navigation.spec.tse2e/react-start/i18n-paraglide/tsconfig.jsone2e/react-start/i18n-paraglide/vite.config.tspackages/react-router/src/Transitioner.tsxpackages/react-router/tests/router.test.tsxpackages/router-core/src/router.tspackages/solid-router/src/Transitioner.tsxpackages/vue-router/src/Transitioner.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.tse2e/react-router/i18n-paraglide/src/routes/about.tsxpackages/react-router/src/Transitioner.tsxe2e/react-start/i18n-paraglide/src/server.tse2e/react-start/i18n-paraglide/src/utils/prerender.tspackages/vue-router/src/Transitioner.tsxe2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/src/routes/index.tsxe2e/react-router/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/src/router.tsxe2e/react-router/i18n-paraglide/tests/navigation.spec.tspackages/solid-router/src/Transitioner.tsxe2e/react-start/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/main.tsxe2e/react-router/i18n-paraglide/playwright.config.tspackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/utils/seo.tse2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tspackages/router-core/src/router.tse2e/react-start/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/playwright.config.tse2e/react-start/i18n-paraglide/tests/navigation.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.tse2e/react-router/i18n-paraglide/src/routes/about.tsxpackages/react-router/src/Transitioner.tsxe2e/react-start/i18n-paraglide/src/server.tse2e/react-start/i18n-paraglide/src/utils/prerender.tspackages/vue-router/src/Transitioner.tsxe2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/src/routes/index.tsxe2e/react-router/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/src/router.tsxe2e/react-router/i18n-paraglide/tests/navigation.spec.tspackages/solid-router/src/Transitioner.tsxe2e/react-start/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/main.tsxe2e/react-router/i18n-paraglide/playwright.config.tspackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/utils/seo.tse2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tspackages/router-core/src/router.tse2e/react-start/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/playwright.config.tse2e/react-start/i18n-paraglide/tests/navigation.spec.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/react-router/i18n-paraglide/package.jsone2e/react-start/i18n-paraglide/package.json
🧠 Learnings (13)
📓 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.
📚 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:
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.tse2e/react-router/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/utils/prerender.tse2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/src/router.tsxe2e/react-start/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/src/main.tsxpackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tspackages/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:
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.tspackages/vue-router/src/Transitioner.tsxpackages/solid-router/src/Transitioner.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxpackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tspackages/router-core/src/router.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/i18n-paraglide/.vscode/settings.jsone2e/react-router/i18n-paraglide/src/routes/about.tsxe2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/.prettierignoree2e/react-router/i18n-paraglide/.gitignoree2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tse2e/react-start/i18n-paraglide/.gitignore
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/react-start/i18n-paraglide/.vscode/settings.jsone2e/react-start/i18n-paraglide/.prettierignoree2e/react-start/i18n-paraglide/src/router.tsxe2e/react-router/i18n-paraglide/tests/navigation.spec.tse2e/react-router/i18n-paraglide/package.jsone2e/react-router/i18n-paraglide/.gitignorepackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.tse2e/react-start/i18n-paraglide/tests/navigation.spec.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/react-router/i18n-paraglide/src/routes/about.tsxe2e/react-router/i18n-paraglide/src/routes/index.tsxe2e/react-start/i18n-paraglide/.prettierignoree2e/react-start/i18n-paraglide/src/routes/index.tsxe2e/react-router/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/src/router.tsxe2e/react-start/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/tsconfig.jsone2e/react-router/i18n-paraglide/src/routes/__root.tsxe2e/react-router/i18n-paraglide/index.htmle2e/react-router/i18n-paraglide/package.jsone2e/react-router/i18n-paraglide/src/main.tsxe2e/react-router/i18n-paraglide/.gitignorepackages/react-router/tests/router.test.tsxe2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.ts
📚 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/react-router/src/Transitioner.tsxpackages/vue-router/src/Transitioner.tsxe2e/react-router/i18n-paraglide/tests/navigation.spec.tspackages/solid-router/src/Transitioner.tsxe2e/react-router/i18n-paraglide/src/routes/__root.tsxpackages/react-router/tests/router.test.tsxpackages/router-core/src/router.tse2e/react-start/i18n-paraglide/tests/navigation.spec.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
e2e/react-start/i18n-paraglide/.prettierignoree2e/react-router/i18n-paraglide/vite.config.tse2e/react-start/i18n-paraglide/tsconfig.jsone2e/react-router/i18n-paraglide/tsconfig.jsone2e/react-router/i18n-paraglide/package.jsone2e/react-router/i18n-paraglide/src/main.tsxe2e/react-start/i18n-paraglide/package.jsone2e/react-start/i18n-paraglide/src/routeTree.gen.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript strict mode with extensive type safety for all code
Applied to files:
e2e/react-start/i18n-paraglide/tsconfig.jsone2e/react-router/i18n-paraglide/tsconfig.json
📚 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:
e2e/react-router/i18n-paraglide/index.html
📚 Learning: 2025-10-09T12:59:14.842Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/public/site.webmanifest:2-3
Timestamp: 2025-10-09T12:59:14.842Z
Learning: In e2e test fixtures (files under e2e directories), empty or placeholder values in configuration files like site.webmanifest are acceptable and should not be flagged unless the test specifically validates those fields.
Applied to files:
e2e/react-router/i18n-paraglide/playwright.config.tse2e/react-start/i18n-paraglide/playwright.config.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/react-start/i18n-paraglide/src/routes/about.tsxe2e/react-start/i18n-paraglide/src/routeTree.gen.ts
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/react-start/i18n-paraglide/tests/navigation.spec.ts
🧬 Code graph analysis (13)
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.ts (1)
e2e/react-start/i18n-paraglide/src/routeTree.gen.ts (1)
FileRoutesByTo(30-33)
e2e/react-router/i18n-paraglide/src/routes/about.tsx (2)
e2e/react-router/i18n-paraglide/src/routes/__root.tsx (1)
Route(10-68)e2e/react-router/i18n-paraglide/src/routes/index.tsx (1)
Route(4-6)
e2e/react-start/i18n-paraglide/src/utils/prerender.ts (1)
packages/router-core/src/route.ts (1)
path(1553-1555)
packages/vue-router/src/Transitioner.tsx (1)
packages/vue-router/src/index.tsx (1)
trimPathRight(8-8)
e2e/react-router/i18n-paraglide/src/routes/index.tsx (2)
e2e/react-router/i18n-paraglide/src/routes/__root.tsx (1)
Route(10-68)e2e/react-router/i18n-paraglide/src/routes/about.tsx (1)
Route(4-6)
e2e/react-start/i18n-paraglide/src/routes/index.tsx (2)
e2e/react-start/i18n-paraglide/src/routes/__root.tsx (1)
Route(12-30)e2e/react-start/i18n-paraglide/src/routes/about.tsx (1)
Route(4-6)
e2e/react-start/i18n-paraglide/src/routes/__root.tsx (2)
e2e/react-start/i18n-paraglide/src/routes/about.tsx (1)
Route(4-6)e2e/react-start/i18n-paraglide/src/routes/index.tsx (1)
Route(11-19)
e2e/react-router/i18n-paraglide/src/routes/__root.tsx (2)
e2e/react-router/i18n-paraglide/src/routes/about.tsx (1)
Route(4-6)e2e/react-router/i18n-paraglide/src/routes/index.tsx (1)
Route(4-6)
packages/react-router/tests/router.test.tsx (2)
packages/react-router/src/index.tsx (7)
createRootRoute(254-254)Outlet(239-239)createRoute(251-251)createMemoryHistory(114-114)createRouter(269-269)RouterProvider(279-279)Link(144-144)packages/react-router/src/Match.tsx (1)
Outlet(316-366)
e2e/react-start/i18n-paraglide/src/utils/seo.ts (1)
scripts/llms-generate.mjs (1)
title(101-101)
e2e/react-start/i18n-paraglide/src/routes/about.tsx (2)
e2e/react-start/i18n-paraglide/src/routes/__root.tsx (1)
Route(12-30)e2e/react-start/i18n-paraglide/src/routes/index.tsx (1)
Route(11-19)
e2e/react-start/i18n-paraglide/src/routeTree.gen.ts (1)
e2e/react-start/i18n-paraglide/src/router.tsx (1)
getRouter(8-18)
packages/router-core/src/router.ts (2)
packages/router-core/src/rewrite.ts (1)
executeRewriteOutput(80-93)packages/router-core/src/utils.ts (1)
decodePath(507-520)
⏰ 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 (48)
e2e/react-start/i18n-paraglide/.vscode/extensions.json (1)
1-3: LGTM!The VSCode extensions recommendation is well-placed. Suggesting the inlang extension improves the developer experience for contributors working on this i18n example.
e2e/react-start/i18n-paraglide/project.inlang/.gitignore (1)
1-1: LGTM!Ignoring the cache directory is standard practice for project configuration. This prevents build artifacts and temporary files from being committed.
e2e/react-router/i18n-paraglide/project.inlang/.gitignore (1)
1-1: LGTM!The single-line gitignore rule is appropriate for the inlang project subdirectory context.
e2e/react-start/i18n-paraglide/.gitignore (1)
1-10: LGTM!The patterns appropriately cover common React Start artifacts (.nitro, .tanstack, .output) and standard development ignores.
Verify that the React Start example does not need explicit rules for
src/routeTree.gen.tsandsrc/paraglide(which the React Router sibling example does ignore). If these files are generated in a different location or handled by dist-ssr, no action is needed.e2e/react-router/i18n-paraglide/.gitignore (1)
1-7: LGTM!The patterns correctly ignore autogenerated files (
src/routeTree.gen.ts,src/paraglide) and build artifacts. Port and test-result files are appropriately excluded.e2e/react-start/i18n-paraglide/public/robots.txt (1)
1-3: LGTM! Standard robots.txt for e2e test example.The permissive configuration is appropriate for test infrastructure.
e2e/react-start/i18n-paraglide/public/manifest.json (1)
1-25: LGTM! Standard PWA manifest for e2e test example.The manifest configuration is appropriate for the test infrastructure supporting the i18n-paraglide example.
e2e/react-router/i18n-paraglide/project.inlang/project_id (1)
1-1: LGTM! Standard Inlang project identifier.This file correctly contains the Inlang project identifier for the e2e test infrastructure.
e2e/react-start/i18n-paraglide/.prettierignore (1)
1-4: LGTM! Appropriate ignore patterns.The ignore patterns correctly exclude build artifacts, lock files, and autogenerated files (routeTree.gen.ts) from Prettier formatting.
e2e/react-start/i18n-paraglide/project.inlang/project_id (1)
1-1: LGTM! Standard Inlang project identifier.This file correctly contains the Inlang project identifier for the React Start e2e test infrastructure.
e2e/react-start/i18n-paraglide/src/styles.css (1)
1-1: LGTM! Correct Tailwind CSS v4 import syntax.The single
@import 'tailwindcss';statement is the proper way to import Tailwind CSS in v4, replacing the older@tailwinddirectives.e2e/react-start/i18n-paraglide/project.inlang/settings.json (1)
1-12: LGTM! Properly configured Inlang project settings.The configuration correctly sets up English and German locales with appropriate plugins and message paths for the e2e test infrastructure.
e2e/react-router/i18n-paraglide/messages/en.json (1)
1-7: LGTM! Properly structured translation messages.The English translation file follows the Inlang message format schema correctly, with appropriate placeholder syntax for dynamic content.
e2e/react-start/i18n-paraglide/src/server.ts (1)
1-8: LGTM! Correct middleware integration pattern.The server entry correctly wraps the React Start handler with Paraglide i18n middleware, enabling locale detection and URL rewriting for the e2e test infrastructure. The type signatures are explicit and appropriate.
e2e/react-start/i18n-paraglide/src/utils/prerender.ts (1)
3-8: The current prerender configuration correctly includes all locale variants. The router's automatic link discovery (enabled viacrawlLinksby default) handles locale expansion seamlessly: when prerendered pages are fetched, the router's output rewrite generates href attributes with locale prefixes (e.g.,/en/about,/de/ueber), which the crawler automatically discovers and prerendered. This is verified by the comprehensive Playwright tests that confirm access to all locale variants (/en,/de,/en/about,/de/ueber).e2e/react-start/i18n-paraglide/tsconfig.json (1)
1-29: LGTM! Well-configured TypeScript setup.The configuration follows best practices with strict mode enabled, modern ES2022 target, and appropriate bundler settings. The path alias setup aligns with the project structure.
e2e/react-start/i18n-paraglide/src/utils/translated-pathnames.ts (2)
47-56: LGTM! Route mappings are clear and consistent.The exported
translatedPathnamescorrectly maps English and German localized paths for the root and about routes.
18-30: The regex patterns are not currently used by any routes in this file.The
toUrlPatternfunction defines patterns for catch-all (/$$), optional parameters ({-$param}), and named parameters ($param), but the project's actual routes (/and/about) are simple static paths. These regex transformations are not exercised by the currenttranslatedPathnamesexport. If dynamic or parameterized routes are added in the future, these patterns should be tested against actual route definitions.e2e/react-router/i18n-paraglide/src/styles.css (1)
1-1: LGTM! Standard Tailwind CSS v4 import.This follows the new Tailwind v4 import syntax as documented in the library context.
e2e/react-router/i18n-paraglide/index.html (1)
1-18: LGTM! Standard HTML entry point.The HTML structure is appropriate for a React application with correct meta tags and script loading.
e2e/react-start/i18n-paraglide/.vscode/settings.json (1)
1-11: LGTM! Appropriate VS Code settings for generated files.These settings correctly exclude
routeTree.gen.tsfrom watching, searching, and mark it as read-only, which aligns with best practices for auto-generated files.e2e/react-start/i18n-paraglide/vite.config.ts (1)
8-47: Plugin order and URL patterns are correct.The
paraglideVitePluginruns first as required to initialize the localization structure before TanStack Start processes the routes. The URL patterns correctly map the defined routes (/and/about) with locale prefixes (/en,/de), and the catch-all pattern/:path(.*)?appropriately handles dynamic or unmatched routes.e2e/react-start/i18n-paraglide/playwright.config.ts (1)
22-27: Configuration follows established patterns across the codebase.The
webServer.commandsetup is consistent with other e2e tests in the repository (e.g.,e2e/vue-start/projects). Bothpnpm buildandpnpm startscripts exist in package.json, and the environment variablesVITE_SERVER_PORTandPORTare properly set before command execution. The srvx static server respects thePORTvariable across the codebase, and the command syntax is standard for all target platforms. No changes needed.e2e/react-start/i18n-paraglide/messages/en.json (1)
1-8: LGTM! Well-structured i18n message file.The message format follows the Inlang schema correctly with appropriate parameterized messages. All message keys (
example_message,server_message,about_message,home_page,about_page) are properly used throughout the codebase.e2e/react-router/i18n-paraglide/messages/de.json (1)
1-7: LGTM! Valid i18n resource file.The German translation file is properly structured with correct schema reference and translation keys that align with the English counterpart.
e2e/react-start/i18n-paraglide/messages/de.json (1)
1-8: LGTM! Valid i18n resource file.The German translation file is properly structured with correct schema reference and includes appropriate keys for the React Start SSR context.
e2e/react-router/i18n-paraglide/tsconfig.json (1)
14-16: LGTM! Path alias configuration is correct.The
@/*alias mapping to./src/*follows common conventions and will work correctly with the project structure.e2e/react-router/i18n-paraglide/project.inlang/settings.json (1)
1-12: LGTM! Valid Inlang configuration.The settings file correctly defines the base locale, supported locales, and message path pattern that aligns with the actual file structure (
./messages/en.json,./messages/de.json).e2e/react-router/i18n-paraglide/src/routes/index.tsx (1)
1-18: LGTM! Route implementation follows TanStack Router patterns correctly.The file-based route is properly configured with
createFileRoute('/'), the component correctly renders the localized message with parameters, and includes appropriate test attributes for e2e testing.e2e/react-start/i18n-paraglide/src/routes/about.tsx (1)
1-10: LGTM! Clean about route implementation.The file-based route is properly configured with
createFileRoute('/about')and the component correctly renders the localized message. The implementation is clean and appropriate for e2e testing.e2e/react-start/i18n-paraglide/package.json (2)
14-16: LGTM! Internal dependencies correctly use workspace protocol.The TanStack packages correctly use
workspace:^for internal dependencies, which aligns with the coding guidelines.
17-18: React 19.0.0 is officially supported by TanStack Router and no compatibility issues are documented. The project is testing against the workspace version, which ensures compatibility with the development branch. No action needed.e2e/react-router/i18n-paraglide/package.json (2)
15-15: LGTM! Internal dependency correctly uses workspace protocol.The
@tanstack/react-routerpackage correctly usesworkspace:^for the internal dependency, which aligns with the coding guidelines.
16-17: TanStack Router is compatible with React 19—both v18.x.x and v19.x.x are supported per the official documentation.e2e/react-router/i18n-paraglide/src/routes/about.tsx (1)
1-10: LGTM!Clean route implementation following the established pattern. The
data-testidattribute enables reliable e2e test targeting.e2e/react-start/i18n-paraglide/src/routes/__root.tsx (1)
1-85: LGTM!Well-structured SSR root component with proper HTML shell rendering. The locale switcher pattern with
data-active-localeprovides a clean way to style the active locale. The absence of abeforeLoadredirect hook is appropriate here since redirects are handled server-side in the Start framework.e2e/react-router/i18n-paraglide/src/main.tsx (2)
10-22: Core router configuration for the i18n rewrite fix.The
rewrite.input/outputconfiguration correctly separates the internal routing URL (de-localized) from the browser-visible URL (localized). This is the key mechanism that prevents the infinite redirect loop by allowing the router to match routes on canonical paths while displaying locale-prefixed paths to users.
24-29: Type registration pattern is correct.Module augmentation properly registers the router type for type-safe usage throughout the application.
e2e/react-router/i18n-paraglide/vite.config.ts (1)
8-49: LGTM!Well-configured Vite setup with Paraglide URL patterns correctly mapping both the canonical routes (
/,/about) and their localized variants. The catch-all pattern at the end ensures all other routes are also properly localized.e2e/react-start/i18n-paraglide/src/routes/index.tsx (1)
5-9: Server function demonstrates i18n on the server side.The
inputValidatoris a passthrough, which is acceptable for this e2e demo. In production code, you'd typically add proper validation.e2e/react-router/i18n-paraglide/src/routes/__root.tsx (2)
10-19: Client-side redirect handling for SPA i18n.The
beforeLoadhook correctly sets the document language and handles locale-based redirects for the SPA scenario. Usingdecision.redirectUrl.hreffor the redirect target is appropriate here.
20-68: LGTM!The component structure is clean with proper navigation links and locale switcher. The
data-testidattributes enable reliable e2e test targeting.e2e/react-start/i18n-paraglide/src/router.tsx (1)
7-18: Router factory with i18n rewrite configuration.The factory pattern is appropriate for SSR scenarios. The
rewriteconfiguration mirrors the SPA setup, ensuring consistent locale URL transformation.Note: Unlike the SPA
main.tsx, this file doesn't include the module augmentation for type safety (declare module '@tanstack/react-router' { interface Register { router: ... } }). If type inference for route parameters is needed elsewhere in the Start app, consider adding type registration.e2e/react-router/i18n-paraglide/playwright.config.ts (1)
1-35: LGTM!The Playwright configuration follows the established patterns in this repository. The dynamic port allocation via
getTestServerPort, webServer setup with build/preview commands, and CI-awarereuseExistingServersetting are all appropriately configured for the i18n e2e tests.e2e/react-router/i18n-paraglide/tests/navigation.spec.ts (3)
1-7: Good test documentation.The header comment clearly explains the purpose of these tests and the rewrite API's bidirectional URL transformation (de-localization for route matching, localization for display). This provides valuable context for future maintainers.
8-105: Comprehensive i18n navigation test coverage.This test suite effectively validates the fix for issue #6089 by covering:
- Page loads without redirect loops (the core regression)
- Locale-prefixed routing (
/de,/de/ueber)- Navigation with translated URLs
- Locale switching with proper URL updates
- Link href localization
The use of
data-testidselectors ensures stable test targeting.
221-246: LGTM!These tests effectively verify that the rewrite output function correctly localizes link
hrefattributes based on the current locale—German links include/deprefix with translated paths, while English links remain unprefixed.e2e/react-start/i18n-paraglide/src/routeTree.gen.ts (1)
1-86: Skipping review of autogenerated file.This file is automatically generated by TanStack Router and should not be manually modified. Based on learnings,
routeTree.gen.tsfiles are excluded from review.
| { | ||
| "compilerOptions": { | ||
| "strict": true, | ||
| "noImplicitAny": false, |
There was a problem hiding this comment.
Remove noImplicitAny: false to align with strict type safety guidelines.
The coding guidelines require "extensive type safety for all code." Setting noImplicitAny: false weakens type checking and contradicts the strict: true setting on Line 3. When strict is enabled, noImplicitAny defaults to true, so this line explicitly disables an important safety check.
As per coding guidelines, use TypeScript strict mode with extensive type safety for all code.
🔎 Recommended fix
"compilerOptions": {
"strict": true,
- "noImplicitAny": false,
"esModuleInterop": true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "noImplicitAny": false, | |
| "compilerOptions": { | |
| "strict": true, | |
| "esModuleInterop": true, |
🤖 Prompt for AI Agents
In e2e/react-router/i18n-paraglide/tsconfig.json around line 4, the compiler
option "noImplicitAny": false must be removed (or set to true) to comply with
strict type safety; edit the tsconfig to delete that line so the "strict": true
setting isn't contradicted (or explicitly set "noImplicitAny": true if you
prefer explicitness), then run the TypeScript build to fix any resulting
implicit-any errors.
| { name: 'og:type', content: 'website' }, | ||
| { name: 'og:title', content: title }, | ||
| { name: 'og:description', content: description }, |
There was a problem hiding this comment.
Use property attribute for Open Graph meta tags.
Open Graph tags should use the property attribute instead of name according to the Open Graph protocol specification. Social media platforms (Facebook, LinkedIn, etc.) look for property="og:*" and may not recognize these tags correctly with name="og:*".
🔎 Proposed fix
- { name: 'og:type', content: 'website' },
- { name: 'og:title', content: title },
- { name: 'og:description', content: description },
+ { property: 'og:type', content: 'website' },
+ { property: 'og:title', content: title },
+ { property: 'og:description', content: description },And for line 27 in the conditional image block:
- { name: 'og:image', content: image },
+ { property: 'og:image', content: image },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/react-start/i18n-paraglide/src/utils/seo.ts around lines 20-22 (and the
conditional image block at line 27), replace meta entries that use name: 'og:*'
with property: 'og:*' so Open Graph tags are rendered with the property
attribute (e.g., property: 'og:type', property: 'og:title', property:
'og:description', and the og:image entry in the conditional block); keep the
content values unchanged and ensure any code that maps these objects to actual
<meta> elements uses the property key when rendering Open Graph tags.
fixes #6089
Summary by CodeRabbit
New Features
Behavior Changes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.