-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(react-start): allow customizing react identifierPrefix
#5418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds an e2e React Start app to test a custom React useId identifierPrefix across SSR and hydration, updates docs with usage guidance, and plumbs router.options.ssr.identifierPrefix through React Router’s SSR render paths and typings. Router core types are expanded to carry SSR options. No existing public APIs removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant Router as TanStack Router
participant RDOMS as ReactDOMServer
participant Server
participant RDOM as ReactDOM (client)
rect rgba(230,240,255,0.5)
note over Router,RDOMS: Server-side render with custom identifierPrefix
Server->>Router: createRouter({ ssr: { identifierPrefix: "myapp" } })
Router->>RDOMS: renderTo(Readable|Pipeable|String, { identifierPrefix: "myapp" })
RDOMS-->>Server: HTML stream/string with ids prefixed "myapp"
Server-->>Browser: HTML response
end
rect rgba(230,255,230,0.5)
note over Browser,RDOM: Client hydration with same prefix
Browser->>RDOM: hydrateRoot(document, <App/>, { identifierPrefix: "myapp" })
RDOM-->>Browser: Hydration completes (ids match)
end
alt Prefix mismatch
Browser-->>User: Hydration warning/error due to id mismatch
else Prefix matches
Browser-->>User: Page renders without hydration mismatch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
View your CI Pipeline Execution ↗ for commit bce4720
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
e2e/react-start/custom-identifier-prefix/public/site.webmanifest (1)
2-3
: Provide meaningful app names in the web manifest.The
name
andshort_name
fields are empty. While this is a test app, providing meaningful values improves testing accuracy and follows PWA best practices. Consider using values like:- "name": "", - "short_name": "", + "name": "TanStack Router Custom Prefix Test", + "short_name": "Custom Prefix",docs/start/framework/react/client-entry-point.md (1)
73-114
: Consider making the hydration warning more prominent.The documentation correctly shows the identifierPrefix usage, but the warning about matching values is only in inline comments. Consider adding a more prominent warning block before or after the code examples:
> [!WARNING] > The `identifierPrefix` values must be identical between client hydration and SSR configuration to prevent hydration errors.This would make the critical requirement more visible to developers.
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
1-25
: Consider extracting NotFound to a shared test utility.This component is duplicated across multiple e2e test apps (server-routes, basic-cloudflare, etc.). Consider extracting it to a shared location like
e2e/test-components
ore2e/e2e-utils
to reduce duplication and improve maintainability.e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts (1)
11-15
: Strengthen the test assertion to verify prefix position.The test uses
toContainText('myapp')
, which only checks if 'myapp' appears anywhere in the text. React'sidentifierPrefix
prepends the value, so the assertion should verify that 'myapp' appears at the expected position in the ID:-await expect( page.getByTestId('tested-element')).toContainText('myapp') +await expect(page.getByTestId('tested-element')).toHaveText(/:myapp:/)or
-await expect( page.getByTestId('tested-element')).toContainText('myapp') +const text = await page.getByTestId('tested-element').textContent() +expect(text).toMatch(/^:myapp:/)This ensures the prefix is correctly applied at the start of the ID.
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
1-53
: Code duplication with other e2e test apps.This component is identical to
DefaultCatchBoundary
implementations in multiple other e2e test apps and examples (e.g.,e2e/react-start/server-routes/src/components/DefaultCatchBoundary.tsx
,e2e/react-start/basic-cloudflare/src/components/DefaultCatchBoundary.tsx
). While this duplication is typical for isolated e2e test apps, consider whether a shared utility component could reduce maintenance burden.The implementation itself is correct and follows the established pattern for error boundaries in this codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
e2e/react-start/custom-identifier-prefix/public/android-chrome-192x192.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/android-chrome-512x512.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/apple-touch-icon.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-16x16.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-32x32.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon.ico
is excluded by!**/*.ico
e2e/react-start/custom-identifier-prefix/public/favicon.png
is excluded by!**/*.png
📒 Files selected for processing (32)
docs/start/framework/react/client-entry-point.md
(1 hunks)e2e/react-start/custom-identifier-prefix/.gitignore
(1 hunks)e2e/react-start/custom-identifier-prefix/.prettierignore
(1 hunks)e2e/react-start/custom-identifier-prefix/package.json
(1 hunks)e2e/react-start/custom-identifier-prefix/playwright.config.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/postcss.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script2.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/site.webmanifest
(1 hunks)e2e/react-start/custom-identifier-prefix/server.js
(1 hunks)e2e/react-start/custom-identifier-prefix/src/client.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/router.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/server.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/styles/app.css
(1 hunks)e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tsconfig.json
(1 hunks)e2e/react-start/custom-identifier-prefix/vite.config.ts
(1 hunks)packages/react-router/src/ssr/renderRouterToStream.tsx
(2 hunks)packages/react-router/src/ssr/renderRouterToString.tsx
(1 hunks)packages/react-router/src/ssr/serializer.ts
(1 hunks)packages/router-core/src/index.ts
(1 hunks)packages/router-core/src/router.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-router/src/ssr/serializer.ts
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
packages/react-router/src/ssr/renderRouterToString.tsx
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
packages/router-core/src/router.ts
e2e/react-start/custom-identifier-prefix/src/router.tsx
packages/router-core/src/index.ts
e2e/react-start/custom-identifier-prefix/src/server.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
e2e/react-start/custom-identifier-prefix/vite.config.ts
packages/react-router/src/ssr/renderRouterToStream.tsx
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/client.tsx
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/src/ssr/serializer.ts
packages/react-router/src/ssr/renderRouterToString.tsx
packages/react-router/src/ssr/renderRouterToStream.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
e2e/react-start/custom-identifier-prefix/tsconfig.json
e2e/react-start/custom-identifier-prefix/public/script.js
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
e2e/react-start/custom-identifier-prefix/public/site.webmanifest
e2e/react-start/custom-identifier-prefix/src/router.tsx
e2e/react-start/custom-identifier-prefix/server.js
e2e/react-start/custom-identifier-prefix/src/server.ts
e2e/react-start/custom-identifier-prefix/package.json
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
e2e/react-start/custom-identifier-prefix/vite.config.ts
e2e/react-start/custom-identifier-prefix/postcss.config.mjs
e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/client.tsx
e2e/react-start/custom-identifier-prefix/public/script2.js
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/src/styles/app.css
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/start/framework/react/client-entry-point.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/start/framework/react/client-entry-point.md
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
packages/router-core/src/index.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/custom-identifier-prefix/package.json
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
🧠 Learnings (2)
📚 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/custom-identifier-prefix/tsconfig.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
🧬 Code graph analysis (13)
packages/react-router/src/ssr/serializer.ts (1)
packages/router-core/src/router.ts (1)
RouterSSROptionsExtensions
(139-141)
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
scripts/llms-generate.mjs (1)
title
(96-96)
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (5)
e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/react-start/basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)examples/react/start-basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/react-start/basic-cloudflare/src/routes/__root.tsx (1)
NotFound
(55-55)examples/react/start-basic-cloudflare/src/routes/__root.tsx (1)
NotFound
(61-61)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (7)
packages/router-core/src/index.ts (2)
ErrorComponentProps
(137-137)rootRouteId
(110-110)e2e/react-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/solid-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)examples/solid/start-basic/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)examples/react/start-basic-cloudflare/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/basic-cloudflare/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)packages/react-router/src/CatchBoundary.tsx (2)
CatchBoundaryImpl
(31-78)CatchBoundary
(5-29)
e2e/react-start/custom-identifier-prefix/playwright.config.ts (1)
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
isSpaMode
(1-1)
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts (1)
e2e/e2e-utils/src/index.ts (1)
test
(7-7)
packages/router-core/src/router.ts (1)
packages/router-core/src/index.ts (1)
RouterSSROptionsExtensions
(247-247)
e2e/react-start/custom-identifier-prefix/src/router.tsx (2)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
scripts/set-ts-version.js (1)
packageJson
(33-33)
e2e/react-start/custom-identifier-prefix/vite.config.ts (1)
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
isSpaMode
(1-1)
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (2)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (1)
Route
(16-75)packages/router-core/src/route.ts (1)
id
(1547-1549)
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts (3)
packages/router-core/src/index.ts (3)
FileRouteTypes
(64-64)FileRoutesByPath
(65-65)Register
(211-211)packages/router-core/src/router.ts (1)
Register
(97-102)e2e/react-start/custom-identifier-prefix/src/router.tsx (1)
getRouter
(6-19)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (3)
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
seo
(1-33)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
🪛 GitHub Actions: autofix.ci
e2e/react-start/custom-identifier-prefix/package.json
[error] 1-1: pnpm install --frozen-lockfile failed: lockfile is outdated. 24 new dependencies were added. Run 'pnpm install' without --frozen-lockfile to update the lockfile.
🔇 Additional comments (27)
e2e/react-start/custom-identifier-prefix/.prettierignore (1)
1-4
: LGTM!The ignore patterns are standard and appropriate for a React Start e2e test project. The exclusion of generated routeTree and build outputs is correct.
e2e/react-start/custom-identifier-prefix/public/script2.js (1)
1-2
: LGTM!Simple test fixture for verifying script loading. The pattern matches script.js, which is appropriate for testing multiple script loading scenarios.
e2e/react-start/custom-identifier-prefix/public/script.js (1)
1-2
: LGTM!Simple test fixture for verifying script loading. Works as expected for e2e test scenarios.
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
1-1
: LGTM!Clean utility for distinguishing SPA vs SSR test modes. The explicit boolean type annotation is good practice.
e2e/react-start/custom-identifier-prefix/postcss.config.mjs (1)
1-6
: LGTM!Standard PostCSS configuration with Tailwind CSS and Autoprefixer using default settings. Appropriate for the e2e test project.
packages/react-router/src/ssr/renderRouterToStream.tsx (2)
27-27
: LGTM!Correctly passes
identifierPrefix
from router SSR options to React's streaming API. The optional chaining safely handles cases where the option is not configured.
50-50
: LGTM!Consistently applies
identifierPrefix
to the pipeable stream API, matching the implementation inrenderToReadableStream
. This ensures consistent behavior across both React SSR streaming modes.e2e/react-start/custom-identifier-prefix/.gitignore (1)
1-20
: LGTM!Comprehensive ignore patterns covering all typical artifacts for a React e2e test project, including build outputs, environment files, test results, and Playwright cache. Well-organized and complete.
e2e/react-start/custom-identifier-prefix/tsconfig.json (1)
1-22
: LGTM! Aligns with strict TypeScript guidelines.The configuration properly enables strict mode with comprehensive type safety as required by the coding guidelines. The inclusion of
public/script*.js
for type-checking is intentional for the test fixtures. Modern module resolution and path mapping are appropriate for the Vite-based project.As per coding guidelines
packages/react-router/src/ssr/renderRouterToString.tsx (1)
15-17
: LGTM! Proper integration of identifierPrefix.The change correctly passes
identifierPrefix
from router SSR options to React'srenderToString
. The optional chaining ensures safe access when the option is not configured.e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts (1)
1-6
: LGTM!Standard teardown implementation that properly cleans up the test server using the e2e utility.
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
1-16
: LGTM! Clean demonstration of the identifierPrefix feature.The route correctly uses
useId
to demonstrate the custom identifier prefix. The generated ID will include the 'myapp' prefix configured in the router and client setup.Minor note: Line 9 has a semicolon while other statements don't, but this doesn't affect functionality.
e2e/react-start/custom-identifier-prefix/src/client.tsx (1)
1-19
: LGTM! Consistent identifierPrefix across client and router configuration.The client hydration setup correctly applies the
identifierPrefix: 'myapp'
to match the SSR configuration insrc/router.tsx
(line 14). The use ofStrictMode
andstartTransition
follows React best practices for hydration.packages/router-core/src/index.ts (1)
247-247
: LGTM! Type export correctly positioned.The
RouterSSROptionsExtensions
export is properly placed alongsideSSROption
in the router types export block, maintaining consistency with the new interface introduced inpackages/router-core/src/router.ts
.e2e/react-start/custom-identifier-prefix/src/styles/app.css (1)
1-22
: LGTM! Standard Tailwind configuration.The CSS setup follows standard Tailwind patterns with appropriate base layer styling and dark mode support.
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
1-6
: LGTM! Clean test setup implementation.The setup correctly reads the package name from
package.json
and initializes the dummy server. The import assertion syntax is appropriate for JSON modules.packages/react-router/src/ssr/serializer.ts (1)
8-10
: LGTM! Type augmentation correctly extends SSR options.The module augmentation properly extends
RouterSSROptionsExtensions
with theidentifierPrefix
property, complementing the basenonce
property defined inpackages/router-core/src/router.ts
.e2e/react-start/custom-identifier-prefix/src/router.tsx (1)
13-15
: LGTM! SSR configuration with identifierPrefix is properly set.The SSR configuration correctly sets
identifierPrefix: 'myapp'
, matching the client hydration configuration insrc/client.tsx
(line 16). This ensures consistent ReactuseId
behavior across server and client rendering.e2e/react-start/custom-identifier-prefix/tailwind.config.mjs (1)
1-4
: LGTM! Standard Tailwind configuration.The configuration correctly specifies the content glob to include all TypeScript/JavaScript files in the
src
directory.packages/router-core/src/router.ts (2)
139-141
: LGTM! Clean refactoring for extensibility.The new
RouterSSROptionsExtensions
interface follows the established pattern (similar toRouterOptionsExtensions
at lines 134-135) and enables module augmentation to extend SSR options in framework-specific packages. This maintains backward compatibility while enabling theidentifierPrefix
extension.
472-472
: LGTM! Type reference aligns with the new interface.The update correctly references the newly defined
RouterSSROptionsExtensions
interface, completing the refactoring and enabling framework-specific SSR option extensions.e2e/react-start/custom-identifier-prefix/playwright.config.ts (2)
1-20
: LGTM! Clean test configuration setup.The configuration correctly uses:
- Import attributes (
with { type: 'json' }
) for JSON imports- Utility functions for port management with mode-specific naming
- Helpful console logging for debugging test mode
24-60
: LGTM! Well-structured Playwright configuration.The configuration appropriately:
- Switches between SPA and SSR build/start commands based on mode
- Passes necessary environment variables (ports, mode) to the test server
- Uses a single worker to avoid test concurrency issues
- Follows standard Playwright patterns
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (4)
1-31
: LGTM! Proper root route setup with SEO.The imports and initial route configuration correctly set up:
- TanStack Router SSR components (
HeadContent
,Scripts
)- SEO metadata through the utility function
- Standard viewport and charset meta tags
32-65
: LGTM! Asset links and inline styles properly configured.The configuration correctly:
- Links standard favicons, manifest, and app stylesheet
- Defines inline responsive styles in the
head
config that target.inline-div
used later (line 115)- Uses the SSR-compatible inline styles pattern
66-93
: LGTM! Proper error handling and dev tools setup.The route configuration correctly:
- Wraps error and not-found components in the root document structure
- Lazy loads RouterDevtools only in development mode
- Returns a no-op component in production to avoid bundle bloat
95-123
: LGTM! Complete SSR-ready document structure.The
RootDocument
component properly:
- Uses
HeadContent
andScripts
components for SSR hydration- Includes the
.inline-div
element (line 115) styled by the inline CSS from the head config (lines 56-63)- Wraps lazy-loaded devtools in a Suspense boundary with null fallback
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: 4
🧹 Nitpick comments (14)
e2e/react-start/custom-identifier-prefix/src/server.ts (1)
7-11
: Consider adding explicit return type annotation.The fetch method delegates correctly to the handler. However, per the strict TypeScript guidelines for this codebase, consider adding an explicit return type annotation for improved type safety and documentation.
As per coding guidelines.
Apply this diff to add the return type:
export default { - fetch(request: Request) { + fetch(request: Request): ReturnType<typeof handler.fetch> { return handler.fetch(request) }, }Alternatively, if you want to be more explicit about the Promise type:
export default { - fetch(request: Request) { + fetch(request: Request): Promise<Response> { return handler.fetch(request) }, }e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
1-4
: Type children precisely (avoid any).Use ReactNode for children to honor strict typing.
As per coding guidelines.
import { Link } from '@tanstack/react-router' +import type { ReactNode } from 'react' -export function NotFound({ children }: { children?: any }) { +export function NotFound({ children }: { children?: ReactNode }) {e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (2)
1-11
: Add an explicit return type for tags.Improves strictness and editor hints.
As per coding guidelines.
-export const seo = ({ +export const seo = ({ title, description, keywords, image, }: { title: string description?: string image?: string keywords?: string -}) => { +}): Array<{ title: string } | { name: string; content: string }> => {
12-30
: Don’t include meta tags with undefined content.Guard optional entries to avoid <meta ... content="undefined">.
- 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 }, + ] + : []), + ]Note: For OG tags, using the property attribute is more canonical, but optional here.
e2e/react-start/custom-identifier-prefix/tsconfig.json (1)
3-21
: Include Vite client types if using import.meta.env.Prevents missing type errors in Vite projects.
"compilerOptions": { "strict": true, + "types": ["vite/client"], "esModuleInterop": true, "jsx": "react-jsx",
e2e/react-start/custom-identifier-prefix/tailwind.config.mjs (1)
1-4
: Consider adding index.html to Tailwind content.Ensures classes in HTML aren’t purged.
export default { - content: ['./src/**/*.{js,jsx,ts,tsx}'], + content: ['./index.html', './src/**/*.{js,jsx,ts,tsx}'], }docs/start/framework/react/client-entry-point.md (1)
73-113
: Good documentation with critical warnings.The new section clearly demonstrates how to configure a custom
identifierPrefix
for React'suseId
hook. The warnings on lines 92 and 107 about identical values are essential to prevent hydration mismatches.Consider adding a brief explanation of when and why developers might want to customize the
identifierPrefix
, such as:
- Avoiding ID collisions when embedding multiple React apps
- Creating more readable/debuggable IDs in development
## Custom `useId` prefix -You can customize the prefix of react's `useId` hook. +You can customize the prefix of React's `useId` hook. This is useful for avoiding ID collisions when embedding multiple React applications on the same page, or for creating more readable IDs during development.e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts (1)
5-9
: Clarify the need for the 404 whitelist.The 404 error whitelist is added, but it's not immediately clear which resource is expected to return a 404. Consider adding a comment explaining what triggers this expected error.
test.use({ whitelistErrors: [ + // Whitelists expected 404 for [resource name/path] during test setup /Failed to load resource: the server responded with a status of 404/, ], })
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
4-6
: Consider adding error handling for better debugging.While the setup function correctly starts the dummy server, it lacks error handling. If
e2eStartDummyServer
fails, tests may proceed with unclear error messages. Consider wrapping the call in a try-catch block to log or rethrow with context.Example:
export default async function setup() { - await e2eStartDummyServer(packageJson.name) + try { + await e2eStartDummyServer(packageJson.name) + } catch (error) { + console.error(`Failed to start dummy server for ${packageJson.name}:`, error) + throw error + } }e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts (1)
4-6
: Consider adding error handling for better debugging.Similar to the setup script, this teardown function lacks error handling. If
e2eStopDummyServer
fails, the teardown will silently fail or produce unclear errors. Consider adding try-catch with logging for better debugging experience.Example:
export default async function teardown() { - await e2eStopDummyServer(packageJson.name) + try { + await e2eStopDummyServer(packageJson.name) + } catch (error) { + console.error(`Failed to stop dummy server for ${packageJson.name}:`, error) + // Don't rethrow in teardown to allow cleanup to complete + } }e2e/react-start/custom-identifier-prefix/package.json (2)
18-21
: Align workspace protocol for internal packages.Per repo guidelines, prefer
workspace:*
for internal deps for consistent linking.As per coding guidelines
- "@tanstack/react-router": "workspace:^", - "@tanstack/react-router-devtools": "workspace:^", - "@tanstack/react-start": "workspace:^", + "@tanstack/react-router": "workspace:*", + "@tanstack/react-router-devtools": "workspace:*", + "@tanstack/react-start": "workspace:*",- "@tanstack/router-e2e-utils": "workspace:^", + "@tanstack/router-e2e-utils": "workspace:*",Also applies to: 30-30
29-29
: Optional: bump Playwright to latest 1.52.x for stability/features.Not required, but upgrading helps keep CI consistent and picks up fixes.
e2e/react-start/custom-identifier-prefix/server.js (1)
6-9
: Coerce ports to numbers to avoid named‑pipe listen.Environment vars are strings; ensure numeric ports.
-const port = process.env.PORT || 3000 -const startPort = process.env.START_PORT || 3001 +const port = process.env.PORT ? Number(process.env.PORT) : 3000 +const startPort = process.env.START_PORT ? Number(process.env.START_PORT) : 3001e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
23-30
: Add explicit button type to avoid unintended form submitsSet type="button" for safety if rendered inside a form.
- <button + <button + type="button" onClick={() => { router.invalidate() }} className={`px-2 py-1 bg-gray-600 dark:bg-gray-700 rounded text-white uppercase font-extrabold`} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
e2e/react-start/custom-identifier-prefix/public/android-chrome-192x192.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/android-chrome-512x512.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/apple-touch-icon.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-16x16.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-32x32.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon.ico
is excluded by!**/*.ico
e2e/react-start/custom-identifier-prefix/public/favicon.png
is excluded by!**/*.png
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
docs/start/framework/react/client-entry-point.md
(1 hunks)e2e/react-start/custom-identifier-prefix/.gitignore
(1 hunks)e2e/react-start/custom-identifier-prefix/.prettierignore
(1 hunks)e2e/react-start/custom-identifier-prefix/package.json
(1 hunks)e2e/react-start/custom-identifier-prefix/playwright.config.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/postcss.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script2.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/site.webmanifest
(1 hunks)e2e/react-start/custom-identifier-prefix/server.js
(1 hunks)e2e/react-start/custom-identifier-prefix/src/client.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/router.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/server.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/styles/app.css
(1 hunks)e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tsconfig.json
(1 hunks)e2e/react-start/custom-identifier-prefix/vite.config.ts
(1 hunks)packages/react-router/src/ssr/renderRouterToStream.tsx
(2 hunks)packages/react-router/src/ssr/renderRouterToString.tsx
(1 hunks)packages/react-router/src/ssr/serializer.ts
(1 hunks)packages/router-core/src/index.ts
(1 hunks)packages/router-core/src/router.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
packages/react-router/src/ssr/renderRouterToString.tsx
packages/router-core/src/index.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
packages/react-router/src/ssr/serializer.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/server.ts
packages/router-core/src/router.ts
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/src/router.tsx
e2e/react-start/custom-identifier-prefix/vite.config.ts
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/src/client.tsx
packages/react-router/src/ssr/renderRouterToStream.tsx
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/server.ts
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/tsconfig.json
e2e/react-start/custom-identifier-prefix/public/script2.js
e2e/react-start/custom-identifier-prefix/src/router.tsx
e2e/react-start/custom-identifier-prefix/vite.config.ts
e2e/react-start/custom-identifier-prefix/src/styles/app.css
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/src/client.tsx
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
e2e/react-start/custom-identifier-prefix/package.json
e2e/react-start/custom-identifier-prefix/server.js
e2e/react-start/custom-identifier-prefix/public/site.webmanifest
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
e2e/react-start/custom-identifier-prefix/postcss.config.mjs
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
e2e/react-start/custom-identifier-prefix/public/script.js
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/src/ssr/renderRouterToString.tsx
packages/react-router/src/ssr/serializer.ts
packages/react-router/src/ssr/renderRouterToStream.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/index.ts
packages/router-core/src/router.ts
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/start/framework/react/client-entry-point.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/start/framework/react/client-entry-point.md
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/custom-identifier-prefix/package.json
🧠 Learnings (3)
📚 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/custom-identifier-prefix/tsconfig.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
📚 Learning: 2025-10-08T08:11:47.064Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.064Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
🧬 Code graph analysis (14)
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
scripts/set-ts-version.js (1)
packageJson
(33-33)
packages/react-router/src/ssr/serializer.ts (2)
packages/router-core/src/index.ts (1)
RouterSSROptionsExtensions
(247-247)packages/router-core/src/router.ts (1)
RouterSSROptionsExtensions
(139-141)
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts (1)
scripts/set-ts-version.js (1)
packageJson
(33-33)
packages/router-core/src/router.ts (1)
packages/router-core/src/index.ts (1)
RouterSSROptionsExtensions
(247-247)
e2e/react-start/custom-identifier-prefix/playwright.config.ts (1)
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
isSpaMode
(1-1)
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (5)
e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/react-start/basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)examples/react/start-basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/react-start/basic-cloudflare/src/routes/__root.tsx (1)
NotFound
(55-55)examples/react/start-basic-cloudflare/src/routes/__root.tsx (1)
NotFound
(61-61)
e2e/react-start/custom-identifier-prefix/src/router.tsx (2)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/vite.config.ts (1)
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
isSpaMode
(1-1)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (4)
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
Route
(4-6)e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
seo
(1-33)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (2)
packages/router-core/src/index.ts (2)
ErrorComponentProps
(137-137)rootRouteId
(110-110)e2e/react-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
examples/solid/start-basic/src/utils/seo.ts (1)
title
(1-33)
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts (1)
e2e/e2e-utils/src/index.ts (1)
test
(7-7)
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (1)
Route
(16-75)
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts (3)
e2e/react-router/js-only-file-based/src/routeTree.gen.js (1)
IndexRoute
(30-34)packages/router-core/src/router.ts (1)
Register
(97-102)e2e/react-start/custom-identifier-prefix/src/router.tsx (1)
getRouter
(6-19)
⏰ 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 (24)
e2e/react-start/custom-identifier-prefix/src/server.ts (3)
1-2
: LGTM!The comment clearly documents the file's purpose as a smoke test for the custom server entry.
3-3
: LGTM!The import statement is correct and follows TypeScript conventions.
5-5
: LGTM!The console log is appropriate for an e2e test and provides useful debugging information.
e2e/react-start/custom-identifier-prefix/public/site.webmanifest (1)
6-13
: Icon files verified
Both android-chrome-192×192.png and android-chrome-512×512.png are present in e2e/react-start/custom-identifier-prefix/public/.e2e/react-start/custom-identifier-prefix/public/script.js (1)
1-2
: LGTM!Simple test helper script for e2e validation. The global flag pattern is appropriate for test verification.
packages/react-router/src/ssr/serializer.ts (1)
8-10
: LGTM!The module augmentation correctly extends
RouterSSROptionsExtensions
with the optionalidentifierPrefix
property. This pattern aligns with the existingSerializerExtensions
augmentation and enables React-specific SSR options to be typed correctly.packages/router-core/src/index.ts (1)
247-247
: LGTM!The export of
RouterSSROptionsExtensions
correctly exposes the SSR options interface as part of the public API, enabling consumers to extend and reference the type.e2e/react-start/custom-identifier-prefix/postcss.config.mjs (1)
1-6
: LGTM!Standard PostCSS configuration for Tailwind CSS processing. The setup is appropriate for the e2e test application.
packages/react-router/src/ssr/renderRouterToStream.tsx (2)
27-27
: LGTM!The
identifierPrefix
option is correctly passed torenderToReadableStream
, sourced from the router's SSR options with appropriate optional chaining. This matches the pattern used for thenonce
option on line 26.
50-50
: LGTM!The
identifierPrefix
option is correctly passed torenderToPipeableStream
, maintaining consistency with therenderToReadableStream
path. Both streaming approaches now support custom identifier prefixes.e2e/react-start/custom-identifier-prefix/public/script2.js (1)
1-2
: LGTM!Consistent test helper pattern with
script.js
. The global flag enables test verification of script loading.e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts (1)
11-15
: LGTM!The test correctly validates that the custom identifier prefix 'myapp' is present in the rendered output. The test structure follows Playwright best practices with proper use of
getByTestId
andtoContainText
.e2e/react-start/custom-identifier-prefix/.gitignore (1)
1-20
: LGTM!The
.gitignore
file comprehensively covers standard Node.js artifacts, build outputs, OS-specific files, environment files, and Playwright-specific directories. All necessary patterns are present.e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
1-1
: LGTM!The utility correctly exports a typed boolean constant based on the environment variable. The explicit type annotation ensures type safety, and the implementation is straightforward.
e2e/react-start/custom-identifier-prefix/src/router.tsx (1)
6-19
: LGTM!The router factory function correctly configures the router with
identifierPrefix: 'myapp'
in the SSR options. This matches the client hydration configuration, ensuring consistency across SSR and client rendering. The router setup is comprehensive with appropriate error boundaries and not-found handlers.e2e/react-start/custom-identifier-prefix/src/styles/app.css (1)
1-22
: Standard Tailwind setup looks good.The base styles properly configure dark/light mode support with
color-scheme
, and the Tailwind directives are correctly ordered. The border and text/background color utilities will work well across themes.e2e/react-start/custom-identifier-prefix/src/client.tsx (1)
9-19
: identifierPrefix consistency verified
TheidentifierPrefix
value'myapp'
matches in bothclient.tsx
androuter.tsx
.packages/react-router/src/ssr/renderRouterToString.tsx (1)
15-17
: Confirm ReactDOMServer.renderToString identifierPrefix support
ReactDOMServer.renderToString accepts an options object with identifierPrefix.e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
4-16
: Looks good — simple route rendering with useId.Clear and sufficient for the e2e check.
e2e/react-start/custom-identifier-prefix/vite.config.ts (1)
14-27
: Vite config LGTM.Plugin order and conditional SPA prerendering are sensible. The ts-ignore is acceptable here for the test case.
e2e/react-start/custom-identifier-prefix/server.js (1)
50-52
: Express catch‑all route pattern is invalid.
'/{*splat}'
isn't recognized by Express; use'*'
(or'/*'
) for SPA fallback.- app.get('/{*splat}', (req, res) => { + app.get('*', (req, res) => { res.sendFile(path.resolve('./dist/client/index.html')) })⛔ Skipped due to learnings
Learnt from: nlynzaad PR: TanStack/router#5284 File: e2e/react-start/basic/server.js:50-0 Timestamp: 2025-09-28T21:41:45.233Z Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
packages/router-core/src/router.ts (1)
139-141
: Extension point is properly wired:RouterSSROptionsExtensions
is exported inpackages/router-core/src/index.ts
, and the React package augments it inpackages/react-router/src/ssr/serializer.ts
withidentifierPrefix
.e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
10-16
: LGTM on boundary setup and route checksHook usage and root detection look correct and consistent with other e2e examples.
e2e/react-start/custom-identifier-prefix/playwright.config.ts (1)
7-7
: Use supported JSON import syntax and verify CI Node version
- Replace
import packageJson from './package.json' with { type: 'json' }
with either:
•import packageJson from './package.json' assert { type: 'json' }
(requires Node 20+ ESM)
•import packageJson from './package.json'
alongside TypeScript’sresolveJsonModule
- Ensure your CI environment runs on Node ≥20 (e.g., via
actions/setup-node
in your workflow)
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
♻️ Duplicate comments (4)
e2e/react-start/custom-identifier-prefix/public/site.webmanifest (1)
2-3
: Past review comment remains valid.The empty
name
andshort_name
fields were already flagged in a previous review. Consider addressing that feedback to avoid PWA validation warnings.e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
12-36
: LGTM!The conditional spreads for optional fields (
description
,keywords
,image
) correctly prevent undefined values in meta tags. This properly addresses the previous review concern about invalid HTML attributes.e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (1)
51-121
: Remove invalid manifest attribute and addlang
on<html>
<link rel="manifest">
doesn’t support acolor
attribute, and<html>
should declare a language for accessibility. Please align with HTML specs.- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' }, ... - <html> + <html lang="en">e2e/react-start/custom-identifier-prefix/server.js (1)
32-46
: Proxy target misconfiguration will duplicate path segments.This issue was already flagged in a previous review. Mounting at
/api
and/_serverFn
while also including that segment intarget
leads to/api/api/...
and/_serverFn/_serverFn/...
upstream.
🧹 Nitpick comments (8)
e2e/react-start/custom-identifier-prefix/src/server.ts (1)
7-11
: LGTM! Consider adding explicit return type for stricter type safety.The delegation pattern is clear and correct. To align with TypeScript strict mode guidelines, consider adding an explicit return type annotation to the fetch method.
Apply this diff to add the return type:
export default { - fetch(request: Request) { + fetch(request: Request): ReturnType<typeof handler.fetch> { return handler.fetch(request) }, }Alternatively, if you prefer to be more explicit about the Response type:
export default { async fetch(request: Request): Promise<Response> { return handler.fetch(request) }, }As per coding guidelines
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
1-25
: Consider extracting to a shared e2e component.The implementation is correct, but this component is duplicated across multiple e2e projects (server-routes, basic-cloudflare). While acceptable for e2e tests, extracting to a shared component could reduce duplication.
docs/start/framework/react/client-entry-point.md (1)
92-93
: Fix grammar in identifierPrefix warningChange “These values must be the identical” to “These values must be identical” to keep the warning clear.
e2e/react-start/custom-identifier-prefix/server.js (5)
16-16
: Usepath.resolve()
for static file directory.The relative path
'./dist/client'
may fail if the process is started from a different working directory.Apply this diff:
- app.use(express.static('./dist/client')) + app.use(express.static(path.resolve('./dist/client')))
18-24
: Consider adding an explicit error handler.While Express provides a default error handler, an explicit one improves observability in e2e tests by logging stack traces or request details.
Add an error handler after the request handler:
app.use(async (req, res, next) => { try { await nodeHandler(req, res) } catch (error) { next(error) } }) app.use((err, req, res, next) => { console.error('Start server error:', err) res.status(500).send('Internal Server Error') })
48-48
: Usepath.resolve()
for static file directory.The relative path
'./dist/client'
may fail if the process is started from a different working directory.Apply this diff:
- app.use(express.static('./dist/client')) + app.use(express.static(path.resolve('./dist/client')))
35-36
: Remove or clarify misleading comments.The comments "Replace with your target server's URL" and "Needed for virtual hosted sites," are misleading:
- This is production e2e test code, not a template to be modified.
changeOrigin: false
means the feature is disabled, contradicting the comment's implication.Consider removing these comments or clarifying them:
app.use( '/api', createProxyMiddleware({ - target: `http://localhost:${startPort}/api`, // Replace with your target server's URL - changeOrigin: false, // Needed for virtual hosted sites, + target: `http://localhost:${startPort}`, + changeOrigin: false, }), ) app.use( '/_serverFn', createProxyMiddleware({ - target: `http://localhost:${startPort}/_serverFn`, // Replace with your target server's URL - changeOrigin: false, // Needed for virtual hosted sites, + target: `http://localhost:${startPort}`, + changeOrigin: false, }), )Also applies to: 43-44
57-67
: Add error handling for server startup.If either server fails to start (e.g., port already in use), the unhandled promise rejection may crash the process or go unnoticed in e2e test environments.
Apply this diff to add error handling:
-createSpaServer().then(async ({ app }) => - app.listen(port, () => { - console.info(`Client Server: http://localhost:${port}`) - }), -) +createSpaServer() + .then(async ({ app }) => + app.listen(port, () => { + console.info(`Client Server: http://localhost:${port}`) + }), + ) + .catch((err) => { + console.error('Failed to start SPA server:', err) + process.exit(1) + }) -createStartServer().then(async ({ app }) => - app.listen(startPort, () => { - console.info(`Start Server: http://localhost:${startPort}`) - }), -) +createStartServer() + .then(async ({ app }) => + app.listen(startPort, () => { + console.info(`Start Server: http://localhost:${startPort}`) + }), + ) + .catch((err) => { + console.error('Failed to start Start server:', err) + process.exit(1) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
e2e/react-start/custom-identifier-prefix/public/android-chrome-192x192.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/android-chrome-512x512.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/apple-touch-icon.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-16x16.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon-32x32.png
is excluded by!**/*.png
e2e/react-start/custom-identifier-prefix/public/favicon.ico
is excluded by!**/*.ico
e2e/react-start/custom-identifier-prefix/public/favicon.png
is excluded by!**/*.png
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
docs/start/framework/react/client-entry-point.md
(1 hunks)e2e/react-start/custom-identifier-prefix/.gitignore
(1 hunks)e2e/react-start/custom-identifier-prefix/.prettierignore
(1 hunks)e2e/react-start/custom-identifier-prefix/package.json
(1 hunks)e2e/react-start/custom-identifier-prefix/playwright.config.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/postcss.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/script2.js
(1 hunks)e2e/react-start/custom-identifier-prefix/public/site.webmanifest
(1 hunks)e2e/react-start/custom-identifier-prefix/server.js
(1 hunks)e2e/react-start/custom-identifier-prefix/src/client.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/router.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
(1 hunks)e2e/react-start/custom-identifier-prefix/src/server.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/src/styles/app.css
(1 hunks)e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
(1 hunks)e2e/react-start/custom-identifier-prefix/tsconfig.json
(1 hunks)e2e/react-start/custom-identifier-prefix/vite.config.ts
(1 hunks)packages/react-router/src/ssr/renderRouterToStream.tsx
(2 hunks)packages/react-router/src/ssr/renderRouterToString.tsx
(1 hunks)packages/react-router/src/ssr/serializer.ts
(1 hunks)packages/router-core/src/index.ts
(1 hunks)packages/router-core/src/router.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
packages/react-router/src/ssr/renderRouterToStream.tsx
packages/router-core/src/router.ts
packages/react-router/src/ssr/serializer.ts
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/src/router.tsx
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
packages/router-core/src/index.ts
packages/react-router/src/ssr/renderRouterToString.tsx
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/server.ts
e2e/react-start/custom-identifier-prefix/src/client.tsx
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
e2e/react-start/custom-identifier-prefix/vite.config.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts
e2e/react-start/custom-identifier-prefix/tailwind.config.mjs
e2e/react-start/custom-identifier-prefix/src/styles/app.css
e2e/react-start/custom-identifier-prefix/tsconfig.json
e2e/react-start/custom-identifier-prefix/public/site.webmanifest
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx
e2e/react-start/custom-identifier-prefix/src/router.tsx
e2e/react-start/custom-identifier-prefix/postcss.config.mjs
e2e/react-start/custom-identifier-prefix/public/script.js
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/public/script2.js
e2e/react-start/custom-identifier-prefix/package.json
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts
e2e/react-start/custom-identifier-prefix/src/server.ts
e2e/react-start/custom-identifier-prefix/src/client.tsx
e2e/react-start/custom-identifier-prefix/tests/identifier-prefix.spec.ts
e2e/react-start/custom-identifier-prefix/server.js
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
e2e/react-start/custom-identifier-prefix/playwright.config.ts
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
e2e/react-start/custom-identifier-prefix/vite.config.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/src/ssr/renderRouterToStream.tsx
packages/react-router/src/ssr/serializer.ts
packages/react-router/src/ssr/renderRouterToString.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
packages/router-core/src/index.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/react-start/custom-identifier-prefix/package.json
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/start/framework/react/client-entry-point.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/start/framework/react/client-entry-point.md
🧠 Learnings (4)
📚 Learning: 2025-10-09T12:59:02.111Z
Learnt from: hokkyss
PR: TanStack/router#5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.111Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/react-start/custom-identifier-prefix/src/styles/app.css
📚 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/custom-identifier-prefix/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 **/package.json : Use workspace:* protocol for internal dependencies in package.json files
Applied to files:
e2e/react-start/custom-identifier-prefix/package.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts
🧬 Code graph analysis (13)
e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
scripts/llms-generate.mjs (1)
title
(96-96)
packages/router-core/src/router.ts (1)
packages/router-core/src/index.ts (1)
RouterSSROptionsExtensions
(247-247)
packages/react-router/src/ssr/serializer.ts (1)
packages/router-core/src/router.ts (1)
RouterSSROptionsExtensions
(139-141)
e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (4)
e2e/react-start/server-routes/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/react-start/basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)e2e/solid-start/server-routes/src/components/NotFound.tsx (1)
NotFound
(3-25)examples/react/start-basic-cloudflare/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/src/router.tsx (2)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (3)
packages/router-core/src/index.ts (2)
ErrorComponentProps
(137-137)rootRouteId
(110-110)e2e/react-start/server-routes/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)packages/react-router/src/CatchBoundary.tsx (2)
CatchBoundaryImpl
(31-78)CatchBoundary
(5-29)
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (1)
Route
(16-75)
e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts (3)
scripts/set-ts-version.js (1)
packageJson
(33-33)e2e/solid-router/basic-file-based/tests/setup/global.teardown.ts (1)
teardown
(4-6)e2e/react-start/basic/tests/setup/global.teardown.ts (1)
teardown
(4-6)
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
scripts/set-ts-version.js (1)
packageJson
(33-33)
e2e/react-start/custom-identifier-prefix/src/routes/__root.tsx (4)
e2e/react-start/custom-identifier-prefix/src/routes/index.tsx (1)
Route
(4-6)e2e/react-start/custom-identifier-prefix/src/utils/seo.ts (1)
seo
(1-37)e2e/react-start/custom-identifier-prefix/src/components/DefaultCatchBoundary.tsx (1)
DefaultCatchBoundary
(10-53)e2e/react-start/custom-identifier-prefix/src/components/NotFound.tsx (1)
NotFound
(3-25)
e2e/react-start/custom-identifier-prefix/playwright.config.ts (1)
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
isSpaMode
(1-1)
e2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts (4)
e2e/react-router/js-only-file-based/src/routeTree.gen.js (1)
IndexRoute
(30-34)packages/router-core/src/index.ts (3)
FileRouteTypes
(64-64)FileRoutesByPath
(65-65)Register
(211-211)packages/router-core/src/router.ts (1)
Register
(97-102)e2e/react-start/custom-identifier-prefix/src/router.tsx (1)
getRouter
(6-19)
e2e/react-start/custom-identifier-prefix/vite.config.ts (1)
e2e/react-start/custom-identifier-prefix/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
🔇 Additional comments (23)
e2e/react-start/custom-identifier-prefix/package.json (1)
1-44
: LGTM! Package configuration follows guidelines.The package.json correctly uses
workspace:*
for internal TanStack dependencies and includes appropriate scripts and external dependencies for the e2e test suite. All past review concerns have been addressed.e2e/react-start/custom-identifier-prefix/tests/setup/global.teardown.ts (1)
1-6
: LGTM! Teardown script follows established pattern.The global teardown correctly imports the e2e utility, reads the package name from the project's package.json, and stops the dummy server. This implementation is consistent with other e2e test suites in the repository.
e2e/react-start/custom-identifier-prefix/public/site.webmanifest (1)
4-18
: LGTM! Standard PWA manifest structure.The icon configurations, theme colors, and display mode are correctly defined for a PWA manifest.
e2e/react-start/custom-identifier-prefix/postcss.config.mjs (1)
1-6
: LGTM!Standard PostCSS configuration for Tailwind CSS with autoprefixer. The setup correctly chains tailwindcss processing followed by autoprefixer for vendor prefixing.
e2e/react-start/custom-identifier-prefix/tailwind.config.mjs (1)
1-4
: LGTM!Standard Tailwind configuration with appropriate content glob pattern. The JSDoc type annotation provides useful IDE support, and the content path correctly targets all React source files for class purging.
e2e/react-start/custom-identifier-prefix/src/styles/app.css (2)
1-3
: LGTM!Standard Tailwind directives correctly ordered: base, components, utilities.
5-22
: LGTM!The base layer styles provide appropriate light/dark mode theming with color-scheme support, consistent border and text colors across themes. The outline suppression pattern on lines 19-21 was previously reviewed and accepted as appropriate for e2e test code.
Based on learnings
e2e/react-start/custom-identifier-prefix/public/script.js (1)
1-2
: LGTM!Standard test utility script for verifying script loading. The implementation correctly logs to console and sets a global flag for test assertions.
e2e/react-start/custom-identifier-prefix/public/script2.js (1)
1-2
: LGTM!Parallel test utility script following the same pattern as script.js. The implementation is consistent and appropriate for e2e testing.
e2e/react-start/custom-identifier-prefix/tests/setup/global.setup.ts (1)
1-6
: LGTM!The global setup correctly uses modern import assertions for JSON and properly initializes the test server. The implementation follows standard Playwright setup patterns.
e2e/react-start/custom-identifier-prefix/tests/utils/isSpaMode.ts (1)
1-1
: LGTM!Simple and effective utility for determining SPA mode. The implementation is clear and type-safe.
e2e/react-start/custom-identifier-prefix/tsconfig.json (1)
1-22
: LGTM!The TypeScript configuration is properly set up with strict mode enabled and modern settings appropriate for a React e2e test project. The path alias and module resolution are correctly configured.
As per coding guidelines
e2e/react-start/custom-identifier-prefix/playwright.config.ts (2)
1-20
: LGTM!The configuration setup is well-structured:
- Top-level await for port allocation is appropriate for modern ES modules
- Import attributes syntax (
with { type: 'json' }
) is correctly used for JSON imports- SPA/SSR mode switching is properly implemented via
isSpaMode
- Logging SPA mode state aids debugging
24-60
: LGTM!The Playwright configuration is correctly structured:
- Single worker ensures test isolation for SSR/hydration scenarios
- Global setup/teardown properly referenced
- Environment variables (
VITE_SERVER_PORT
,START_PORT
,PORT
, etc.) correctly passed to the web server- Conditional command selection based on
isSpaMode
enables testing both SPA and SSR modesreuseExistingServer
appropriately disabled in CI for clean test runse2e/react-start/custom-identifier-prefix/src/routeTree.gen.ts (1)
1-68
: Skipping review of auto-generated file.This file is auto-generated by TanStack Router and should not be manually modified or reviewed.
Based on learnings.
e2e/react-start/custom-identifier-prefix/server.js (1)
50-50
: Invalid Express route pattern syntax.Express.js does not recognize
'/{*splat}'
as a valid route pattern. This catch-all route will not work as intended and requests will not match.Apply this diff to use valid Express syntax:
- app.get('/{*splat}', (req, res) => { + app.get('/*', (req, res) => { res.sendFile(path.resolve('./dist/client/index.html')) })Note: Also applying
path.resolve()
to thesendFile
path for consistency.⛔ Skipped due to learnings
Learnt from: nlynzaad PR: TanStack/router#5284 File: e2e/react-start/basic/server.js:50-0 Timestamp: 2025-09-28T21:41:45.233Z Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
packages/router-core/src/index.ts (1)
247-247
: LGTM! Proper public API exposure for framework extensions.The export of
RouterSSROptionsExtensions
enables framework-specific packages (likereact-router
) to augment SSR options via module augmentation while keeping the core router framework-agnostic.packages/router-core/src/router.ts (2)
139-141
: LGTM! Well-designed extensibility pattern.Extracting SSR options into a dedicated interface enables framework-specific packages to augment it via module augmentation without coupling the core router to any framework.
472-472
: LGTM! Type refactor maintains backward compatibility.The change from inline
{ nonce?: string }
toRouterSSROptionsExtensions
is a non-breaking refactor that maintains the same structure while enabling extensibility.packages/react-router/src/ssr/renderRouterToStream.tsx (2)
27-27
: LGTM! Consistent implementation across streaming paths.The
identifierPrefix
is correctly propagated torenderToReadableStream
, maintaining consistency with the other SSR render methods and using safe optional chaining.
50-50
: LGTM! Complete coverage of streaming variants.The
identifierPrefix
is correctly propagated torenderToPipeableStream
, ensuring all React SSR rendering paths support the custom identifier prefix.packages/react-router/src/ssr/serializer.ts (1)
8-10
: LGTM! Exemplary use of module augmentation pattern.The code correctly extends
RouterSSROptionsExtensions
via module augmentation to add the React-specificidentifierPrefix
option. This keeps the core router framework-agnostic while enabling React-specific SSR customization.As per coding guidelines: React-specific extensions are properly isolated in the
react-router
package.packages/react-router/src/ssr/renderRouterToString.tsx (1)
15-17
: Approved: React 19 renderToString supports the identifierPrefix option
ReadableStream: React.JSX.Element | ||
} | ||
|
||
interface RouterSSROptionsExtensions { |
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.
why is this in serializer.ts?
) | ||
``` | ||
|
||
## Custom `useId` prefix |
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.
this seems to be the wrong file for this, as it affects not only the client entrypoint but also getRouter
try { | ||
let html = ReactDOMServer.renderToString(children) | ||
let html = ReactDOMServer.renderToString(children, { | ||
identifierPrefix: router.options.ssr?.identifierPrefix, |
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.
i am not sure this edge case really warrants putting this on the router options.
i would rather add this as an option to renderRouterToString
etc and then use a custom server handler
https://tanstack.com/start/latest/docs/framework/react/server-entry-point#custom-server-handlers
Previously, to customize
identifierPrefix
on the server, we need to create a customrenderRouterToStream
and passidentifierPrefix
in. This PR removes that hassleSummary by CodeRabbit