Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fresh-ducks-hydrate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/react-router': patch
---

Avoid wrapping root components that may render the HTML document in a Suspense boundary during SSR and hydration.
3 changes: 2 additions & 1 deletion e2e/react-start/dev-ssr-styles/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
"test:e2e:dev:bundled-dev:nitro": "MODE=dev VITE_USE_NITRO=true E2E_VITE_BUNDLED_DEV=true playwright test --project=chromium",
"test:e2e:dev:disabled:bundled-dev:nitro": "MODE=dev SSR_STYLES=disabled VITE_USE_NITRO=true E2E_VITE_BUNDLED_DEV=true playwright test --project=chromium",
"test:e2e:dev:custom-basepath:bundled-dev:nitro": "MODE=dev SSR_STYLES=custom-basepath VITE_USE_NITRO=true E2E_VITE_BUNDLED_DEV=true playwright test --project=chromium",
"test:e2e:prod": "MODE=prod playwright test tests/issue-8053-root-document-hydration.spec.ts --project=chromium",
"test:e2e:bundled-dev": "pnpm run test:e2e:dev:bundled-dev && pnpm run test:e2e:dev:disabled:bundled-dev && pnpm run test:e2e:dev:custom-basepath:bundled-dev",
"test:e2e:nitro:bundled-dev": "pnpm run test:e2e:dev:bundled-dev:nitro && pnpm run test:e2e:dev:disabled:bundled-dev:nitro && pnpm run test:e2e:dev:custom-basepath:bundled-dev:nitro",
"test:e2e": "rm -rf port*.txt; pnpm run test:e2e:dev && pnpm run test:e2e:dev:disabled && pnpm run test:e2e:dev:custom-basepath && pnpm run test:e2e:bundled-dev",
"test:e2e": "rm -rf port*.txt; pnpm run test:e2e:dev && pnpm run test:e2e:dev:disabled && pnpm run test:e2e:dev:custom-basepath && pnpm run test:e2e:bundled-dev && pnpm run test:e2e:prod",
"test:e2e:nitro": "rm -rf port*.txt; pnpm run test:e2e:dev:nitro && pnpm run test:e2e:dev:disabled:nitro && pnpm run test:e2e:dev:custom-basepath:nitro && pnpm run test:e2e:nitro:bundled-dev"
},
"dependencies": {
Expand Down
12 changes: 12 additions & 0 deletions e2e/react-start/dev-ssr-styles/src/routes/__root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Scripts,
createRootRoute,
} from '@tanstack/react-router'
import { useEffect } from 'react'
import '~/styles/app.css'

export const Route = createRootRoute({
Expand All @@ -13,16 +14,27 @@ export const Route = createRootRoute({
{ name: 'viewport', content: 'width=device-width, initial-scale=1' },
],
}),
pendingComponent: () => null,
component: RootComponent,
})

