From 629150f0a1a674bf4fce34719a8f81ca395b78d6 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 24 Sep 2026 14:15:17 +0000 Subject: [PATCH 1/2] test(ui): drain React's scheduler before jsdom teardown (BLO-23426) React 19 schedules a passive-effect flush after every commit with passive flags -- including the flushSync unmount most ui tests do in afterEach -- and that scheduler task reads `window.event` before anything else (react-dom-client.development.js:17920). When a jsdom file's environment is torn down before the setImmediate fires, it throws "window is not defined" as an unhandled error, and vitest exits 1 on a fully green run. It has ejected merge-queue groups with every test passing: #1987 (35903530339, 20 errors, BuiltInBundlePanel) and #1976 (35993984182, 4 errors, ArtifactGroupCard). The blamed file changes run to run because any component can leave the task behind, so the fix is teardown ordering in the one place every ui file shares: a setup-file afterAll, which vitest runs last (after-hooks are stack-ordered), drains a few macrotasks while jsdom is still installed. Co-Authored-By: Claude Opus 5.5 (1M context) --- ui/vitest.setup.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ui/vitest.setup.ts b/ui/vitest.setup.ts index 12d640ac069f..1464120d6b07 100644 --- a/ui/vitest.setup.ts +++ b/ui/vitest.setup.ts @@ -1,3 +1,5 @@ +import { afterAll } from "vitest"; + const storageEntries = new Map(); function installStorageMock(target: Record) { @@ -38,3 +40,17 @@ if (typeof window !== "undefined" && window.localStorage !== globalThis.localSto if (typeof Element !== "undefined" && typeof Element.prototype.scrollIntoView !== "function") { Element.prototype.scrollIntoView = function scrollIntoView() {}; } + +// React 19 schedules a passive-effect flush after every commit with passive +// flags (a flushSync unmount in afterEach included), and that scheduler task +// reads `window.event` before anything else. If a jsdom file finishes and the +// environment is torn down before the task runs, it throws "window is not +// defined" as an unhandled error and turns an all-green workspaces-a run red +// (BLO-23426; merge groups 35903530339, 35993984182). Setup-file hooks run +// last, so draining a few macrotasks here lets that work finish while jsdom +// is still installed. +if (typeof window !== "undefined") { + afterAll(async () => { + for (let i = 0; i < 3; i++) await new Promise((resolve) => setImmediate(resolve)); + }); +} From 50e488241e40a3645461674b082cb8f909d9abcc Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 24 Sep 2026 14:36:30 +0000 Subject: [PATCH 2/2] test(ui): pin the scheduler drain with a test that fails when it is a no-op commitperclip rightly asked for a test. The drain now lives in ui/src/lib/drainReactScheduler.ts, which vitest.setup.ts registers as the jsdom afterAll. The new test flushSync-renders and unmounts a component with a useEffect, drains, then requires that three more immediate turns produce no further `window.event` reads. A read on those turns is exactly what throws "window is not defined" after a real teardown. It fails with the drain made a no-op (expected 6 to be 4) and still passes if React ever stops deferring the task. Co-Authored-By: Claude Opus 5.5 (1M context) --- ui/src/lib/drainReactScheduler.test.tsx | 36 +++++++++++++++++++++++++ ui/src/lib/drainReactScheduler.ts | 13 +++++++++ ui/vitest.setup.ts | 18 +++++-------- 3 files changed, 56 insertions(+), 11 deletions(-) create mode 100644 ui/src/lib/drainReactScheduler.test.tsx create mode 100644 ui/src/lib/drainReactScheduler.ts diff --git a/ui/src/lib/drainReactScheduler.test.tsx b/ui/src/lib/drainReactScheduler.test.tsx new file mode 100644 index 000000000000..caf6a2ee6ab8 --- /dev/null +++ b/ui/src/lib/drainReactScheduler.test.tsx @@ -0,0 +1,36 @@ +// @vitest-environment jsdom + +import { useEffect } from "react"; +import { flushSync } from "react-dom"; +import { createRoot } from "react-dom/client"; +import { afterEach, expect, it } from "vitest"; +import { drainReactScheduler } from "./drainReactScheduler"; + +afterEach(() => { + delete (window as { event?: unknown }).event; +}); + +it("leaves no scheduler task that reads window pending after an unmount", async () => { + let reads = 0; + Object.defineProperty(window, "event", { + configurable: true, + get: () => { + reads++; + return undefined; + }, + }); + function WithEffect() { + useEffect(() => () => {}, []); + return null; + } + const root = createRoot(document.createElement("div")); + flushSync(() => root.render()); + flushSync(() => root.unmount()); + + await drainReactScheduler(); + const afterDrain = reads; + // Anything the drain left queued runs on these turns and reads window.event + // again -- after a real teardown, that read is the "window is not defined". + for (let i = 0; i < 3; i++) await new Promise((resolve) => setImmediate(resolve)); + expect(reads).toBe(afterDrain); +}); diff --git a/ui/src/lib/drainReactScheduler.ts b/ui/src/lib/drainReactScheduler.ts new file mode 100644 index 000000000000..039b64c8ac08 --- /dev/null +++ b/ui/src/lib/drainReactScheduler.ts @@ -0,0 +1,13 @@ +/** + * Runs the work React has already queued on its scheduler, while the test + * environment still exists. + * + * React 19 schedules a passive-effect flush after every commit with passive + * flags (a flushSync unmount included), and that task reads `window.event` + * before anything else. In Node the scheduler posts it with setImmediate, and + * immediates run in order, so awaiting our own turns runs everything queued + * before the call. Three turns cover a task that yields and re-posts itself. + */ +export async function drainReactScheduler(): Promise { + for (let i = 0; i < 3; i++) await new Promise((resolve) => setImmediate(resolve)); +} diff --git a/ui/vitest.setup.ts b/ui/vitest.setup.ts index 1464120d6b07..e3146fee633a 100644 --- a/ui/vitest.setup.ts +++ b/ui/vitest.setup.ts @@ -1,4 +1,5 @@ import { afterAll } from "vitest"; +import { drainReactScheduler } from "./src/lib/drainReactScheduler"; const storageEntries = new Map(); @@ -41,16 +42,11 @@ if (typeof Element !== "undefined" && typeof Element.prototype.scrollIntoView != Element.prototype.scrollIntoView = function scrollIntoView() {}; } -// React 19 schedules a passive-effect flush after every commit with passive -// flags (a flushSync unmount in afterEach included), and that scheduler task -// reads `window.event` before anything else. If a jsdom file finishes and the -// environment is torn down before the task runs, it throws "window is not -// defined" as an unhandled error and turns an all-green workspaces-a run red -// (BLO-23426; merge groups 35903530339, 35993984182). Setup-file hooks run -// last, so draining a few macrotasks here lets that work finish while jsdom -// is still installed. +// A scheduler task React left behind (see drainReactScheduler) that fires after +// jsdom is torn down throws "window is not defined" as an unhandled error and +// turns an all-green workspaces-a run red (BLO-23426; merge groups 35903530339, +// 35993984182). After-hooks run in reverse registration order, so this one runs +// after every test file's own hooks and before the environment goes away. if (typeof window !== "undefined") { - afterAll(async () => { - for (let i = 0; i < 3; i++) await new Promise((resolve) => setImmediate(resolve)); - }); + afterAll(drainReactScheduler); }