-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(router-core): consider __root__ in path parsing of trailing underscores #5262
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
Conversation
WalkthroughAdjusts router-core path parsing to preserve Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Playwright
participant CFG as playwright.config.ts
participant Vite as Dev Server
participant App as App (Vite)
participant Router as TanStack Router
Dev->>CFG: start tests
CFG->>CFG: read MODE -> isSpaMode
CFG->>Vite: start webServer (spaModeCommand | ssrModeCommand) with env
Vite-->>Dev: server ready
Dev->>App: open URL
App->>Router: initialize routes (file-route registration)
Router->>App: render matched components
note over App,Router: RouterDevtools loads inside Suspense (dev only)
sequenceDiagram
participant File as Route file
participant API as createFileRoute
participant Router as Router runtime
File->>API: createFileRoute('/path')({ config })
API-->>File: Route (path-bound)
File->>Router: export/register Route
Router-->>Router: register route with explicit path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
|
View your CI Pipeline Execution ↗ for commit ebb9887
☁️ 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-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: |
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: 27
🧹 Nitpick comments (33)
e2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsx (1)
11-14: Preserve other search params and avoid extra history entries on default redirect.Use functional search to merge existing params and set replace: true to prevent an extra history entry.
- throw redirect({ - to: '/search-params/loader-throws-redirect', - search: { step: 'a' }, - }) + throw redirect({ + to: '/search-params/loader-throws-redirect', + search: (prev) => ({ ...prev, step: 'a' }), + replace: true, + })e2e/react-start/basic-spa/src/routes/inline-scripts.tsx (1)
4-16: Optional: add stable IDs to inline scripts for robust de‑duplicationIf head script de‑dup relies on keys, giving each inline script an id helps prevent duplicates across SSR/CSR hydration.
Apply this diff:
{ + id: 'inline-script-1', children: 'window.INLINE_SCRIPT_1 = true; console.log("Inline script 1 executed");', }, { + id: 'inline-script-2', children: 'window.INLINE_SCRIPT_2 = "test"; console.log("Inline script 2 executed");', type: 'text/javascript', },e2e/react-start/basic-spa/src/routes/search-params/default.tsx (1)
8-17: Optional: DRY the repeated context checkThe beforeLoad and loader checks are identical. Extract into a small helper for clarity.
Apply this diff in-place:
- beforeLoad: ({ context }) => { - if (context.hello !== 'world') { - throw new Error('Context hello is not "world"') - } - }, - loader: ({ context }) => { - if (context.hello !== 'world') { - throw new Error('Context hello is not "world"') - } - }, + beforeLoad: ({ context }) => assertWorld(context), + loader: ({ context }) => assertWorld(context),Add this helper near the top of the file:
const assertWorld = (context: { hello?: unknown }) => { if (context.hello !== 'world') { throw new Error('Context hello is not "world"') } }e2e/react-start/basic-spa/src/utils/users.tsx (1)
1-5: Nit: this utility doesn’t use JSX; consider .ts instead of .tsx.Slightly improves build ergonomics and signals intent.
packages/router-core/src/path.ts (2)
279-281: Good fix: preserve "root" while stripping trailing underscores elsewhere.This aligns with the intended behavior (strip on route segments, preserve special/base segments). Consider avoiding the magic string and using a shared constant, and prefer
endsWithfor clarity.Apply this diff:
- !basePathValues && part !== '__root__' && part.slice(-1) === '_' + !basePathValues && part !== ROOT_ROUTE_ID && part.endsWith('_') ? part.slice(0, -1) : partAdd near the top of this module (or import from a shared constants file if one exists):
const ROOT_ROUTE_ID = '__root__'This reduces typos and centralizes the identifier. Based on learnings.
214-225: Cache key likely conflates base vs route parsing (potentially incorrect results).
parsePathnamecaches bypathnameonly, but parsing differs whenbasePathValuesis true vs false. If the same string is parsed both ways sharing the same cache, results can be wrong. Safer to include the mode in the cache key.Apply this diff:
export const parsePathname = ( pathname?: string, cache?: ParsePathnameCache, basePathValues?: boolean, ): ReadonlyArray<Segment> => { if (!pathname) return [] - const cached = cache?.get(pathname) + const cacheKey = `${basePathValues ? 'b' : 'r'}|${pathname}` + const cached = cache?.get(cacheKey) if (cached) return cached const parsed = baseParsePathname(pathname, basePathValues) - cache?.set(pathname, parsed) + cache?.set(cacheKey, parsed) return parsed }Please verify if a shared
parseCacheinstance is passed to bothparseBasePathSegmentsandparseRoutePathSegmentsin any hot paths. If yes, this change prevents subtle mismatches. Based on learnings.e2e/react-start/basic-spa/src/styles/app.css (1)
1-22: LGTM: Tailwind layers and dark mode base styles.The
.using-mouse * { outline: none }pattern is acceptable for e2e. If this ever ships to prod, consider preferring:focus-visibleutilities to preserve keyboard focus indicators.e2e/react-start/basic-spa/src/utils/seo.ts (1)
12-33: Avoid emitting meta tags with undefined content.Guard optional fields so consumers don’t get
content="undefined".Apply this diff:
- const tags = [ - { title }, - { name: 'description', content: description }, - { name: 'keywords', content: keywords }, - { name: 'twitter:title', content: title }, - { name: 'twitter:description', content: description }, - { name: 'twitter:creator', content: '@tannerlinsley' }, - { name: 'twitter:site', content: '@tannerlinsley' }, - { name: 'og:type', content: 'website' }, - { name: 'og:title', content: title }, - { name: 'og:description', content: description }, - ...(image - ? [ - { name: 'twitter:image', content: image }, - { name: 'twitter:card', content: 'summary_large_image' }, - { name: 'og:image', content: image }, - ] - : []), - ] + const tags = [ + { title }, + ...(description ? [{ name: 'description', content: description }] : []), + ...(keywords ? [{ name: 'keywords', content: keywords }] : []), + { name: 'twitter:title', content: title }, + ...(description ? [{ name: 'twitter:description', content: description }] : []), + { name: 'twitter:creator', content: '@tannerlinsley' }, + { name: 'twitter:site', content: '@tannerlinsley' }, + { name: 'og:type', content: 'website' }, + { name: 'og:title', content: title }, + ...(description ? [{ name: 'og:description', content: description }] : []), + ...(image + ? [ + { name: 'twitter:image', content: image }, + { name: 'twitter:card', content: 'summary_large_image' }, + { name: 'og:image', content: image }, + ] + : []), + ]e2e/react-start/basic-spa/src/components/RedirectOnClick.tsx (1)
17-24: Prevent accidental form submit and unhandled promise rejections
- Add type="button" to avoid submitting if used inside a form.
- Handle the promise so redirect rejections don’t surface as unhandled errors.
Apply this diff:
return ( <button + type="button" data-testid="redirect-on-click" - onClick={() => - execute({ data: { target, reloadDocument, externalHost } }) - } + onClick={async () => { + try { + await execute({ data: { target, reloadDocument, externalHost } }) + } catch { + // Redirect is handled by the framework + } + }} > click me </button> )e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx (1)
1-10: Validate and coerce search params to expected typesEnsure reloadDocument is a boolean (not a "true"/"false" string) and externalHost is a string. Add validateSearch with zod for predictable behavior in SPA mode.
Apply this diff:
-import { createFileRoute } from '@tanstack/react-router' +import { createFileRoute } from '@tanstack/react-router' +import { z } from 'zod' import { throwRedirect } from '~/components/throwRedirect' export const Route = createFileRoute('/redirect/$target/serverFn/via-beforeLoad')({ + validateSearch: z.object({ + reloadDocument: z.boolean().optional(), + externalHost: z.string().optional(), + }), beforeLoad: ({ params: { target }, search: { reloadDocument, externalHost }, }) => throwRedirect({ data: { target, reloadDocument, externalHost } }), component: () => <div>{Route.fullPath}</div>, })e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsx (1)
9-11: Optional: await the serverFn for clarityThe router will await the loader’s returned promise, but making it explicit reads clearer.
- loader: ({ params: { target }, deps: { reloadDocument, externalHost } }) => - throwRedirect({ data: { target, reloadDocument, externalHost } }), + loader: async ({ + params: { target }, + deps: { reloadDocument, externalHost }, + }) => { + await throwRedirect({ data: { target, reloadDocument, externalHost } }) + },e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx (1)
17-17: Gate console.error in productionConsole noise in CI/production can be distracting; consider gating behind an env check.
- console.error(error) + if (import.meta.env.DEV) console.error(error)e2e/react-start/basic-spa/src/routes/scripts.tsx (1)
6-16: Avoid undefined entries and prefer absolute paths for public assetsPrevent undefined in scripts array and use absolute paths for stability with varying base URLs.
head: () => ({ scripts: [ - { - src: 'script.js', - }, - isProd - ? undefined - : { - src: 'script2.js', - }, + { src: '/script.js' }, + ...(isProd ? [] : [{ src: '/script2.js' }]), ], }),e2e/react-start/basic-spa/src/components/throwRedirect.ts (1)
12-23: Optional: validate external URL before redirectGuard against malformed externalHost by validating it; fall back to example.com if invalid. Keeps behavior robust for test inputs.
- .handler((ctx) => { + .handler((ctx) => { if (ctx.data.target === 'internal') { throw redirect({ to: '/posts', reloadDocument: ctx.data.reloadDocument, }) } - const href = ctx.data.externalHost ?? 'http://example.com' + const href = (() => { + const candidate = ctx.data.externalHost ?? 'http://example.com' + try { + return new URL(candidate).toString() + } catch { + return 'http://example.com' + } + })() throw redirect({ href, }) })e2e/react-start/basic-spa/src/routes/users.tsx (2)
24-27: Keepidtype consistent withUserAppending a string
idwidens the array element type and weakens type safety. Use a numeric sentinel to stay compatible withUser.- {[ - ...users, - { id: 'i-do-not-exist', name: 'Non-existent User', email: '' }, - ].map((user) => { + {[ + ...users, + { id: -1, name: 'Non-existent User', email: '' }, + ].map((user) => {
7-14: Minor: simplify loader with try/catchReadability nit: using try/catch avoids promise chaining.
- loader: async () => { - return await axios - .get<Array<User>>('/api/users') - .then((r) => r.data) - .catch(() => { - throw new Error('Failed to fetch users') - }) - }, + loader: async () => { + try { + const { data } = await axios.get<Array<User>>('/api/users') + return data + } catch { + throw new Error('Failed to fetch users') + } + },e2e/react-start/basic-spa/src/routes/posts.tsx (1)
13-13: Remove redundantasyncNo
awaitused; keep it simple.- loader: async () => fetchPosts(), + loader: () => fetchPosts(),e2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsx (1)
12-14: Guard against undefinedexternalHostRedirecting with
href: undefinedcan throw; validate before redirect.- case 'external': - throw redirect({ href: externalHost }) + case 'external': + if (!externalHost) { + throw new Error('externalHost is required for external redirects') + } + throw redirect({ href: externalHost })e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsx (1)
6-8: Provide a safe default forreloadDocumentPrevents passing
undefinedinto the server fn flow.- const { reloadDocument, externalHost } = Route.useSearch() + const { reloadDocument = false, externalHost } = Route.useSearch()e2e/react-start/basic-spa/src/routes/api/users.$id.ts (2)
18-18: Minor: prefer template strings for URL assembly.Slightly clearer and avoids implicit string coercion.
- const res = await axios.get<User>(`${queryURL}/users/` + params.id) + const res = await axios.get<User>(`${queryURL}/users/${params.id}`)
11-15: Optional: deduplicate env base-URL selection.This logic appears in multiple files; consider extracting a getExternalApiBaseUrl() helper to keep it consistent.
e2e/react-start/basic-spa/tests/not-found.spec.ts (1)
41-43: Use Playwright's waitForTimeout instead of manual Promise.Cleaner and integrates with Playwright timing controls.
- await new Promise((r) => setTimeout(r, 250)) + await page.waitForTimeout(250)e2e/react-start/basic-spa/tests/navigation.spec.ts (1)
17-17: Make the post link selector less brittle.Use a case-insensitive regex to reduce reliance on an exact truncated title.
- await page.getByRole('link', { name: 'sunt aut facere repe' }).click() + await page.getByRole('link', { name: /sunt aut facere/i }).click()e2e/react-start/basic-spa/src/utils/posts.tsx (1)
11-15: Optional: centralize external API base URL logic.Extract to a shared util to avoid drift with other modules (e.g., users route).
e2e/react-start/basic-spa/playwright.config.ts (2)
6-10: Avoid JSON import attributes to keep config compatible across Node/TS setups
import packageJson ... with { type: 'json' }requires ESM + Node support for import attributes. This can break under Node 18 or TS configs without resolveJsonModule/import attributes. Prefer reading package.json via fs to avoid env fragility. Also confirm CI uses a Node/TS combo that supports top‑level await in TS config files.Proposed change:
-import packageJson from './package.json' with { type: 'json' } +import fs from 'node:fs' + +const pkgName: string = JSON.parse( + fs.readFileSync(new URL('./package.json', import.meta.url), 'utf-8'), +).name + -const PORT = await getTestServerPort(packageJson.name) -const EXTERNAL_PORT = await getDummyServerPort(packageJson.name) +const PORT = await getTestServerPort(pkgName) +const EXTERNAL_PORT = await getDummyServerPort(pkgName)Verification checklist:
- CI Node version >= 20 if you want to keep import attributes and top-level await in TS configs.
- tsconfig for this e2e fixture: "moduleResolution" supports ESM, and "resolveJsonModule": true if sticking with JSON imports. As per coding guidelines (TypeScript strict mode), keep type-safety intact.
28-33: Set env via webServer.env and add a timeout for resiliencyInlining env vars in the command is brittle across shells. Prefer webServer.env and add a timeout to avoid indefinite waits if the dev server fails.
Apply:
webServer: { - command: `VITE_EXTERNAL_PORT=${EXTERNAL_PORT} VITE_SERVER_PORT=${PORT} PORT=${PORT} pnpm run dev:e2e --port=${PORT}`, + command: `pnpm run dev:e2e --port=${PORT}`, url: baseURL, reuseExistingServer: !process.env.CI, stdout: 'pipe', + timeout: 120_000, + env: { + VITE_EXTERNAL_PORT: String(EXTERNAL_PORT), + VITE_SERVER_PORT: String(PORT), + PORT: String(PORT), + }, },e2e/react-start/basic-spa/tests/script-duplication.spec.ts (6)
9-16: Make script selector robust and use locator assertions to reduce flakeExact match
script[src="script.js"]is brittle (absolute vs relative URL). Prefer ends-with andtoHaveCounton a locator.Apply:
- const scriptCount = await page.evaluate(() => { - return document.querySelectorAll('script[src="script.js"]').length - }) - - expect(scriptCount).toBe(1) + await expect(page.locator('script[src$="/script.js"]')).toHaveCount(1)
26-41: Repeat the robust selector pattern for client-side nav checksSame concern as above; use locator + toHaveCount for both checks.
Apply:
- const firstNavCount = await page.evaluate(() => { - return document.querySelectorAll('script[src="script.js"]').length - }) - expect(firstNavCount).toBe(1) + await expect(page.locator('script[src$="/script.js"]')).toHaveCount(1) @@ - const secondNavCount = await page.evaluate(() => { - return document.querySelectorAll('script[src="script.js"]').length - }) - expect(secondNavCount).toBe(1) + await expect(page.locator('script[src$="/script.js"]')).toHaveCount(1)
61-66: Use locator count at the end of multi-nav test to avoid timing pitfallsSwitch to locator with ends-with selector for consistency and less flakiness.
Apply:
- const finalCount = await page.evaluate(() => { - return document.querySelectorAll('script[src="script.js"]').length - }) - expect(finalCount).toBe(1) + await expect(page.locator('script[src$="/script.js"]')).toHaveCount(1)
76-96: Inline script detection by raw text is brittle; use locator hasText filterExact textContent matching can break on formatting/minification. Use locator text filters.
Apply:
- const script1Count = await page.evaluate(() => { - const scripts = Array.from(document.querySelectorAll('script:not([src])')) - return scripts.filter( - (script) => - script.textContent && - script.textContent.includes('window.INLINE_SCRIPT_1 = true'), - ).length - }) + const script1Locator = page.locator('script:not([src])', { + hasText: 'window.INLINE_SCRIPT_1 = true', + }) - const script2Count = await page.evaluate(() => { - const scripts = Array.from(document.querySelectorAll('script:not([src])')) - return scripts.filter( - (script) => - script.textContent && - script.textContent.includes('window.INLINE_SCRIPT_2 = "test"'), - ).length - }) + const script2Locator = page.locator('script:not([src])', { + hasText: 'window.INLINE_SCRIPT_2 = "test"', + }) - expect(script1Count).toBe(1) - expect(script2Count).toBe(1) + await expect(script1Locator).toHaveCount(1) + await expect(script2Locator).toHaveCount(1)
111-120: Same robustness for inline scripts during client-side navigationUse locator filters instead of manual evaluation.
Apply:
- const firstNavScript1Count = await page.evaluate(() => { - const scripts = Array.from(document.querySelectorAll('script:not([src])')) - return scripts.filter( - (script) => - script.textContent && - script.textContent.includes('window.INLINE_SCRIPT_1 = true'), - ).length - }) - expect(firstNavScript1Count).toBe(1) + await expect( + page.locator('script:not([src])', { + hasText: 'window.INLINE_SCRIPT_1 = true', + }), + ).toHaveCount(1)
139-142: Stabilize script readiness with waitForFunction when asserting globalsTo reduce flakiness, wait until globals are set before asserting.
Apply:
- // Verify the scripts are still working - expect(await page.evaluate('window.INLINE_SCRIPT_1')).toBe(true) - expect(await page.evaluate('window.INLINE_SCRIPT_2')).toBe('test') + await page.waitForFunction(() => (window as any).INLINE_SCRIPT_1 === true) + await page.waitForFunction(() => (window as any).INLINE_SCRIPT_2 === 'test')e2e/react-start/basic-spa/tests/redirect.spec.ts (1)
1-1: Optional: Prefer URLSearchParams over legacynode:querystring
node:querystringis legacy;URLSearchParamsis modern and built-in.Example:
const q = new URLSearchParams({ externalHost: `http://localhost:${EXTERNAL_HOST_PORT}/` }).toString()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
e2e/react-start/basic-spa/public/android-chrome-192x192.pngis excluded by!**/*.pnge2e/react-start/basic-spa/public/android-chrome-512x512.pngis excluded by!**/*.pnge2e/react-start/basic-spa/public/apple-touch-icon.pngis excluded by!**/*.pnge2e/react-start/basic-spa/public/favicon-16x16.pngis excluded by!**/*.pnge2e/react-start/basic-spa/public/favicon-32x32.pngis excluded by!**/*.pnge2e/react-start/basic-spa/public/favicon.icois excluded by!**/*.icoe2e/react-start/basic-spa/public/favicon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (74)
e2e/react-start/basic-spa/.gitignore(1 hunks)e2e/react-start/basic-spa/.prettierignore(1 hunks)e2e/react-start/basic-spa/package.json(1 hunks)e2e/react-start/basic-spa/playwright.config.ts(1 hunks)e2e/react-start/basic-spa/postcss.config.mjs(1 hunks)e2e/react-start/basic-spa/public/script.js(1 hunks)e2e/react-start/basic-spa/public/script2.js(1 hunks)e2e/react-start/basic-spa/public/site.webmanifest(1 hunks)e2e/react-start/basic-spa/src/client.tsx(1 hunks)e2e/react-start/basic-spa/src/components/CustomMessage.tsx(1 hunks)e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx(1 hunks)e2e/react-start/basic-spa/src/components/NotFound.tsx(1 hunks)e2e/react-start/basic-spa/src/components/RedirectOnClick.tsx(1 hunks)e2e/react-start/basic-spa/src/components/throwRedirect.ts(1 hunks)e2e/react-start/basic-spa/src/routeTree.gen.ts(1 hunks)e2e/react-start/basic-spa/src/router.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/__root.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/_layout.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/api.users.ts(1 hunks)e2e/react-start/basic-spa/src/routes/api/users.$id.ts(1 hunks)e2e/react-start/basic-spa/src/routes/deferred.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/inline-scripts.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/links.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/not-found/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/not-found/route.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/not-found/via-loader.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/posts.$postId.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/posts.index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/posts.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/scripts.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/search-params/default.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/search-params/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/search-params/route.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/stream.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/users.$userId.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/users.index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/users.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/대한민국.tsx(1 hunks)e2e/react-start/basic-spa/src/server.ts(1 hunks)e2e/react-start/basic-spa/src/styles/app.css(1 hunks)e2e/react-start/basic-spa/src/utils/posts.tsx(1 hunks)e2e/react-start/basic-spa/src/utils/seo.ts(1 hunks)e2e/react-start/basic-spa/src/utils/users.tsx(1 hunks)e2e/react-start/basic-spa/tailwind.config.mjs(1 hunks)e2e/react-start/basic-spa/tests/navigation.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/not-found.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/params.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/redirect.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/script-duplication.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/search-params.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/setup/global.setup.ts(1 hunks)e2e/react-start/basic-spa/tests/setup/global.teardown.ts(1 hunks)e2e/react-start/basic-spa/tests/streaming.spec.ts(1 hunks)e2e/react-start/basic-spa/tsconfig.json(1 hunks)e2e/react-start/basic-spa/vite.config.ts(1 hunks)packages/router-core/src/path.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic-spa/tsconfig.jsone2e/react-start/basic-spa/public/script2.jse2e/react-start/basic-spa/src/utils/users.tsxe2e/react-start/basic-spa/public/site.webmanifeste2e/react-start/basic-spa/tailwind.config.mjse2e/react-start/basic-spa/src/routes/redirect/index.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target.tsxe2e/react-start/basic-spa/src/routes/api.users.tse2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic-spa/playwright.config.tse2e/react-start/basic-spa/src/routes/posts.index.tsxe2e/react-start/basic-spa/src/components/RedirectOnClick.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsxe2e/react-start/basic-spa/vite.config.tse2e/react-start/basic-spa/src/routes/users.$userId.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic-spa/tests/params.spec.tse2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic-spa/src/routes/links.tsxe2e/react-start/basic-spa/src/routes/not-found/via-loader.tsxe2e/react-start/basic-spa/src/utils/posts.tsxe2e/react-start/basic-spa/public/script.jse2e/react-start/basic-spa/src/routes/search-params/route.tsxe2e/react-start/basic-spa/src/routes/posts.$postId.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic-spa/src/router.tsxe2e/react-start/basic-spa/src/routeTree.gen.tse2e/react-start/basic-spa/src/routes/stream.tsxe2e/react-start/basic-spa/src/routes/index.tsxe2e/react-start/basic-spa/src/styles/app.csse2e/react-start/basic-spa/src/components/NotFound.tsxe2e/react-start/basic-spa/src/utils/seo.tse2e/react-start/basic-spa/tests/redirect.spec.tse2e/react-start/basic-spa/tests/search-params.spec.tse2e/react-start/basic-spa/src/routes/users.index.tsxe2e/react-start/basic-spa/src/server.tse2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic-spa/tests/setup/global.teardown.tse2e/react-start/basic-spa/src/routes/_layout.tsxe2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic-spa/tests/script-duplication.spec.tse2e/react-start/basic-spa/src/routes/redirect/$target/index.tsxe2e/react-start/basic-spa/tests/not-found.spec.tse2e/react-start/basic-spa/src/routes/not-found/index.tsxe2e/react-start/basic-spa/src/components/CustomMessage.tsxe2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsxe2e/react-start/basic-spa/src/routes/scripts.tsxe2e/react-start/basic-spa/src/routes/search-params/index.tsxe2e/react-start/basic-spa/tests/setup/global.setup.tse2e/react-start/basic-spa/postcss.config.mjse2e/react-start/basic-spa/src/components/throwRedirect.tse2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic-spa/src/routes/posts.tsxe2e/react-start/basic-spa/tests/navigation.spec.tse2e/react-start/basic-spa/src/routes/users.tsxe2e/react-start/basic-spa/src/routes/deferred.tsxe2e/react-start/basic-spa/package.jsone2e/react-start/basic-spa/tests/streaming.spec.tse2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/대한민국.tsxe2e/react-start/basic-spa/src/routes/search-params/default.tsxe2e/react-start/basic-spa/src/client.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic-spa/src/routes/inline-scripts.tsxe2e/react-start/basic-spa/src/routes/api/users.$id.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/basic-spa/src/utils/users.tsxe2e/react-start/basic-spa/src/routes/redirect/index.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target.tsxe2e/react-start/basic-spa/src/routes/api.users.tse2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic-spa/playwright.config.tse2e/react-start/basic-spa/src/routes/posts.index.tsxpackages/router-core/src/path.tse2e/react-start/basic-spa/src/components/RedirectOnClick.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsxe2e/react-start/basic-spa/vite.config.tse2e/react-start/basic-spa/src/routes/users.$userId.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic-spa/tests/params.spec.tse2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic-spa/src/routes/links.tsxe2e/react-start/basic-spa/src/routes/not-found/via-loader.tsxe2e/react-start/basic-spa/src/utils/posts.tsxe2e/react-start/basic-spa/src/routes/search-params/route.tsxe2e/react-start/basic-spa/src/routes/posts.$postId.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic-spa/src/router.tsxe2e/react-start/basic-spa/src/routeTree.gen.tse2e/react-start/basic-spa/src/routes/stream.tsxe2e/react-start/basic-spa/src/routes/index.tsxe2e/react-start/basic-spa/src/components/NotFound.tsxe2e/react-start/basic-spa/src/utils/seo.tse2e/react-start/basic-spa/tests/redirect.spec.tse2e/react-start/basic-spa/tests/search-params.spec.tse2e/react-start/basic-spa/src/routes/users.index.tsxe2e/react-start/basic-spa/src/server.tse2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic-spa/tests/setup/global.teardown.tse2e/react-start/basic-spa/src/routes/_layout.tsxe2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic-spa/tests/script-duplication.spec.tse2e/react-start/basic-spa/src/routes/redirect/$target/index.tsxe2e/react-start/basic-spa/tests/not-found.spec.tse2e/react-start/basic-spa/src/routes/not-found/index.tsxe2e/react-start/basic-spa/src/components/CustomMessage.tsxe2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsxe2e/react-start/basic-spa/src/routes/scripts.tsxe2e/react-start/basic-spa/src/routes/search-params/index.tsxe2e/react-start/basic-spa/tests/setup/global.setup.tse2e/react-start/basic-spa/src/components/throwRedirect.tse2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic-spa/src/routes/posts.tsxe2e/react-start/basic-spa/tests/navigation.spec.tse2e/react-start/basic-spa/src/routes/users.tsxe2e/react-start/basic-spa/src/routes/deferred.tsxe2e/react-start/basic-spa/tests/streaming.spec.tse2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/대한민국.tsxe2e/react-start/basic-spa/src/routes/search-params/default.tsxe2e/react-start/basic-spa/src/client.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic-spa/src/routes/inline-scripts.tsxe2e/react-start/basic-spa/src/routes/api/users.$id.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic-spa/src/routes/redirect/index.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/redirect/$target.tsxe2e/react-start/basic-spa/src/routes/api.users.tse2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic-spa/src/routes/posts.index.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsxe2e/react-start/basic-spa/src/routes/users.$userId.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic-spa/src/routes/links.tsxe2e/react-start/basic-spa/src/routes/not-found/via-loader.tsxe2e/react-start/basic-spa/src/routes/search-params/route.tsxe2e/react-start/basic-spa/src/routes/posts.$postId.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic-spa/src/routes/stream.tsxe2e/react-start/basic-spa/src/routes/index.tsxe2e/react-start/basic-spa/src/routes/users.index.tsxe2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic-spa/src/routes/_layout.tsxe2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/index.tsxe2e/react-start/basic-spa/src/routes/not-found/index.tsxe2e/react-start/basic-spa/src/routes/scripts.tsxe2e/react-start/basic-spa/src/routes/search-params/index.tsxe2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic-spa/src/routes/posts.tsxe2e/react-start/basic-spa/src/routes/users.tsxe2e/react-start/basic-spa/src/routes/deferred.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic-spa/src/routes/대한민국.tsxe2e/react-start/basic-spa/src/routes/search-params/default.tsxe2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic-spa/src/routes/inline-scripts.tsxe2e/react-start/basic-spa/src/routes/api/users.$id.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/path.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/basic-spa/package.json
🧠 Learnings (6)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript in strict mode with extensive type safety across the codebase
Applied to files:
e2e/react-start/basic-spa/tsconfig.json
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#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/react-start/basic-spa/.prettierignore
📚 Learning: 2025-09-22T00:56:49.237Z
Learnt from: nlynzaad
PR: TanStack/router#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/router-core/src/path.ts
📚 Learning: 2025-09-22T00:56:53.426Z
Learnt from: nlynzaad
PR: TanStack/router#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/src/path.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#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/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/scripts.tsxe2e/react-start/basic-spa/src/routes/inline-scripts.tsx
🧬 Code graph analysis (47)
e2e/react-start/basic-spa/src/routes/redirect/index.tsx (2)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)
e2e/react-start/basic-spa/src/routes/redirect/$target/via-beforeLoad.tsx (2)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx (2)
e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)e2e/react-start/basic-spa/src/components/throwRedirect.ts (1)
throwRedirect(4-23)
e2e/react-start/basic-spa/src/routes/redirect/$target.tsx (1)
e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)
e2e/react-start/basic-spa/src/routes/api.users.ts (2)
e2e/react-start/basic-spa/src/routes/api/users.$id.ts (1)
Route(12-32)e2e/react-start/basic-spa/src/utils/users.tsx (1)
User(1-5)
e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsx (1)
e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/posts.index.tsx (1)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)
e2e/react-start/basic-spa/src/components/RedirectOnClick.tsx (1)
e2e/react-start/basic-spa/src/components/throwRedirect.ts (1)
throwRedirect(4-23)
e2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsx (3)
e2e/react-start/basic-spa/src/routes/_layout.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/users.$userId.tsx (3)
e2e/react-start/basic-spa/src/routes/api/users.$id.ts (1)
Route(12-32)e2e/react-start/basic-spa/src/utils/users.tsx (1)
User(1-5)e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsx (4)
e2e/react-start/basic-spa/src/routes/__root.tsx (1)
Route(16-75)e2e/react-start/basic-spa/src/routes/_layout.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsx (3)
e2e/react-start/basic-spa/src/routes/_layout.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/links.tsx (2)
e2e/react-start/basic-spa/src/routes/index.tsx (1)
Route(4-6)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)
e2e/react-start/basic-spa/src/routes/not-found/via-loader.tsx (3)
e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/utils/posts.tsx (1)
e2e/react-router/js-only-file-based/src/posts.js (1)
queryURL(5-5)
e2e/react-start/basic-spa/src/routes/search-params/route.tsx (2)
e2e/react-start/basic-spa/src/routes/deferred.tsx (1)
Route(18-29)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/routes/posts.$postId.tsx (2)
e2e/react-start/basic-spa/src/utils/posts.tsx (1)
fetchPost(17-33)e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsx (3)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/router.tsx (2)
e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/src/routeTree.gen.ts (1)
e2e/react-start/basic-spa/src/router.tsx (1)
getRouter(6-16)
e2e/react-start/basic-spa/src/routes/stream.tsx (2)
e2e/react-start/basic-spa/src/routes/deferred.tsx (1)
Route(18-29)e2e/react-start/basic-spa/src/routes/index.tsx (1)
Route(4-6)
e2e/react-start/basic-spa/src/routes/index.tsx (1)
e2e/react-start/basic-spa/src/components/CustomMessage.tsx (1)
CustomMessage(3-10)
e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/tests/redirect.spec.ts (1)
e2e/react-start/basic/tests/redirect.spec.ts (2)
internalNavigationTestMatrix(17-208)test(132-168)
e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)
e2e/react-start/basic-spa/src/routes/__root.tsx (3)
e2e/react-start/basic-spa/src/utils/seo.ts (1)
seo(1-33)e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsx (2)
e2e/react-start/basic-spa/src/utils/posts.tsx (1)
fetchPost(17-33)e2e/react-start/basic-spa/src/routes/posts.$postId.tsx (1)
PostErrorComponent(16-18)
e2e/react-start/basic-spa/tests/setup/global.teardown.ts (9)
e2e/solid-router/basic-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/react-router/basic-esbuild-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/basic-virtual-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/basic-solid-query-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/rspack-basic-virtual-named-export-config-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/basic-virtual-named-export-config-file-based/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/react-router/generator-cli-only/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/basic-solid-query/tests/setup/global.teardown.ts (1)
teardown(4-6)e2e/solid-router/basic/tests/setup/global.teardown.ts (1)
teardown(4-6)
e2e/react-start/basic-spa/src/routes/_layout.tsx (3)
e2e/react-start/basic-spa/src/routes/_layout/_layout-2.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-a.tsx (1)
Route(3-5)e2e/react-start/basic-spa/src/routes/_layout/_layout-2/layout-b.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/search-params/loader-throws-redirect.tsx (2)
e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/routes/redirect/$target/index.tsx (3)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)
e2e/react-start/basic-spa/tests/not-found.spec.ts (1)
e2e/e2e-utils/src/index.ts (1)
test(7-7)
e2e/react-start/basic-spa/src/routes/not-found/index.tsx (3)
e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (1)
Route(3-15)e2e/react-start/basic-spa/src/routes/not-found/via-loader.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx (1)
e2e/react-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)
e2e/react-start/basic-spa/src/routes/scripts.tsx (2)
e2e/react-start/basic-spa/src/routes/inline-scripts.tsx (1)
Route(3-18)packages/router-core/src/ssr/tsrScript.ts (1)
p(7-9)
e2e/react-start/basic-spa/src/routes/search-params/index.tsx (2)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)
e2e/react-start/basic-spa/tests/setup/global.setup.ts (3)
e2e/solid-router/basic-file-based/tests/setup/global.setup.ts (1)
setup(4-6)e2e/react-router/basic-file-based-code-splitting/tests/setup/global.setup.ts (1)
setup(4-6)e2e/react-router/basic/tests/setup/global.setup.ts (1)
setup(4-6)
e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here/index.tsx (1)
e2e/react-start/basic-spa/src/routes/foo/$bar/$qux/_here.tsx (1)
Route(3-5)
e2e/react-start/basic-spa/src/routes/posts.tsx (1)
e2e/react-start/basic-spa/src/utils/posts.tsx (1)
fetchPosts(35-42)
e2e/react-start/basic-spa/src/routes/users.tsx (4)
e2e/react-start/basic-spa/src/routes/api.users.ts (1)
Route(13-28)e2e/react-start/basic-spa/src/routes/api/users.$id.ts (1)
Route(12-32)e2e/react-start/basic-spa/src/utils/users.tsx (1)
User(1-5)examples/solid/start-basic/src/routes/users.tsx (1)
UsersComponent(18-48)
e2e/react-start/basic-spa/src/routes/deferred.tsx (1)
examples/solid/start-basic/src/routes/deferred.tsx (2)
Deferred(31-62)deferredStuff(19-27)
e2e/react-start/basic-spa/src/routes/redirect/$target/via-loader.tsx (2)
e2e/react-start/basic-spa/src/routes/links.tsx (1)
Route(3-47)e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)
e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-loader.tsx (1)
e2e/react-start/basic-spa/src/components/throwRedirect.ts (1)
throwRedirect(4-23)
e2e/react-start/basic-spa/src/routes/not-found/via-beforeLoad.tsx (3)
e2e/react-start/basic-spa/src/routes/not-found/index.tsx (1)
Route(3-31)e2e/react-start/basic-spa/src/routes/not-found/route.tsx (1)
Route(4-8)e2e/react-start/basic-spa/src/routes/not-found/via-loader.tsx (1)
Route(3-15)
e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/via-useServerFn.tsx (1)
e2e/react-start/basic-spa/src/components/RedirectOnClick.tsx (1)
RedirectOnClick(10-26)
e2e/react-start/basic-spa/src/routes/inline-scripts.tsx (2)
e2e/react-start/basic-spa/src/routes/__root.tsx (1)
Route(16-75)packages/router-core/src/ssr/tsrScript.ts (1)
p(7-9)
e2e/react-start/basic-spa/src/routes/api/users.$id.ts (2)
e2e/react-start/basic-spa/src/routes/api.users.ts (1)
Route(13-28)e2e/react-start/basic-spa/src/utils/users.tsx (1)
User(1-5)
🪛 Biome (2.1.2)
e2e/react-start/basic-spa/src/routes/stream.tsx
[error] 51-51: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
e2e/react-start/basic-spa/src/routes/deferred.tsx
[error] 43-43: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 53-53: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
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: 1
🧹 Nitpick comments (1)
e2e/react-start/basic-spa/tests/redirect.spec.ts (1)
79-100: Keep the dev-only whitelist scoped to the direct-visit suiteLine 82 calls
test.useinside the loop; because Playwright keeps those settings for every test declared after that call, the hydration warning gets whitelisted for the rest of this file whenever NODE_ENV isdevelopment, hiding real regressions in later suites. Please wrap these direct-visit tests in their owntest.describeand apply the whitelist there so other tests still surface unexpected hydration errors.- internalDirectVisitTestMatrix.forEach(({ thrower, reloadDocument }) => { - if (process.env.NODE_ENV === 'development') { - test.use({ - whitelistErrors: [ - /A tree hydrated but some attributes of the server rendered HTML/, - ], - }) - } - test(`internal target, direct visit: thrower: ${thrower}, reloadDocument: ${reloadDocument}`, async ({ - page, - }) => { - await page.goto(`/redirect/internal/via-${thrower}`) - await page.waitForLoadState('networkidle') - - const url = `http://localhost:${PORT}/posts` - - await page.waitForURL(url) - expect(page.url()).toBe(url) - await page.waitForLoadState('networkidle') - await expect(page.getByTestId('PostsIndexComponent')).toBeInViewport() - }) - }) + test.describe('internal direct visit', () => { + if (process.env.NODE_ENV === 'development') { + test.use({ + whitelistErrors: [ + /A tree hydrated but some attributes of the server rendered HTML/, + ], + }) + } + + internalDirectVisitTestMatrix.forEach(({ thrower, reloadDocument }) => { + test(`internal target, direct visit: thrower: ${thrower}, reloadDocument: ${reloadDocument}`, async ({ + page, + }) => { + await page.goto(`/redirect/internal/via-${thrower}`) + await page.waitForLoadState('networkidle') + + const url = `http://localhost:${PORT}/posts` + + await page.waitForURL(url) + expect(page.url()).toBe(url) + await page.waitForLoadState('networkidle') + await expect(page.getByTestId('PostsIndexComponent')).toBeInViewport() + }) + }) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
e2e/react-start/basic-spa/package.json(1 hunks)e2e/react-start/basic-spa/src/routes/__root.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/not-found/route.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/stream.tsx(1 hunks)e2e/react-start/basic-spa/tests/redirect.spec.ts(1 hunks)e2e/react-start/basic-spa/tests/search-params.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/react-start/basic-spa/src/routes/redirect/$target.tsx
- e2e/react-start/basic-spa/tests/search-params.spec.ts
- e2e/react-start/basic-spa/package.json
- e2e/react-start/basic-spa/src/routes/redirect/$target/serverFn/index.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/tests/redirect.spec.tse2e/react-start/basic-spa/src/routes/stream.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/src/routes/stream.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic-spa/src/routes/__root.tsxe2e/react-start/basic-spa/src/routes/not-found/route.tsxe2e/react-start/basic-spa/tests/redirect.spec.tse2e/react-start/basic-spa/src/routes/stream.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#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/react-start/basic-spa/src/routes/__root.tsx
🧬 Code graph analysis (4)
e2e/react-start/basic-spa/src/routes/__root.tsx (3)
e2e/react-start/basic-spa/src/utils/seo.ts (1)
seo(1-33)e2e/react-start/basic-spa/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary(10-53)e2e/react-start/basic-spa/src/components/NotFound.tsx (1)
NotFound(3-25)
e2e/react-start/basic-spa/src/routes/not-found/route.tsx (2)
e2e/react-start/basic-spa/src/routes/__root.tsx (1)
Route(16-75)e2e/react-start/basic-spa/src/routes/redirect/$target.tsx (1)
Route(4-21)
e2e/react-start/basic-spa/tests/redirect.spec.ts (1)
e2e/react-start/basic/tests/redirect.spec.ts (2)
internalNavigationTestMatrix(17-208)test(132-168)
e2e/react-start/basic-spa/src/routes/stream.tsx (1)
e2e/react-start/basic-spa/src/routes/deferred.tsx (1)
Route(18-29)
🪛 Biome (2.1.2)
e2e/react-start/basic-spa/src/routes/stream.tsx
[error] 64-64: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
⏰ 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). (1)
- GitHub Check: Test
🔇 Additional comments (2)
e2e/react-start/basic-spa/src/routes/stream.tsx (1)
62-74: Switch<Await>to JSX children to satisfy BiomeBiome still errors on passing the render fn via the
childrenprop (noChildrenProp), so this will fail CI until converted to JSX children.- <Await - promise={promise} - children={(promiseData) => ( + <Await promise={promise}> + {(promiseData) => ( <div className="p-2" data-testid="promise-data"> {promiseData} <div data-testid="stream-data"> {streamData.map((d) => ( <div key={d}>{d}</div> ))} </div> </div> - )} - /> + )} + </Await>e2e/react-start/basic-spa/src/routes/__root.tsx (1)
51-51: Drop unsupportedcolorattribute from the manifest link
rel="manifest"links don’t honor acolorattribute, so this value is ignored. Let’s remove it to keep the head clean.- { rel: 'manifest', href: '/site.webmanifest', color: '#ffffff' }, + { rel: 'manifest', href: '/site.webmanifest' },
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
e2e/react-start/basic-spa/tests/redirect.spec.ts(1 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:
e2e/react-start/basic-spa/tests/redirect.spec.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic-spa/tests/redirect.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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.
⏰ 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
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
e2e/react-start/basic-spa/package.json(1 hunks)e2e/react-start/basic-spa/playwright.config.ts(1 hunks)e2e/react-start/basic-spa/src/routes/posts.$postId.tsx(1 hunks)e2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsx(1 hunks)e2e/react-start/basic-spa/tests/navigation.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/react-start/basic-spa/package.json
- e2e/react-start/basic-spa/src/routes/posts.$postId.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/basic-spa/playwright.config.tse2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic-spa/tests/navigation.spec.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic-spa/playwright.config.tse2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic-spa/tests/navigation.spec.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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.
🧬 Code graph analysis (2)
e2e/react-start/basic-spa/src/routes/posts_.$postId.deep.tsx (2)
e2e/react-start/basic-spa/src/routes/posts.$postId.tsx (1)
Route(7-14)e2e/react-start/basic-spa/src/utils/posts.tsx (1)
fetchPost(17-33)
e2e/react-start/basic-spa/tests/navigation.spec.ts (1)
e2e/e2e-utils/src/index.ts (1)
test(7-7)
⏰ 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 (1)
e2e/react-start/basic-spa/tests/navigation.spec.ts (1)
65-68: Stop basing script expectations onprocess.env.NODE_ENV.
process.env.NODE_ENVreflects the Playwright runner, not the built app mode, so these assertions misfire (e.g. a prod build exercised under a dev runner). Detect the actual script presence instead and derive the expectation from that, e.g.:- expect(await page.evaluate('window.SCRIPT_2')).toBe( - process.env.NODE_ENV === 'development' ? true : undefined, - ) + const hasScript2 = await page.evaluate( + () => !!document.querySelector('script[src*="script2.js"]'), + ) + expect(await page.evaluate('window.SCRIPT_2')).toBe( + hasScript2 ? true : undefined, + )Apply the same change in the direct navigation test below so both cases read the DOM rather than the runner environment.
Also applies to: 75-77
| const PORT = await getTestServerPort(packageJson.name) | ||
| const EXTERNAL_PORT = await getDummyServerPort(packageJson.name) | ||
| const baseURL = `http://localhost:${PORT}` | ||
| const spaModeCommand = `pnpm build && pnpm dev:e2e --port=${PORT}` |
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.
Testing using dev for spa mode?
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.
If this is a limitation of the single playwright entry, then maybe we can just have a spa mode specific entry like we have in the scroll restoration sandbox.
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.
using dev for spa only to ensure we are mounting in the shell and running the server. not entirely sure yet how to mount the shell and server.ts correctly for production in the test.
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.
you would need to serve the shell (and the other static files) via a custom http server and proxy through the requests for server functions etc to the actual start backend.
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: 2
🧹 Nitpick comments (8)
e2e/react-start/basic/src/routes/users.tsx (1)
6-16: Prefer async/await over mixed .then/.catch in loaderSimplifies flow and preserves stack traces.
Apply:
export const Route = createFileRoute('/users')({ loader: async () => { - return await axios - .get<Array<User>>('/api/users') - .then((r) => r.data) - .catch(() => { - throw new Error('Failed to fetch users') - }) + try { + const { data } = await axios.get<Array<User>>('/api/users') + return data + } catch { + throw new Error('Failed to fetch users') + } }, component: UsersComponent, })e2e/react-start/basic/tests/redirect.spec.ts (6)
9-9: Use simpler relative import for isSpaModeAvoids brittle ../tests/ hopping.
Apply:
-import { isSpaMode } from '../tests/utils/isSpaMode' +import { isSpaMode } from './utils/isSpaMode'
39-60: Use page.waitForRequest instead of manual event/promise raceSimpler, less error-prone, no lingering listeners.
Apply:
- await page.waitForLoadState('networkidle') - let requestHappened = false - - const requestPromise = new Promise<void>((resolve) => { - page.on('request', (request) => { - if ( - request.url().startsWith(`http://localhost:${PORT}/_serverFn/`) - ) { - requestHappened = true - resolve() - } - }) - }) - await link.focus() - - const expectRequestHappened = preload && !reloadDocument - const timeoutPromise = new Promise((resolve) => - setTimeout(resolve, expectRequestHappened ? 5000 : 500), - ) - await Promise.race([requestPromise, timeoutPromise]) - expect(requestHappened).toBe(expectRequestHappened) + await page.waitForLoadState('networkidle') + await link.focus() + const expectRequestHappened = preload && !reloadDocument + const req = await page + .waitForRequest( + (r) => + r.url().startsWith(`http://localhost:${PORT}/_serverFn/`), + { timeout: expectRequestHappened ? 5000 : 500 }, + ) + .catch(() => null) + expect(!!req).toBe(expectRequestHappened)
61-63: Use page.once for domcontentloaded flagPrevents multiple triggers toggling state unexpectedly.
Apply:
- page.on('domcontentloaded', () => { + page.once('domcontentloaded', () => { fullPageLoad = true })
83-90: Set whitelistErrors once per describe instead of inside the loopReduces repetition and makes scope explicit.
Apply within the 'internal' describe, before building the matrix:
// Place near Line 19, before internalNavigationTestMatrix if (isSpaMode) { test.use({ whitelistErrors: [ /A tree hydrated but some attributes of the server rendered HTML/, ], }) }Then remove the per-iteration block:
- if (isSpaMode) { - test.use({ - whitelistErrors: [ - /A tree hydrated but some attributes of the server rendered HTML/, - ], - }) - }
164-166: Use page.once for domcontentloaded in serverFn matrixSame rationale: avoid accumulating listeners.
Apply:
- page.on('domcontentloaded', () => { + page.once('domcontentloaded', () => { fullPageLoad = true })
216-219: Use page.once for domcontentloaded in useServerFn suiteConsistent listener semantics across tests.
Apply:
- page.on('domcontentloaded', () => { + page.once('domcontentloaded', () => { fullPageLoad = true })e2e/react-start/basic/vite.config.ts (1)
24-27: Refresh the @ts-ignore rationaleThe ignore comment still talks about
verboseFileRoutes, but we’re now suppressing the type check for thespaoption. Updating the comment to match the new reason would prevent future confusion.Here’s an updated comment that matches the current code:
- // @ts-ignore we want to keep one test with verboseFileRoutes off even though the option is hidden + // @ts-ignore: the SPA option isn’t exposed in the plugin’s public types yet
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
e2e/react-start/basic/package.json(1 hunks)e2e/react-start/basic/playwright.config.ts(2 hunks)e2e/react-start/basic/src/routeTree.gen.ts(1 hunks)e2e/react-start/basic/src/routes/__root.tsx(1 hunks)e2e/react-start/basic/src/routes/_layout.tsx(1 hunks)e2e/react-start/basic/src/routes/_layout/_layout-2.tsx(1 hunks)e2e/react-start/basic/src/routes/_layout/_layout-2/layout-a.tsx(1 hunks)e2e/react-start/basic/src/routes/_layout/_layout-2/layout-b.tsx(1 hunks)e2e/react-start/basic/src/routes/api.users.ts(2 hunks)e2e/react-start/basic/src/routes/api/users.$id.ts(2 hunks)e2e/react-start/basic/src/routes/deferred.tsx(2 hunks)e2e/react-start/basic/src/routes/foo/$bar/$qux/_here.tsx(1 hunks)e2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsx(1 hunks)e2e/react-start/basic/src/routes/index.tsx(1 hunks)e2e/react-start/basic/src/routes/inline-scripts.tsx(1 hunks)e2e/react-start/basic/src/routes/links.tsx(1 hunks)e2e/react-start/basic/src/routes/not-found/index.tsx(1 hunks)e2e/react-start/basic/src/routes/not-found/route.tsx(1 hunks)e2e/react-start/basic/src/routes/not-found/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic/src/routes/not-found/via-loader.tsx(1 hunks)e2e/react-start/basic/src/routes/posts.$postId.tsx(2 hunks)e2e/react-start/basic/src/routes/posts.index.tsx(1 hunks)e2e/react-start/basic/src/routes/posts.tsx(1 hunks)e2e/react-start/basic/src/routes/posts_.$postId.deep.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/index.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/serverFn/index.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-loader.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/$target/via-loader.tsx(1 hunks)e2e/react-start/basic/src/routes/redirect/index.tsx(1 hunks)e2e/react-start/basic/src/routes/scripts.tsx(1 hunks)e2e/react-start/basic/src/routes/search-params/default.tsx(1 hunks)e2e/react-start/basic/src/routes/search-params/index.tsx(1 hunks)e2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsx(1 hunks)e2e/react-start/basic/src/routes/search-params/route.tsx(1 hunks)e2e/react-start/basic/src/routes/stream.tsx(1 hunks)e2e/react-start/basic/src/routes/users.$userId.tsx(1 hunks)e2e/react-start/basic/src/routes/users.index.tsx(1 hunks)e2e/react-start/basic/src/routes/users.tsx(1 hunks)e2e/react-start/basic/src/routes/대한민국.tsx(1 hunks)e2e/react-start/basic/tests/navigation.spec.ts(2 hunks)e2e/react-start/basic/tests/redirect.spec.ts(2 hunks)e2e/react-start/basic/tests/search-params.spec.ts(2 hunks)e2e/react-start/basic/tests/utils/isSpaMode.ts(1 hunks)e2e/react-start/basic/vite.config.ts(2 hunks)packages/router-core/src/path.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/router-core/src/path.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:
e2e/react-start/basic/tests/utils/isSpaMode.tse2e/react-start/basic/tests/navigation.spec.tse2e/react-start/basic/tests/search-params.spec.tse2e/react-start/basic/src/routes/users.tsxe2e/react-start/basic/src/routes/redirect/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2.tsxe2e/react-start/basic/src/routes/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic/src/routes/search-params/route.tsxe2e/react-start/basic/src/routes/search-params/index.tsxe2e/react-start/basic/src/routes/api.users.tse2e/react-start/basic/src/routes/posts.index.tsxe2e/react-start/basic/src/routes/scripts.tsxe2e/react-start/basic/src/routes/api/users.$id.tse2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic/src/routes/_layout.tsxe2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routes/redirect/$target/index.tsxe2e/react-start/basic/playwright.config.tse2e/react-start/basic/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic/src/routes/대한민국.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic/src/routes/not-found/index.tsxe2e/react-start/basic/vite.config.tse2e/react-start/basic/src/routes/links.tsxe2e/react-start/basic/tests/redirect.spec.tse2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic/src/routes/redirect/$target.tsxe2e/react-start/basic/src/routes/inline-scripts.tsxe2e/react-start/basic/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic/src/routes/search-params/default.tsxe2e/react-start/basic/src/routes/stream.tsxe2e/react-start/basic/src/routes/deferred.tsxe2e/react-start/basic/src/routes/not-found/route.tsxe2e/react-start/basic/src/routes/not-found/via-loader.tsxe2e/react-start/basic/src/routes/users.$userId.tsxe2e/react-start/basic/src/routes/posts.$postId.tsxe2e/react-start/basic/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic/src/routes/users.index.tsxe2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic/src/routes/posts.tsxe2e/react-start/basic/src/routeTree.gen.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic/tests/utils/isSpaMode.tse2e/react-start/basic/package.jsone2e/react-start/basic/tests/navigation.spec.tse2e/react-start/basic/tests/search-params.spec.tse2e/react-start/basic/src/routes/users.tsxe2e/react-start/basic/src/routes/redirect/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2.tsxe2e/react-start/basic/src/routes/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic/src/routes/search-params/route.tsxe2e/react-start/basic/src/routes/search-params/index.tsxe2e/react-start/basic/src/routes/api.users.tse2e/react-start/basic/src/routes/posts.index.tsxe2e/react-start/basic/src/routes/scripts.tsxe2e/react-start/basic/src/routes/api/users.$id.tse2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic/src/routes/_layout.tsxe2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routes/redirect/$target/index.tsxe2e/react-start/basic/playwright.config.tse2e/react-start/basic/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic/src/routes/대한민국.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic/src/routes/not-found/index.tsxe2e/react-start/basic/vite.config.tse2e/react-start/basic/src/routes/links.tsxe2e/react-start/basic/tests/redirect.spec.tse2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic/src/routes/redirect/$target.tsxe2e/react-start/basic/src/routes/inline-scripts.tsxe2e/react-start/basic/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic/src/routes/search-params/default.tsxe2e/react-start/basic/src/routes/stream.tsxe2e/react-start/basic/src/routes/deferred.tsxe2e/react-start/basic/src/routes/not-found/route.tsxe2e/react-start/basic/src/routes/not-found/via-loader.tsxe2e/react-start/basic/src/routes/users.$userId.tsxe2e/react-start/basic/src/routes/posts.$postId.tsxe2e/react-start/basic/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic/src/routes/users.index.tsxe2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic/src/routes/posts.tsxe2e/react-start/basic/src/routeTree.gen.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/basic/package.json
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic/src/routes/users.tsxe2e/react-start/basic/src/routes/redirect/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2.tsxe2e/react-start/basic/src/routes/index.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-a.tsxe2e/react-start/basic/src/routes/search-params/route.tsxe2e/react-start/basic/src/routes/search-params/index.tsxe2e/react-start/basic/src/routes/api.users.tse2e/react-start/basic/src/routes/posts.index.tsxe2e/react-start/basic/src/routes/scripts.tsxe2e/react-start/basic/src/routes/api/users.$id.tse2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsxe2e/react-start/basic/src/routes/posts_.$postId.deep.tsxe2e/react-start/basic/src/routes/_layout.tsxe2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routes/redirect/$target/index.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/via-loader.tsxe2e/react-start/basic/src/routes/대한민국.tsxe2e/react-start/basic/src/routes/_layout/_layout-2/layout-b.tsxe2e/react-start/basic/src/routes/not-found/index.tsxe2e/react-start/basic/src/routes/links.tsxe2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsxe2e/react-start/basic/src/routes/redirect/$target.tsxe2e/react-start/basic/src/routes/inline-scripts.tsxe2e/react-start/basic/src/routes/foo/$bar/$qux/_here.tsxe2e/react-start/basic/src/routes/not-found/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsxe2e/react-start/basic/src/routes/redirect/$target/serverFn/index.tsxe2e/react-start/basic/src/routes/search-params/default.tsxe2e/react-start/basic/src/routes/stream.tsxe2e/react-start/basic/src/routes/deferred.tsxe2e/react-start/basic/src/routes/not-found/route.tsxe2e/react-start/basic/src/routes/not-found/via-loader.tsxe2e/react-start/basic/src/routes/users.$userId.tsxe2e/react-start/basic/src/routes/posts.$postId.tsxe2e/react-start/basic/src/routes/redirect/$target/via-loader.tsxe2e/react-start/basic/src/routes/users.index.tsxe2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsxe2e/react-start/basic/src/routes/posts.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#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/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsx
🧬 Code graph analysis (17)
e2e/react-start/basic/tests/navigation.spec.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/tests/search-params.spec.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/src/routes/search-params/route.tsx (2)
e2e/react-start/basic/src/routes/search-params/default.tsx (1)
Route(4-28)e2e/react-start/basic/src/routes/search-params/index.tsx (1)
Route(3-5)
e2e/react-start/basic/src/routes/search-params/index.tsx (3)
e2e/react-router/basic-file-based/src/routes/search-params/index.tsx (1)
Route(3-5)e2e/solid-router/basic-file-based/src/routes/search-params/index.tsx (1)
Route(3-5)e2e/solid-start/basic/src/routes/search-params/index.tsx (1)
Route(3-5)
e2e/react-start/basic/src/routes/api.users.ts (1)
e2e/react-start/basic/src/routes/api/users.$id.ts (1)
Route(12-32)
e2e/react-start/basic/src/routes/scripts.tsx (1)
e2e/solid-start/basic/src/routes/scripts.tsx (1)
Route(5-19)
e2e/react-start/basic/src/routes/api/users.$id.ts (1)
e2e/react-start/basic/src/routes/api.users.ts (1)
Route(13-28)
e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsx (1)
e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx (1)
Route(4-12)
e2e/react-start/basic/playwright.config.ts (2)
e2e/e2e-utils/src/derivePort.ts (2)
getTestServerPort(25-27)getDummyServerPort(21-23)e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/vite.config.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/tests/redirect.spec.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/src/routes/inline-scripts.tsx (2)
e2e/react-start/basic/src/routes/__root.tsx (1)
Route(16-75)e2e/solid-start/basic/src/routes/inline-scripts.tsx (1)
Route(3-18)
e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-beforeLoad.tsx (1)
e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsx (1)
Route(4-18)
e2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsx (3)
e2e/react-start/basic/src/routes/redirect/$target/via-loader.tsx (1)
Route(3-17)e2e/solid-start/basic/src/routes/redirect/$target/via-beforeLoad.tsx (1)
Route(3-16)e2e/solid-start/basic/src/routes/redirect/$target/via-loader.tsx (1)
Route(3-17)
e2e/react-start/basic/src/routes/search-params/default.tsx (2)
e2e/react-start/basic/src/routes/search-params/index.tsx (1)
Route(3-5)e2e/react-router/basic-file-based/src/routes/search-params/default.tsx (1)
Route(4-28)
e2e/react-start/basic/src/routes/redirect/$target/via-loader.tsx (2)
e2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsx (1)
Route(3-16)e2e/solid-start/basic/src/routes/redirect/$target/via-loader.tsx (1)
Route(3-17)
e2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsx (2)
e2e/react-start/basic/src/routes/search-params/route.tsx (1)
Route(3-8)e2e/solid-start/basic/src/routes/search-params/loader-throws-redirect.tsx (1)
Route(4-26)
⏰ 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: Preview
- GitHub Check: Test
🔇 Additional comments (31)
e2e/react-start/basic/src/routes/__root.tsx (1)
181-183: LGTM: Suspense wrapper matches lazy devtools usageWrapping the lazily-loaded
RouterDevtoolsinReact.Suspenseavoids runtime warnings when the lazy import resolves later, while keeping production builds unaffected with the null fallback. Nicely done.e2e/react-start/basic/src/routes/api/users.$id.ts (1)
12-32: Looks goodPath-first invocation and server handler wiring stay intact; no issues spotted.
e2e/react-start/basic/src/routes/posts_.$postId.deep.tsx (1)
5-13: Nice upgradePath binding stays accurate and the dedicated error wrapper keeps the UX consistent.
e2e/react-start/basic/src/routes/users.index.tsx (1)
3-5: Route path literal is correct
The generator’s FileRoutesByPath includes/users/(not/users) for the index route, socreateFileRoute('/users/')already matches.e2e/react-start/basic/src/routes/redirect/$target/index.tsx (1)
3-76: Verify trailing slash semantics
Ensure the generated route path for/redirect/$target/includes the trailing slash; if it’s emitted as/redirect/$target(no slash),Route.fullPath–based links will misbehave. Inspect your route manifest (e.g.routeTree.gen.ts) in the build output.e2e/react-start/basic/src/routes/대한민국.tsx (1)
3-5: Explicit path keeps the Hangul segment intactSwitching to the path-first
createFileRoute('/대한민국')call locks in the intended Unicode segment so the new root parsing change can’t strip it. Looks solid.e2e/react-start/basic/src/routes/search-params/loader-throws-redirect.tsx (1)
4-26: Path-first binding lines up with the redirect targetExplicitly registering
/search-params/loader-throws-redirectensures the loader’s redirect target stays consistent after the root-id parsing tweak. The rest of the loader/search wiring still behaves as before.e2e/react-start/basic/src/routes/redirect/$target/via-loader.tsx (1)
3-17: Route id now mirrors the loader redirect casesThe added path literal
/redirect/$target/via-loaderkeeps the loader’s internal redirects aligned with the generated route id, so the SPA mode regression is avoided.e2e/react-start/basic/src/routes/posts.index.tsx (1)
3-5: Index route retains its trailing-slash identityLocking the index route to
/posts/preserves the expected trailing-slash semantics for the posts overview after the root-route handling change. Looks correct.e2e/react-start/basic/src/routes/scripts.tsx (1)
5-19: Explicit/scriptsroute id matches head/component wiringThe head metadata and dev/prod script toggles now sit on a stable
/scriptsid, which should keep SPA navigation behaving. Nice.e2e/react-start/basic/src/routes/posts.$postId.tsx (2)
7-14: Path-first route + notFoundComponent look correctSwitch to createFileRoute('/posts/$postId') and adding notFoundComponent aligns with the PR goals and improves UX for missing posts.
16-18: No external imports of PostErrorComponent found
Removed export in routes/posts.$postId.tsx doesn’t break any imports.e2e/react-start/basic/src/routes/foo/$bar/$qux/_here.tsx (1)
3-5: Good: path-first route, underscore segment preservedcreateFileRoute('/foo/$bar/$qux/_here') is consistent with preserving underscores in base path segments. Parent layout without trailing slash pairs well with the index child route.
Based on learnings
e2e/react-start/basic/src/routes/foo/$bar/$qux/_here/index.tsx (2)
1-1: LGTMImport of createFileRoute is correct.
3-6: Index route path is consistentUsing a trailing slash in createFileRoute('/foo/$bar/$qux/_here/') correctly denotes the index route under the _here segment.
e2e/react-start/basic/src/routes/users.tsx (1)
1-1: LGTMImport changes are aligned with the path-first route pattern.
e2e/react-start/basic/src/routes/links.tsx (1)
3-47: Path-first route binding looks solidBinding
createFileRoutewith/linkskeeps the file path and route id aligned with the updated parsing logic, andRoute.useNavigatestill resolves correctly. Nice job.e2e/react-start/basic/src/routes/inline-scripts.tsx (1)
3-18: Inline-scripts route update aligns with the new APISupplying the explicit
/inline-scriptspath keeps this route consistent with the other SPA-mode fixtures and avoids ambiguity in the generated route ids. Looks good.e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-loader.tsx (1)
4-12: Dynamic redirect route remains intactPassing the explicit
/redirect/$target/serverFn/via-loaderpath preserves the dynamic segment while keeping the loader logic unchanged. This should exercise the trailing-underscore parsing fix as intended.e2e/react-start/basic/src/routes/not-found/via-beforeLoad.tsx (1)
3-15: Not-found guard stays effectiveThe explicit
/not-found/via-beforeLoadpath keeps this route’s notFound guard wired up while matching the updated path parser expectations. All good.e2e/react-start/basic/src/routes/redirect/$target/serverFn/via-useServerFn.tsx (1)
4-6: Path-first route binding looks correctThe bound path matches the file-system location and preserves the existing route behavior. Nice cleanup.
e2e/react-start/basic/src/routes/redirect/$target.tsx (1)
4-21: Dynamic segment routing stays intactUsing
createFileRoute('/redirect/$target')keeps the param parsing and search middleware wired exactly as before while aligning with the new API shape.e2e/react-start/basic/src/routes/redirect/$target/via-beforeLoad.tsx (1)
3-16: SPA redirect flow remains consistentThe path string mirrors the file-based segment, so the beforeLoad redirect logic continues to fire for both internal and external targets.
e2e/react-start/basic/src/routes/_layout/_layout-2/layout-a.tsx (1)
3-5: Layout route registration looks goodBinding
/_layout/_layout-2/layout-aup front matches the generated layout structure without altering the component output.e2e/react-start/basic/src/routes/search-params/index.tsx (1)
3-5: Index route path stays canonicalThe trailing-slash path keeps the index route discoverable and consistent with other platform examples.
e2e/react-start/basic/src/routes/_layout/_layout-2/layout-b.tsx (1)
1-5: Path binding matches the layout hierarchyProviding the explicit
/_layout/_layout-2/layout-bpath keeps this layout’s route id aligned with the file-system hierarchy while still letting the new root-id logic handle underscore stripping correctly. Looks solid.e2e/react-start/basic/src/routes/search-params/route.tsx (1)
1-8: SPA context route wired correctlyThe switch to
createFileRoute('/search-params')keeps the parent context loader intact and ensures the root-id logic can honor the segment. All good here.e2e/react-start/basic/src/routes/redirect/$target/serverFn/index.tsx (1)
1-86: Dynamic serverFn route path looks correctThe path-first invocation preserves the
$targetparameter and trailing slash for the index route, so the generated links will continue to resolve as expected.e2e/react-start/basic/src/routes/not-found/via-loader.tsx (1)
1-15: Not-found loader route alignedExplicitly binding
/not-found/via-loaderkeeps the loader +notFoundpairing working with the updated parser. No issues spotted.e2e/react-start/basic/src/routes/search-params/default.tsx (1)
4-28: Default search params route remains consistentThe new path binding mirrors the folder structure and keeps validation/loader logic untouched. Implementation looks good.
e2e/react-start/basic/src/routeTree.gen.ts (1)
550-555:/linkspreLoaderRoute now points at the right moduleGood catch—wiring
/linkstoLinksRouteImportkeeps the generated typings aligned with the actual route definition and avoids the inline-scripts mismatch. Looks solid.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
e2e/solid-start/basic/package.json(1 hunks)e2e/solid-start/basic/playwright.config.ts(2 hunks)e2e/solid-start/basic/tests/navigation.spec.ts(2 hunks)e2e/solid-start/basic/tests/utils/isSpaMode.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/solid-start/basic/tests/utils/isSpaMode.tse2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/tests/navigation.spec.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/tests/utils/isSpaMode.tse2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/tests/navigation.spec.tse2e/solid-start/basic/package.json
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-start/basic/package.json
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#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
PR: TanStack/router#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: CR
PR: TanStack/router#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/
Learnt from: CR
PR: TanStack/router#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
Learnt from: CR
PR: TanStack/router#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/
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-devtools,*-router-devtools}/** : Keep router devtools packages in packages/router-devtools/ and packages/*-router-devtools/
🧬 Code graph analysis (2)
e2e/solid-start/basic/playwright.config.ts (2)
e2e/e2e-utils/src/derivePort.ts (2)
getTestServerPort(25-27)getDummyServerPort(21-23)e2e/solid-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/solid-start/basic/tests/navigation.spec.ts (1)
e2e/solid-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). (1)
- GitHub Check: Test
| const spaModeCommand = `pnpm build && pnpm dev:e2e --port=${PORT}` | ||
| const ssrModeCommand = `pnpm build && pnpm start` |
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.
Fix SPA webServer command argument forwarding.
pnpm dev:e2e --port=${PORT} treats --port as a pnpm CLI flag, so pnpm bails out with “Unknown option '--port'” and the Playwright web server never starts in SPA mode. Forward the flag with -- (or drop it and rely on the PORT env you already set) so the dev server actually launches.
-const spaModeCommand = `pnpm build && pnpm dev:e2e --port=${PORT}`
+const spaModeCommand = `pnpm build && pnpm dev:e2e -- --port=${PORT}`🤖 Prompt for AI Agents
In e2e/solid-start/basic/playwright.config.ts around lines 12 to 13, the SPA
webServer command forwards --port directly to pnpm which treats it as a pnpm CLI
flag and errors; change the SPA command so the port flag is forwarded to the
script (either add a double-dash separator before the flag, e.g. use pnpm
dev:e2e -- --port=${PORT}, or remove the --port argument entirely and rely on
the existing PORT env variable) so the dev server actually starts in SPA mode.
With the changes to better consider non-nested routes the root id of
__root__is being affected, which has an impact on start projects with spa mode enabled.This updates the path parsing to take
__root__into account, and adds a set of e2e tests specific for spa mode based on the start-basic e2e test suite.This resolves #5171
Summary by CodeRabbit