-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(start): normalize path encoding #6389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds special-character routes and tests, rehomes Unicode routes under a new specialChars subtree, makes SPA/start server startup mode-aware, updates prerender/vite exclusions and e2e scripts, and normalizes incoming request paths in the start server via decodePath. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StartServer as Start Server
participant SpaServer as SPA Server
Client->>StartServer: request to start servers (with MODE env)
alt MODE == "spa"
StartServer->>SpaServer: build & start SPA server
SpaServer-->>StartServer: SPA ready
StartServer->>StartServer: start Start server (SSR/static)
else
StartServer->>StartServer: start Start server only
end
Note over Client,StartServer: runtime request handling
Client->>StartServer: GET /path
StartServer->>StartServer: serve static (redirect: !isPrerender) or SSR
StartServer-->>Client: response
sequenceDiagram
participant Request
participant Handler as createStartHandler
participant Decoder as decodePath
participant Manifest as getManifest
participant Router as executeRouter
Request->>Handler: incoming HTTP request (request.url)
Handler->>Decoder: decodePath(request.url)
Decoder-->>Handler: decoded pathname + normalizedHref
Handler->>Manifest: load manifest
Manifest-->>Handler: manifest
Handler->>Router: execute router/SSR with normalizedHref
Router-->>Handler: response
Handler-->>Request: send final response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 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 (3)📚 Learning: 2025-11-02T16:16:24.898ZApplied to files:
📚 Learning: 2025-12-24T22:47:44.320ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied 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 (2)
✏️ Tip: You can disable this entire section by setting Comment |
|
View your CI Pipeline Execution ↗ for commit 453dfef
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@e2e/react-start/basic/server.js`:
- Around line 5-6: The imports in server.js currently include TypeScript
extensions which Node can't resolve at runtime; update the import statements for
isSpaMode and isPrerender to remove the ".ts" extension so they match the
vite.config.ts usage (i.e., change "import { isSpaMode } from
'./tests/utils/isSpaMode.ts'" and "import { isPrerender } from
'./tests/utils/isPrerender.ts'" to import the same modules without the ".ts"
suffix), ensuring the runtime can resolve the modules without a TS loader.
In `@e2e/solid-start/basic/tests/prerendering.spec.ts`:
- Around line 20-22: The assertion uses join(distDir,
'/specialChars/대한민국/index.html') which drops distDir because of the leading
slash; update the call that builds the path (the expect using existsSync and
join with distDir) to use a relative path segment (e.g.,
'specialChars/대한민국/index.html' or separate segments 'specialChars', '대한민국',
'index.html') so join(distDir, ...) correctly produces a path under distDir;
adjust the expect accordingly in prerendering.spec.ts where existsSync, join,
and distDir are used.
In `@e2e/vue-start/basic/server.js`:
- Around line 5-6: The server entry imports TypeScript modules
'./tests/utils/isSpaMode.ts' and './tests/utils/isPrerender.ts' which Node will
not load when running `node server.js`; either update the start script to run
the server with a TS loader (e.g., change the package.json "start" script to use
`tsx server.js`) or convert the imported utilities to JavaScript and update the
imports in server.js to './tests/utils/isSpaMode.js' and
'./tests/utils/isPrerender.js' so Node can resolve them without a TypeScript
runner; modify whichever approach you choose and ensure the identifiers
isSpaMode and isPrerender are imported from the matching files.
In `@e2e/vue-start/basic/src/routes/specialChars/route.tsx`:
- Around line 1-2: Remove the unnecessary React import: the line importing React
(import * as React from 'react') should be deleted from the route file that
currently imports Link, Outlet, createFileRoute so the file only imports from
'@tanstack/vue-router' (i.e., keep the import of Link, Outlet, createFileRoute
and remove the import of React).
In `@e2e/vue-start/basic/src/routes/specialChars/대한민국.tsx`:
- Around line 9-12: Remove the accidental "Test" text and update the h3 with
data-testid "special-non-latin-heading" so the heading matches the full route
path (include the "/specialChars" prefix, e.g. "/specialChars/대한민국"); keep the
surrounding JSX structure intact and ensure the paragraph text remains
unchanged.
🧹 Nitpick comments (4)
packages/start-server-core/src/createStartHandler.ts (1)
216-233: Avoid decoding query/hash viadecodePath.
IfdecodePathexpects a pathname-only input, passing the fullpath?search#hashcan corrupt encoded separators (e.g.,%23,%26) by turning them into#/&beforenew URL()parsing. Consider decoding justpathnameand keepingsearch/hashfrom the raw URL. Please verifydecodePath’s behavior here.Proposed adjustment
- const decodedPath = decodePath(request.url.replace(origin, '')) - const decodedURL = new URL(decodedPath, origin) - const searchParams = new URLSearchParams(decodedURL.search) - const normalizedHref = - decodedURL.pathname + - (searchParams.size > 0 ? '?' : '') + - searchParams.toString() + - decodedURL.hash - - const url = new URL(normalizedHref, decodedURL.origin) - const href = url.href.replace(url.origin, '') + const rawUrl = new URL(request.url, origin) + const decodedPathname = decodePath(rawUrl.pathname) + const searchParams = new URLSearchParams(rawUrl.search) + const normalizedHref = + decodedPathname + + (searchParams.size > 0 ? '?' : '') + + searchParams.toString() + + rawUrl.hash + + const url = new URL(normalizedHref, rawUrl.origin) + const href = url.href.replace(url.origin, '')e2e/solid-start/basic/server.js (1)
61-79: Minor: Unnecessaryasynckeywords in promise callbacks.The
asynckeyword in the.then()callbacks is not needed since there are noawaitexpressions inside. This is a minor nitpick and doesn't affect functionality.Suggested simplification
if (isSpaMode) { - createSpaServer().then(async ({ app }) => + createSpaServer().then(({ app }) => app.listen(port, () => { console.info(`Client Server: http://localhost:${port}`) }), ) - createStartServer().then(async ({ app }) => + createStartServer().then(({ app }) => app.listen(startPort, () => { console.info(`Start Server: http://localhost:${startPort}`) }), ) } else { - createStartServer().then(async ({ app }) => + createStartServer().then(({ app }) => app.listen(port, () => { console.info(`Start Server: http://localhost:${port}`) }), ) }e2e/vue-start/basic/tests/prerendering.spec.ts (1)
20-22: Minor inconsistency: unnecessary leading slash in path.The path
/specialChars/대한민국/index.htmlhas a leading slash, while other paths in this file (e.g.,'posts/index.html','deferred/index.html') don't. Whilepath.join()normalizes this correctly, removing the leading slash would improve consistency.Suggested fix
expect( - existsSync(join(distDir, '/specialChars/대한민국/index.html')), + existsSync(join(distDir, 'specialChars/대한민국/index.html')), ).toBe(true)e2e/react-start/virtual-routes/tests/special-characters.spec.ts (1)
10-10: Minor: Test suite name could be more specific.The outer describe block is named "Unicode route rendering" but the tests specifically cover ASCII special characters (pipe
|). Consider renaming to "Special character route rendering" for accuracy, or this can remain as-is if the suite will be extended with Unicode tests later.Suggested rename
-test.describe('Unicode route rendering', () => { +test.describe('Special character route rendering', () => {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
e2e/react-start/basic/package.jsone2e/react-start/basic/playwright.config.tse2e/react-start/basic/server.jse2e/react-start/basic/src/routeTree.gen.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/src/routes/specialChars/search.tsxe2e/react-start/basic/src/routes/specialChars/대한민국.tsxe2e/react-start/basic/src/routes/대한민국.tsxe2e/react-start/basic/tests/params.spec.tse2e/react-start/basic/tests/prerendering.spec.tse2e/react-start/basic/tests/special-characters.spec.tse2e/react-start/basic/vite.config.tse2e/react-start/virtual-routes/routes.tse2e/react-start/virtual-routes/src/routeTree.gen.tse2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/root.tsxe2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/package.jsone2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/server.jse2e/solid-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/basic/src/routes/specialChars/대한민국.tsxe2e/solid-start/basic/src/routes/대한민국.tsxe2e/solid-start/basic/tests/params.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/special-characters.spec.tse2e/solid-start/basic/vite.config.tse2e/solid-start/virtual-routes/routes.tse2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/src/routes/root.tsxe2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/package.jsone2e/vue-start/basic/playwright.config.tse2e/vue-start/basic/server.jse2e/vue-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/대한민국.tsxe2e/vue-start/basic/tests/params.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/vite.config.tse2e/vue-start/virtual-routes/routes.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/virtual-routes/src/routes/root.tsxe2e/vue-start/virtual-routes/tests/special-characters.spec.tspackages/start-server-core/src/createStartHandler.ts
💤 Files with no reviewable changes (5)
- e2e/solid-start/basic/src/routes/대한민국.tsx
- e2e/vue-start/basic/tests/params.spec.ts
- e2e/react-start/basic/src/routes/대한민국.tsx
- e2e/solid-start/basic/tests/params.spec.ts
- e2e/react-start/basic/tests/params.spec.ts
🧰 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/virtual-routes/src/routeTree.gen.tse2e/vue-start/basic/vite.config.tse2e/react-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/react-start/basic/src/routes/specialChars/대한민국.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/basic/playwright.config.tse2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/virtual-routes/src/routes/root.tsxe2e/react-start/virtual-routes/src/routes/root.tsxe2e/solid-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/virtual-routes/src/routes/root.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/vite.config.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/playwright.config.tse2e/react-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxpackages/start-server-core/src/createStartHandler.tse2e/solid-start/virtual-routes/routes.tse2e/vue-start/basic/src/routes/specialChars/대한민국.tsxe2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/basic/src/routes/specialChars/대한민국.tsxe2e/solid-start/basic/vite.config.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.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/virtual-routes/src/routeTree.gen.tse2e/vue-start/basic/vite.config.tse2e/react-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/react-start/basic/src/routes/specialChars/대한민국.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/basic/playwright.config.tse2e/react-start/basic/server.jse2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/virtual-routes/src/routes/root.tsxe2e/react-start/virtual-routes/src/routes/root.tsxe2e/solid-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/virtual-routes/src/routes/root.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/vite.config.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/playwright.config.tse2e/react-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/server.jse2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxpackages/start-server-core/src/createStartHandler.tse2e/solid-start/virtual-routes/routes.tse2e/vue-start/basic/src/routes/specialChars/대한민국.tsxe2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/basic/server.jse2e/solid-start/basic/src/routes/specialChars/대한민국.tsxe2e/solid-start/basic/vite.config.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol
workspace:*for internal dependencies in package.json files
Files:
e2e/vue-start/basic/package.jsone2e/solid-start/basic/package.jsone2e/react-start/basic/package.json
🧠 Learnings (15)
📚 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/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/src/routes/root.tsxe2e/react-start/virtual-routes/src/routes/root.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routes/specialChars/route.tsxe2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/solid-start/virtual-routes/routes.tse2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.ts
📚 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/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/virtual-routes/src/routes/root.tsxe2e/solid-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/tests/special-characters.spec.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/react-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/virtual-routes/routes.tse2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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/react-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/basic/vite.config.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/react-start/basic/src/routes/specialChars/대한민국.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/basic/vite.config.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.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/virtual-routes/src/routeTree.gen.tse2e/vue-start/basic/vite.config.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/src/routes/root.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routes/specialChars/route.tsxe2e/solid-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/solid-start/virtual-routes/routes.tse2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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 **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/react-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/routes.tse2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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/react-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/basic/package.jsone2e/react-start/basic/server.jse2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/basic/vite.config.tse2e/vue-start/basic/server.jse2e/react-start/virtual-routes/routes.tspackages/start-server-core/src/createStartHandler.tse2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/solid-start/basic/server.jse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/react-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routes/pipe.tsxe2e/solid-start/virtual-routes/src/routes/pipe.tsxe2e/react-start/virtual-routes/src/routes/pipe.tsxe2e/vue-start/basic/src/routes/specialChars/route.tsxe2e/solid-start/virtual-routes/src/routeTree.gen.tse2e/vue-start/virtual-routes/src/routeTree.gen.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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/vue-start/basic/vite.config.tse2e/react-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/routes.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/solid-start/virtual-routes/routes.tse2e/solid-start/basic/vite.config.tse2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/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/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/tests/special-characters.spec.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/react-start/basic/tests/special-characters.spec.ts
📚 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-start/basic/tests/prerendering.spec.tse2e/solid-start/basic/tests/prerendering.spec.tse2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/tests/prerendering.spec.tse2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/tests/special-characters.spec.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/react-start/basic/tests/special-characters.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-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/vue-start/basic/src/routes/specialChars/$param.tsxe2e/vue-start/basic/tests/special-characters.spec.tse2e/react-start/basic/src/routes/specialChars/route.tsxe2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/react-start/basic/tests/special-characters.spec.tse2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
e2e/react-start/virtual-routes/tests/special-characters.spec.tse2e/vue-start/basic/package.jsone2e/solid-start/basic/package.jsone2e/solid-start/virtual-routes/tests/special-characters.spec.tse2e/solid-start/basic/tests/special-characters.spec.tse2e/vue-start/basic/tests/special-characters.spec.tse2e/vue-start/virtual-routes/tests/special-characters.spec.tse2e/react-start/basic/tests/special-characters.spec.tse2e/react-start/basic/package.json
📚 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: Implement type-safe routing with search params and path params
Applied to files:
e2e/vue-start/basic/src/routes/specialChars/search.tsxe2e/react-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/basic/src/routes/specialChars/search.tsxe2e/solid-start/basic/src/routes/specialChars/$param.tsxe2e/react-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-12-24T22:47:44.320Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6211
File: e2e/react-start/i18n-paraglide/src/server.ts:6-6
Timestamp: 2025-12-24T22:47:44.320Z
Learning: In TanStack Router projects using `inlang/paraglide-js`, the callback passed to `paraglideMiddleware` should use `() => handler.fetch(req)` (referencing the outer `req`) instead of `({ request }) => handler.fetch(request)`. This is intentional because the router needs the untouched URL to perform its own rewrite logic with `deLocalizeUrl`/`localizeUrl`. The middleware's processed request would delocalize the URL and interfere with the router's rewrite handling.
Applied to files:
packages/start-server-core/src/createStartHandler.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/start-server-core/src/createStartHandler.ts
🧬 Code graph analysis (17)
e2e/react-start/basic/src/routes/specialChars/$param.tsx (2)
e2e/react-start/basic/src/routes/specialChars/route.tsx (1)
Route(4-6)e2e/solid-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)
e2e/vue-start/basic/src/routes/specialChars/search.tsx (3)
e2e/react-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)e2e/solid-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)e2e/vue-start/basic/src/routes/specialChars/route.tsx (1)
Route(4-6)
e2e/vue-start/virtual-routes/src/routes/pipe.tsx (2)
e2e/react-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)e2e/solid-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)
e2e/react-start/basic/server.js (2)
e2e/vue-start/basic/server.js (6)
app(16-16)app(34-34)createSpaServer(33-59)port(8-8)createStartServer(12-31)startPort(10-10)e2e/vue-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/src/routes/specialChars/search.tsx (2)
e2e/solid-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)e2e/vue-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)
e2e/solid-start/virtual-routes/src/routes/pipe.tsx (2)
e2e/react-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)e2e/vue-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)
e2e/react-start/virtual-routes/src/routes/pipe.tsx (2)
e2e/solid-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)e2e/vue-start/virtual-routes/src/routes/pipe.tsx (1)
Route(3-5)
e2e/solid-start/basic/src/routes/specialChars/search.tsx (3)
e2e/react-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)e2e/solid-start/basic/src/routes/specialChars/route.tsx (1)
Route(4-6)e2e/vue-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)
e2e/vue-start/basic/src/routes/specialChars/$param.tsx (2)
e2e/react-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)e2e/solid-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)
e2e/react-start/basic/src/routes/specialChars/route.tsx (2)
e2e/react-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)e2e/react-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)
e2e/solid-start/basic/src/routes/specialChars/route.tsx (2)
e2e/solid-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)e2e/solid-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)
e2e/vue-start/basic/server.js (2)
e2e/react-start/basic/server.js (5)
app(16-16)app(34-34)createSpaServer(33-59)port(8-8)startPort(10-10)e2e/vue-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/vue-start/basic/src/routes/specialChars/route.tsx (4)
e2e/react-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)e2e/react-start/basic/src/routes/specialChars/route.tsx (1)
Route(4-6)e2e/vue-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)e2e/vue-start/basic/src/routes/specialChars/search.tsx (1)
Route(4-9)
e2e/vue-start/virtual-routes/routes.ts (1)
packages/virtual-file-routes/src/api.ts (1)
route(66-84)
e2e/react-start/virtual-routes/routes.ts (1)
packages/virtual-file-routes/src/api.ts (1)
route(66-84)
e2e/solid-start/basic/src/routes/specialChars/$param.tsx (1)
e2e/react-start/basic/src/routes/specialChars/$param.tsx (1)
Route(3-5)
e2e/solid-start/basic/server.js (1)
e2e/vue-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
⏰ 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 (51)
packages/start-server-core/src/createStartHandler.ts (4)
9-13: Import change looks good.
86-90: Simplified manifest lookup looks fine.
333-350: executeRouter simplification + manifest usage looks solid.
497-563: Router execution wiring in server routes looks consistent.e2e/vue-start/basic/playwright.config.ts (1)
19-19: SPA startup command alignment looks good.
Using the sharedpnpm startscript keeps SPA mode consistent with other modes and scripts.e2e/solid-start/basic/playwright.config.ts (1)
19-19: SPA startup command alignment looks good.
The unifiedpnpm startusage is consistent with other configurations.e2e/react-start/basic/vite.config.ts (1)
25-25: Prerender exclusion update is appropriate.
Excluding/specialChars/searchmatches the new route structure and avoids mismatched prerender output.e2e/vue-start/basic/package.json (1)
13-13: Start script consolidation looks good.
Directly invokingnode server.jskeeps the start path straightforward and predictable.e2e/react-start/basic/tests/prerendering.spec.ts (1)
20-22: Updated prerender assertion aligns with new route layout.
The expected path now matches the SpecialChars route grouping.e2e/solid-start/basic/vite.config.ts (1)
18-29: Prerender exclusion update looks good.
Adding/specialChars/searchkeeps prerendering aligned with the new SpecialChars route group.e2e/react-start/basic/playwright.config.ts (1)
19-20: SPA command update looks good.
The SPA mode command now matches the unified start flow.e2e/react-start/basic/src/routes/specialChars/대한민국.tsx (1)
1-8: Route addition looks good.
Simple and clear route component for the new non‑Latin path.e2e/solid-start/basic/package.json (1)
13-13: Start script update looks good.
node server.jsaligns with the new startup flow.e2e/solid-start/basic/src/routes/specialChars/route.tsx (1)
1-45: Good coverage for special-character navigation examples.The links and nested outlet provide clear fixtures for Unicode path, param, and search-param cases.
e2e/vue-start/basic/src/routes/specialChars/search.tsx (1)
1-22: Looks good for search-param validation and rendering.The search validation and display wiring is consistent with the route intent.
e2e/react-start/virtual-routes/src/routes/pipe.tsx (1)
1-11: Simple and correct pipe route fixture.The route and component are straightforward and match the test intent.
e2e/solid-start/virtual-routes/src/routes/pipe.tsx (1)
1-11: Solid pipe route fixture looks good.Clear and minimal component for the special-character route.
e2e/react-start/basic/src/routes/specialChars/search.tsx (1)
1-19: Looks good: /specialChars/search route wiring.Clear route setup and rendering for the special-character search test.
e2e/react-start/basic/server.js (1)
18-20: Mode-specific startup flow looks good.The conditional SPA vs Start boot sequence and prerender-aware static serving read cleanly.
Also applies to: 61-79
e2e/react-start/basic/src/routes/specialChars/route.tsx (1)
1-44: SpecialChars navigation setup looks good.The links exercise unicode path, param, and search variants with clear test hooks.
e2e/vue-start/basic/server.js (1)
18-20: Mode-specific startup flow looks good.The conditional SPA vs Start boot sequence and prerender-aware static serving read cleanly.
Also applies to: 61-79
e2e/vue-start/basic/vite.config.ts (1)
16-29: Prerender exclusion update looks good.Excluding
/specialChars/searchmatches the dynamic search-param behavior.e2e/react-start/basic/src/routes/specialChars/$param.tsx (1)
1-15: LGTM!Clean implementation of a test route for special characters. The component correctly uses
Route.useParams()to access the dynamicparamand renders it with adata-testidfor e2e testing. This aligns with the PR objective of testing Unicode and special character handling in route parameters.e2e/solid-start/basic/src/routes/specialChars/search.tsx (1)
1-20: LGTM!Correct Solid-specific implementation. The use of
search().searchParamproperly accesses the reactive signal returned byRoute.useSearch(), which is the idiomatic pattern for SolidJS. The zod validation ensures type-safe search parameters.e2e/solid-start/basic/src/routes/specialChars/$param.tsx (1)
1-15: LGTM!Correct Solid-specific implementation. The
params().paramaccess pattern properly handles SolidJS reactivity whereuseParams()returns an accessor function. This is consistent with the search route in the same directory.e2e/solid-start/basic/server.js (1)
18-20: LGTM!Good addition of the explanatory comment. The
redirect: !isPrerenderoption correctly disables Express's automatic trailing-slash redirects during prerender testing, ensuring uniform test behavior.e2e/vue-start/basic/src/routes/specialChars/route.tsx (1)
8-45: LGTM!Well-structured test route that exercises the key scenarios from the PR objectives:
- Non-Latin Unicode path (
대한민국)- Unicode + pipe character in route params (
대|)- Unicode + pipe character in search params
The
data-testidattributes enable reliable e2e testing, and the use ofclass(vsclassName) inactivePropsis correct for Vue.e2e/vue-start/basic/src/routes/specialChars/$param.tsx (1)
1-15: LGTM!The route component is well-structured and consistent with the React and Solid counterparts. The Vue-specific reactive access pattern (
params.value.param) is correctly used, and thedata-testidattribute aligns with the e2e test expectations.e2e/vue-start/basic/tests/special-characters.spec.ts (1)
1-104: Comprehensive test coverage for special character routing.The test suite thoroughly covers both direct navigation and router-driven navigation for non-Latin characters and special characters in paths and search params. The intentional difference in expected URL encoding between direct navigation (Line 76:
searchParam=%EB%8C%80|) and router navigation (Line 94:searchParam=%EB%8C%80%7C) correctly documents the browser/server encoding differences that this PR aims to normalize.e2e/react-start/basic/tests/special-characters.spec.ts (1)
1-104: LGTM!The test suite is consistent with the Vue counterpart, ensuring uniform behavior across frameworks for special character handling. The coverage for direct navigation, router navigation, path params, and search params is comprehensive.
e2e/react-start/basic/package.json (2)
13-19: LGTM - Server switch and cleanup scripts are appropriate.The change from
srvxtonode server.jsaligns with the PR's goal of fixing non-latin asset filename encoding. The added cleanup commands (rm -rf dist; rm -rf port*.txt;) ensure consistent test state. Based on learnings, Unix-specific commands are acceptable for e2e test scripts in this repository.
44-44:srvxis actively used and should not be removed.The
server.jsfile imports and usessrvx(line 1:import { toNodeHandler } from 'srvx/node') to convert the server's fetch handler into a Node.js-compatible handler. Removing this dependency would break the start server functionality.Likely an incorrect or invalid review comment.
e2e/vue-start/virtual-routes/src/routes/root.tsx (1)
72-80: LGTM!The new pipe route Link follows the established pattern of other navigation links in the file, includes a proper
data-testidfor e2e testing, and aligns with the PR objective of testing special character (|) handling in routes.e2e/solid-start/virtual-routes/src/routes/root.tsx (1)
79-87: LGTM!The new pipe route Link follows the established pattern of other navigation links in the file, includes a proper
data-testidfor e2e testing, and is consistent with the Vue implementation. This aligns with the PR objective of testing special character (|) handling in virtual routes where|cannot be used in Windows filenames.e2e/solid-start/virtual-routes/routes.ts (1)
24-24: LGTM!The new route correctly tests pipe character handling in paths via virtual routes, which is the appropriate approach since
|cannot be used in Windows filenames. This aligns with the PR objective to verify special character encoding normalization.e2e/react-start/virtual-routes/src/routes/root.tsx (1)
79-87: LGTM!The navigation link follows the established pattern and includes the appropriate
data-testidfor e2e test targeting. The unencoded pipe character in thetoprop is correct—the router handles encoding normalization as intended by this PR.e2e/vue-start/virtual-routes/routes.ts (1)
24-24: LGTM!Consistent with the React and Solid Start implementations, correctly extending the Vue virtual routes to cover pipe character handling.
e2e/react-start/virtual-routes/routes.ts (1)
24-24: LGTM!The route definition is consistent with the Vue and Solid Start implementations, providing proper test coverage for pipe character handling in React Start virtual routes.
e2e/react-start/virtual-routes/tests/special-characters.spec.ts (2)
17-27: LGTM!The direct navigation test correctly validates the core bug fix from issue
#6288—navigating directly to a route with a pipe character no longer causes redirect loops. The test properly verifies URL encoding normalization (|→%7C) and element visibility.
29-41: LGTM!The router navigation test provides good coverage by verifying that in-app navigation (which already worked) continues to function correctly alongside the direct navigation fix.
e2e/solid-start/basic/tests/special-characters.spec.ts (4)
1-12: Well-structured test setup.Good use of
whitelistErrorsto handle expected 404 responses and abeforeEachhook to establish consistent starting state.
14-38: Non-latin route tests look correct.Both direct navigation and router navigation tests properly validate URL encoding of Korean characters (
대한민국→%EB%8C%80%ED%95%9C%EB%AF%BC%EA%B5%AD) and element visibility.
40-66: Path param tests with special characters look correct.Good coverage of the pipe character (
|) combined with non-latin characters in path parameters. The decoded param assertion (expect(param).toBe('대|')) confirms the fix from issue#6288is working.
68-103: Verify intentional encoding asymmetry in search params.Line 76 expects the pipe unencoded (
searchParam=%EB%8C%80|) on direct navigation, while line 94 expects it encoded (searchParam=%EB%8C%80%7C) on router navigation. This appears to document the actual browser vs router encoding behavior difference mentioned in the PR objectives. Please confirm this asymmetry is intentional and reflects expected runtime behavior.e2e/vue-start/virtual-routes/src/routes/pipe.tsx (1)
1-11: Consistent implementation across frameworks.The route definition matches the React and Solid implementations, using the same path (
/special|pipe) and test ID (special-pipe-route-heading). This ensures consistent e2e test coverage across all framework starters.e2e/solid-start/basic/src/routes/specialChars/대한민국.tsx (1)
1-9: Clean route implementation.The route properly defines the non-latin path segment and renders a testable component. The consolidation under the
specialCharsgroup improves test organization.e2e/vue-start/virtual-routes/tests/special-characters.spec.ts (1)
1-43: Well-structured pipe character tests.The tests properly cover both direct and router navigation for the pipe character edge case (issue
#6288). The URL encoding assertion (/special%7Cpipe) confirms the normalization fix is working correctly.e2e/solid-start/virtual-routes/tests/special-characters.spec.ts (1)
1-43: Consistent test implementation with Vue counterpart.The test structure mirrors the Vue virtual-routes test, ensuring consistent coverage of the pipe character fix across frameworks. This consistency aids maintainability.
e2e/vue-start/virtual-routes/src/routeTree.gen.ts (1)
1-347: Skipping review of autogenerated file.This file is automatically generated by TanStack Router and should not be manually modified or reviewed. The changes correctly add the new
/special|piperoute to support testing pipe characters in route paths. Based on learnings,routeTree.gen.tsfiles should not be reviewed.e2e/react-start/virtual-routes/src/routeTree.gen.ts (1)
1-347: Skipping review of autogenerated file.This file is automatically generated by TanStack Router and should not be manually modified or reviewed. The changes correctly add the new
/special|piperoute to support testing pipe characters in route paths. Based on learnings,routeTree.gen.tsfiles should not be reviewed.e2e/solid-start/basic/src/routeTree.gen.ts (1)
1-1228: Skipping review of autogenerated file.This file is automatically generated by TanStack Router and should not be manually modified or reviewed. The changes correctly add the new
/specialCharsroute group with subroutes for testing Unicode characters (/대한민국) and dynamic params (/$param,/search). Based on learnings,routeTree.gen.tsfiles should not be reviewed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
fixes #6288 and follows on the work done in #6293
Server runtimes and browser runtimes can decode/encode characters differently in paths and search params. One such example is the
|character. Server runtimes generally strictly follow the WHATWG URL Standard, while browsers may differ for legacy reasons. This behavior also differs from browser to browser. For example, in url paths|is not encoded on the server but is encoded on chromium, but not on firefox, while "대" is encoded on both sides.Another anomaly is that in Node
new URLSearchParams()andnew URL()also decode/encode characters differently.new URLSearchParams()encodes|whilenew URL()does not, and in this instance chromium treats search params differently than paths, i.e.|is not encoded in search params.This PR addresses these disparities between how browsers and server runtimes can handle character encoding. It normalizes the URL when received by start during direct navigation. This issue does not affect router since the runtime (browser) does not change.
In addition, this PR also adds additional tests for testing both direct and router navigations to routes that either has non-latin and/or special printable characters and unifies these tests under a new test route (
/대한민국was moved to this route).Since pipe signs are not allowed in filenames on Windows file systems the test for
|in the url path was added to the virtual-routes e2e test and not included in the above set in start-basic-e2e tests.One final change in this PR is the server that is being used to serve the start app for testing. During the update of these tests it was picked up that srvx does not resolve non-latin character filenames in a decoded form i.e.
/대한민국would during the build step be built as/assets/대한민국-[hash].jsbut it would be served asassets/%EB%8C%80%ED%95%9C%EB%AF%BC%EA%B5%AD-[hash].jsresulting in a failure to load the asset. Previous tests did not wait for the page to finish loading the assets before testing, and hence the tests passed. By waiting for the page to complete loading before verifying the tests, the page would load and now error since the asset could not be loaded resulting in the tests failing correctly. Changing the test server from srvx to express resolved this issue, the asset is loaded correctly, and the tests pass.Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.