diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx index ea84c3c5c3b..08e8c5e483c 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx @@ -434,6 +434,58 @@ describe('the hybrid shell screen', () => { expect(dependencies.storageRefreshes).toBe(2) }) + /** + * A route that moved before the host existed (CodeRabbit on `:271`). + * + * The effect recorded the route's key and then published, so a publish the hook refused for + * having no host counted as delivered anyway. This pins the delivery contract across that gap: + * the caller is told once, and only when a frame carrying the route actually reached the page. + * + * It does not reproduce a lost tap, and the fix beside it is hygiene rather than a repair: the + * host is built from the route the render holds, so a route that moved before it existed is in + * the first `init` regardless, and `publishRoute` then answers "did not move". What the fix + * removes is a key recorded for a frame nobody sent. + */ + it('reports a route that moved before the host existed once, when the page receives it', async () => { + dependencies.client = null + const delivered: { pathname: string; params?: Record }[] = [] + const screen = (params: Record) => + createElement(MobileWebShellScreen, { + hostId: 'host-1', + route: { pathname: '/h/host-1', params }, + fallback: createElement(NativeFallback), + onRouteDelivered: (route) => delivered.push(route) + }) + dependencies.state = readyState('session-one') + const rendered: { tree: ReactTestRenderer | null } = { tree: null } + await act(async () => { + rendered.tree = create(screen({ paneKey: '' })) + }) + const tree = rendered.tree + if (tree === null) { + throw new Error('screen did not render') + } + mounted.push(tree) + // The tap, with no host to take it: nothing reached the page, so nothing is reported. + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + expect(delivered).toEqual([]) + dependencies.client = createFakeRpcClient() + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + // Still nothing: the host now holds that route and has not sent anything yet. + expect(delivered).toEqual([]) + await act(async () => { + byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ + nativeEvent: { json: clientFrame({ type: 'ready' }) } + }) + }) + // The `init` that answered the ask carried it, so the caller may spend the param — once. + expect(delivered).toEqual([{ pathname: '/h/host-1', params: { paneKey: 'pane-1' } }]) + }) + it('ends that wait on the page asking for a session', async () => { dependencies.client = createFakeRpcClient() const tree = await render(readyState('session-one')) diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index 377785c14f1..1511851411f 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -273,10 +273,15 @@ export function MobileWebShellScreen({ if (key === publishedRouteKey.current) { return } - publishedRouteKey.current = key - if (publishRoute(route)) { - onRouteDelivered?.(route) + // Recorded only once a frame has gone out. A publish the hook refuses — no host yet, which is + // the gap between a ready session and its mounted bridge — leaves the key unrecorded, so the + // render that brings the host publishes the route the page never received. `publishRoute`'s + // identity changes with the inputs the host is built from, which is what re-runs this. + if (!publishRoute(route)) { + return } + publishedRouteKey.current = key + onRouteDelivered?.(route) }, [onRouteDelivered, publishRoute, route]) // A profile read that rejected never becomes a host, so the session would otherwise sit in diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts index ef993c1d8fe..0258bbf6866 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts @@ -255,6 +255,11 @@ export function useMobileWebShellBridge(args: { ), // Fenced on the session the same way inbound frames are: a host left over from a session this // render has moved past must not be handed this one's route. + // + // Keyed on everything the host is built from, not on the session alone: a caller that holds a + // route the host was not there to take retries when this identity changes, and the host's own + // effect is a layout effect, so by the time a passive effect sees the new identity the host + // behind it exists. publishRoute: useCallback( (route: BridgeInitRoute) => { const mounted = hostRef.current @@ -262,7 +267,7 @@ export function useMobileWebShellBridge(args: { mounted !== null && mounted.sessionId === sessionId && mounted.host.publishRoute(route) ) }, - [sessionId] + [buildId, client, sessionId, snapshot] ) } }