refactor(router-core): consistent Unicode handling #5736
Conversation
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 86c9b8c
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/router-core/src/path.ts (1)
366-372: Trailing-slash branch usessubstring(1)(likely typo).This removes the first char instead of the trailing slash. It’s currently benign (the variable isn’t reused), but it’s incorrect and could bite later.
Apply this fix:
- if (pathname.slice(-1) === '/') { - pathname = pathname.substring(1) + if (pathname.slice(-1) === '/') { + // Remove trailing slash (symmetry with the leading slash handling) + pathname = pathname.slice(0, -1) segments.push({ type: SEGMENT_TYPE_PATHNAME, value: '/', }) }
🧹 Nitpick comments (4)
packages/react-router/tests/useNavigate.test.tsx (1)
2664-2777: React: encoded/unicode path tests — LGTM.Covers the same matrix and validations as Solid. For extra stability, you could await
router.latestLoadPromiseafter clicking before assertions, but it’s not strictly necessary here.packages/router-core/src/path.ts (1)
24-26: Duplicate doc comments.Several JSDoc headers are duplicated line-adjacent. Trim duplicates to reduce noise.
Also applies to: 36-38, 43-45, 49-51, 55-57, 77-81
packages/router-core/src/utils.ts (2)
492-497: Consider documenting why % and \ are in the ignore list.The reduced ignore list (from many URI-reserved characters to just
%and\) is a significant change. A brief comment explaining the reasoning would help future maintainers:
%must be preserved to avoid double-decoding valid percent-encoded sequences\preservation purpose could be clarified (Windows path compatibility? Escape sequences?)+// Ignore list contains characters that should not be decoded: +// - % must be preserved to avoid double-decoding (e.g., %2520 -> %20, not space) +// - \ is preserved for [reason - Windows paths? escape sequences?] const DECODE_IGNORE_LIST = Array.from( new Map([ ['%', '%25'], ['\\', '%5C'], ]).values(), )
536-537: Consider documenting the switch fromdecodeURIComponenttodecodeURI.The change from
decodeURIComponenttodecodeURIis significant and appears intentional:
decodeURIpreserves characters like%2F(/) which are semantically important in pathsdecodeURIComponentwould aggressively decode all reserved charactersThis aligns with the reduced ignore list and the PR's goal of consistent encoding. A brief comment explaining this design choice would be valuable for maintainability.
+ // Use decodeURI (not decodeURIComponent) to preserve path-semantic characters like / + // Only characters in DECODE_IGNORE_LIST are explicitly preserved try { return decodeURI(part)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
e2e/react-router/basic-file-based/src/routeTree.gen.ts(21 hunks)e2e/react-router/basic-file-based/src/routes/__root.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/params-ps/index.tsx(2 hunks)e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/search-params/index.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/search-params/route.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/대한민국.tsx(0 hunks)e2e/react-router/basic-file-based/src/routes/대한민국/route.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsx(1 hunks)e2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsx(1 hunks)e2e/react-router/basic-file-based/tests/app.spec.ts(1 hunks)e2e/react-router/basic-file-based/tests/params.spec.ts(3 hunks)e2e/react-router/basic-file-based/tests/search-params.spec.ts(4 hunks)e2e/solid-router/basic-file-based/src/routeTree.gen.ts(21 hunks)e2e/solid-router/basic-file-based/src/routes/__root.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/params-ps/index.tsx(2 hunks)e2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/search-params/index.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/search-params/route.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/대한민국.tsx(0 hunks)e2e/solid-router/basic-file-based/src/routes/대한민국/route.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsx(1 hunks)e2e/solid-router/basic-file-based/src/routes/대한민국/🚀.$id.tsx(1 hunks)e2e/solid-router/basic-file-based/tests/app.spec.ts(1 hunks)e2e/solid-router/basic-file-based/tests/params.spec.ts(3 hunks)e2e/solid-router/basic-file-based/tests/search-params.spec.ts(4 hunks)packages/react-router/tests/link.test.tsx(4 hunks)packages/react-router/tests/navigate.test.tsx(2 hunks)packages/react-router/tests/router.test.tsx(8 hunks)packages/react-router/tests/useNavigate.test.tsx(1 hunks)packages/router-core/src/index.ts(1 hunks)packages/router-core/src/path.ts(2 hunks)packages/router-core/src/router.ts(1 hunks)packages/router-core/src/utils.ts(2 hunks)packages/router-core/tests/path.test.ts(0 hunks)packages/router-core/tests/utils.test.ts(1 hunks)packages/solid-router/tests/link.test.tsx(4 hunks)packages/solid-router/tests/navigate.test.tsx(2 hunks)packages/solid-router/tests/router.test.tsx(8 hunks)packages/solid-router/tests/useNavigate.test.tsx(1 hunks)packages/start-plugin-core/src/prerender.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/router-core/tests/path.test.ts
- e2e/react-router/basic-file-based/src/routes/대한민국.tsx
- e2e/solid-router/basic-file-based/src/routes/대한민국.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-router/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxpackages/start-plugin-core/src/prerender.tse2e/react-router/basic-file-based/src/routes/search-params/index.tsxpackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/solid-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/tests/search-params.spec.tspackages/router-core/tests/utils.test.tse2e/react-router/basic-file-based/src/routes/대한민국/route.tsxe2e/react-router/basic-file-based/tests/params.spec.tspackages/react-router/tests/useNavigate.test.tsxe2e/react-router/basic-file-based/src/routes/search-params/route.tsxe2e/solid-router/basic-file-based/tests/search-params.spec.tspackages/react-router/tests/link.test.tsxe2e/react-router/basic-file-based/src/routes/params-ps/index.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxe2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxpackages/router-core/src/utils.tse2e/solid-router/basic-file-based/src/routes/search-params/index.tsxpackages/router-core/src/path.tse2e/react-router/basic-file-based/src/routes/__root.tsxpackages/router-core/src/router.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/router-core/src/index.tspackages/react-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routes/search-params/route.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/route.tsxpackages/solid-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tspackages/solid-router/tests/router.test.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-router/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxe2e/react-router/basic-file-based/src/routes/search-params/index.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/solid-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/tests/search-params.spec.tse2e/react-router/basic-file-based/src/routes/대한민국/route.tsxe2e/react-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/src/routes/search-params/route.tsxe2e/solid-router/basic-file-based/tests/search-params.spec.tse2e/react-router/basic-file-based/src/routes/params-ps/index.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxe2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxe2e/solid-router/basic-file-based/src/routes/search-params/index.tsxe2e/react-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/src/routes/search-params/route.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/route.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routeTree.gen.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxe2e/react-router/basic-file-based/src/routes/search-params/index.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/react-router/basic-file-based/src/routes/대한민국/route.tsxe2e/react-router/basic-file-based/src/routes/search-params/route.tsxe2e/react-router/basic-file-based/src/routes/params-ps/index.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxe2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxe2e/solid-router/basic-file-based/src/routes/search-params/index.tsxe2e/react-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/src/routes/search-params/route.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/route.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-plugin-core/src/prerender.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/useNavigate.test.tsxpackages/react-router/tests/link.test.tsxpackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/solid-router/tests/navigate.test.tsxpackages/solid-router/tests/router.test.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/tests/utils.test.tspackages/router-core/src/utils.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/src/index.ts
🧠 Learnings (16)
📓 Common learnings
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.
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.
📚 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-router/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/tests/app.spec.tspackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/tests/search-params.spec.tspackages/router-core/tests/utils.test.tse2e/react-router/basic-file-based/tests/params.spec.tspackages/react-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/tests/search-params.spec.tspackages/react-router/tests/link.test.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxpackages/router-core/src/path.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/solid-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tspackages/solid-router/tests/router.test.tsxe2e/react-router/basic-file-based/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-router/basic-file-based/tests/app.spec.ts
📚 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/basic-file-based/tests/app.spec.tse2e/solid-router/basic-file-based/tests/app.spec.tspackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/tests/search-params.spec.tspackages/router-core/tests/utils.test.tse2e/react-router/basic-file-based/tests/params.spec.tspackages/react-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/tests/search-params.spec.tse2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxpackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/solid-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tspackages/solid-router/tests/router.test.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
e2e/solid-router/basic-file-based/src/routes/__root.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxpackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxpackages/react-router/tests/useNavigate.test.tsxe2e/react-router/basic-file-based/src/routes/params-ps/index.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxe2e/react-router/basic-file-based/src/routes/__root.tsxpackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/route.tsxpackages/solid-router/tests/navigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tspackages/solid-router/tests/router.test.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
e2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsxe2e/solid-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/tests/params.spec.tse2e/react-router/basic-file-based/src/routes/params-ps/index.tsxe2e/solid-router/basic-file-based/src/routes/params-ps/index.tsxpackages/react-router/tests/router.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tspackages/solid-router/tests/router.test.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-22T00:56:49.237Z
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.
Applied to files:
e2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxpackages/router-core/tests/utils.test.tse2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxpackages/router-core/src/utils.tspackages/router-core/src/path.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/router.test.tsx
📚 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/start-plugin-core/src/prerender.tspackages/react-router/tests/link.test.tsxe2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsxpackages/router-core/src/utils.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/navigate.test.tsxpackages/solid-router/tests/router.test.tsx
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-plugin/** : Use unplugin for universal bundler plugins in the router-plugin package
Applied to files:
packages/start-plugin-core/src/prerender.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-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/src/routes/** : Place file-based routes under src/routes/ directories
Applied to files:
e2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsxe2e/solid-router/basic-file-based/src/routes/대한민국/route.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-22T00:56:53.426Z
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.
Applied to files:
packages/router-core/tests/utils.test.tspackages/router-core/src/utils.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/router.test.tsx
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to e2e/** : Store end-to-end tests under the e2e/ directory
Applied to files:
e2e/solid-router/basic-file-based/tests/search-params.spec.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/src/index.tse2e/solid-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routeTree.gen.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/solid-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Applied to files:
e2e/solid-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routeTree.gen.ts
🧬 Code graph analysis (10)
e2e/solid-router/basic-file-based/src/routes/__root.tsx (2)
packages/react-router/src/link.tsx (1)
Link(567-598)packages/solid-router/src/index.tsx (1)
Link(229-229)
e2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx (3)
e2e/solid-router/basic-file-based/src/routes/__root.tsx (1)
Route(12-22)e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx (1)
Route(4-6)e2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/$bar.route.tsx (1)
Route(9-11)
packages/solid-router/tests/useNavigate.test.tsx (2)
packages/react-router/src/useNavigate.tsx (1)
useNavigate(25-42)packages/solid-router/src/useNavigate.tsx (1)
useNavigate(11-25)
packages/router-core/tests/utils.test.ts (1)
packages/router-core/src/utils.ts (1)
decodePathSegment(499-559)
packages/react-router/tests/useNavigate.test.tsx (1)
packages/react-router/src/useNavigate.tsx (1)
useNavigate(25-42)
e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx (5)
e2e/react-router/basic-file-based/src/routes/__root.tsx (1)
Route(12-22)e2e/react-router/basic-file-based/src/routes/params-ps/index.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/$bar.$baz.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/$bar.route.tsx (1)
Route(9-11)e2e/react-router/basic-file-based/src/routes/params-ps/named/index.tsx (1)
Route(3-7)
e2e/react-router/basic-file-based/src/routes/__root.tsx (1)
packages/react-router/src/link.tsx (1)
Link(567-598)
packages/router-core/src/router.ts (2)
packages/router-core/src/index.ts (1)
decodePathSegment(281-281)packages/router-core/src/utils.ts (1)
decodePathSegment(499-559)
packages/react-router/tests/router.test.tsx (1)
packages/react-router/src/index.tsx (2)
createMemoryHistory(118-118)RouterProvider(283-283)
packages/solid-router/tests/navigate.test.tsx (1)
packages/solid-router/src/router.ts (1)
createRouter(76-78)
⏰ 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 (42)
packages/react-router/tests/link.test.tsx (2)
6487-6527: Test coverage improvements align with PR objectives.The updated test data effectively validates the new Unicode handling behavior:
- The
@character addition enriches special character coverage alongside Unicode (대) and emoji (🚀)- The new
expectedLocationfield cleanly separates validation of encoded hrefs from decoded pathnames- The distinction between
expectedPath(percent-encoded) andexpectedLocation(Unicode-decoded but preserving encoded special chars like%5Cand%25) correctly reflects the PR's approach to consistent pathname decoding
6583-6588: Improved test assertions validate both encoded and decoded states.The refactored assertions now explicitly verify both:
window.location.pathname(encoded) matchesexpectedPathrouter.latestLocation.pathname(decoded) matchesexpectedLocationThis dual validation ensures the router correctly maintains decoded pathnames internally while generating properly encoded hrefs, directly addressing issue #5008 mentioned in the PR objectives.
packages/solid-router/tests/link.test.tsx (1)
6479-6586: LGTM! Well-structured tests for Unicode/encoded path handling.The tests correctly validate the PR's core behavior: pathnames are decoded internally (
router.latestLocation.pathname) while remaining encoded in the browser URL (window.location.pathname). The distinction between these two states (lines 6581-6582) is crucial for ensuring consistent Unicode handling across the router.The test coverage is comprehensive:
- Unicode characters (Korean 대, emoji 🚀)
- Special characters (@ ! % [ ] \ / .)
- Different param types (splat with prefix/suffix, wildcard, path param)
- Edge cases (backslashes, percent signs, forward slashes)
Note: Some characters like
%5C(backslash),%25(percent), and%2F(forward slash in path params) intentionally remain encoded even in the decoded location to avoid ambiguity.packages/solid-router/tests/navigate.test.tsx (2)
1-1: LGTM! Import change enables parameterized test syntax.Adding
testto the vitest imports allows the use oftest.each()for the new parameterized test cases in the "encoded and unicode paths" test suite.
1267-1341: Excellent test coverage for Unicode and special character handling.This test suite thoroughly validates the encoding/decoding behavior with:
- Unicode characters (대, 🚀) correctly decoded in pathname but encoded in href
- Special characters (%, /, , [, ], @, !) properly encoded
- The important distinction between splat params (where
/is preserved) and regular params (where/is encoded as%2F)The parameterized approach using
test.each()is clean and maintainable. The inclusion of both_splatand*keys in the params objects (e.g., lines 1276-1278) appropriately tests that the router accepts both naming conventions for wildcard parameters.packages/solid-router/tests/router.test.tsx (2)
337-337: LGTM! Pathname correctly reflects decoded Unicode characters.These assertions correctly verify that
pathnamecontains the decoded form of the emoji (🚀) regardless of whether the input was already decoded or percent-encoded (%F0%9F%9A%80). This aligns with the PR's objective of consistently decoding pathnames upfront.Also applies to: 367-367
381-388: LGTM! Dual assertions correctly verify both encoded and decoded forms.These assertions properly validate the new encoding behavior:
hrefcontains the fully percent-encoded URLpathnamecontains the decoded form while preserving structural encodings like%2F(which would change path structure if decoded to/)This ensures encoded slashes in params don't inadvertently create new path segments while still decoding user-facing characters like spaces and emoji.
Also applies to: 586-588, 617-619, 632-638, 649-656
packages/start-plugin-core/src/prerender.ts (1)
183-183: URL constructor correctly handles decoded paths—change is correct.The modification removes unnecessary
encodeURI()wrapping frompage.pathbefore passing towithBase, aligning with the PR's objective to keep pathnames decoded throughout the routing system. The JavaScriptURLconstructor at line 93 handles Unicode characters and encodes them internally for HTTP. Existing tests—including comprehensive Unicode parameter tests with paths like/대한민국and special characters—confirm this works end-to-end.packages/react-router/tests/navigate.test.tsx (1)
1296-1371: Great coverage for encoded/unicode navigation.The cases exercise splat, prefix/suffix, and path params and assert encoded href vs decoded pathname correctly. Looks solid.
packages/router-core/tests/utils.test.ts (1)
603-614: decodePathSegment case-normalization — LGTM.Verifies lowercase
%2fand mixed case normalize to%2Fand remain ignored. Nice addition.packages/solid-router/tests/useNavigate.test.tsx (1)
2645-2759: Solid: encoded/unicode path tests — LGTM.Assertions on encoded window path, decoded router path, and params echo are spot on.
packages/router-core/src/index.ts (1)
281-282: Public export ofdecodePathSegment— LGTM.Makes the decoding utility accessible for tests and consumers, consistent with the refactor.
packages/router-core/src/path.ts (1)
361-361: Keep raw pathname segments — LGTM.Assigning
value: part(no decode) aligns with decoding atparseLocationand ensures consistent matching/encoding elsewhere.packages/router-core/src/router.ts (1)
1176-1176: LGTM! Core pathname decoding implemented correctly.This change ensures all pathnames in router state are decoded, fixing the mismatch where resolved pathnames were encoded but link params were not. The decoding happens upfront during location parsing, eliminating the need to decode individual segments during path matching.
packages/react-router/tests/router.test.tsx (4)
347-347: Excellent test coverage for Unicode/emoji decoding.These tests confirm that pathnames containing Unicode characters (like 🚀) are properly decoded, whether provided directly or as percent-encoded sequences. This validates the PR's Unicode handling improvements.
Also applies to: 377-377
391-397: Clear validation of pathname vs href encoding distinction.This test properly validates that
hrefpreserves the original encoding whilepathnamecontains the decoded form. Note that forward slashes remain encoded as%2Fin the pathname (preserved by the decode logic), while spaces are decoded from%20.
600-602: Thorough splat parameter encoding tests.These tests comprehensively validate that splat routes handle encoding correctly across different scenarios (emojis, spaces, encoded slashes). The consistent pattern of encoded
hrefvs decodedpathnameis well-established.Also applies to: 631-633, 649-652, 667-670
750-822: Excellent parameterized test coverage for path segment encoding.This data-driven test suite is particularly well-designed, covering:
- Unicode characters (é, 🚀)
- Special characters requiring encoding (%, &, /)
- Edge cases like nested encoding (%25)
- Mixed encoded and unencoded sequences
The test structure clearly documents expected behavior for each scenario, making it easy to understand and maintain.
packages/router-core/src/utils.ts (1)
551-558: Well-designed normalization and optimization.The implementation includes good optimizations:
- Early return when no percent-encoded sequences exist (line 551)
- Normalization of percent-encodings to uppercase before processing (lines 554-556)
This normalization ensures the case-sensitive
includes()check at line 517 works reliably with the uppercaseDECODE_IGNORE_LISTvalues.e2e/solid-router/basic-file-based/src/routes/search-params/route.tsx (1)
5-5: LGTM! Test execution optimization.Reducing the artificial delay from 1000ms to 100ms will speed up test execution without affecting functionality.
e2e/react-router/basic-file-based/src/routes/search-params/route.tsx (1)
5-5: LGTM! Test execution optimization.Reducing the artificial delay from 1000ms to 100ms will speed up test execution without affecting functionality.
e2e/react-router/basic-file-based/src/routes/__root.tsx (1)
133-140: LGTM! Unicode path test fixture.The new Link to
/대한민국provides a test fixture for Unicode path handling, consistent with the PR's objective to ensure proper encoding/decoding of non-Latin paths.e2e/react-router/basic-file-based/src/routes/params-ps/index.tsx (2)
21-29: LGTM! Comprehensive special character testing in path params.The new link with
'foo%\\/🚀대'provides excellent edge case coverage, testing percent signs, backslashes, forward slashes, emoji, and non-Latin characters in a single path parameter.
70-78: LGTM! Pre-encoded parameter test coverage.The new link tests wildcard path params with pre-encoded strings, complementing the special character tests and ensuring consistent encoding behavior across different parameter types.
e2e/solid-router/basic-file-based/src/routes/__root.tsx (1)
133-140: LGTM! Unicode path test fixture.The new Link to
/대한민국provides a test fixture for Unicode path handling, consistent with the PR's objective to ensure proper encoding/decoding of non-Latin paths.e2e/react-router/basic-file-based/tests/app.spec.ts (1)
320-328: LGTM! Excellent e2e coverage for Unicode routing.This test validates the core PR functionality:
- Navigation to non-Latin paths works correctly
- Content renders with the decoded Unicode path
- Browser URL displays the percent-encoded form as expected
This ensures the router's internal decoding strategy doesn't break browser standards.
e2e/solid-router/basic-file-based/src/routes/search-params/index.tsx (1)
24-40: Verification confirms intentional behavior with proper test coverage.The test suite validates that passing a literal pre-encoded string
'%EB%8C%80%ED%95%9C%EB%AF%BC%EA%B5%AD'is correctly handled by URL-encoding the percent signs to%25. The test at lines 120-134 confirms the router properly processes this case—the encoded string in the URL parameter becomes%25EB%258C%2580...and displays as the literal encoded string on the page. This complements the prior test for direct Unicode handling, providing comprehensive coverage of parameter encoding behavior.e2e/react-router/basic-file-based/src/routes/search-params/index.tsx (1)
24-40: No issues found; approval is appropriate.The test at lines 123-140 in
search-params.spec.tscomprehensively validates the behavior. Passing the literal string'%EB%8C%80%ED%95%9C%EB%AF%BC%EA%B5%AD'is intentional and correct—the router properly encodes the literal percent signs as%25in the URL, then decodes them back when parsing, with assertions confirming the round-trip works as expected. This is not double-encoding; it's correct handling of literal data containing special characters. The code changes demonstrate solid test coverage for both Unicode and pre-encoded parameter scenarios.e2e/react-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx (1)
39-46: LGTM! Good test coverage for special characters.The Link correctly tests URL parameter handling with a mix of Unicode characters (🚀, 대), literal forward slash, and percent-encoded sequences (%2F). This aligns with the PR's objective to ensure consistent encoding/decoding behavior.
e2e/solid-router/basic-file-based/tests/app.spec.ts (1)
308-316: LGTM! Well-structured Unicode route test.The test correctly verifies both the decoded content rendering and the encoded URL representation for non-Latin routes, which is essential for validating the consistent Unicode handling introduced in this PR.
e2e/solid-router/basic-file-based/src/routes/params-ps/index.tsx (2)
21-29: LGTM! Comprehensive special character test case.The Link correctly tests named parameters with a complex mix of special characters, including percent-encoding, escape sequences, emoji, and non-Latin characters.
70-78: LGTM! Good test for pre-encoded parameters.The Link tests wildcard parameter handling with an already percent-encoded value, which is important for verifying that the router doesn't double-encode parameters.
e2e/react-router/basic-file-based/src/routes/대한민국/wildcard.$.tsx (1)
1-19: LGTM! Clean wildcard route implementation.The route correctly implements wildcard parameter handling under a Unicode path, with proper test identifiers for e2e verification. The component follows established React Router patterns.
e2e/solid-router/basic-file-based/src/routes/params-ps/named/$foo/route.tsx (1)
44-51: LGTM! Consistent special character test coverage.The Link correctly mirrors the React Router equivalent, ensuring consistent test coverage for special character handling across both frameworks.
e2e/solid-router/basic-file-based/tests/search-params.spec.ts (4)
8-8: LGTM! Good improvement to test stability.The
waitForURLcalls ensure that navigation completes before assertions run, reducing the likelihood of flaky tests due to race conditions.Also applies to: 21-21, 77-77, 89-89
30-48: LGTM! Comprehensive Unicode search parameter test.The test correctly verifies both direct navigation with Unicode characters and the proper percent-encoding in the resulting URL. The emoji and Korean characters provide good coverage for multi-byte UTF-8 sequences.
50-72: LGTM! Good test for double-encoding prevention.The test verifies that already-encoded parameters (with escaped percent signs) are handled correctly without additional encoding, ensuring the router respects intentionally encoded values.
98-118: LGTM! Complete navigation test coverage.These tests verify Unicode and encoded parameter handling through Link-based navigation, complementing the direct visit tests and ensuring consistent behavior across different navigation methods.
Also applies to: 120-142
e2e/react-router/basic-file-based/src/routes/대한민국/🚀.$id.tsx (1)
1-19: LGTM! Excellent Unicode path segment test.The route effectively tests Unicode handling with both Korean characters and emoji in the path segments, while maintaining clean parameterized routing patterns. The test identifiers are well-chosen for e2e verification.
e2e/solid-router/basic-file-based/src/routes/대한민국/wildcard.$.tsx (1)
1-19: LGTM! Correct Solid Router wildcard implementation.The route correctly implements wildcard parameter handling with Solid Router's reactive patterns (using
params()as a function). This provides consistent Unicode test coverage across both React and Solid Router implementations.e2e/react-router/basic-file-based/tests/search-params.spec.ts (1)
30-72: Nice addition of encoded search param coverage. The direct-visit cases for both decoded and pre-encoded query strings make the regression path explicit and guard against double-decoding issues.e2e/react-router/basic-file-based/tests/params.spec.ts (1)
387-503: Unicode navigation flow looks solid. Exercising both named and wildcard children with encoded params (and validating active state flips) gives great coverage for the pathname decoding refactor.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-router/tests/router.test.tsx (1)
550-563: <!-- [scratchpad_start]
[task verification_1/1]
Final verification of review comment about pathname vs href encoding[observations]
NAVIGATE TESTS (lines 562, 579):
- Line 562: navigate({ params: { slug: '@jane' } }) → expects pathname='/posts/%40jane' (ENCODED)
- Line 579: navigate({ params: { slug: '@jane' }, pathParamsAllowedCharacters: ['@'] }) → expects pathname='/posts/@jane' (DECODED)
INITIALENTRIES TESTS (lines 350, 364, 605, 619):
- Input encoded URLs with %25, %26, %F0%9F%9A%80
- Pathname assertions preserve the encoded form from URL string
REFERENCE PATTERN (line 600-602):
- initialEntries: ['/🚀'] (unicode)
- href: '/%F0%9F%9A%80' (encoded)
- pathname: '/🚀' (decoded)
[analysis]
KEY INSIGHT: Line 600-602 demonstrates the correct semantics:
- pathname should be DECODED
- href should be ENCODED
Line 562 VIOLATES this pattern (navigate test with encoded pathname).
Line 579 FOLLOWS this pattern (navigate test with decoded pathname because @ allowed).InitialEntries tests (357, 367, 611, 621) have different semantics - they load encoded URLs and pathname reflects what was in the URL string, which is encoded. These may be correct for that use case.
Review is CORRECT: Line 562 should assert via href, and optionally add decoded pathname assertion.
Review correctly identifies the semantic issue in line 562 and partially in line 579.[done]
Review comment is VALID and ACCURATE. No issues found with the analysis.
</scratchpad_end] -->Correct review comment - pathname should be decoded under current semantics; use href for encoded assertions
The verification confirms the review's analysis is accurate. The test at line 562 violates the documented encoding semantics shown throughout the test file:
- Line 600-602 demonstrates:
hrefcontains encoded values (/%F0%9F%9A%80), whilepathnamecontains decoded values (/🚀)- Line 562 asserts an encoded value (
/posts/%40jane) againstpathname, which contradicts this pattern- Line 579 correctly asserts the decoded value (
/posts/@jane) againstpathname(because@is inpathParamsAllowedCharacters)The identified encoded pathname assertions at lines 357, 367, 611, and 621 are from
initialEntriestests (pre-loaded URLs), which have different semantics thannavigate()tests—they preserve the encoded form as given in the URL string.Apply the suggested diff to line 562, and optionally add the decoded pathname assertion as suggested.
🧹 Nitpick comments (2)
packages/solid-router/tests/link.test.tsx (1)
6494-6537: Consider adding documentation for the encoding expectations.The test cases validate three distinct path representations (
expectedPath,expectedLocation,params) which differ in their encoding levels. While comprehensive, this complexity could benefit from inline comments explaining:
expectedPath- fully URL-encoded (as seen inwindow.location.pathname)expectedLocation- Unicode decoded but special characters like\,%remain encoded (as stored inrouter.latestLocation.pathname)params- fully decoded values extracted from the pathFor example, on line 6500,
expectedLocationshows/foo/prefix@대test[s%5C/.%5C/parameter%25!🚀@]where:
대and🚀are decoded (Unicode)%5Cand%25remain encoded (special chars)This prevents ambiguity in the pathname (e.g., literal
/vs encoded%2F), but it's not immediately obvious from the test alone.Consider adding a comment block above the test cases:
+// Each test case validates three representations: +// - expectedPath: fully encoded pathname (window.location.pathname) +// - expectedLocation: Unicode decoded, special chars encoded (router.latestLocation.pathname) +// - params: fully decoded route parameters const testCases = [packages/react-router/tests/router.test.tsx (1)
812-812: Test title mentions $output but variable is now { path, url }The display name still says “should resolve $input to $output”. Consider renaming for clarity since the test uses path and url.
Apply this diff:
- ])('should resolve $input to $output', async ({ input, path, url }) => { + ])('should resolve $input to path=$path and url=$url', async ({ input, path, url }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
e2e/solid-router/basic-file-based/src/routeTree.gen.ts(21 hunks)packages/react-router/tests/link.test.tsx(4 hunks)packages/react-router/tests/router.test.tsx(8 hunks)packages/router-core/src/index.ts(1 hunks)packages/router-core/src/router.ts(3 hunks)packages/router-core/src/utils.ts(2 hunks)packages/router-core/tests/utils.test.ts(7 hunks)packages/solid-router/tests/link.test.tsx(4 hunks)packages/solid-router/tests/router.test.tsx(8 hunks)packages/solid-router/tests/useNavigate.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/router-core/src/router.ts
- packages/router-core/src/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/tests/utils.test.tspackages/react-router/tests/router.test.tsxpackages/router-core/src/utils.tspackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/link.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/tests/utils.test.tspackages/router-core/src/utils.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/link.test.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
🧠 Learnings (13)
📓 Common learnings
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.
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.
📚 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:
packages/router-core/tests/utils.test.tspackages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/link.test.tsxe2e/solid-router/basic-file-based/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/tests/router.test.tsxpackages/router-core/src/utils.tspackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/react-router/tests/link.test.tsx
📚 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:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/link.test.tsxpackages/solid-router/tests/router.test.tsxpackages/solid-router/tests/useNavigate.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-22T00:56:53.426Z
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.
Applied to files:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/router.test.tsx
📚 Learning: 2025-09-22T00:56:49.237Z
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.
Applied to files:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/router.test.tsx
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/react-router/tests/router.test.tsxpackages/solid-router/tests/router.test.tsxe2e/solid-router/basic-file-based/src/routeTree.gen.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/solid-router/basic-file-based/src/routeTree.gen.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/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Applied to files:
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/src/routes/** : Place file-based routes under src/routes/ directories
Applied to files:
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
🧬 Code graph analysis (5)
packages/router-core/tests/utils.test.ts (2)
packages/router-core/src/index.ts (1)
decodePath(281-281)packages/router-core/src/utils.ts (1)
decodePath(499-559)
packages/react-router/tests/router.test.tsx (1)
packages/react-router/src/index.tsx (2)
createMemoryHistory(118-118)RouterProvider(283-283)
packages/router-core/src/utils.ts (1)
packages/router-core/src/index.ts (1)
decodePath(281-281)
packages/solid-router/tests/router.test.tsx (1)
packages/router-core/src/route.ts (1)
path(1551-1553)
packages/solid-router/tests/useNavigate.test.tsx (1)
packages/solid-router/src/useNavigate.tsx (1)
useNavigate(11-25)
⏰ 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 (10)
packages/solid-router/tests/useNavigate.test.tsx (1)
2645-2759: Excellent test coverage for Unicode and encoded path handling!The test suite comprehensively verifies navigation with Unicode characters and special characters across different path patterns (prefix, suffix, wildcard, path param). The assertions correctly validate both the encoded form (
window.location.pathname) and decoded form (router.latestLocation.pathname), which aligns perfectly with the PR's objective of consistent Unicode handling.packages/solid-router/tests/router.test.tsx (5)
337-337: LGTM! Pathname correctly asserts decoded emoji.The test properly verifies that
router.state.location.pathnamecontains the decoded Unicode emoji '🚀' rather than its percent-encoded form '%F0%9F%9A%80'.
367-367: LGTM! Correctly verifies decoding of percent-encoded input.
381-388: LGTM! Excellent test for nuanced encoding behavior.This test correctly validates that:
hrefpreserves full percent-encodingpathnamedecodes spaces (%20 → ' ') but preserves encoded slashes (%2F) to maintain path structure integrityThis is exactly the right behavior for path parameters containing special characters.
586-587: LGTM! Comprehensive splat route encoding tests.These tests properly validate the encoding/decoding behavior for splat routes (
/$), covering:
- Emoji characters in both encoded and unencoded input forms
- Complex multi-segment paths with preserved encoding for slashes
- Proper href encoding vs pathname decoding
Also applies to: 617-618, 632-638, 649-656
736-808: LGTM! Improved parameterized test structure with comprehensive encoding coverage.The refactored test structure using separate
path(decoded) andurl(encoded) properties is much clearer than the previous singleoutputapproach. The test cases comprehensively cover:
- Unicode characters (é)
- Emoji (🚀)
- Percent-encoded special characters (%25, %26)
- Complex combinations with preserved slashes and decoded spaces
The assertions at lines 806-807 correctly validate that
pathnamecontains the decoded path whileURL.pathnamepreserves the encoded form.Note: The past review concern about line 807 was already resolved—
new URL().pathnamedoes preserve percent-encoding as confirmed by the author.packages/solid-router/tests/link.test.tsx (1)
6593-6598: LGTM! Comprehensive validation of path encoding at multiple levels.The test correctly validates the three distinct path representations:
window.location.pathname- the fully encoded browser URLrouter.latestLocation.pathname- the router's partially decoded internal pathname (Unicode decoded, special chars encoded)- Route params - fully decoded parameter values
This ensures the router maintains correct encoding state at each layer, which directly addresses the PR's goal of consistent Unicode handling and active link matching with special characters.
packages/react-router/tests/router.test.tsx (2)
391-396: Good split: encoded href vs decoded pathnameAsserting encoded href (or URL) alongside a decoded pathname here is correct and aligns with the new invariant (pathname decoded; href/URL encoded). LGTM. Based on learnings
Also applies to: 600-602, 631-633, 649-651, 667-669
347-347: Emoji decode expectations look rightBoth direct and percent-encoded rocket cases resolve to a decoded pathname. LGTM.
Also applies to: 370-377
packages/react-router/tests/link.test.tsx (1)
6501-6520: Encoded href + decoded internal pathname checks are solid
- href (or window.location.pathname) asserted encoded,
- router.latestLocation.pathname asserted decoded,
- params echoed as expected (including both _splat and '*').
Looks good and covers tricky characters.
If you want to mirror consistency with other suites, you can also assert the router’s encoded URL surface:
+ expect(new URL(router.state.location.url).pathname).toBe(expectedPath)Also applies to: 6545-6603
|
requested changes made. quick note on the actionable comment from code rabbit. in the specific test |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-router/tests/router.test.tsx (1)
370-378: Clarify test description to distinguish from line 340.This test has the same description as the test at line 340, but uses a percent-encoded input (
%F0%9F%9A%80) instead of the raw emoji. The description should clarify that this test verifies decoding behavior.Consider updating the test description to something like:
- it('state.location.pathname, should have the params.slug value of "🚀"', async () => { + it('state.location.pathname, should decode %F0%9F%9A%80 to "🚀"', async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-router/tests/router.test.tsx(8 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/react-router/tests/router.test.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/router.test.tsx
🧠 Learnings (8)
📓 Common learnings
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.
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: 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-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:
packages/react-router/tests/router.test.tsx
📚 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/tests/router.test.tsx
📚 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:
packages/react-router/tests/router.test.tsx
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
packages/react-router/tests/router.test.tsx
📚 Learning: 2025-09-22T00:56:53.426Z
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.
Applied to files:
packages/react-router/tests/router.test.tsx
📚 Learning: 2025-09-22T00:56:49.237Z
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.
Applied to files:
packages/react-router/tests/router.test.tsx
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/react-router/tests/router.test.tsx
🧬 Code graph analysis (1)
packages/react-router/tests/router.test.tsx (1)
packages/react-router/src/index.tsx (2)
createMemoryHistory(118-118)RouterProvider(283-283)
⏰ 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 (7)
packages/react-router/tests/router.test.tsx (7)
340-348: LGTM!The test correctly verifies that the pathname contains the decoded emoji character.
380-398: LGTM!The test correctly verifies that:
hrefmaintains the fully encoded form with%2Fand%20pathnamedecodes unreserved characters like spaces while preserving reserved character encoding like%2FThis behavior aligns with the PR's use of
decodeURIas noted in the comments.
593-602: LGTM!The test correctly verifies that when a raw emoji is used in the URL:
hrefis percent-encoded for URL validitypathnamecontains the decoded emoji character
624-633: LGTM!The test correctly verifies that percent-encoded input is decoded in the pathname while remaining encoded in the href. This aligns with the expected behavior confirmed in past reviews.
635-652: LGTM!The test correctly distinguishes between encoded href and selectively decoded pathname, consistent with
decodeURIsemantics.
654-670: LGTM!The test correctly verifies that an unencoded space in the input is:
- Encoded to
%20in the href for URL validity- Kept as a space character in the pathname for display
749-826: LGTM! Excellent test coverage.This parametrized test comprehensively covers various encoding scenarios including:
- Unicode characters (é, 🚀)
- Percent encoding (
%25,%26)- Mixed encoded/decoded inputs
- Complex multi-character sequences
The test implementation correctly validates both the decoded
pathnameand the encoded URL pathname, ensuring consistent handling across all scenarios.
This PR follows on from #5663 and ensures that the path names are always in a consistent state by decoding the pathname when parsing the location. This ensures that the pathname is always in a decoded state and any further encoding/decoding that is required for navigation and matching is done in a consistent manner. While the pathname is decoded the URL is not ensuring the href and urls are always in a valid state.
One case where this currently fails currently is when matching a link as active or not when the params contain special characters, see #5008. While the resolved pathname are in encoded form, the params are on the link is not, and this can cause mismatching to occur.
As an added bonus, by decoding the path upfront we do not need to decode each path segment individually when parsing the paths for matching.
This PR adds a bunch of tests to ensure this is treated consistently throughout router at the core, code-based routing in React and Solid as well as in the browser environment with file-based routing in React and Solid e2e tests.
Summary by CodeRabbit