From 2bca660fd64781af03770f83e4d8f3d75e910510 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Wed, 26 Aug 2026 12:32:08 -0700 Subject: [PATCH 1/7] fix(test, frontend): bound the workflow-snapshot render's cost under jsdom `report-generation.service.spec.ts` drives the real html2canvas, which clones the whole document rather than the element it is pointed at. The unit-test builder runs spec files with `isolate: false`, so one jsdom document is shared by every spec file in a worker and the clone drags in whatever DOM the files before it left behind. On the macOS runner that clone was measured at 12-37s against a 20s test timeout, failing "fails when the editor cannot be rendered" while ubuntu and windows passed. The job log carried 5,691 `Not implemented: window.getComputedStyle(elt, pseudoElt)` stack traces for six renders -- about 950 per clone, so roughly 475 foreign elements were being cloned each time. The same run locally emits 56. Two changes, both test-only: - The snapshot suite parks the document's existing body nodes for the duration of the file, so the clone's cost depends only on the DOM these tests build. It spans the file rather than each test because the renders outlive the tests that start them. - `jsdom-svg-polyfill.ts` stops jsdom announcing `getComputedStyle(elt, pseudoElt)` and `scrollTo` on the virtual console, which vitest forwards to `console.error` as a full stack trace per call. Dropping `pseudoElt` changes no behaviour: jsdom complains and then ignores it, returning the element's own declaration either way. Measured against a seeded 500-node document, the traces alone cost ~30% of clone time (18.8s -> 13.1s across the file's six renders). This continues #7886, which stopped the four image-inlining tests waiting on the render; the flake moved to the two tests that still await one. Co-Authored-By: Claude Opus 5 (1M context) --- .../report-generation.service.spec.ts | 26 +++++++++++++ frontend/src/jsdom-svg-polyfill.ts | 39 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts index 90926b74315..0beee313843 100644 --- a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts +++ b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts @@ -330,6 +330,28 @@ describe("ReportGenerationService", () => { }); describe("generateWorkflowSnapshot", () => { + /** + * html2canvas clones the whole document, not just the element it is pointed at, and the + * unit-test builder runs spec files with `isolate: false` — so one jsdom document is shared + * by every spec file in a worker, and the renders below drag in whatever DOM the files + * before this one left behind. That is what failed the macOS leg: a clone that costs ~60ms + * against this file's own DOM was measured at 12–37s there, past the 20s test timeout, + * while ubuntu and windows passed. Park the foreign nodes for the duration of the file and + * put them back after, so the render's cost depends only on what these tests build. The + * renders started here outlive the tests that start them, so this has to span the file + * rather than each test. + */ + let parkedNodes: ChildNode[]; + + beforeAll(() => { + parkedNodes = Array.from(document.body.childNodes); + parkedNodes.forEach(node => node.remove()); + }); + + afterAll(() => { + parkedNodes.forEach(node => document.body.appendChild(node)); + }); + it("fails when the editor is not on the page", async () => { await expect(firstValueFrom(service.generateWorkflowSnapshot("myflow"))).rejects.toBe( "Workflow editor element not found" @@ -431,6 +453,10 @@ describe("ReportGenerationService", () => { editor = document.createElement("div"); editor.id = "workflow-editor"; document.body.appendChild(editor); + // The render these tests start is left to fail on its own, but jsdom announces its + // missing 2D context on the virtual console, so an otherwise clean run carries a stack + // trace per test. Hand back what jsdom hands back after complaining, minus the complaint. + vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(null as never); }); afterEach(() => { diff --git a/frontend/src/jsdom-svg-polyfill.ts b/frontend/src/jsdom-svg-polyfill.ts index 1fe62e21477..7d3fe63fc09 100644 --- a/frontend/src/jsdom-svg-polyfill.ts +++ b/frontend/src/jsdom-svg-polyfill.ts @@ -192,6 +192,45 @@ G.ResizeObserver ??= class { disconnect(): void {} }; +// `window.getComputedStyle(elt, pseudoElt)` and `window.scrollTo` — jsdom +// implements neither. Both route through its `notImplemented` helper, which +// emits a `jsdomError` on the virtual console; vitest's jsdom environment +// forwards that to `console.error`, one full stack trace per call. html2canvas +// calls both on every render — a `:before` and an `:after` lookup for each +// cloned node, plus one `scrollTo` per document clone — so the +// report-generation spec, which drives the real renderer on purpose (`vi.mock` +// can't reach it under the builder's `isolate: false`), buries the run in +// traces while all of its tests pass. +// Dropping `pseudoElt` changes no behaviour: jsdom complains and then ignores +// the argument, returning the element's own declaration either way. `scrollTo` +// has nothing to move — jsdom has no layout. +const jsdomGetComputedStyle = G.getComputedStyle as ((elt: Element, pseudoElt?: string | null) => unknown) | undefined; +if (typeof jsdomGetComputedStyle === "function") { + const withoutPseudoElement = ((elt: Element) => jsdomGetComputedStyle(elt)) as AnyFn; + G.getComputedStyle = withoutPseudoElement; + if (G.window) G.window.getComputedStyle = withoutPseudoElement; +} +const inertScrollTo: AnyFn = () => undefined; +G.scrollTo = inertScrollTo; +if (G.window) G.window.scrollTo = inertScrollTo; +// The `scrollTo` html2canvas actually reaches for belongs to the throwaway +// iframe it clones the page into, not to this window, and jsdom installs the +// method on each window instance rather than on a shared prototype — so it has +// to be neutered as each `contentWindow` is handed out. +const iframeProto = G.HTMLIFrameElement?.prototype; +const contentWindow = iframeProto && Object.getOwnPropertyDescriptor(iframeProto, "contentWindow"); +if (contentWindow?.get) { + const getContentWindow = contentWindow.get; + Object.defineProperty(iframeProto, "contentWindow", { + ...contentWindow, + get(): unknown { + const frameWindow = getContentWindow.call(this) as Record | null; + if (frameWindow) frameWindow.scrollTo = inertScrollTo; + return frameWindow; + }, + }); +} + // `WebSocket` — y-websocket schedules a reconnect timer the moment a // collaborative-editing service is constructed. When that timer fires AFTER // vitest has begun tearing down the jsdom window, jsdom's WebSocket From a60a72ad699cf382c37088a97cea70fc1991bac3 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:38:54 -0700 Subject: [PATCH 2/7] fix(test, frontend): install the jsdom noise patches once per global `setupFiles` is re-evaluated once per spec file, so the two patches added for html2canvas's `notImplemented` traces wrapped their own wrapper on every evaluation -- 41 layers deep by the end of a full run. The file already guards its process-level handlers and its loader hook against the same accumulation. Mark the installed function and the installed `contentWindow` getter, and skip the patch when the mark is already there. The mark rides on what was installed rather than on a `globalThis` flag so that a jsdom window replaced mid-run is still patched -- a flag would suppress the re-patch and let the traces back in. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/jsdom-svg-polyfill.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/frontend/src/jsdom-svg-polyfill.ts b/frontend/src/jsdom-svg-polyfill.ts index 7d3fe63fc09..f4ae569cee4 100644 --- a/frontend/src/jsdom-svg-polyfill.ts +++ b/frontend/src/jsdom-svg-polyfill.ts @@ -204,9 +204,18 @@ G.ResizeObserver ??= class { // Dropping `pseudoElt` changes no behaviour: jsdom complains and then ignores // the argument, returning the element's own declaration either way. `scrollTo` // has nothing to move — jsdom has no layout. +// Both patches wrap a global that this setup file is re-evaluated against once +// per spec file, so each marks what it installed and does nothing when it finds +// its own mark — otherwise the wrappers nest one layer deeper per spec file. +// The mark rides on the installed function rather than on a `globalThis` flag +// so that a jsdom window replaced mid-run still gets patched. +const NOISE_PATCH_MARK = Symbol.for("texera.jsdomNotImplementedNoisePatched"); +const alreadyPatched = (fn: unknown): boolean => typeof fn === "function" && NOISE_PATCH_MARK in (fn as object); +const markPatched = (fn: AnyFn): AnyFn => Object.assign(fn, { [NOISE_PATCH_MARK]: true }); + const jsdomGetComputedStyle = G.getComputedStyle as ((elt: Element, pseudoElt?: string | null) => unknown) | undefined; -if (typeof jsdomGetComputedStyle === "function") { - const withoutPseudoElement = ((elt: Element) => jsdomGetComputedStyle(elt)) as AnyFn; +if (typeof jsdomGetComputedStyle === "function" && !alreadyPatched(jsdomGetComputedStyle)) { + const withoutPseudoElement = markPatched(((elt: Element) => jsdomGetComputedStyle(elt)) as AnyFn); G.getComputedStyle = withoutPseudoElement; if (G.window) G.window.getComputedStyle = withoutPseudoElement; } @@ -219,15 +228,15 @@ if (G.window) G.window.scrollTo = inertScrollTo; // to be neutered as each `contentWindow` is handed out. const iframeProto = G.HTMLIFrameElement?.prototype; const contentWindow = iframeProto && Object.getOwnPropertyDescriptor(iframeProto, "contentWindow"); -if (contentWindow?.get) { +if (contentWindow?.get && !alreadyPatched(contentWindow.get)) { const getContentWindow = contentWindow.get; Object.defineProperty(iframeProto, "contentWindow", { ...contentWindow, - get(): unknown { + get: markPatched(function (this: unknown): unknown { const frameWindow = getContentWindow.call(this) as Record | null; if (frameWindow) frameWindow.scrollTo = inertScrollTo; return frameWindow; - }, + } as AnyFn), }); } From e37a25d94aabb13c9597be82d23127a5ff2a7085 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:39:09 -0700 Subject: [PATCH 3/7] fix(test, frontend): drop the unreachable main-window scrollTo stub Neutering `scrollTo` on this window was dead code. html2canvas's only call against it is `restoreOwnerScroll`, guarded by `x !== pageXOffset || y !== pageYOffset`; jsdom has no layout, so both offsets are 0 and the call never happens. The call that does happen belongs to the clone iframe's window, which the `contentWindow` patch below already covers. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/jsdom-svg-polyfill.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frontend/src/jsdom-svg-polyfill.ts b/frontend/src/jsdom-svg-polyfill.ts index f4ae569cee4..d1a3067df4d 100644 --- a/frontend/src/jsdom-svg-polyfill.ts +++ b/frontend/src/jsdom-svg-polyfill.ts @@ -219,13 +219,13 @@ if (typeof jsdomGetComputedStyle === "function" && !alreadyPatched(jsdomGetCompu G.getComputedStyle = withoutPseudoElement; if (G.window) G.window.getComputedStyle = withoutPseudoElement; } +// The only `scrollTo` html2canvas aims at this window is guarded by a +// scroll-offset check that cannot fire under jsdom — there is no layout, so +// both offsets are 0 and the call is skipped. The one it does make belongs to +// the throwaway iframe it clones the page into, and jsdom installs the method +// on each window instance rather than on a shared prototype, so it has to be +// neutered as each `contentWindow` is handed out. const inertScrollTo: AnyFn = () => undefined; -G.scrollTo = inertScrollTo; -if (G.window) G.window.scrollTo = inertScrollTo; -// The `scrollTo` html2canvas actually reaches for belongs to the throwaway -// iframe it clones the page into, not to this window, and jsdom installs the -// method on each window instance rather than on a shared prototype — so it has -// to be neutered as each `contentWindow` is handed out. const iframeProto = G.HTMLIFrameElement?.prototype; const contentWindow = iframeProto && Object.getOwnPropertyDescriptor(iframeProto, "contentWindow"); if (contentWindow?.get && !alreadyPatched(contentWindow.get)) { From 4fa165d2f285f0735239c8c7395c69c23139f71e Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:39:30 -0700 Subject: [PATCH 4/7] fix(test, frontend): keep jsdom's rejection of shadow-DOM pseudo-elements Dropping `pseudoElt` wholesale was not behaviour-preserving after all. jsdom tests the argument against `/^::(?:part|slotted)\(/i` and throws a `TypeError` before it reaches `notImplemented`, so a shadow-DOM pseudo-element would have been answered with the element's own declaration instead of the rejection. Nothing in `src` passes one today; forward those arguments so nothing has to. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/jsdom-svg-polyfill.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/frontend/src/jsdom-svg-polyfill.ts b/frontend/src/jsdom-svg-polyfill.ts index d1a3067df4d..7aef063d7d0 100644 --- a/frontend/src/jsdom-svg-polyfill.ts +++ b/frontend/src/jsdom-svg-polyfill.ts @@ -201,9 +201,12 @@ G.ResizeObserver ??= class { // report-generation spec, which drives the real renderer on purpose (`vi.mock` // can't reach it under the builder's `isolate: false`), buries the run in // traces while all of its tests pass. -// Dropping `pseudoElt` changes no behaviour: jsdom complains and then ignores -// the argument, returning the element's own declaration either way. `scrollTo` -// has nothing to move — jsdom has no layout. +// Dropping `pseudoElt` changes no behaviour for the arguments jsdom only +// complains about: it ignores them and returns the element's own declaration +// either way. The exception is a shadow-DOM pseudo-element (`::part(…)` / +// `::slotted(…)`), which jsdom rejects with a `TypeError` before it reaches +// `notImplemented` — those are forwarded so the rejection still happens. +// `scrollTo` has nothing to move — jsdom has no layout. // Both patches wrap a global that this setup file is re-evaluated against once // per spec file, so each marks what it installed and does nothing when it finds // its own mark — otherwise the wrappers nest one layer deeper per spec file. @@ -213,9 +216,13 @@ const NOISE_PATCH_MARK = Symbol.for("texera.jsdomNotImplementedNoisePatched"); const alreadyPatched = (fn: unknown): boolean => typeof fn === "function" && NOISE_PATCH_MARK in (fn as object); const markPatched = (fn: AnyFn): AnyFn => Object.assign(fn, { [NOISE_PATCH_MARK]: true }); +const SHADOW_DOM_PSEUDO = /^::(?:part|slotted)\(/i; const jsdomGetComputedStyle = G.getComputedStyle as ((elt: Element, pseudoElt?: string | null) => unknown) | undefined; if (typeof jsdomGetComputedStyle === "function" && !alreadyPatched(jsdomGetComputedStyle)) { - const withoutPseudoElement = markPatched(((elt: Element) => jsdomGetComputedStyle(elt)) as AnyFn); + const withoutPseudoElement = markPatched(((elt: Element, pseudoElt?: string | null) => + pseudoElt !== undefined && pseudoElt !== null && SHADOW_DOM_PSEUDO.test(String(pseudoElt)) + ? jsdomGetComputedStyle(elt, pseudoElt) + : jsdomGetComputedStyle(elt)) as AnyFn); G.getComputedStyle = withoutPseudoElement; if (G.window) G.window.getComputedStyle = withoutPseudoElement; } From 3b5cca8c9b3d556303e2afa419974fb8c7a1f3f8 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:39:49 -0700 Subject: [PATCH 5/7] test(frontend): park the head's foreign nodes alongside the body's html2canvas clones from `documentElement`, so `` is walked as well -- measured at 185 elements there against the body's 463. Park both, restoring each node to the parent it came from. Co-Authored-By: Claude Opus 5 (1M context) --- .../report-generation.service.spec.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts index 0beee313843..29874c47ae3 100644 --- a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts +++ b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts @@ -331,25 +331,27 @@ describe("ReportGenerationService", () => { describe("generateWorkflowSnapshot", () => { /** - * html2canvas clones the whole document, not just the element it is pointed at, and the - * unit-test builder runs spec files with `isolate: false` — so one jsdom document is shared - * by every spec file in a worker, and the renders below drag in whatever DOM the files - * before this one left behind. That is what failed the macOS leg: a clone that costs ~60ms - * against this file's own DOM was measured at 12–37s there, past the 20s test timeout, - * while ubuntu and windows passed. Park the foreign nodes for the duration of the file and - * put them back after, so the render's cost depends only on what these tests build. The - * renders started here outlive the tests that start them, so this has to span the file - * rather than each test. + * html2canvas clones the whole document — from `documentElement`, not from the element it is + * pointed at — and the unit-test builder runs spec files with `isolate: false`, so one jsdom + * document is shared by every spec file in a worker and the renders below drag in whatever DOM + * the files before this one left behind, in `` as much as in the body. That is what + * failed the macOS leg: a clone that costs ~60ms against this file's own DOM was measured at + * 12–37s there, past the 20s test timeout, while ubuntu and windows passed. Park the foreign + * nodes of both for the duration of the file and put them back after, so the render's cost + * depends only on what these tests build. The renders started here outlive the tests that + * start them, so this has to span the file rather than each test. */ - let parkedNodes: ChildNode[]; + let parkedNodes: [ParentNode, ChildNode][]; beforeAll(() => { - parkedNodes = Array.from(document.body.childNodes); - parkedNodes.forEach(node => node.remove()); + parkedNodes = [document.head, document.body].flatMap(parent => + Array.from(parent.childNodes).map((node): [ParentNode, ChildNode] => [parent, node]) + ); + parkedNodes.forEach(([, node]) => node.remove()); }); afterAll(() => { - parkedNodes.forEach(node => document.body.appendChild(node)); + parkedNodes.forEach(([parent, node]) => parent.appendChild(node)); }); it("fails when the editor is not on the page", async () => { From f892b1e144478141faf2f654ee9af397f423fd47 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:40:02 -0700 Subject: [PATCH 6/7] test(frontend): remove the clone iframes a failed render leaves behind `DocumentCloner.destroy` runs only on the render's success path, so each render these tests leave failing strands its clone iframe in the body -- three of them by the time the suite ends. Removing them before the parked nodes go back keeps the next spec file's renders from cloning them. Co-Authored-By: Claude Opus 5 (1M context) --- .../report-generation/report-generation.service.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts index 29874c47ae3..613ce638d08 100644 --- a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts +++ b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts @@ -351,6 +351,10 @@ describe("ReportGenerationService", () => { }); afterAll(() => { + // html2canvas only detaches the iframe it clones into on the render's success path, so each + // render these tests leave failing strands one in the body. Drop them before the parked + // nodes go back, otherwise the next spec file's renders clone them. + document.body.querySelectorAll("iframe.html2canvas-container").forEach(node => node.remove()); parkedNodes.forEach(([parent, node]) => parent.appendChild(node)); }); From 6a29d2d614b32cda18a3d48e9f528e37f7c2e3a7 Mon Sep 17 00:00:00 2001 From: Neilk1021 Date: Thu, 27 Aug 2026 11:40:17 -0700 Subject: [PATCH 7/7] docs(test, frontend): the parking hooks scope to the suite, not the file `beforeAll` / `afterAll` inside this `describe` span the suite, and a render that outlives them sees the restored DOM either way. Say "suite". Co-Authored-By: Claude Opus 5 (1M context) --- .../report-generation/report-generation.service.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts index 613ce638d08..abcfbb24ba6 100644 --- a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts +++ b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts @@ -335,11 +335,11 @@ describe("ReportGenerationService", () => { * pointed at — and the unit-test builder runs spec files with `isolate: false`, so one jsdom * document is shared by every spec file in a worker and the renders below drag in whatever DOM * the files before this one left behind, in `` as much as in the body. That is what - * failed the macOS leg: a clone that costs ~60ms against this file's own DOM was measured at + * failed the macOS leg: a clone that costs ~60ms against this suite's own DOM was measured at * 12–37s there, past the 20s test timeout, while ubuntu and windows passed. Park the foreign - * nodes of both for the duration of the file and put them back after, so the render's cost + * nodes of both for the duration of the suite and put them back after, so the render's cost * depends only on what these tests build. The renders started here outlive the tests that - * start them, so this has to span the file rather than each test. + * start them, so this has to span the suite rather than each test. */ let parkedNodes: [ParentNode, ChildNode][];