From 433d40686dd6d4b4f0ed365efd170b9ecfe433e7 Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Fri, 11 Sep 2026 04:07:33 -0700 Subject: [PATCH] fix(desktop): republish composer clearance after an effect replay useComposerMetrics dedupes its --composer-measured-height writes against a "last published bucket" ref. The unmount cleanup cleared the surface vars but left that ref alone, so after a non-final unmount (StrictMode replay, Suspense hide) the re-mount measured the same dock, saw an unchanged bucket, and never wrote the var back. The thread then read the :root estimate (~62px) under a dock that could be 200px tall, and the status stack covered the last turn until a real resize fired. Reset the bucket refs in the same cleanup that clears the vars. Adds a StrictMode regression test that fails on the old code. --- .../hooks/use-composer-metrics.test.tsx | 122 ++++++++++++++++++ .../composer/hooks/use-composer-metrics.ts | 11 ++ 2 files changed, 133 insertions(+) create mode 100644 apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.test.tsx diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.test.tsx new file mode 100644 index 0000000000..cf082d0202 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.test.tsx @@ -0,0 +1,122 @@ +import { cleanup, render } from '@testing-library/react' +import { type RefObject, StrictMode, useRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { COMPOSER_HEIGHT_VAR, COMPOSER_SURFACE_HEIGHT_VAR } from '@/app/chat/surface-vars' + +import { useComposerMetrics } from './use-composer-metrics' + +vi.mock('@assistant-ui/react', () => ({ + useAuiState: (selector: (state: { composer: { text: string } }) => unknown) => selector({ composer: { text: '' } }) +})) + +// The shared ResizeObserver delivers once per observed element after +// observe(); replay that by hand so the hook's initial measurement runs where +// it would in Chromium. +const observers: FakeResizeObserver[] = [] + +class FakeResizeObserver implements ResizeObserver { + readonly targets = new Set() + + constructor(readonly callback: ResizeObserverCallback) { + observers.push(this) + } + + disconnect() { + this.targets.clear() + } + + observe(target: Element) { + this.targets.add(target) + } + + unobserve(target: Element) { + this.targets.delete(target) + } +} + +const deliverResize = () => { + for (const observer of observers) { + const entries = [...observer.targets].map(target => ({ target }) as ResizeObserverEntry) + + if (entries.length > 0) { + observer.callback(entries, observer) + } + } +} + +const sized = + (ref: RefObject, height: number) => + (node: HTMLDivElement | null): void => { + if (node) { + node.getBoundingClientRect = () => + ({ bottom: height, height, left: 0, right: 640, top: 0, width: 640, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect + } + + ref.current = node + } + +function Harness({ dockHeight, surfaceHeight }: { dockHeight: number; surfaceHeight: number }) { + const composerDockRef = useRef(null) + const composerRef = useRef(null) + const composerSurfaceRef = useRef(null) + const editorRef = useRef(null) + + useComposerMetrics({ composerDockRef, composerRef, composerSurfaceRef, editorRef, poppedOut: false }) + + return ( +
+
+
+
+
+
+ +
+
+ ) +} + +const surfaceOf = (container: HTMLElement) => container.querySelector('[data-chat-surface]')! + +describe('useComposerMetrics — published clearance survives an effect replay', () => { + beforeEach(() => { + observers.length = 0 + vi.stubGlobal('ResizeObserver', FakeResizeObserver) + }) + + afterEach(() => { + cleanup() + vi.unstubAllGlobals() + }) + + it('republishes the dock height after StrictMode replays the cleanup', () => { + // StrictMode replays every effect: mount → cleanup → mount. The cleanup + // clears the surface vars; the replayed measurement must write them back + // even though the dock's size has not changed since the first write. + const { container } = render( + + + + ) + + deliverResize() + + const surface = surfaceOf(container) + + expect(surface.style.getPropertyValue(COMPOSER_HEIGHT_VAR)).toBe('200px') + expect(surface.style.getPropertyValue(COMPOSER_SURFACE_HEIGHT_VAR)).toBe('120px') + }) + + it('skips the write when the bucketed height is unchanged within one mount', () => { + const { container } = render() + + deliverResize() + + const setProperty = vi.spyOn(surfaceOf(container).style, 'setProperty') + + deliverResize() + + expect(setProperty).not.toHaveBeenCalled() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts index 9af4e1be7c..dbc0ab24b0 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts @@ -193,6 +193,7 @@ export function useComposerMetrics({ syncComposerMetrics() }, [poppedOut, syncComposerMetrics]) + // eslint-disable-next-line no-restricted-syntax -- resets a publish-dedupe cache in cleanup, not a mirrored atom useEffect(() => { // Resolve the owning surface while the composer is still attached; the // unmount cleanup runs after React detached the node, where closest() can @@ -202,6 +203,16 @@ export function useComposerMetrics({ return () => { clearSurfaceVar(root, COMPOSER_HEIGHT_VAR) clearSurfaceVar(root, COMPOSER_SURFACE_HEIGHT_VAR) + // The bucket refs mirror what is published, so clearing the vars must + // clear them too. This cleanup also runs on a non-final unmount (a + // StrictMode effect replay, a Suspense hide); the re-mount then + // re-measures the same dock, and with the refs still holding the old + // bucket the unchanged-skip check swallowed the republish. The thread + // fell back to the :root estimate (~62px) under a dock that could be + // 200px tall, and the status stack sat on top of the last turn until a + // real resize happened to fire. + lastBucketedHeightRef.current = 0 + lastBucketedSurfaceHeightRef.current = 0 } }, [composerRef])