function RootComponent() {
useEffect(() => {
document.documentElement.dataset.hydrated = 'true'
Comment thread
Sheraff marked this conversation as resolved.
Resolved
}, [])

return (
<html>
<head>
<HeadContent />
</head>
<body>
<div data-testid="issue-8053-ssr-node" />
<script
dangerouslySetInnerHTML={{
__html: `window.__issue8053SsrNode ??= document.currentScript.previousElementSibling`,
}}
/>
<Outlet />
<Scripts />
</body>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { expect } from '@playwright/test'
import { test } from '@tanstack/router-e2e-utils'

test('#8053: a root pendingComponent preserves the SSR document during hydration', async ({
page,
}) => {
await page.addInitScript(() => {
const state = { removed: false }
;(window as any).__issue8053Hydration = state

new MutationObserver((records) => {
const ssrNode = (window as any).__issue8053SsrNode as Node | undefined

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Replace any with typed Window properties.

window as any removes type checks for the SSR-node and hydration-state contract. Declare these test properties on Window and access them directly.

Proposed fix
+declare global {
+  interface Window {
+    __issue8053Hydration?: { removed: boolean }
+    __issue8053SsrNode?: Node
+  }
+}
+
-    ;(window as any).__issue8053Hydration = state
+    window.__issue8053Hydration = state
...
-      const ssrNode = (window as any).__issue8053SsrNode as Node | undefined
+      const ssrNode = window.__issue8053SsrNode
...
-      captured: !!(window as any).__issue8053SsrNode,
-      removed: (window as any).__issue8053Hydration.removed,
+      captured: !!window.__issue8053SsrNode,
+      removed: window.__issue8053Hydration!.removed,

As per coding guidelines: **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.

Also applies to: 45-50

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@e2e/react-start/dev-ssr-styles/tests/issue-8053-root-document-hydration.spec.ts`
around lines 9 - 12, Declare __issue8053Hydration and __issue8053SsrNode on the
Window interface with their appropriate types, then update the hydration-state
assignment and MutationObserver access to use window.__issue8053Hydration and
window.__issue8053SsrNode directly. Remove both window as any casts while
preserving the existing SSR-node and hydration-state behavior.

Source: Coding guidelines

if (!ssrNode) {
return
}

for (const record of records) {
for (const removedNode of record.removedNodes) {
if (
removedNode === ssrNode ||
(removedNode instanceof Element && removedNode.contains(ssrNode))
) {
state.removed = true
}
}
}
}).observe(document, { childList: true, subtree: true })
})

const hydrationErrors: Array<string> = []
Comment thread
Sheraff marked this conversation as resolved.
Resolved
page.on('console', (message) => {
if (message.type() === 'error') {
hydrationErrors.push(message.text())
}
})
page.on('pageerror', (error) => {
hydrationErrors.push(error.message)
})

const response = await page.goto('/')
expect(response?.ok()).toBe(true)
await expect(page.locator('html')).toHaveAttribute('data-hydrated', 'true')

expect(
await page.evaluate(() => ({
captured: !!(window as any).__issue8053SsrNode,
removed: (window as any).__issue8053Hydration.removed,
replaced:
(window as any).__issue8053SsrNode !==
document.querySelector('[data-testid="issue-8053-ssr-node"]'),
})),
).toEqual({
captured: true,
removed: false,
replaced: false,
})
expect(
hydrationErrors.filter((error) =>
/hydrat|did not match|server rendered HTML/i.test(error),
),
).toEqual([])
})
24 changes: 22 additions & 2 deletions packages/react-router/src/Match.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ const outletMatchSelectionEqual = (
b: OutletMatchSelection,
) => a[0] === b[0] && a[1] === b[1]

function canWrapRouteInSuspense(route: AnyRoute, resolvedNoSsr: boolean) {
return (
!route.isRoot ||
!!(route.options as RootRouteOptions).shellComponent ||
!!route.options.wrapInSuspense ||
resolvedNoSsr
)
}

export const Match = React.memo(function MatchImpl({
routeId,
}: {
Expand Down Expand Up @@ -89,10 +98,14 @@ function MatchView({
: route.options.notFoundComponent

const resolvedNoSsr = match.ssr === false || match.ssr === 'data-only'
// A root component may render the document itself. Only place its Suspense
// boundary inside an explicit shell, unless the route forcefully opts in.
const canWrapInSuspense = canWrapRouteInSuspense(route, resolvedNoSsr)
const ResolvedSuspenseBoundary =
canWrapInSuspense &&
(route.options.wrapInSuspense ??
pendingElement ??
((route.options.errorComponent as any)?.preload || resolvedNoSsr))
pendingElement ??
((route.options.errorComponent as any)?.preload || resolvedNoSsr))
? React.Suspense
: SafeFragment

Expand Down Expand Up @@ -198,6 +211,13 @@ export const MatchInner = React.memo(function MatchInnerImpl({
}, [key, route.options.component, router.options.defaultComponent])

if (match.status === 'pending') {
const resolvedNoSsr = match.ssr === false || match.ssr === 'data-only'
if (!canWrapRouteInSuspense(route, resolvedNoSsr)) {
// Replacing an SSR document root with pending UI would remove <html>.
// Hydrated matches retain their prior data, so keep rendering it while
// pure CSR roots can safely render the pending element directly.
return router.ssr ? out : renderPending(router, route)
}
if (router._tx) {
throw router._tx[5]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ test('a global not-found destination does not retain the mounted root success',
})

expect(await screen.findByTestId('pending')).toBeVisible()
expect(screen.getByTestId('content')).not.toBeVisible()
expect(screen.queryByTestId('content')).not.toBeInTheDocument()
Comment thread
Sheraff marked this conversation as resolved.
Resolved

await act(async () => {
missingLoader.resolve()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { expect, test } from 'vitest'
import { Outlet, createRootRoute, createRoute, createRouter } from '../src'
import {
RouterServer,
createRequestHandler,
renderRouterToStream,
} from '../src/ssr/server'

// https://github.com/TanStack/router/issues/8053
test('a root pendingComponent does not wrap the SSR document in Suspense', async () => {
const rootRoute = createRootRoute({
pendingComponent: () => null,
component: () => (
<html>
<head>
<title>Root document</title>
</head>
<body>
<Outlet />
</body>
</html>
),
})
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => <main>Root content</main>,
})
const handler = createRequestHandler({
request: new Request('http://localhost/'),
createRouter: () =>
createRouter({
routeTree: rootRoute.addChildren([indexRoute]),
isServer: true,
}),
})

const response = await handler(({ request, router, responseHeaders }) =>
renderRouterToStream({
request,
router,
responseHeaders,
children: <RouterServer router={router} />,
}),
)
const html = await response.text()
const serverDocument = new DOMParser().parseFromString(html, 'text/html')

expect(response.status).toBe(200)
expect(serverDocument.body.textContent).toContain('Root content')
expect(html).not.toContain('<!--html--><!--head--><!--body-->')
})
32 changes: 17 additions & 15 deletions packages/react-router/tests/root-pending-min.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ test('a post-hydration root reload keeps its fallback through pendingMinMs', asy
})

expect(rootLoader).toHaveBeenCalledTimes(1)
expect(screen.getByTestId('root-pending')).toBeInTheDocument()
expect(screen.getByTestId('root-content')).not.toBeVisible()
expect(screen.queryByTestId('root-pending')).not.toBeInTheDocument()
expect(screen.getByTestId('root-content')).toHaveTextContent('Generation 1')

await act(async () => {
reloadGate.resolve()
Expand All @@ -95,7 +95,7 @@ test('a post-hydration root reload keeps its fallback through pendingMinMs', asy
await act(async () => {
await vi.advanceTimersByTimeAsync(99)
})
expect(screen.getByTestId('root-pending')).toBeInTheDocument()
expect(screen.queryByTestId('root-pending')).not.toBeInTheDocument()

await act(async () => {
await vi.advanceTimersByTimeAsync(1)
Expand All @@ -105,7 +105,7 @@ test('a post-hydration root reload keeps its fallback through pendingMinMs', asy
expect(screen.getByTestId('root-content')).toHaveTextContent('Generation 2')
})

test('root route hydration preserves component state across its Suspense boundary', async () => {
test('root route hydration without a pending boundary preserves component state', async () => {
const mounts = vi.fn()
const unmounts = vi.fn()
const initializers = vi.fn(() => 'preserved')
Expand All @@ -127,14 +127,13 @@ test('root route hydration preserves component state across its Suspense boundar
})
await router.load()

// Model the server render and the client router produced by hydrate(). The
// outer root boundary is intentionally absent from both trees, while the
// route's own pending boundary is present in both.
// Model the server render and the client router produced by hydrate(). A root
// without a separate shell stays unwrapped on both sides of hydration.
router.ssr = { manifest: { routes: {} } }
router.isServer = true
const html = renderToString(<RouterProvider router={router} />)
router.isServer = false
expect(html).toContain('<!--$-->')
expect(html).not.toContain('<!--$-->')
expect(html).toContain('preserved')

const container = document.createElement('div')
Expand All @@ -153,19 +152,23 @@ test('root route hydration preserves component state across its Suspense boundar
})

expect(container).toHaveTextContent('preserved')
// One initializer belongs to the server render and one to client
// hydration. The stable boundary must preserve that hydrated client
// instance instead of creating a third one.
// One initializer belongs to the server render and one to client hydration.
expect(initializers).toHaveBeenCalledTimes(2)
expect(mounts).toHaveBeenCalledTimes(1)
expect(unmounts).not.toHaveBeenCalled()
expect(consoleError).not.toHaveBeenCalled()
})

test('server rendering uses the root pending boundary for route component suspension', async () => {
test('server rendering uses the root pending boundary inside its document shell', async () => {
const gate = createControlledPromise<void>()
const rootRoute = createRootRoute({
pendingComponent: () => <div>Server root pending</div>,
shellComponent: ({ children }) => (
<html>
<head />
<body>{children}</body>
</html>
),
component: () => {
throw gate
},
Expand All @@ -179,8 +182,7 @@ test('server rendering uses the root pending boundary for route component suspen

const html = renderToString(<RouterProvider router={router} />)

// renderToString cannot wait for Suspense, but the root route's stable
// boundary contains the suspension and emits its fallback. Streaming SSR
// can wait for the same boundary instead.
// renderToString cannot wait for Suspense, but the boundary inside the root
// shell contains the suspension and emits its fallback.
expect(html).toContain('Server root pending')
})
Loading