From dd246b42c7c3f93c652054e1eefc2b126306b994 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Mon, 3 Aug 2026 01:14:09 +0200 Subject: [PATCH 1/3] test(react): drain the post-unmount scheduler timer in UI suites RichCodeBlock block mode, Command and Tooltip each unmounted their React root with a bare `flushSync(() => root.unmount())`. That leaves a scheduler callback queued on a real timer, so Deno's leak sanitizer failed the whole suite with "A timer was started in this test, but never completed" even though every step passed. Detection depended on how the runner distributed files across workers, so the same file passed alone and failed in a batch. Replace those call sites with the local `unmount(root)` helper already used by select.test.tsx, floating.test.tsx and overlay-surfaces.test.tsx, which flushes the unmount and then awaits a macrotask so the timer completes inside the test that created it. red: `deno test src/react/components/ui/command.test.tsx` fails the suite on leak detection with 0 failed steps green: each file passes 5x in isolation and the full src/react sweep is clean across repeated runs --- .../chat/chat/components/code-block.test.tsx | 13 +++++-- src/react/components/ui/command.test.tsx | 19 +++++++--- src/react/components/ui/tooltip.test.tsx | 38 +++++++++++-------- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/react/components/chat/chat/components/code-block.test.tsx b/src/react/components/chat/chat/components/code-block.test.tsx index 0ea216e834..5563127572 100644 --- a/src/react/components/chat/chat/components/code-block.test.tsx +++ b/src/react/components/chat/chat/components/code-block.test.tsx @@ -1,5 +1,5 @@ import { flushSync } from "react-dom"; -import { createRoot } from "react-dom/client"; +import { createRoot, type Root } from "react-dom/client"; import { renderToString } from "react-dom/server"; import { JSDOM } from "npm:jsdom@28.0.0"; import { assert, assertEquals, assertStringIncludes } from "#veryfront/testing/assert"; @@ -48,6 +48,13 @@ async function settle(): Promise { flushSync(() => {}); } +// Unmounting leaves a scheduler callback queued; drain it so the leak +// sanitizer does not attribute that timer to the test. +async function unmount(root: Root): Promise { + flushSync(() => root.unmount()); + await new Promise((resolve) => setTimeout(resolve, 0)); +} + describe("RichCodeBlock — inline mode", () => { it("renders an inline element with no language label or copy button", () => { const html = renderToString(); @@ -122,7 +129,7 @@ describe("RichCodeBlock — block mode", () => { "Unable to copy code", ); assertEquals(document.querySelectorAll("textarea").length, 0); - flushSync(() => root.unmount()); + await unmount(root); } finally { dom.restore(); } @@ -167,7 +174,7 @@ describe("RichCodeBlock — block mode", () => { assertStringIncludes(rootElement.textContent ?? "", "new code"); assert(!(rootElement.textContent ?? "").includes("old code")); - flushSync(() => root.unmount()); + await unmount(root); } finally { dom.restore(); } diff --git a/src/react/components/ui/command.test.tsx b/src/react/components/ui/command.test.tsx index 97ab5e4f11..66dd240342 100644 --- a/src/react/components/ui/command.test.tsx +++ b/src/react/components/ui/command.test.tsx @@ -1,6 +1,6 @@ import type * as React from "react"; import { flushSync } from "react-dom"; -import { createRoot } from "react-dom/client"; +import { createRoot, type Root } from "react-dom/client"; import { renderToString } from "react-dom/server"; import { JSDOM } from "npm:jsdom@28.0.0"; import { assert, assertEquals } from "#veryfront/testing/assert.ts"; @@ -92,6 +92,13 @@ function createChangeEvent(value: string): React.ChangeEvent { return event as unknown as React.ChangeEvent; } +// Unmounting leaves a scheduler callback queued; drain it so the leak +// sanitizer does not attribute that timer to the test. +async function unmount(root: Root): Promise { + flushSync(() => root.unmount()); + await new Promise((resolve) => setTimeout(resolve, 0)); +} + describe("Command", () => { it("exposes a listbox contract and tracks pointer-active options", async () => { const dom = new JSDOM( @@ -166,7 +173,7 @@ describe("Command", () => { flushSync(() => disabled.click()); assertEquals(selected, ["beta"]); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -219,7 +226,7 @@ describe("Command", () => { flushSync(() => cancelled.click()); assertEquals(calls, ["click", "select", "cancel-click"]); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -328,7 +335,7 @@ describe("Command", () => { return document.getElementById(active ?? "")?.textContent === "Beta"; }); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -410,7 +417,7 @@ describe("Command", () => { await waitFor(() => document.querySelector("[data-empty]") === null); assert(document.querySelector("[data-always]")); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -442,7 +449,7 @@ describe("Command", () => { assertEquals(empty.getAttribute("aria-disabled"), "true"); assertEquals(empty.getAttribute("aria-live"), "polite"); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); diff --git a/src/react/components/ui/tooltip.test.tsx b/src/react/components/ui/tooltip.test.tsx index db1ec18280..8863000ff0 100644 --- a/src/react/components/ui/tooltip.test.tsx +++ b/src/react/components/ui/tooltip.test.tsx @@ -1,6 +1,6 @@ import type { RefCallback } from "react"; import { flushSync } from "react-dom"; -import { createRoot, hydrateRoot } from "react-dom/client"; +import { createRoot, hydrateRoot, type Root } from "react-dom/client"; import { renderToString } from "react-dom/server"; import { JSDOM } from "npm:jsdom@28.0.0"; import { assert, assertEquals } from "#veryfront/testing/assert.ts"; @@ -100,6 +100,14 @@ function escape(window: JSDOM["window"], target: EventTarget): KeyboardEvent { return event; } +// Unmounting leaves a scheduler callback queued; drain it so the leak +// sanitizer does not attribute that timer to the test. +async function unmount(root: Root | undefined): Promise { + if (!root) return; + flushSync(() => root.unmount()); + await new Promise((resolve) => setTimeout(resolve, 0)); +} + describe("Tooltip", () => { it("gives the default trigger a keyboard path and honors an explicit tab index", async () => { const dom = createDom(); @@ -140,7 +148,7 @@ describe("Tooltip", () => { assert(tooltipId); assertEquals(defaultTrigger.getAttribute("aria-describedby"), tooltipId); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -225,7 +233,7 @@ describe("Tooltip", () => { assertEquals(childFocusCalls, 1); assertEquals(recoverableErrors, []); } finally { - root?.unmount(); + await unmount(root); restore(); } }); @@ -263,7 +271,7 @@ describe("Tooltip", () => { await waitFor(() => document.querySelector('[role="tooltip"]') === null); assertEquals(trigger.hasAttribute("aria-describedby"), false); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -313,7 +321,7 @@ describe("Tooltip", () => { }); await waitFor(() => document.querySelector('[role="tooltip"]') !== null); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -356,7 +364,7 @@ describe("Tooltip", () => { assertEquals(calls, ["child", "trigger"]); assert(document.querySelector('[role="tooltip"]') === null); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -391,7 +399,7 @@ describe("Tooltip", () => { flushSync(() => unhover(dom.window, trigger)); await waitFor(() => document.querySelector('[role="tooltip"]') === null); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -439,7 +447,7 @@ describe("Tooltip", () => { await new Promise((resolve) => setTimeout(resolve, 50)); assert(document.querySelector('[role="tooltip"]') === null); } finally { - if (rootMounted) flushSync(() => root.unmount()); + if (rootMounted) await unmount(root); restore(); } }); @@ -530,7 +538,7 @@ describe("Tooltip", () => { flushSync(() => escape(targetDom.window, targetDocument)); await waitFor(() => targetDocument.querySelector('[role="tooltip"]') === null); } finally { - flushSync(() => root.unmount()); + await unmount(root); targetDom.window.close(); restore(); } @@ -571,7 +579,7 @@ describe("Tooltip", () => { assertEquals(tooltip.style.overflowWrap, "normal"); assertEquals(tooltip.style.whiteSpace, "pre-wrap"); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); @@ -599,12 +607,12 @@ describe("Tooltip", () => { flushSync(() => hover(dom.window, trigger)); await waitFor(() => document.querySelector('[role="tooltip"]') !== null); } finally { - flushSync(() => root.unmount()); + await unmount(root); restore(); } }); - it("caps excessive provider delays and cancels the owner-window timer", () => { + it("caps excessive provider delays and cancels the owner-window timer", async () => { const dom = createDom(); const restore = installDom(dom); const root = createRoot(document.getElementById("root")!); @@ -654,7 +662,7 @@ describe("Tooltip", () => { flushSync(() => unhover(dom.window, trigger)); assertEquals(clearedHandles, [47]); } finally { - flushSync(() => root.unmount()); + await unmount(root); if (setTimeoutDescriptor) { Object.defineProperty(dom.window, "setTimeout", setTimeoutDescriptor); } else delete (dom.window as unknown as Record).setTimeout; @@ -704,7 +712,7 @@ describe("Tooltip", () => { assertEquals(cleanupCalls, 0); assertEquals(nullCalls, 0); } finally { - flushSync(() => root.unmount()); + await unmount(root); assertEquals(attachedElement, null); assertEquals(cleanupCalls, 1); assertEquals(nullCalls, 0); @@ -754,7 +762,7 @@ describe("Tooltip", () => { assert(document.querySelector('[role="tooltip"]')); assertEquals(refCalls, ["attach:1", "cleanup:1", "attach:2"]); } finally { - flushSync(() => root.unmount()); + await unmount(root); assertEquals(refCalls, [ "attach:1", "cleanup:1", From 990078dd08d219ec1895878f884a55ac5625c7ab Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Mon, 3 Aug 2026 01:14:22 +0200 Subject: [PATCH 2/3] test(select): await the close request instead of racing the invalid render "fails default-open dynamic duplicates before user interaction" failed non-deterministically. It waited for `aria-invalid="true"` and for the content to disappear, then asserted `onOpenChange` had already fired with `false`. Those are two different phases: `isOpen = rawOpen && !effectiveDisabled` suppresses the popup synchronously in the invalid render, while the close request to the owner is raised from the root's `useIsomorphicLayoutEffect`. In Deno that hook resolves to `React.useEffect` because `typeof document === "undefined"` when the module is imported -- JSDOM is installed per test, after import -- so the request lands a tick after the DOM already shows the suppressed state, and the assertion read `openChanges` in between. The component is correct: it does emit `onOpenChange(false)`, and the documented contract separates suppressing the popup from requesting closure. Wait for the request before asserting its contents, so the test still pins "exactly one change, and it is false" without assuming the two phases share a tick. red: deleting the `setOpen(false)` request makes the step time out on "default-open close request" green: 12/12 in isolation, and the full src/react sweep is clean --- src/react/components/ui/select.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/react/components/ui/select.test.tsx b/src/react/components/ui/select.test.tsx index b9cf60c220..2ce5fb1cac 100644 --- a/src/react/components/ui/select.test.tsx +++ b/src/react/components/ui/select.test.tsx @@ -1112,6 +1112,12 @@ describe("Select", () => { () => document.getElementById("dynamic-default-list") === null, "default-open duplicate content suppression", ); + // Suppression is synchronous with the invalid render; the close request + // to the owner is an effect that lands afterwards. + await waitFor( + () => openChanges.length > 0, + "default-open close request", + ); assertEquals(rootElement.contains(trigger), true); assertEquals(trigger.disabled, true); assertEquals(openChanges, [false]); From 691f4b6a0dd07470c3650bf8e7b6da40b64988ca Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Mon, 3 Aug 2026 02:03:15 +0200 Subject: [PATCH 3/3] test(react): make cleanup failures safe --- .../chat/chat/components/code-block.test.tsx | 22 +++++++++++-------- src/react/components/ui/select.test.tsx | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/react/components/chat/chat/components/code-block.test.tsx b/src/react/components/chat/chat/components/code-block.test.tsx index 5563127572..ee1ba87bb1 100644 --- a/src/react/components/chat/chat/components/code-block.test.tsx +++ b/src/react/components/chat/chat/components/code-block.test.tsx @@ -50,7 +50,8 @@ async function settle(): Promise { // Unmounting leaves a scheduler callback queued; drain it so the leak // sanitizer does not attribute that timer to the test. -async function unmount(root: Root): Promise { +async function unmount(root: Root | undefined): Promise { + if (!root) return; flushSync(() => root.unmount()); await new Promise((resolve) => setTimeout(resolve, 0)); } @@ -97,6 +98,7 @@ describe("RichCodeBlock — block mode", () => { it("reports failed copies without leaking the fallback textarea", async () => { const dom = installDom(); + let root: Root | undefined; Object.defineProperty(dom.window.navigator, "clipboard", { configurable: true, value: { writeText: () => Promise.reject(new Error("denied")) }, @@ -113,8 +115,9 @@ describe("RichCodeBlock — block mode", () => { try { const rootElement = document.getElementById("root"); assert(rootElement, "root fixture exists"); - const root = createRoot(rootElement); - flushSync(() => root.render()); + const mountedRoot = createRoot(rootElement); + root = mountedRoot; + flushSync(() => mountedRoot.render()); const button = rootElement.querySelector("button"); assert(button, "copy control renders"); @@ -129,14 +132,15 @@ describe("RichCodeBlock — block mode", () => { "Unable to copy code", ); assertEquals(document.querySelectorAll("textarea").length, 0); - await unmount(root); } finally { + await unmount(root); dom.restore(); } }); it("does not show stale success after the displayed code changes", async () => { const dom = installDom(); + let root: Root | undefined; const pending: Array<() => void> = []; Object.defineProperty(dom.window.navigator, "clipboard", { configurable: true, @@ -151,14 +155,15 @@ describe("RichCodeBlock — block mode", () => { try { const rootElement = document.getElementById("root"); assert(rootElement, "root fixture exists"); - const root = createRoot(rootElement); - flushSync(() => root.render()); + const mountedRoot = createRoot(rootElement); + root = mountedRoot; + flushSync(() => mountedRoot.render()); const firstButton = rootElement.querySelector("button"); assert(firstButton, "first copy control renders"); firstButton.dispatchEvent(new dom.window.MouseEvent("click", { bubbles: true })); - flushSync(() => root.render()); + flushSync(() => mountedRoot.render()); const secondButton = rootElement.querySelector("button"); assert(secondButton, "updated copy control renders"); secondButton.dispatchEvent(new dom.window.MouseEvent("click", { bubbles: true })); @@ -173,9 +178,8 @@ describe("RichCodeBlock — block mode", () => { assertEquals(secondButton.textContent?.trim(), "Copied"); assertStringIncludes(rootElement.textContent ?? "", "new code"); assert(!(rootElement.textContent ?? "").includes("old code")); - - await unmount(root); } finally { + await unmount(root); dom.restore(); } }); diff --git a/src/react/components/ui/select.test.tsx b/src/react/components/ui/select.test.tsx index 2ce5fb1cac..11a3b11ff9 100644 --- a/src/react/components/ui/select.test.tsx +++ b/src/react/components/ui/select.test.tsx @@ -1115,7 +1115,7 @@ describe("Select", () => { // Suppression is synchronous with the invalid render; the close request // to the owner is an effect that lands afterwards. await waitFor( - () => openChanges.length > 0, + () => openChanges.length === 1, "default-open close request", ); assertEquals(rootElement.contains(trigger), true);