feat: Add page-level selective hydration with hydrate route option#6092
feat: Add page-level selective hydration with hydrate route option#6092tannerlinsley wants to merge 2 commits intomainfrom
Conversation
github-actions
bot
added
documentation
📝 WalkthroughWalkthroughThis PR introduces page-level selective hydration for TanStack Start's React framework. It adds route-level configuration options ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 5a864b0
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (35)
e2e/react-start/basic-hydrate-false/src/routes/_layout/_layout-2/layout-b.tsx (1)
7-9: Consider adding explicit return type for strict TypeScript mode.The component is simple and works correctly. For full TypeScript strict mode compliance (per coding guidelines), you could optionally add an explicit return type, though it's not critical for this e2e test file.
-function LayoutBComponent() { +function LayoutBComponent(): JSX.Element { return <div>I'm layout B!</div> }e2e/react-start/basic-hydrate-false/src/utils/users.tsx (1)
7-9: Ensure consistent types for strict TypeScript.The
PORTconstant has inconsistent types:process.env.VITE_SERVER_PORTisstring | undefined, while the fallback isnumber. This results inPORTbeingstring | number, which violates strict type safety guidelines.Apply this diff to maintain type consistency by parsing the environment variable to a number:
-const PORT = process.env.VITE_SERVER_PORT || 3000 +const PORT = Number(process.env.VITE_SERVER_PORT) || 3000Alternatively, keep it as a string consistently:
-const PORT = process.env.VITE_SERVER_PORT || 3000 +const PORT = process.env.VITE_SERVER_PORT || '3000'As per coding guidelines, use TypeScript strict mode with extensive type safety for all code.
e2e/react-start/basic-hydrate-false/src/routes/search-params/default.tsx (1)
8-17: Consider extracting duplicate validation logic.Both
beforeLoadandloaderhooks contain identical validation logic checkingcontext.hello !== 'world'. While this duplication may be intentional for e2e testing to verify both hooks independently, consider extracting to a shared helper for better maintainability.Apply this diff to extract the validation:
+const validateContext = (context: { hello?: string }) => { + if (context.hello !== 'world') { + throw new Error('Context hello is not "world"') + } +} + export const Route = createFileRoute('/search-params/default')({ validateSearch: z.object({ default: z.string().default('d1'), }), beforeLoad: ({ context }) => { - if (context.hello !== 'world') { - throw new Error('Context hello is not "world"') - } + validateContext(context) }, loader: ({ context }) => { - if (context.hello !== 'world') { - throw new Error('Context hello is not "world"') - } + validateContext(context) },e2e/react-start/basic-hydrate-false/src/routes/search-params/index.tsx (1)
3-5: Consider explicithydrateoption for test clarity.Since this is an e2e test for the selective hydration feature, explicitly setting the
hydrateoption (even ifundefinedto test default behavior) would make the test's intent clearer to future readers. If this route is intentionally testing inheritance from a parent route ordefaultHydrate, consider adding a comment explaining the test scenario.Example:
export const Route = createFileRoute('/search-params/')({ component: RouteComponent, + // Testing hydrate inheritance from parent route + // hydrate: undefined, })e2e/react-start/basic-hydrate-false/src/routes/index.tsx (1)
8-64: Consider adding explicit return type for strict TypeScript compliance.The
Homecomponent implementation is clean and correct. For consistency with TypeScript strict mode guidelines, consider adding an explicit return type annotation.Apply this diff:
-function Home() { +function Home(): React.JSX.Element { return (e2e/react-start/basic-hydrate-false/tsconfig.json (1)
2-15:allowJs: true+ includingpublic/script*.jsmay create noisy/unintended typecheck surface
If the goal is “don’t fail on JS”, consider explicitly settingcheckJs: false(or excluding that folder) to avoid accidental TS diagnostics frompublic/.packages/start-server-core/src/router-manifest.ts (1)
14-18: Make asset injection idempotent (avoid duplicate pushes across repeated calls/requests).
getStartManifest()mutatesstartManifest.routes[rootRouteId].assetsand unconditionallypushes dev + client-entry scripts. IftsrStartManifest()(or the root route object) is cached/reused, assets will accumulate and duplicate.Suggested direction: before pushing, detect existing entries (e.g.,
data-tsr-client-entry, or a dedicated marker for React Refresh) and skip if present.Also applies to: 19-36, 38-46
e2e/react-start/basic-hydrate-false/src/routes/links.tsx (2)
33-43: Addtype="button"to prevent accidental form submission.These
<button>elements will default totype="submit"if rendered inside a form in the future.-<button onClick={() => navigate({ to: '/posts' })}> +<button type="button" onClick={() => navigate({ to: '/posts' })}> navigate to /posts </button> <button + type="button" onClick={() => navigate({ to: '/posts', reloadDocument: true })} > navigate to /posts (reloadDocument=true) </button>
3-47: Use the recommended type-safe pattern foruseNavigate.
Route.useNavigate()(Line 5) works at runtime, but TanStack React Router's documented best practice is to useuseNavigate({ from: Route.fullPath })to ensure type safety for params and search. This gives better IDE support and catches routing errors earlier.Update the import and adjust the hook call:
-import { Link, createFileRoute } from '@tanstack/react-router' +import { Link, createFileRoute, useNavigate } from '@tanstack/react-router' export const Route = createFileRoute('/links')({ component: () => { - const navigate = Route.useNavigate() + const navigate = useNavigate({ from: Route.fullPath }) return (e2e/react-start/basic-hydrate-false/src/routes/multi-cookie-redirect/target.tsx (2)
12-18: Consider reading cookies in a loader to avoid hydration mismatch.The current implementation reads cookies client-side in
useEffect, which creates a hydration mismatch: the server renders empty strings while the client renders actual cookie values. This violates React's hydration contract and may cause warnings.Apply this pattern to read cookies server-side:
+import { createServerFn } from '@tanstack/start' + +const getCookies = createServerFn('GET', async (_, { request }) => { + const cookieHeader = request.headers.get('Cookie') || '' + const session = cookieHeader.match(/session=([^;]+)/)?.[1] || '' + const csrf = cookieHeader.match(/csrf=([^;]+)/)?.[1] || '' + const theme = cookieHeader.match(/theme=([^;]+)/)?.[1] || '' + return { session, csrf, theme } +}) + export const Route = createFileRoute('/multi-cookie-redirect/target')({ + loader: async () => getCookies(), component: RouteComponent, }) function RouteComponent() { - const [cookies, setCookies] = React.useState<Record<string, string>>({}) - - useEffect(() => { - setCookies({ - session: Cookies.get('session') || '', - csrf: Cookies.get('csrf') || '', - theme: Cookies.get('theme') || '', - }) - }, []) + const cookies = Route.useLoaderData()This ensures the server and client render identical content, eliminating the hydration mismatch.
14-16: Optional: Add error handling for cookie access.While
js-cookieis generally safe, wrapping the cookie reads in a try-catch would add robustness against unexpected errors.useEffect(() => { + try { setCookies({ session: Cookies.get('session') || '', csrf: Cookies.get('csrf') || '', theme: Cookies.get('theme') || '', }) + } catch (error) { + console.error('Failed to read cookies:', error) + setCookies({ session: '', csrf: '', theme: '' }) + } }, [])e2e/react-start/basic-hydrate-false/src/routes/api/users.$id.ts (1)
18-18: Inconsistent string concatenation style.Mixing template literal syntax with concatenation is inconsistent. Use a full template literal for clarity.
Apply this diff:
- const res = await axios.get<User>(`${queryURL}/users/` + params.id) + const res = await axios.get<User>(`${queryURL}/users/${params.id}`)packages/router-core/src/Matches.ts (1)
203-203: Consider usingHydrateOptionfor consistency.The
PreValidationErrorHandlingRouteMatchinterface usesbooleandirectly, whileRouteMatchusesHydrateOption(which is a type alias forboolean). While functionally equivalent, usingHydrateOptionconsistently would better express the semantic intent.Apply this diff for consistency:
- hydrate?: boolean + hydrate?: HydrateOptionNote: This requires importing
HydrateOptionin the type parameters section if not already available at that scope.docs/start/framework/react/guide/selective-hydration.md (2)
313-318: Add language specifier to fenced code block.The warning message example lacks a language identifier. Consider adding
textorplaintextto satisfy markdown linting rules.-``` +```text ⚠️ [TanStack Router] Conflicting hydrate options detected in route matches. Some routes have hydrate: false while others have hydrate: true. The page will NOT be hydrated, but this may not be the intended behavior. Please ensure all routes in the match have consistent hydrate settings.--- `217-228`: **Clarify the inheritance vs. conflict distinction.** The example shows `$postId` with explicit `hydrate: true` while parent `blog` has `hydrate: false`. According to the conflict detection section (lines 302-318), this creates a conflict (explicit `true` vs explicit `false`), not pure inheritance. The explanation "inherits `false` from its parent" may confuse readers since it's actually a conflict scenario where the page won't hydrate. Consider either: 1. Changing `$postId { hydrate: true }` to `$postId { /* omitted */ }` to show true inheritance, or 2. Referencing the conflict detection section here. </blockquote></details> <details> <summary>e2e/react-start/basic-hydrate-false/package.json (1)</summary><blockquote> `6-22`: **POSIX-only e2e scripts (`MODE=...`, `rm -rf`)—confirm CI/OS expectations.** If these scripts ever need to run on Windows runners, consider `cross-env` for `MODE=...` and a cross-platform cleanup (or do cleanup inside the Playwright setup). </blockquote></details> <details> <summary>e2e/react-start/basic-hydrate-false/server.js (1)</summary><blockquote> `6-9`: **Consider normalizing `PORT` / `START_PORT` types.** `process.env.* || 3000` yields `string | number`; parsing to integers avoids surprises and keeps logging consistent. </blockquote></details> <details> <summary>e2e/react-start/basic-hydrate-false/src/routes/__root.tsx (1)</summary><blockquote> `95-187`: **Consider adding `<html lang="en">` (a11y) unless tests require omission.** ```diff - <html> + <html lang="en">e2e/react-start/basic-hydrate-false/src/utils/posts.tsx (2)
11-15: Prefer a single-expressionqueryURL(avoid mutable module state).-let queryURL = 'https://jsonplaceholder.typicode.com' - -if (import.meta.env.VITE_NODE_ENV === 'test') { - queryURL = `http://localhost:${import.meta.env.VITE_EXTERNAL_PORT}` -} +const queryURL = + import.meta.env.VITE_NODE_ENV === 'test' + ? `http://localhost:${import.meta.env.VITE_EXTERNAL_PORT}` + : 'https://jsonplaceholder.typicode.com'
5-9:PostType.idtype may not match upstream JSONPlaceholder (numbervsstring).If you want this to reflect the actual API payload, consider
id: number(orstring | numberif you’re normalizing elsewhere).e2e/react-start/basic-hydrate-false/src/components/NotFound.tsx (1)
3-3: Use React.ReactNode for the children prop type.The
anytype bypasses TypeScript's type checking. For better type safety and adherence to strict mode, useReact.ReactNodeinstead.Apply this diff:
-export function NotFound({ children }: { children?: any }) { +export function NotFound({ children }: { children?: React.ReactNode }) {e2e/react-start/basic-hydrate-false/src/routes/deferred.tsx (2)
40-49: Use JSX children syntax instead of children prop.The
Awaitcomponent uses achildrenprop, but React's canonical way is to pass children as JSX content between opening and closing tags.Apply this diff to use JSX children syntax:
- <Suspense fallback={<div>Loading person...</div>}> - <Await - promise={deferredPerson} - children={(data) => ( - <div data-testid="deferred-person"> - {data.name} - {data.randomNumber} - </div> - )} - /> - </Suspense> + <Suspense fallback={<div>Loading person...</div>}> + <Await promise={deferredPerson}> + {(data) => ( + <div data-testid="deferred-person"> + {data.name} - {data.randomNumber} + </div> + )} + </Await> + </Suspense>
50-55: Use JSX children syntax instead of children prop.Same issue as the previous
Awaitcomponent - use JSX children syntax for better readability and to follow React conventions.Apply this diff:
- <Suspense fallback={<div>Loading stuff...</div>}> - <Await - promise={deferredStuff} - children={(data) => <h3 data-testid="deferred-stuff">{data}</h3>} - /> - </Suspense> + <Suspense fallback={<div>Loading stuff...</div>}> + <Await promise={deferredStuff}> + {(data) => <h3 data-testid="deferred-stuff">{data}</h3>} + </Await> + </Suspense>e2e/react-start/basic-hydrate-false/src/routes/redirect/$target/via-loader.tsx (1)
8-15: Add default case to handle unexpected target values.The switch statement lacks a default case, which means unexpected
targetvalues will be silently ignored and the component will render. While this may be intentional for testing, explicit handling improves clarity.Apply this diff to add a default case:
loader: ({ params: { target }, deps: { externalHost, reloadDocument } }) => { switch (target) { case 'internal': throw redirect({ to: '/posts', reloadDocument }) case 'external': throw redirect({ href: externalHost }) + default: + // Allow component to render for testing unexpected values + return } },e2e/react-start/basic-hydrate-false/src/routes/stream.tsx (1)
30-45: Add cleanup to cancel the stream reader on unmount.The
useEffectlacks a cleanup function to cancel the stream reader when the component unmounts, which could lead to memory leaks or errors if the component is unmounted before the stream completes.Apply this diff to add cleanup:
useEffect(() => { async function fetchStream() { const reader = stream.getReader() let chunk while (!(chunk = await reader.read()).done) { let value = chunk.value if (typeof value !== 'string') { value = decoder.decode(value, { stream: !chunk.done }) } setStreamData((prev) => [...prev, value]) } } fetchStream() + + return () => { + // Cancel the stream reader on unmount + stream.cancel() + } }, [])e2e/react-start/basic-hydrate-false/src/routes/users.$userId.tsx (1)
15-17: Consider preserving original error details.The catch block throws a generic error message, losing the original error details. While acceptable for test code, consider whether preserving the original error would aid debugging.
Optional improvement:
.catch((error) => { - throw new Error('Failed to fetch user') + throw new Error(`Failed to fetch user: ${error.message}`) })e2e/react-start/basic-hydrate-false/tests/hydrate.spec.ts (3)
4-40: Harden “no main bundle scripts” and “non-interactive” assertions (current checks are too broad).
html.includes('type="module"')can false-fail (any module script trips it), and the “non-interactive” claim isn’t asserted via user action.- const hasMainBundleScript = html.includes('type="module"') - expect(hasMainBundleScript).toBe(false) + // More specific: no TanStack client entry marker/scripts + expect(html).not.toContain('data-tsr-client-entry') + expect(html).not.toContain('virtual:tanstack-start-client-entry') @@ - // Button with onClick should not work - await expect(page.getByTestId('inactive-button')).toBeVisible() + // Button with onClick should not work (no hydration) + const btn = page.getByTestId('inactive-button') + await expect(btn).toBeVisible() + await btn.click() + await expect(page.getByTestId('click-count')).toHaveText('0')(If there’s no
click-countin the fixture, assert any other stable “post-click” change does not occur.)
58-68: Modulepreload assertion may be overly global.
If the page legitimately includes othermodulepreloadlinks (unrelated to client hydration),rel="modulepreload"will be too coarse. Consider asserting absence of router/app preloads specifically (eg byhrefprefix/known chunk markers) if available.
120-129: Prefer role-based navigation for resilience.
page.click('a[href="/static"]')can break if multiple anchors share that href (or if a base path is introduced).- await page.click('a[href="/static"]') + await page.getByRole('link', { name: 'Static', exact: true }).click()e2e/react-start/basic-hydrate-false/src/routes/not-found/via-loader.tsx (1)
17-23: Optional: makedata-servervalue less surprising.
data-server={typeof window}yields"undefined"/"object"strings; if you want a boolean-ish marker, considerdata-server={String(typeof window === 'undefined')}.packages/react-router/src/Scripts.tsx (1)
7-14: Remove duplicated docblock.
Two identical “Render body script tags…” blocks appear back-to-back./** * Render body script tags collected from route matches and SSR manifests. * Should be placed near the end of the document body. */ -/** - * Render body script tags collected from route matches and SSR manifests. - * Should be placed near the end of the document body. - */e2e/react-start/basic-hydrate-false/tests/script-duplication.spec.ts (2)
4-16: Make scriptsrcselector resilient (/script.jsvsscript.js, base paths).
Exactscript[src="script.js"]is brittle.- return document.querySelectorAll('script[src="script.js"]').length + return document.querySelectorAll('script[src$="script.js"]').length
18-43: Same brittleness in navigation tests; switch to$=selector too.- return document.querySelectorAll('script[src="script.js"]').length + return document.querySelectorAll('script[src$="script.js"]').lengthAlso applies to: 45-67
e2e/react-start/basic-hydrate-false/tests/not-found.spec.ts (1)
42-45: Consider using a more deterministic wait strategy.The fixed 250ms
setTimeoutfor preload wait may be flaky in slower CI environments. Consider waiting for a network request or usingpage.waitForResponseto confirm preload completion.if (preload) { await link.focus() - await new Promise((r) => setTimeout(r, 250)) + // Wait for preload network request to complete + await page.waitForLoadState('networkidle') }e2e/react-start/basic-hydrate-false/src/routes/users.tsx (1)
8-16: Consider preserving original error context.The catch block discards the original error, which may make debugging harder. Consider including the original error as the cause.
.catch(() => { - throw new Error('Failed to fetch users') + throw new Error('Failed to fetch users', { cause: error }) })Note: This would require capturing the error:
.catch((error) => { ... })
| "@tanstack/react-router": "workspace:^", | ||
| "@tanstack/react-router-devtools": "workspace:^", | ||
| "@tanstack/react-start": "workspace:^", |
There was a problem hiding this comment.
Use workspace:* for internal workspace deps (repo guideline).
Per coding guidelines for **/package.json, switch internal deps to workspace:* (e.g. @tanstack/react-router, @tanstack/react-start, @tanstack/router-e2e-utils).
"dependencies": {
- "@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:*",
"express": "^5.1.0",
...
},
"devDependencies": {
"@playwright/test": "^1.50.1",
"@tailwindcss/postcss": "^4.1.15",
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",
...
}Also applies to: 38-38
🤖 Prompt for AI Agents
In e2e/react-start/basic-hydrate-false/package.json around lines 24-26 (and also
line 38), the internal workspace dependencies use "workspace:^" but must follow
the repo guideline and use "workspace:*"; update the listed internal packages
(e.g. @tanstack/react-router, @tanstack/react-router-devtools,
@tanstack/react-start, @tanstack/router-e2e-utils where present) to use the
exact specifier "workspace:*" instead of "workspace:^" so the package.json
conforms to the workspace dependency convention.
| createSpaServer().then(async ({ app }) => | ||
| app.listen(port, () => { | ||
| console.info(`Client Server: http://localhost:${port}`) | ||
| }), | ||
| ) | ||
|
|
||
| createStartServer().then(async ({ app }) => | ||
| app.listen(startPort, () => { | ||
| console.info(`Start Server: http://localhost:${startPort}`) | ||
| }), | ||
| ) |
There was a problem hiding this comment.
Add .catch(...) on server startup promises to avoid silent boot failures.
Right now a failed dynamic import (or port bind error) can become an unhandled rejection with incomplete logs. Add a .catch((e) => { console.error(e); process.exitCode = 1 }) (or similar) on both startup chains.
🤖 Prompt for AI Agents
In e2e/react-start/basic-hydrate-false/server.js around lines 57 to 67, the
Promise chains returned by createSpaServer() and createStartServer() lack catch
handlers so startup failures (dynamic import errors or port bind failures) can
become unhandled rejections; add .catch((e) => { console.error(e);
process.exitCode = 1 }) to each chain (or equivalent error logging and setting a
non‑zero exit code) so errors are logged and the process indicates failure.
| head: () => ({ | ||
| meta: [ | ||
| { | ||
| charSet: 'utf-8', | ||
| }, | ||
| { | ||
| name: 'viewport', | ||
| content: 'width=device-width, initial-scale=1', | ||
| }, | ||
| ...seo({ | ||
| title: | ||
| 'TanStack Start | Type-Safe, Client-First, Full-Stack React Framework', | ||
| description: `TanStack Start is a type-safe, client-first, full-stack React framework. `, | ||
| }), | ||
| ], | ||
| links: [ | ||
| { rel: 'stylesheet', href: appCss }, | ||
| { | ||
| rel: 'apple-touch-icon', | ||
| sizes: '180x180', | ||
| href: '/apple-touch-icon.png', | ||
| }, | ||
| { | ||
| rel: 'icon', | ||
| type: 'image/png', | ||
| sizes: '32x32', | ||
| href: '/favicon-32x32.png', | ||
| }, | ||
| { | ||
| rel: 'icon', | ||
| type: 'image/png', | ||
| sizes: '16x16', | ||
| href: '/favicon-16x16.png', | ||
| }, | ||
| { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, | ||
| { rel: 'icon', href: '/favicon.ico' }, | ||
| ], | ||
| styles: [ | ||
| { | ||
| media: 'all and (min-width: 500px)', | ||
| children: ` | ||
| .inline-div { | ||
| color: white; | ||
| background-color: gray; | ||
| max-width: 250px; | ||
| }`, | ||
| }, | ||
| ], | ||
| }), |
There was a problem hiding this comment.
Fix invalid hex color (#fffff → #ffffff).
- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' },
+ { rel: 'manifest', href: '/site.webmanifest', color: '#ffffff' },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| head: () => ({ | |
| meta: [ | |
| { | |
| charSet: 'utf-8', | |
| }, | |
| { | |
| name: 'viewport', | |
| content: 'width=device-width, initial-scale=1', | |
| }, | |
| ...seo({ | |
| title: | |
| 'TanStack Start | Type-Safe, Client-First, Full-Stack React Framework', | |
| description: `TanStack Start is a type-safe, client-first, full-stack React framework. `, | |
| }), | |
| ], | |
| links: [ | |
| { rel: 'stylesheet', href: appCss }, | |
| { | |
| rel: 'apple-touch-icon', | |
| sizes: '180x180', | |
| href: '/apple-touch-icon.png', | |
| }, | |
| { | |
| rel: 'icon', | |
| type: 'image/png', | |
| sizes: '32x32', | |
| href: '/favicon-32x32.png', | |
| }, | |
| { | |
| rel: 'icon', | |
| type: 'image/png', | |
| sizes: '16x16', | |
| href: '/favicon-16x16.png', | |
| }, | |
| { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, | |
| { rel: 'icon', href: '/favicon.ico' }, | |
| ], | |
| styles: [ | |
| { | |
| media: 'all and (min-width: 500px)', | |
| children: ` | |
| .inline-div { | |
| color: white; | |
| background-color: gray; | |
| max-width: 250px; | |
| }`, | |
| }, | |
| ], | |
| }), | |
| head: () => ({ | |
| meta: [ | |
| { | |
| charSet: 'utf-8', | |
| }, | |
| { | |
| name: 'viewport', | |
| content: 'width=device-width, initial-scale=1', | |
| }, | |
| ...seo({ | |
| title: | |
| 'TanStack Start | Type-Safe, Client-First, Full-Stack React Framework', | |
| description: `TanStack Start is a type-safe, client-first, full-stack React framework. `, | |
| }), | |
| ], | |
| links: [ | |
| { rel: 'stylesheet', href: appCss }, | |
| { | |
| rel: 'apple-touch-icon', | |
| sizes: '180x180', | |
| href: '/apple-touch-icon.png', | |
| }, | |
| { | |
| rel: 'icon', | |
| type: 'image/png', | |
| sizes: '32x32', | |
| href: '/favicon-32x32.png', | |
| }, | |
| { | |
| rel: 'icon', | |
| type: 'image/png', | |
| sizes: '16x16', | |
| href: '/favicon-16x16.png', | |
| }, | |
| { rel: 'manifest', href: '/site.webmanifest', color: '#ffffff' }, | |
| { rel: 'icon', href: '/favicon.ico' }, | |
| ], | |
| styles: [ | |
| { | |
| media: 'all and (min-width: 500px)', | |
| children: ` | |
| .inline-div { | |
| color: white; | |
| background-color: gray; | |
| max-width: 250px; | |
| }`, | |
| }, | |
| ], | |
| }), |
🤖 Prompt for AI Agents
In e2e/react-start/basic-hydrate-false/src/routes/__root.tsx around lines 17 to
65, the manifest link uses an invalid 5-digit hex color '#fffff'; update it to a
valid 6-digit hex '#ffffff' (or another valid CSS color) so the manifest link's
color attribute is valid.
| filter: (page: { path: string }) => | ||
| ![ | ||
| '/this-route-does-not-exist', | ||
| '/redirect', | ||
| '/i-do-not-exist', | ||
| '/not-found/via-beforeLoad', | ||
| '/not-found/via-head', | ||
| '/not-found/via-loader', | ||
| '/users', | ||
| ].some((p) => page.path.includes(p)), |
There was a problem hiding this comment.
Refine filter logic to avoid unintended exclusions.
The filter function uses page.path.includes(p), which performs substring matching rather than exact path matching. This could cause false positives—for example, excluding /users will also exclude /users/123, /users/profile, or any path containing the substring /users.
Consider using exact matching or startsWith for more precise path filtering:
filter: (page: { path: string }) =>
![
'/this-route-does-not-exist',
'/redirect',
'/i-do-not-exist',
'/not-found/via-beforeLoad',
'/not-found/via-head',
'/not-found/via-loader',
'/users',
- ].some((p) => page.path.includes(p)),
+ ].some((p) => page.path === p || page.path.startsWith(`${p}/`)),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| filter: (page: { path: string }) => | |
| ![ | |
| '/this-route-does-not-exist', | |
| '/redirect', | |
| '/i-do-not-exist', | |
| '/not-found/via-beforeLoad', | |
| '/not-found/via-head', | |
| '/not-found/via-loader', | |
| '/users', | |
| ].some((p) => page.path.includes(p)), | |
| filter: (page: { path: string }) => | |
| ![ | |
| '/this-route-does-not-exist', | |
| '/redirect', | |
| '/i-do-not-exist', | |
| '/not-found/via-beforeLoad', | |
| '/not-found/via-head', | |
| '/not-found/via-loader', | |
| '/users', | |
| ].some((p) => page.path === p || page.path.startsWith(`${p}/`)), |
🤖 Prompt for AI Agents
In e2e/react-start/basic-hydrate-false/vite.config.ts around lines 17 to 26, the
filter currently uses page.path.includes(p) which causes unintended exclusions
by matching substrings (e.g., excluding /users also excludes /users/123); change
the logic to perform exact matches for the listed full paths (use page.path ===
p) or use prefix matching where appropriate (use page.path.startsWith(p) for
routes meant to exclude whole subpaths), updating the array entries to reflect
whether they are exact routes or prefixes and ensure the filter negates those
precise checks.
|
|
||
| const prerenderConfiguration = { | ||
| enabled: true, | ||
| filter: (page: { path: string }) => |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the vite.config.ts file to get full context
cat -n e2e/react-start/basic-hydrate-false/vite.config.tsRepository: TanStack/router
Length of output: 1441
🏁 Script executed:
# Search for prerender configuration and filter type definitions in the codebase
rg -nP --type=ts 'filter.*=.*\(page' -A3 -B3Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for TanStack Start plugin types and prerender options
rg -nP --type=ts 'PrerenderOptions|PrerenderConfig|type.*Prerender' -A5 -B2Repository: TanStack/router
Length of output: 2480
🏁 Script executed:
# Check for @tanstack/react-start imports and their types
rg -nP '@tanstack/react-start' --type=ts -A2 -B2Repository: TanStack/router
Length of output: 50372
🏁 Script executed:
# Search for any type definitions or interfaces related to filter function
ast-grep --pattern 'interface $_ {
filter: ($_) => $$$
}'Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Read the schema.ts file to see the full pagePrerenderOptionsSchema definition
sed -n '90,200p' packages/start-plugin-core/src/schema.tsRepository: TanStack/router
Length of output: 3003
🏁 Script executed:
# Search for filter property in prerender options/config
rg -n 'filter.*:\s*' packages/start-plugin-core/src/schema.ts -A2 -B2Repository: TanStack/router
Length of output: 341
🏁 Script executed:
# Check if there are any exported types from the start plugin
rg -n 'export.*type.*Prerender' packages/start-plugin-core/src/ -A5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for the actual filter function type definition
rg -n '\bfilter\b.*=>' packages/start-plugin-core/src/schema.ts -B5 -A5Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Find pageBaseSchema definition
rg -n 'pageBaseSchema\s*=' packages/start-plugin-core/src/schema.ts -A10Repository: TanStack/router
Length of output: 436
🏁 Script executed:
# Check if there are any type exports that correspond to the schema
rg -n 'infer.*pageSchema\|z\.infer' packages/start-plugin-core/src/schema.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for any index.ts or types.ts that might export the types
fd -e ts packages/start-plugin-core/src/ | head -20Repository: TanStack/router
Length of output: 488
🏁 Script executed:
# Check for exported types in schema.ts or index files
rg -n 'export' packages/start-plugin-core/src/schema.ts | head -20Repository: TanStack/router
Length of output: 277
🏁 Script executed:
# Check the main index file for the start-plugin-core package
cat packages/start-plugin-core/src/index.ts 2>/dev/null || cat packages/start-plugin-core/index.ts 2>/dev/null || echo "No index file found"Repository: TanStack/router
Length of output: 265
🏁 Script executed:
# Look for TypeScript declaration files
fd . packages/start-plugin-core/src/ -name "*.d.ts" -o -name "index.ts"Repository: TanStack/router
Length of output: 289
Use the exported Page type for the filter function parameter.
The inline type { path: string } is incomplete. The @tanstack/react-start/plugin/vite prerender filter should accept the full Page object which includes optional sitemap, fromCrawl, and prerender properties. Import and use the proper type to align with strict TypeScript type safety.
🤖 Prompt for AI Agents
In e2e/react-start/basic-hydrate-false/vite.config.ts around line 17, the
prerender filter parameter is typed as an inline { path: string } which is
incomplete; import and use the exported Page type from
'@tanstack/react-start/plugin/vite' (e.g. add an import for Page) and change the
filter signature to accept page: Page so the function receives the full Page
shape (including optional sitemap, fromCrawl, prerender) and satisfies strict
TypeScript checks.
| const { injectedHeadScripts } = await import( | ||
| 'tanstack-start-injected-head-scripts:v' | ||
| ) | ||
| if (injectedHeadScripts) { | ||
| script = `${injectedHeadScripts + ';'}${script}` | ||
| // Add React Refresh script (keep for HMR even when hydrate: false) | ||
| rootRoute.assets.push({ | ||
| tag: 'script', | ||
| attrs: { | ||
| type: 'module', | ||
| async: true, | ||
| }, | ||
| children: injectedHeadScripts, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Harden script children strings (escape clientEntry; consider guarding injected scripts).
children: \import('${startManifest.clientEntry}')`will break ifclientEntryever contains'`. Safer:- children:
import('${startManifest.clientEntry}'),
- children:
import(${JSON.stringify(startManifest.clientEntry)}),
- Dev-only `injectedHeadScripts` is injected verbatim into a `<script>` tag. If it can include `</script>`, it can terminate the tag early. If there’s any chance it’s not strictly controlled, consider sanitizing/encoding or enforcing a trusted source invariant.
Also applies to: 39-46
<details>
<summary>🤖 Prompt for AI Agents</summary>
In packages/start-server-core/src/router-manifest.ts around lines 21-35 (and
similarly at 39-46), the code injects raw script strings using children:
import('${startManifest.clientEntry}') and verbatim injectedHeadScripts, which
can break if clientEntry contains single quotes or if injectedHeadScripts
contains a closing </script>; to fix, always escape/serialize the clientEntry
when building the script string (e.g. replace the inline '${...}'
interpolation with a safe serialization such as JSON.stringify or an equivalent
escaping helper so any quotes are escaped), and treat injectedHeadScripts as
untrusted: either restrict this path to dev-only with an explicit invariant
check, sanitize/encode the content to disallow </script> (or base64/URL-encode
and decode in a safe runtime insertion), or assert a trusted-source invariant
before injecting; apply these changes at both locations mentioned.
</details>
<!-- fingerprinting:phantom:poseidon:falcon -->
<!-- This is an auto-generated comment by CodeRabbit -->
| 'data-tsr-client-entry': 'true', | ||
| }, | ||
| children: script, | ||
| children: `import('${startManifest.clientEntry}')`, |
There was a problem hiding this comment.
I think we could use a bit more commenting here, just so we explcitly point it out for anyone looking at this in the future, that when hydrate: false, then these scripts tagged with data-tsr-client-entry='true' will outright not be loaded into the DOM.
Also, for the default use-case, we should probably use the preview packages and init a few apps to make sure that HMR really is working just fine. Mostly, since the current flow of having the HMR script be synchronously imported in before even starting to bring in the client entry was something that got settled on (by Manuel) after a decent bit of trial and error.
| })) | ||
|
|
||
| // If hydrate is false, remove client entry imports but keep React Refresh for HMR | ||
| if (!shouldHydrate) { |
There was a problem hiding this comment.
This bit, might make more sense for it to be function that lives in router-core, to then be consumed by React, Solid, and Vue.
| @@ -0,0 +1,30 @@ | |||
| export function getHydrateStatus( | |||
There was a problem hiding this comment.
Also to be bounced into router-core.
| }) | ||
|
|
||
| // Warn about conflicting hydrate options | ||
| if (hydrateStatus.hasConflict) { |
There was a problem hiding this comment.
we don't have such a warning for ssr: false etc., we just use the "least common denominator".
do we need this warning here really?
also, if we do, we should only emit in DEV
| // ❌ Renders on client ❌ Data loads on client ✅ Interactive on client | ||
|
|
||
| // This combination doesn't make sense | ||
| ssr: false, hydrate: false |
There was a problem hiding this comment.
we probably should warn if this combination happens?
There was a problem hiding this comment.
since ssr and hydrate props can conflict, should we use a single config option to express both?
There was a problem hiding this comment.
or should we make it configurable whether ssr:false wins over hydrate:false?
|
|
||
| ## Inheritance | ||
|
|
||
| A child route inherits the `hydrate` configuration of its parent. If **any route** in the match has `hydrate: false`, the entire page will not be hydrated: |
There was a problem hiding this comment.
should this be flippable via a config?
so that you can also express
"If any route in the match has hydrate: true, the entire page will be hydrated"
tannerlinsley
force-pushed
the
feat/selective-hydration
branch
from
9c4b380 to
8f342d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/start/framework/react/guide/selective-hydration.md`:
- Around line 313-318: Update the fenced code block containing the TanStack
Router warning in selective-hydration.md to include a language specifier (e.g.,
plaintext or text) so the warning is syntax-highlighted correctly; locate the
block with the warning text "⚠️ [TanStack Router] Conflicting hydrate options
detected in route matches..." and change the opening triple backticks to include
the specifier (```plaintext).
| ``` | ||
| ⚠️ [TanStack Router] Conflicting hydrate options detected in route matches. | ||
| Some routes have hydrate: false while others have hydrate: true. | ||
| The page will NOT be hydrated, but this may not be the intended behavior. | ||
| Please ensure all routes in the match have consistent hydrate settings. | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The warning message code block is missing a language specifier, which prevents proper syntax highlighting. Since this is a plain text warning message, specify plaintext or text.
📝 Proposed fix
-```
+```plaintext
⚠️ [TanStack Router] Conflicting hydrate options detected in route matches.
Some routes have hydrate: false while others have hydrate: true.
The page will NOT be hydrated, but this may not be the intended behavior.
Please ensure all routes in the match have consistent hydrate settings.</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 313-313: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/start/framework/react/guide/selective-hydration.md` around lines 313 -
318, Update the fenced code block containing the TanStack Router warning in
selective-hydration.md to include a language specifier (e.g., plaintext or text)
so the warning is syntax-highlighted correctly; locate the block with the
warning text "⚠️ [TanStack Router] Conflicting hydrate options detected in route
matches..." and change the opening triple backticks to include the specifier
(```plaintext).
43c8661 to
2ecfbb7
Compare
Introduce route-level and default hydration controls, skip hydration client-entry/modulepreload assets when hydration is disabled, and add focused react-start e2e/unit coverage with documentation.
tannerlinsley
force-pushed
the
feat/selective-hydration
branch
from
2ecfbb7 to
c3acaab
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/react-start/basic/src/routes/hydrate-true.tsx (1)
4-9: Consider explicitly settinghydrate: truefor test completeness.The route is named
/hydrate-truebut doesn't explicitly set thehydrateoption. Comparing tohydrate-false.tsxwhich explicitly setshydrate: false, this test route should probably also explicitly sethydrate: trueto test the explicit opt-in behavior rather than relying on the default.♻️ Suggested change
export const Route = createFileRoute('/hydrate-true')({ + hydrate: true, loader: () => ({ message: 'hydrate true route rendered', }), component: HydrateTrueComponent, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/react-start/basic/src/routes/hydrate-true.tsx` around lines 4 - 9, The Route currently created with createFileRoute('/hydrate-true') sets a loader and component but does not explicitly set hydrate; update the Route definition to include hydrate: true in the route options object passed to createFileRoute so the test explicitly opts into hydration (i.e., add hydrate: true alongside loader and component in the object used to create the Route).packages/react-router/src/Scripts.tsx (1)
20-22: Consider consolidatinggetHydrateStatuscalls.
getHydrateStatusis called three times per render cycle (lines 21, 26, 65). While this is a pure function and the selectors are memoized, you could potentially hoist the hydration status to a singleuseRouterStatecall and derive the dependent computations from it, reducing redundant iterations over matches.This is a minor optimization concern and the current implementation is functionally correct.
Also applies to: 26-26, 65-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/Scripts.tsx` around lines 20 - 22, Hoist the hydration status into a single useRouterState invocation: call useRouterState once with select: (state) => getHydrateStatus(state.matches, router) (the current hydrateStatus variable) and then derive any other computations that previously invoked getHydrateStatus (the other select calls around lines referencing getHydrateStatus) from that single hydrateStatus value; update places that call getHydrateStatus again to read from hydrateStatus instead so you avoid three separate iterations over matches while keeping logic in functions/components like Scripts.tsx unchanged otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/start/framework/react/guide/selective-hydration.md`:
- Around line 320-325: The fenced code block in
docs/start/framework/react/guide/selective-hydration.md containing the warning
lines starting with "⚠️ [TanStack Router] Conflicting hydrate options detected
in route matches." is missing a language specifier; update that fenced block to
include a language token (use plaintext or text, e.g., ```plaintext) so the
warning is rendered with proper formatting.
---
Nitpick comments:
In `@e2e/react-start/basic/src/routes/hydrate-true.tsx`:
- Around line 4-9: The Route currently created with
createFileRoute('/hydrate-true') sets a loader and component but does not
explicitly set hydrate; update the Route definition to include hydrate: true in
the route options object passed to createFileRoute so the test explicitly opts
into hydration (i.e., add hydrate: true alongside loader and component in the
object used to create the Route).
In `@packages/react-router/src/Scripts.tsx`:
- Around line 20-22: Hoist the hydration status into a single useRouterState
invocation: call useRouterState once with select: (state) =>
getHydrateStatus(state.matches, router) (the current hydrateStatus variable) and
then derive any other computations that previously invoked getHydrateStatus (the
other select calls around lines referencing getHydrateStatus) from that single
hydrateStatus value; update places that call getHydrateStatus again to read from
hydrateStatus instead so you avoid three separate iterations over matches while
keeping logic in functions/components like Scripts.tsx unchanged otherwise.
| if (hydrateOption === false) { | ||
| hasExplicitFalse = true | ||
| } else if (hydrateOption === true && routeHydrate !== undefined) { | ||
| hasExplicitTrue = true | ||
| } |
There was a problem hiding this comment.
Could we skip some iterations w/ a for loop and a break here?
| if (hydrateOption === false) { | |
| hasExplicitFalse = true | |
| } else if (hydrateOption === true && routeHydrate !== undefined) { | |
| hasExplicitTrue = true | |
| } | |
| if (hydrateOption === false) { | |
| hasExplicitFalse = true | |
| } else if (hydrateOption === true && routeHydrate !== undefined) { | |
| hasExplicitTrue = true | |
| } | |
| if (hasExplicitFalse && hasExplicitTrue) break |
4 participants
Summary
hydrateoption and routerdefaultHydrateoption.e2e/react-start/basicfixture and remove the duplicatedbasic-hydrate-falsefixture.API
Behavior
hydratedefaults totrueunless overridden bydefaultHydrate.hydrate: false, the page does not hydrate.hydrate: false, hydration-specific scripts/preloads are excluded:Implementation Highlights
hydrate/defaultHydrateoptions and match support.getHydrateStatusutility<Scripts />client-entry outputdata-tsr-client-entryto allow robust filtering.Tests
packages/react-router/tests/Scripts.test.tsxfor client-entry stripping behavior.e2e/react-start/basic/src/routes/hydrate-false.tsxe2e/react-start/basic/src/routes/hydrate-true.tsxe2e/react-start/basic/tests/hydrate-false.spec.tse2e/react-start/basic-hydrate-false.Docs
docs/start/framework/react/guide/selective-hydration.mddocs/start/config.jsonCompatibility
Summary by CodeRabbit
New Features
hydrateanddefaultHydrateconfiguration options for improved performance on server-rendered pages.Documentation
Tests