Skip to content

Commit eabb2c3

Browse files
committed
fix(session): keep server-rendered rows when the session settles during hydration
The session read settles above every route's Suspense boundary. When a boundary's hydration is still waiting on its route chunk, that urgent context change makes React drop its server HTML and rebuild it on the client, silently, with no hydration error. On Links every row's favicon was requested again, since a 204 is not reusable within a document. useDeferredValue holds the change until hydration finishes. The first-paint check only began to see this when routing the socket token turned off its HTTP cache, so the scripts it loads are no longer already held. Its comment now says so, and a duplicate read's failure now lists the favicon images added and removed and any hydration errors.
1 parent 8c4bfcb commit eabb2c3

3 files changed

Lines changed: 97 additions & 6 deletions

File tree

‎apps/sim/app/_shell/providers/session-provider.test.tsx‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import { act, useContext } from 'react'
4+
import { act, type ComponentType, lazy, Suspense, useContext } from 'react'
55
import { createDeferred } from '@sim/testing/helpers/deferred'
66
import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
77
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
8-
import { createRoot, type Root } from 'react-dom/client'
8+
import { createRoot, hydrateRoot, type Root } from 'react-dom/client'
9+
import { renderToString } from 'react-dom/server'
910
import { beforeEach, describe, expect, it, vi } from 'vitest'
1011

1112
vi.mock('@/lib/auth/client', () => authClientMock)
@@ -250,4 +251,39 @@ describe('SessionProvider', () => {
250251
invalidateSpy.mockRestore()
251252
h.unmount()
252253
})
254+
255+
it('keeps server-rendered rows when the session settles while their chunk is still loading', async () => {
256+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
257+
setSearch('')
258+
const session = createDeferred<{ data: AppSession }>()
259+
mockGetSession.mockReturnValue(session.promise)
260+
const chunk = createDeferred<{ default: ComponentType }>()
261+
const Row = () => <img alt='' src='/api/favicon?host=example.com' />
262+
const Chunked = lazy(() => chunk.promise)
263+
const page = (Content: ComponentType) => (
264+
<QueryClientProvider client={new QueryClient()}>
265+
<SessionProvider>
266+
<main>
267+
<Suspense fallback={null}>
268+
<Content />
269+
</Suspense>
270+
</main>
271+
</SessionProvider>
272+
</QueryClientProvider>
273+
)
274+
const container = document.createElement('div')
275+
container.innerHTML = renderToString(page(Row))
276+
const served = container.querySelector('img')
277+
expect(served).not.toBeNull()
278+
279+
const root = await act(async () => hydrateRoot(container, page(Chunked)))
280+
await flush()
281+
await act(async () => session.resolve({ data: FRESH_SESSION }))
282+
await flush()
283+
await act(async () => chunk.resolve({ default: Row }))
284+
await flush()
285+
286+
expect(container.querySelector('img')).toBe(served)
287+
act(() => root.unmount())
288+
})
253289
})

‎apps/sim/app/_shell/providers/session-provider.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22

33
import type React from 'react'
4-
import { createContext, useEffect, useMemo } from 'react'
4+
import { createContext, useDeferredValue, useEffect, useMemo } from 'react'
55
import { createLogger } from '@sim/logger'
66
import { useQueryClient } from '@tanstack/react-query'
77
import { client } from '@/lib/auth/client'
@@ -150,5 +150,10 @@ export function SessionProvider({ children }: { children: React.ReactNode }) {
150150
[data, isPending, error, refetch]
151151
)
152152

153-
return <SessionContext.Provider value={value}>{children}</SessionContext.Provider>
153+
// The read settles above every route's Suspense boundary. An urgent context change makes React
154+
// discard the server HTML of a boundary whose hydration is waiting on its chunk, so each row is
155+
// rebuilt and asks for its images again; deferred, the change waits for hydration to finish.
156+
const settledValue = useDeferredValue(value)
157+
158+
return <SessionContext.Provider value={settledValue}>{children}</SessionContext.Provider>
154159
}

