diff --git a/.changeset/6378-boot-redirect-splash.md b/.changeset/6378-boot-redirect-splash.md new file mode 100644 index 0000000000..739b8c31a9 --- /dev/null +++ b/.changeset/6378-boot-redirect-splash.md @@ -0,0 +1,47 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/console': patch +--- + +The console boot no longer flashes a fully-white frame after the splash has painted +(objectui#6378). + +Cause, established by measurement before any fix was written — the card named +`LoadingScreen`'s unmount timing and `RouteFader` as suspects and both are exonerated. +A CDP `Page.startScreencast` frame ledger (every frame classified with the card's own +rule: white when no colour channel falls below 242) was correlated against a DOM-state +ledger on the same clock (`performance.timeOrigin`), against the production +`apps/console` bundle with the boot endpoints mocked. `RouteFader` never mounts on the +boot path at all, and `LoadingScreen` unmounts exactly when its own gate says to. What +is wrong is what REPLACES it: every readiness gate renders the splash while it waits and +a bare `` the moment it decides. `` renders `null` and react-router +runs the navigation as a transition, so the destination tree renders while the commit +that already dropped the splash is what the compositor is showing — 41–147 ms during +which `#root` holds no view and the viewport is the bare page background. The flash is +intermittent only because it depends on a frame being swapped inside that window; the +window itself was present on every measured boot. + +`RedirectWithSplash` (new, `@object-ui/app-shell`) pairs the same `` with the +same `LoadingScreen` the gate one line above was already rendering, so the handoff +changes no pixels and the transition runs underneath an unchanged screen. The console's +three boot redirects use it: the auth gate's `/login` bounce, the `/` landing resolver, +and the catch-all route. The nested organization `index` redirect deliberately does not — +it fires under an already-painted layout, where covering the screen would be the +regression. + +Acceptance campaign — same instrument on both sides, the two arms INTERLEAVED inside one +process and one browser so drift in this shared container's load lands on both equally. +102 paired boots per arm across five cells (signed-out `/`, signed-in `/`, an unmatched +entry, and the card's two throttled network profiles). The empty-viewport window: 102/102 +pre-fix, 0/102 post-fix. The white frame itself, pooled over the three cells where the +pre-fix build actually flashed: 67/87 (77%) pre-fix, 0/87 post-fix — 95% upper bound on +the residual rate 3.4%, against a card-reported defect rate of ~1/3. The two throttled +cells are reported but NOT pooled: the pre-fix build flashed 0 times there, so before and +after agree and those cells prove nothing about the pixels (they still separate 15/15 vs +0/15 on the DOM window). + +`e2e/console-boot-indicator.spec.ts` gains the deterministic half as a gate — after +React's first commit the viewport centre must never stop being covered. That reading is +what makes an intermittent defect gateable: the flash needs a frame to be swapped inside +the window, but the window itself was present on every measured boot. Verified red-first, +6/6 red on the pre-fix bundle and 6/6 green on this one. diff --git a/apps/console/src/App.tsx b/apps/console/src/App.tsx index 129c418e34..e7cb02e2f5 100644 --- a/apps/console/src/App.tsx +++ b/apps/console/src/App.tsx @@ -38,6 +38,7 @@ import { DefaultAiChatPage, getProductName, getFaviconUrl, + RedirectWithSplash, } from '@object-ui/app-shell'; import { AppContent } from './AppContent'; @@ -366,7 +367,17 @@ export function App() { } /> - } /> + {/* `RedirectWithSplash`, not a bare `` (objectui#6378). + * This is a BOOT redirect: a URL the router does not know is + * commonly the very first thing a session renders (a stale deep + * link, or a console served under a mount path with no + * `` for `resolveBasename` to read), so it fires with + * the splash freshly torn down and nothing else on screen. + * Measured on that entry: 35-95 ms of empty `#root`. The nested + * `index` redirect above is deliberately NOT changed — it fires + * under an already-painted organization layout, where covering the + * screen with a splash would be the regression. */} + } /> diff --git a/apps/console/src/__tests__/App.docsPortalLazy.test.tsx b/apps/console/src/__tests__/App.docsPortalLazy.test.tsx index 98e159ef37..b5d8b9da99 100644 --- a/apps/console/src/__tests__/App.docsPortalLazy.test.tsx +++ b/apps/console/src/__tests__/App.docsPortalLazy.test.tsx @@ -88,6 +88,10 @@ vi.mock('@object-ui/app-shell', () => ({ ConsoleShell: passthrough, ConsoleToaster: () => null, LoadingScreen: stub('loading-screen'), + // objectui#6378 — App.tsx's catch-all route element. Stubbed like every + // other chrome symbol here; that it renders the splash while redirecting + // is pinned by `packages/app-shell/src/chrome/RedirectWithSplash.test.tsx`. + RedirectWithSplash: stub('redirect-with-splash'), RequireAiSurface: passthrough, SystemRedirect: () => null, DefaultHomeLayout: passthrough, diff --git a/apps/console/src/__tests__/internalFormShell.test.tsx b/apps/console/src/__tests__/internalFormShell.test.tsx index 3b2fc69be9..d3c3a31f27 100644 --- a/apps/console/src/__tests__/internalFormShell.test.tsx +++ b/apps/console/src/__tests__/internalFormShell.test.tsx @@ -60,6 +60,9 @@ vi.mock('@object-ui/app-shell', () => ({ // The route elements are built when App renders, so this export is // read even by a test that never visits /docs. LoadingScreen: stub('loading-screen'), + // App.tsx's catch-all route element (objectui#6378). Read for the same + // reason `LoadingScreen` is: the route table is built when App renders. + RedirectWithSplash: stub('redirect-with-splash'), RequireAiSurface: passthrough, SystemRedirect: () => null, DefaultHomeLayout: ({ children }: { children?: ReactNode }) => ( diff --git a/apps/console/src/components/ProtectedRoute.tsx b/apps/console/src/components/ProtectedRoute.tsx index cafc292210..f3d7064043 100644 --- a/apps/console/src/components/ProtectedRoute.tsx +++ b/apps/console/src/components/ProtectedRoute.tsx @@ -11,9 +11,14 @@ */ import type { ReactNode } from 'react'; -import { Navigate, useLocation } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { AuthGuard } from '@object-ui/auth'; -import { ConnectedShell, RequireOrganization, LoadingFallback } from '@object-ui/app-shell'; +import { + ConnectedShell, + RequireOrganization, + LoadingFallback, + RedirectWithSplash, +} from '@object-ui/app-shell'; /** * Where an unauthenticated visitor to a protected route goes. @@ -27,7 +32,11 @@ export function LoginRedirect() { const location = useLocation(); const redirect = location.pathname + location.search; const search = redirect && redirect !== '/' ? `?redirect=${encodeURIComponent(redirect)}` : ''; - return ; + // `RedirectWithSplash`, not a bare `` (objectui#6378): this element + // replaces the `LoadingFallback` the SAME `AuthGuard` was rendering one state + // earlier, and a bare redirect renders null — measured, that left the viewport + // blank for 41 ms while `/login` rendered at transition priority. + return ; } /** diff --git a/apps/console/src/components/RootLandingRedirect.route.test.tsx b/apps/console/src/components/RootLandingRedirect.route.test.tsx index be734aeb30..5b69c995c7 100644 --- a/apps/console/src/components/RootLandingRedirect.route.test.tsx +++ b/apps/console/src/components/RootLandingRedirect.route.test.tsx @@ -65,7 +65,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'; import { render, screen } from '@testing-library/react'; -import { MemoryRouter, Routes, Route, useLocation } from 'react-router-dom'; +import { MemoryRouter, Routes, Route, Navigate, useLocation } from 'react-router-dom'; import type { ReactNode } from 'react'; const { passthrough } = vi.hoisted(() => ({ @@ -92,6 +92,20 @@ vi.mock('@object-ui/app-shell', () => ({ ConnectedShell: passthrough, RequireOrganization: passthrough, LoadingFallback: () =>
, + // objectui#6378 — what `LoginRedirect` and `RootLandingRedirect` now + // render instead of a bare ``. Stubbed to the SAME navigation + // plus its own marker, so every location assertion below keeps measuring + // exactly what it measured before and `loading-fallback` keeps meaning + // "a gate is waiting". That the real component also paints the splash is + // pinned where the real component lives — + // `packages/app-shell/src/chrome/RedirectWithSplash.test.tsx` — and + // end-to-end by `e2e/console-boot-indicator.spec.ts`. + RedirectWithSplash: ({ to, replace }: { to: string; replace?: boolean }) => ( + <> +
+ + + ), SETUP_APP_PACKAGE_ID: 'com.objectstack.setup', SETUP_APP_NAME: 'setup', useMetadata: () => ({ diff --git a/apps/console/src/components/RootLandingRedirect.tsx b/apps/console/src/components/RootLandingRedirect.tsx index 4a56774733..452871571c 100644 --- a/apps/console/src/components/RootLandingRedirect.tsx +++ b/apps/console/src/components/RootLandingRedirect.tsx @@ -34,10 +34,10 @@ */ import { useEffect, useRef } from 'react'; -import { Navigate } from 'react-router-dom'; import { useMetadata, LoadingFallback, + RedirectWithSplash, SETUP_APP_PACKAGE_ID, SETUP_APP_NAME, } from '@object-ui/app-shell'; @@ -177,5 +177,10 @@ export function RootLandingRedirect() { }, [unresolved, refresh]); if (loading || unresolved) return ; - return ; + // `RedirectWithSplash`, not a bare `` (objectui#6378). This is the + // widest measured instance of the boot blank: the line above holds the splash + // while the app list loads, and handing off to a null-rendering redirect left + // `#root` empty for 147 ms while `/home` rendered at transition priority. The + // splash is unchanged across the handoff, so the boot reads as one screen. + return ; } diff --git a/e2e/console-boot-indicator.spec.ts b/e2e/console-boot-indicator.spec.ts index 7c4d3ea853..7676146799 100644 --- a/e2e/console-boot-indicator.spec.ts +++ b/e2e/console-boot-indicator.spec.ts @@ -194,3 +194,156 @@ test.describe('Console boot indicator', () => { await expect(page.locator(SPLASH)).toHaveCount(0); }); }); + +/** + * objectui#6378 — the window AFTER `LoadingScreen` has painted. + * + * A different window from the one above, and independent of it: #2628 covers + * the gap BEFORE React commits (the bundle download), and this one is a + * fully-white frame that appears after the splash has already painted, on + * roughly one boot in three. + * + * ⚠️ WHAT THIS CAN AND CANNOT SEE — the same caveat as the tests above, applied + * to a defect that is intermittent by nature. Playwright has no frame buffer, so + * this asserts nothing about pixels; the pixel ledger (CDP + * `Page.startScreencast` at everyNthFrame:1, every frame classified white/not) + * is recorded in the pull request, as it is for #2628. + * + * What IS observable here, and is the whole mechanism: whether the DOM ever + * holds nothing for the viewport to show. The flash is intermittent only + * because it depends on the compositor happening to swap a frame inside the + * window — the WINDOW ITSELF was present on every measured boot. So the DOM + * reading is the deterministic half of an intermittent defect, which makes it + * the right thing to gate on: it goes red on the broken build every time, + * where a pixel assertion would go red about one run in three. + * + * The window is produced by the console's boot redirects. Every readiness gate + * renders the splash while it WAITS and a bare `` the moment it + * DECIDES; `` renders null and react-router runs the navigation as a + * transition, so the destination tree renders while the commit that dropped the + * splash is already on screen. Measured on the production bundle: 41–147 ms of + * empty `#root`. + */ +interface CoverProbe { + reactMountAt?: number; + /** Samples taken at every mutation and every animation frame after mount. */ + uncovered: Array<{ t: number; centre: string | null; path: string }>; + covered: number; + lastSampleAt?: number; +} + +declare global { + interface Window { + __coverProbe?: CoverProbe; + } +} + +test.describe('Console boot continuity', () => { + test('never hands the viewport to an empty document between splash and destination', async ({ + page, + }) => { + // Boot endpoints mocked so the boot RESOLVES rather than sitting on the + // splash forever — an unresolved boot never reaches the handoff this test + // is about, and would pass it vacuously. Signed-out is the shortest + // complete boot: `/console/` is not a route the router knows (the built + // document carries no ``, so the basename is `/`), so it takes + // the catch-all redirect, then the auth gate's redirect to `/login` — + // two of the three redirects this fix covers, in one boot. + await page.route('**/api/**', async (route) => { + const path = new URL(route.request().url()).pathname; + const json = (body: unknown) => route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(body), + }); + if (path.endsWith('/api/v1/runtime/config')) { + return json({ success: true, data: { productName: 'ObjectOS', logoUrl: '', faviconUrl: '' } }); + } + if (path.includes('/api/v1/i18n/locales')) return json({ success: true, data: [{ code: 'en', name: 'English' }] }); + if (path.includes('/api/v1/i18n/translations')) return json({ success: true, data: {} }); + if (path.includes('/api/v1/auth/get-session')) return json(null); + if (path.includes('/api/v1/discovery')) { + return json({ name: 'ObjectStack', mode: 'production', services: { auth: { status: 'ok', handlerReady: true } } }); + } + return json({ success: true, data: null }); + }); + + await page.addInitScript(() => { + const probe: CoverProbe = { uncovered: [], covered: 0 }; + window.__coverProbe = probe; + + // "Covered" = a hit test at the viewport centre lands on something the + // app is responsible for. The pre-React indicator is a SIBLING of + // `#root`, not a child, so it is named explicitly: during the handoff + // both are legitimately on screen and either one alone is enough. + const sample = (t: number) => { + const el = document.elementFromPoint( + Math.floor(window.innerWidth / 2), + Math.floor(window.innerHeight / 2), + ); + const ok = !!el && !!(el.closest('#root') || el.closest('#boot-splash')); + probe.lastSampleAt = t; + if (ok) probe.covered++; + else { + probe.uncovered.push({ + t, + centre: el ? el.tagName.toLowerCase() + (el.id ? `#${el.id}` : '') : null, + path: location.pathname, + }); + } + }; + + const observer = new MutationObserver(() => { + const root = document.getElementById('root'); + if (probe.reactMountAt === undefined && root && root.firstChild) { + probe.reactMountAt = performance.now(); + } + // Only from React's first commit: before it, coverage is #2628's + // subject and is asserted by the tests above. + if (probe.reactMountAt !== undefined) sample(performance.now()); + }); + // `document`, NOT `document.documentElement` — an init script runs at + // document-start, where there is no `` element yet and the + // observe() call throws (measured, in the test above). + observer.observe(document, { childList: true, subtree: true }); + + // A mutation log alone cannot see a window that outlives the commit that + // opened it, which is exactly this defect's shape: one commit empties the + // tree and the next one fills it, tens of frames later. The frame loop is + // what samples the time in between. + const tick = () => { + if (probe.reactMountAt !== undefined) sample(performance.now()); + requestAnimationFrame(tick); + }; + requestAnimationFrame(tick); + }); + + await page.goto(`${CONSOLE_BASE}/`); + await waitForReactMount(page); + + // GHOST-ASSERTION GUARD. "Nothing uncovered" is also true of a page that + // never booted, never redirected, or was observed too late — so the boot is + // required to have COMPLETED, on its own evidence, before the invariant + // below is allowed to mean anything. + await expect(page.locator('#login-email')).toBeVisible({ timeout: 30_000 }); + // ...and let the last frames of the handoff be sampled. + await page.waitForTimeout(500); + + const probe = await page.evaluate(() => window.__coverProbe); + expect(probe?.reactMountAt, "React's first commit was not observed").toBeDefined(); + expect(probe?.covered ?? 0, 'the coverage probe never sampled a covered frame').toBeGreaterThan(5); + + const uncovered = probe?.uncovered ?? []; + const window0 = uncovered[0]; + const spanMs = uncovered.length + ? Math.round(uncovered[uncovered.length - 1].t - uncovered[0].t) + : 0; + expect( + uncovered.length, + `the viewport was empty for ${uncovered.length} sample(s) spanning ~${spanMs}ms after React's ` + + `first commit — first at t=${Math.round(window0?.t ?? 0)}ms on ${window0?.path} with the ` + + `centre hit test landing on <${window0?.centre}>. A boot redirect that renders null hands the ` + + `screen back to the bare page background; that is the white flash of objectui#6378.`, + ).toBe(0); + }); +}); diff --git a/packages/app-shell/src/chrome/RedirectWithSplash.test.tsx b/packages/app-shell/src/chrome/RedirectWithSplash.test.tsx new file mode 100644 index 0000000000..0d2094ba1e --- /dev/null +++ b/packages/app-shell/src/chrome/RedirectWithSplash.test.tsx @@ -0,0 +1,126 @@ +/** + * objectui#6378 — the console boot must never hand the viewport to an empty + * document. + * + * ## What this file can and cannot see + * + * The defect is a TIMING one: a bare `` renders null, and the + * destination tree then renders at transition priority, so for 41–147 ms + * (measured on the production bundle — a CDP screencast frame ledger correlated + * against a DOM-state ledger on the same clock) `#root` holds no view and the + * compositor may swap a fully white frame. + * + * jsdom has no compositor and no CSS engine, so **nothing here measures that + * window**. Asserting "no flash" in jsdom would be a test that passes because + * the phenomenon cannot exist there, which is precisely the shape this card + * exists to avoid. What this file pins is the STRUCTURAL half, which is where + * the fix lives: the element the gate hands off to renders the splash, and it + * still performs the same navigation with the same history semantics. The + * timing claim is measured only in a real browser — + * `e2e/console-boot-indicator.spec.ts` carries the end-to-end invariant and the + * pixel ledger is recorded in the pull request. + * + * The two halves are pinned SEPARATELY on purpose. A single "it redirects" + * assertion stays green when the splash is dropped (the navigation is + * unaffected), and losing the splash is the only regression this component + * exists to prevent. + * + * Nothing in react-router is stubbed: the real `` renders null, and + * that nullness IS the thing being worked around, so replacing it with a marker + * would hide the subject. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useLocation, useNavigationType } from 'react-router-dom'; + +// Imported at module scope, not inside a test: `@object-ui/components` is a +// heavy barrel and resolving it mid-assertion would race the RTL timeouts +// (AGENTS.md § 测试纪律). +import '@object-ui/components'; + +import { RedirectWithSplash } from './RedirectWithSplash.js'; + +/** Reports where the router ended up and how it got there. */ +function RouterProbe() { + const location = useLocation(); + const type = useNavigationType(); + return ( + <> +
{location.pathname + location.search}
+
{type}
+ + ); +} + +describe('RedirectWithSplash', () => { + it('paints the splash — the screen the gate hands off is never handed back empty', () => { + // Rendered OUTSIDE a `` sink so the element under test stays + // mounted after it navigates; what is asserted is what it renders, not what + // survives the route change. + const { container } = render( + + + , + ); + + // The real `LoadingScreen`, not a marker: its product name, its status line + // and its step list are exactly what the gate one state earlier was already + // painting, and the point of the fix is that those pixels do not change + // across the handoff. + expect(screen.getByRole('heading', { level: 1 }).textContent).toBe('ObjectOS'); + expect(screen.getByText('Initializing application…')).toBeTruthy(); + expect(screen.getByText('Connecting to data source')).toBeTruthy(); + + // ...and it is the full-viewport splash root, so nothing shows through + // around it. Class names, not computed style — jsdom resolves no Tailwind + // (see the header), so a style assertion here would measure nothing. + expect( + container.querySelector('div.h-screen.bg-background'), + 'LoadingScreen renders its full-viewport root', + ).toBeTruthy(); + }); + + it('lands the router on the destination, with the search string intact', () => { + render( + + + + } /> + } /> + + , + ); + + expect(screen.getByTestId('path').textContent).toBe('/login?redirect=%2Fapps'); + expect(screen.getByTestId('login-sink')).toBeTruthy(); + }); + + it('honours `replace` — a boot redirect must not fossilize `/` in history', () => { + render( + + + + } /> + } /> + + , + ); + + expect(screen.getByTestId('nav-type').textContent).toBe('REPLACE'); + }); + + it('leaves a push a push when the caller omits `replace`', () => { + render( + + + + } /> + } /> + + , + ); + + expect(screen.getByTestId('nav-type').textContent).toBe('PUSH'); + }); +}); diff --git a/packages/app-shell/src/chrome/RedirectWithSplash.tsx b/packages/app-shell/src/chrome/RedirectWithSplash.tsx new file mode 100644 index 0000000000..57b2b700c6 --- /dev/null +++ b/packages/app-shell/src/chrome/RedirectWithSplash.tsx @@ -0,0 +1,74 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `RedirectWithSplash` — a boot-path redirect that keeps the splash painted. + * + * ## Why this exists (objectui#6378 — measured, not guessed) + * + * Every readiness gate on the console boot path renders the splash while it + * WAITS and a bare `` the moment it DECIDES: + * + * ```tsx + * if (loading) return ; // splash on screen + * return ; // renders NOTHING + * ``` + * + * `` renders `null` and performs its navigation from an effect, and + * react-router wraps that navigation in a transition — so the destination tree + * renders at transition priority while the commit that already dropped the + * splash is what the compositor is showing. For the whole of that window + * `#root` holds no view at all and the viewport is the bare page background. + * + * Measured on the production `apps/console` bundle with the boot endpoints + * mocked, correlating a CDP `Page.startScreencast` frame ledger against a + * DOM-state ledger on the same clock (`performance.timeOrigin`): the window is + * 41–147 ms wide, and whenever the compositor happens to swap a frame inside it + * the user sees a full-viewport white flash. Both suspects named on the card + * were exonerated by that ledger — `RouteFader` never mounts on the boot path + * at all, and `LoadingScreen` unmounts exactly when its own gate says to. What + * is wrong is what REPLACES it. + * + * ## Why the splash, and why it cannot itself flicker + * + * The component renders the SAME `LoadingScreen` the gate one line above was + * already rendering, so the handoff changes no pixels: the transition now runs + * underneath an unchanged screen instead of underneath a blank one. That is + * also why this is not "add a spinner" — a different holding image would + * introduce a visual change where today there is a blank, and the goal is for + * the boot to look like one continuous screen until the destination paints. + * + * ## Scope + * + * Deliberately minimal: `to` + `replace`, the only shape the console's boot + * redirects use. It is a boot-chrome affordance, not a general `` + * replacement — a redirect that fires while a view is already on screen should + * keep that view, not cover it with a splash. + */ + +import { Navigate } from 'react-router-dom'; +import { LoadingScreen } from './LoadingScreen.js'; + +export interface RedirectWithSplashProps { + /** Where to go — the same value a bare `` would take. */ + to: string; + /** Replace the current history entry instead of pushing a new one. */ + replace?: boolean; +} + +export function RedirectWithSplash({ to, replace }: RedirectWithSplashProps) { + return ( + <> + {/* Order is not load-bearing — `` renders null — but the + splash is written first so the element that OCCUPIES the screen reads + first at every call site. */} + + + + ); +} diff --git a/packages/app-shell/src/chrome/index.ts b/packages/app-shell/src/chrome/index.ts index 862a95b81e..47095c813b 100644 --- a/packages/app-shell/src/chrome/index.ts +++ b/packages/app-shell/src/chrome/index.ts @@ -4,6 +4,7 @@ export { OnboardingWalkthrough } from './OnboardingWalkthrough.js'; export { ConditionalAuthWrapper } from './ConditionalAuthWrapper.js'; export { ConsoleToaster } from './ConsoleToaster.js'; export { RouteFader } from './RouteFader.js'; +export { RedirectWithSplash, type RedirectWithSplashProps } from './RedirectWithSplash.js'; export { ErrorBoundary } from './ErrorBoundary.js'; export { LoadingScreen } from './LoadingScreen.js'; export { ThemeProvider, useTheme } from './ThemeProvider.js'; diff --git a/packages/app-shell/src/index.ts b/packages/app-shell/src/index.ts index be4860bcb2..6742326c8c 100644 --- a/packages/app-shell/src/index.ts +++ b/packages/app-shell/src/index.ts @@ -129,6 +129,8 @@ export { ConsoleToaster, presentNotificationToast, RouteFader, + RedirectWithSplash, + type RedirectWithSplashProps, toastWithUndo, type ToastWithUndoOptions, ErrorBoundary,