Merge branch 'wip/fix-ui' into improvement/org-view

Author: waleedlatif1

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

@@ -1,11 +1,12 @@
 /**
  * @vitest-environment jsdom
  */
-import { act, useContext } from 'react'
+import { act, type ComponentType, lazy, Suspense, useContext } from 'react'
 import { createDeferred } from '@sim/testing/helpers/deferred'
 import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
-import { createRoot, type Root } from 'react-dom/client'
+import { createRoot, hydrateRoot, type Root } from 'react-dom/client'
+import { renderToString } from 'react-dom/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 vi.mock('@/lib/auth/client', () => authClientMock)
@@ -250,4 +251,39 @@ describe('SessionProvider', () => {
     invalidateSpy.mockRestore()
     h.unmount()
   })
+
+  it('keeps server-rendered rows when the session settles while their chunk is still loading', async () => {
+    ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
+    setSearch('')
+    const session = createDeferred<{ data: AppSession }>()
+    mockGetSession.mockReturnValue(session.promise)
+    const chunk = createDeferred<{ default: ComponentType }>()
+    const Row = () => <img alt='' src='/api/favicon?host=example.com' />
+    const Chunked = lazy(() => chunk.promise)
+    const page = (Content: ComponentType) => (
+      <QueryClientProvider client={new QueryClient()}>
+        <SessionProvider>
+          <main>
+            <Suspense fallback={null}>
+              <Content />
+            </Suspense>
+          </main>
+        </SessionProvider>
+      </QueryClientProvider>
+    )
+    const container = document.createElement('div')
+    container.innerHTML = renderToString(page(Row))
+    const served = container.querySelector('img')
+    expect(served).not.toBeNull()
+
+    const root = await act(async () => hydrateRoot(container, page(Chunked)))
+    await flush()
+    await act(async () => session.resolve({ data: FRESH_SESSION }))
+    await flush()
+    await act(async () => chunk.resolve({ default: Row }))
+    await flush()
+
+    expect(container.querySelector('img')).toBe(served)
+    act(() => root.unmount())
+  })
 })

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

@@ -1,7 +1,7 @@
 'use client'
 
 import type React from 'react'
-import { createContext, useEffect, useMemo } from 'react'
+import { createContext, useDeferredValue, useEffect, useMemo } from 'react'
 import { createLogger } from '@sim/logger'
 import { useQueryClient } from '@tanstack/react-query'
 import { client } from '@/lib/auth/client'
@@ -150,5 +150,10 @@ export function SessionProvider({ children }: { children: React.ReactNode }) {
     [data, isPending, error, refetch]
   )
 
-  return <SessionContext.Provider value={value}>{children}</SessionContext.Provider>
+  // The read settles above every route's Suspense boundary. An urgent context change makes React
+  // discard the server HTML of a boundary whose hydration is waiting on its chunk, so each row is
+  // rebuilt and asks for its images again; deferred, the change waits for hydration to finish.
+  const settledValue = useDeferredValue(value)
+
+  return <SessionContext.Provider value={settledValue}>{children}</SessionContext.Provider>
 }

apps/sim/components/favicon.tsx

@@ -32,25 +32,21 @@ interface FaviconProps {
 export function Favicon({ host, fallback, className, onLoad }: FaviconProps) {
   const [, rerender] = useReducer((failures: number) => failures + 1, 0)
   if (!host || knownMisses.has(host)) return fallback
-  const recordMiss = () => {
-    knownMisses.set(host, true)
-    rerender()
-  }
   return (
     <img
       ref={(image) => {
-        if (!image?.complete) return
-        if (image.naturalWidth > 0) onLoad?.()
-        // A failure before hydration fires no onError, and a remount would ask for the icon again.
-        else void image.decode().catch(recordMiss)
+        if (onLoad && image?.complete && image.naturalWidth > 0) onLoad()
       }}
       src={faviconSrc(host)}
       alt=''
       loading='lazy'
       decoding='async'
       className={cn('rounded-sm', className)}
       onLoad={onLoad}
-      onError={recordMiss}
+      onError={() => {
+        knownMisses.set(host, true)
+        rerender()
+      }}
     />
   )
 }

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

@@ -47,6 +47,35 @@ const PER_WORKSPACE_READ = [
 ]
 const isPerWorkspaceRead = (path: string) => PER_WORKSPACE_READ.some((read) => read.test(path))
 
+type FaviconTrace = Window & { faviconTrace?: string[] }
+
+/**
+ * Runs in the page: logs each favicon `<img>` the document gains or loses. React's streamed reveal
+ * moves one (it stays connected), and a discarded boundary removes it for good.
+ */
+function traceFaviconImages() {
+  const trace: string[] = []
+  ;(window as FaviconTrace).faviconTrace = trace
+  const faviconsIn = (node: Node) =>
+    node instanceof Element
+      ? [node, ...node.querySelectorAll('img')].filter(
+          (element) => element.tagName === 'IMG' && element.getAttribute('src')?.includes('favicon')
+        )
+      : []
+  const log = (change: string, nodes: NodeList) => {
+    const stamp = `${Math.round(performance.now())}ms`
+    for (const node of nodes)
+      for (const image of faviconsIn(node))
+        trace.push(`${stamp} ${change} ${image.getAttribute('src')} connected=${image.isConnected}`)
+  }
+  new MutationObserver((mutations) => {
+    for (const { addedNodes, removedNodes } of mutations) {
+      log('added', addedNodes)
+      log('removed', removedNodes)
+    }
+  }).observe(document, { childList: true, subtree: true })
+}
+
 export async function firstPaintChecks() {
   section('6 First paint')
   const pages = [
@@ -180,11 +209,21 @@ export async function firstPaintChecks() {
         async () => {
           await withSession(persona, async (session) => {
             const { page } = session
-            // A returning member's browser already holds the app's scripts.
+            // `open` routes the socket token, which turns off the context's HTTP cache, so both loads
+            // fetch every script: hydration can wait on a route's chunk, as on a first visit.
             await page.goto(orgUrl(`/${entry.path}`))
             await entry.primary(page).first().waitFor({ timeout: 30_000 })
             await session.settle()
             session.requests.length = 0
+            const hydrationErrors: string[] = []
+            page.on('pageerror', ({ message }) => {
+              if (/hydrat|#4(18|19|23|25)\b/i.test(message)) hydrationErrors.push(message)
+            })
+            page.on('console', (message) => {
+              if (/hydrat|#4(18|19|23|25)\b/i.test(message.text()))
+                hydrationErrors.push(message.text())
+            })
+            await page.addInitScript(traceFaviconImages)
             const started = performance.now()
             await page.goto(orgUrl(`/${entry.path}`), { waitUntil: 'commit' })
             await entry.primary(page).first().waitFor({ timeout: 30_000 })
@@ -222,7 +261,18 @@ export async function firstPaintChecks() {
               prefetchedAndRefetched: refetched,
               perWorkspaceReads,
             })
-            assert.deepEqual(duplicates, [], 'A read was fetched twice on first paint')
+            const faviconTrace = await page.evaluate(
+              () => (window as FaviconTrace).faviconTrace ?? []
+            )
+            assert.deepEqual(
+              duplicates,
+              [],
+              [
+                'A read was fetched twice on first paint',
+                `Hydration errors: ${JSON.stringify(hydrationErrors)}`,
+                `Favicon images: ${JSON.stringify(faviconTrace)}`,
+              ].join('\n')
+            )
             assert.deepEqual(refetched, [], 'A prefetched key was fetched again on first paint')
             assert.deepEqual(perWorkspaceReads, [], 'The composer read every workspace on load')
           })