‎apps/sim/scripts/org-redesign-e2e/checks/first-paint.ts‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,35 @@ const PER_WORKSPACE_READ = [
4747
]
4848
const isPerWorkspaceRead = (path: string) => PER_WORKSPACE_READ.some((read) => read.test(path))
4949

50+
type FaviconTrace = Window & { faviconTrace?: string[] }
51+
52+
/**
53+
* Runs in the page: logs each favicon `<img>` the document gains or loses. React's streamed reveal
54+
* moves one (it stays connected), and a discarded boundary removes it for good.
55+
*/
56+
function traceFaviconImages() {
57+
const trace: string[] = []
58+
;(window as FaviconTrace).faviconTrace = trace
59+
const faviconsIn = (node: Node) =>
60+
node instanceof Element
61+
? [node, ...node.querySelectorAll('img')].filter(
62+
(element) => element.tagName === 'IMG' && element.getAttribute('src')?.includes('favicon')
63+
)
64+
: []
65+
const log = (change: string, nodes: NodeList) => {
66+
const stamp = `${Math.round(performance.now())}ms`
67+
for (const node of nodes)
68+
for (const image of faviconsIn(node))
69+
trace.push(`${stamp} ${change} ${image.getAttribute('src')} connected=${image.isConnected}`)
70+
}
71+
new MutationObserver((mutations) => {
72+
for (const { addedNodes, removedNodes } of mutations) {
73+
log('added', addedNodes)
74+
log('removed', removedNodes)
75+
}
76+
}).observe(document, { childList: true, subtree: true })
77+
}
78+
5079
export async function firstPaintChecks() {
5180
section('6 First paint')
5281
const pages = [
@@ -177,11 +206,21 @@ export async function firstPaintChecks() {
177206
async () => {
178207
await withSession(persona, async (session) => {
179208
const { page } = session
180-
// A returning member's browser already holds the app's scripts.
209+
// `open` routes the socket token, which turns off the context's HTTP cache, so both loads
210+
// fetch every script: hydration can wait on a route's chunk, as on a first visit.
181211
await page.goto(orgUrl(`/${entry.path}`))
182212
await entry.primary(page).first().waitFor({ timeout: 30_000 })
183213
await session.settle()
184214
session.requests.length = 0
215+
const hydrationErrors: string[] = []
216+
page.on('pageerror', ({ message }) => {
217+
if (/hydrat|#4(18|19|23|25)\b/i.test(message)) hydrationErrors.push(message)
218+
})
219+
page.on('console', (message) => {
220+
if (/hydrat|#4(18|19|23|25)\b/i.test(message.text()))
221+
hydrationErrors.push(message.text())
222+
})
223+
await page.addInitScript(traceFaviconImages)
185224
const started = performance.now()
186225
await page.goto(orgUrl(`/${entry.path}`), { waitUntil: 'commit' })
187226
await entry.primary(page).first().waitFor({ timeout: 30_000 })
@@ -219,7 +258,18 @@ export async function firstPaintChecks() {
219258
prefetchedAndRefetched: refetched,
220259
perWorkspaceReads,
221260
})
222-
assert.deepEqual(duplicates, [], 'A read was fetched twice on first paint')
261+
const faviconTrace = await page.evaluate(
262+
() => (window as FaviconTrace).faviconTrace ?? []
263+
)
264+
assert.deepEqual(
265+
duplicates,
266+
[],
267+
[
268+
'A read was fetched twice on first paint',
269+
`Hydration errors: ${JSON.stringify(hydrationErrors)}`,
270+
`Favicon images: ${JSON.stringify(faviconTrace)}`,
271+
].join('\n')
272+
)
223273
assert.deepEqual(refetched, [], 'A prefetched key was fetched again on first paint')
224274
assert.deepEqual(perWorkspaceReads, [], 'The composer read every workspace on load')
225275
})

0 commit comments

Comments
 (0)