Skip to content

Commit ab51232

Browse files
fix: don't reload OOPIFs (#1273)
# why - we needed to reload OOPIFs so that our shadow-root piercing JS was able to intercept the `attachShadow()` call - this is wasteful, and can cause unpredictable behaviour # what changed - swapped iframe reload logic for new logic that clones, and replaces shadow root nodes - this triggers the patched `attachShadow()` function # test plan - existing unit tests & evals should sufficiently cover shadow root piercing
1 parent a9b58f1 commit ab51232

File tree

8 files changed

+112
-100
lines changed

8 files changed

+112
-100
lines changed

.changeset/clear-women-float.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@browserbasehq/stagehand": patch
3+
---
4+
5+
fix: trigger shadow root rerender in OOPIFs by cloning & replacing instead of reloading

packages/core/lib/v3/dom/genDomScripts.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,26 @@ const script = fs.readFileSync(path.join(outDir, "v3-index.js"), "utf8");
2525
const content = `export const v3ScriptContent = ${JSON.stringify(script)};`;
2626

2727
fs.writeFileSync(path.join(outDir, "scriptV3Content.ts"), content);
28+
29+
esbuild.buildSync({
30+
entryPoints: [path.join(here, "rerenderMissingShadows.entry.ts")],
31+
bundle: true,
32+
format: "iife",
33+
platform: "browser",
34+
target: "es2020",
35+
minify: true,
36+
legalComments: "none",
37+
outfile: path.join(outDir, "rerender-index.js"),
38+
});
39+
40+
const rerenderScript = fs.readFileSync(
41+
path.join(outDir, "rerender-index.js"),
42+
"utf8",
43+
);
44+
const rerenderContent = `export const reRenderScriptContent = ${JSON.stringify(
45+
rerenderScript,
46+
)};`;
47+
fs.writeFileSync(
48+
path.join(outDir, "reRenderScriptContent.ts"),
49+
rerenderContent,
50+
);
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { rerenderMissingShadowHosts } from "./rerenderMissingShadows.runtime";
2+
3+
rerenderMissingShadowHosts();
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
export function rerenderMissingShadowHosts(): void {
2+
try {
3+
const piercer = window.__stagehandV3__;
4+
if (!piercer || typeof piercer.getClosedRoot !== "function") return;
5+
6+
const needsReset: Element[] = [];
7+
const walker = document.createTreeWalker(document, NodeFilter.SHOW_ELEMENT);
8+
while (walker.nextNode()) {
9+
const el = walker.currentNode as Element;
10+
const tag = el.tagName?.toLowerCase() ?? "";
11+
if (!tag.includes("-")) continue;
12+
if (typeof customElements?.get !== "function") continue;
13+
if (!customElements.get(tag)) continue;
14+
const hasOpen = !!el.shadowRoot;
15+
const hasClosed = !!piercer.getClosedRoot(el);
16+
if (hasOpen || hasClosed) continue;
17+
needsReset.push(el);
18+
}
19+
20+
for (const host of needsReset) {
21+
try {
22+
const clone = host.cloneNode(true);
23+
host.replaceWith(clone);
24+
} catch {
25+
// ignore individual failures
26+
}
27+
}
28+
29+
if (piercer.stats && needsReset.length) {
30+
console.info("[v3-piercer] rerender", { count: needsReset.length });
31+
}
32+
} catch (err) {
33+
console.info("[v3-piercer] rerender error", { message: String(err ?? "") });
34+
}
35+
}

packages/core/lib/v3/tests/default-page-tracking.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { test, expect } from "@playwright/test";
22
import { V3 } from "../v3";
33
import { v3DynamicTestConfig } from "./v3.dynamic.config";
44

5+
test.describe.configure({ mode: "parallel" });
56
test.describe("V3 default page tracking", () => {
67
let v3: V3;
78

@@ -40,7 +41,7 @@ test.describe("V3 default page tracking", () => {
4041
const root = await ctx.awaitActivePage();
4142
await root!.goto(
4243
"https://browserbase.github.io/stagehand-eval-sites/sites/five-tab/",
43-
{ waitUntil: "networkidle", timeoutMs: 15000 },
44+
{ waitUntil: "load", timeoutMs: 15000 },
4445
);
4546
// 2) Click button on the page to open a new tab → page2.html
4647
await root.locator("xpath=/html/body/button").click();

packages/core/lib/v3/understudy/cdp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export class CdpConnection implements CDPSessionLike {
9393
await this.send("Target.setAutoAttach", {
9494
autoAttach: true,
9595
flatten: true,
96-
waitForDebuggerOnStart: true,
96+
waitForDebuggerOnStart: false,
9797
filter: [
9898
{ type: "worker", exclude: true },
9999
{ type: "shared_worker", exclude: true },

packages/core/lib/v3/understudy/context.ts

Lines changed: 20 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,15 @@ export class V3Context {
133133
}
134134
}
135135

136-
private async ensurePiercer(session: CDPSessionLike): Promise<void> {
136+
private async ensurePiercer(session: CDPSessionLike): Promise<boolean> {
137137
const key = this.sessionKey(session);
138-
if (this._piercerInstalled.has(key)) return;
138+
if (this._piercerInstalled.has(key)) return true;
139139

140-
await installV3PiercerIntoSession(session);
141-
this._piercerInstalled.add(key);
140+
const installed = await installV3PiercerIntoSession(session);
141+
if (installed) {
142+
this._piercerInstalled.add(key);
143+
}
144+
return installed;
142145
}
143146

144147
/** Mark a page target as the most-recent one (active). */
@@ -286,11 +289,7 @@ export class V3Context {
286289
this.conn.on<Protocol.Target.AttachedToTargetEvent>(
287290
"Target.attachedToTarget",
288291
async (evt) => {
289-
await this.onAttachedToTarget(
290-
evt.targetInfo,
291-
evt.sessionId,
292-
evt.waitingForDebugger === true,
293-
);
292+
await this.onAttachedToTarget(evt.targetInfo, evt.sessionId);
294293
},
295294
);
296295

@@ -365,38 +364,25 @@ export class V3Context {
365364
private async onAttachedToTarget(
366365
info: Protocol.Target.TargetInfo,
367366
sessionId: SessionId,
368-
waitingForDebugger?: boolean, // Chrome paused this target at doc-start?
369367
): Promise<void> {
370368
const session = this.conn.getSession(sessionId);
371369
if (!session) return;
372370

373371
// Init guard
374372
if (this._sessionInit.has(sessionId)) return;
375373
this._sessionInit.add(sessionId);
374+
await session.send("Runtime.runIfWaitingForDebugger").catch(() => {});
375+
376+
// Register for Runtime events before enabling it so we don't miss initial contexts.
377+
executionContexts.attachSession(session);
378+
379+
const piercerReady = await this.ensurePiercer(session);
380+
if (!piercerReady) return;
376381

377-
// Page domain first so frame events flow
378-
const pageEnabled = await session
379-
.send("Page.enable")
380-
.then(() => true)
381-
.catch(() => false);
382-
if (!pageEnabled) {
383-
if (waitingForDebugger) {
384-
await session.send("Runtime.runIfWaitingForDebugger").catch(() => {});
385-
}
386-
return;
387-
}
388382
await session
389383
.send("Page.setLifecycleEventsEnabled", { enabled: true })
390384
.catch(() => {});
391385

392-
// Proactively resume ASAP so navigation isn't stuck at about:blank
393-
await session.send("Runtime.runIfWaitingForDebugger").catch(() => {});
394-
395-
// Capture main-world creations & inject piercer after resume
396-
executionContexts.attachSession(session);
397-
await session.send("Runtime.enable").catch(() => {});
398-
await this.ensurePiercer(session);
399-
400386
// Top-level handling
401387
if (isTopLevelPage(info)) {
402388
const page = await Page.create(
@@ -422,10 +408,6 @@ export class V3Context {
422408
this._pushActive(info.targetId);
423409
this.installFrameEventBridges(sessionId, page);
424410

425-
// Resume only if Chrome actually paused
426-
if (waitingForDebugger) {
427-
await session.send("Runtime.runIfWaitingForDebugger").catch(() => {});
428-
}
429411
return;
430412
}
431413

@@ -458,73 +440,18 @@ export class V3Context {
458440
owner.adoptOopifSession(session, childMainId);
459441
this.sessionOwnerPage.set(sessionId, owner);
460442
this.installFrameEventBridges(sessionId, owner);
443+
// Prime the execution-context registry so later lookups succeed even if
444+
// the frame navigates before we issue a command.
445+
void executionContexts
446+
.waitForMainWorld(session, childMainId)
447+
.catch(() => {});
461448
} else {
462449
this.pendingOopifByMainFrame.set(childMainId, sessionId);
463450
}
464451
} catch {
465452
// page.getFrameTree failed. Most likely was an ad iframe
466453
// that opened & closed before we could attach. ignore
467454
}
468-
469-
/* TODO: for child OOPIFs, we rely on reloads for our script to be applied.
470-
* This is not ideal. We should figure out a way to pause them, and inject them
471-
* from the beginning.
472-
*/
473-
// --- If Chrome did NOT pause this iframe (waitingForDebugger=false),
474-
// we missed doc-start. Do a one-time, best-effort child reload
475-
// so Page.addScriptToEvaluateOnNewDocument applies at creation time.
476-
if (info.type === "iframe" && !waitingForDebugger) {
477-
try {
478-
// Ensure lifecycle events are enabled to observe load
479-
await session
480-
.send("Page.setLifecycleEventsEnabled", { enabled: true })
481-
.catch(() => {});
482-
483-
const loadWait = new Promise<void>((resolve) => {
484-
const handler = (evt: Protocol.Page.LifecycleEventEvent) => {
485-
if (evt.name === "load") {
486-
session.off("Page.lifecycleEvent", handler);
487-
resolve();
488-
}
489-
};
490-
session.on("Page.lifecycleEvent", handler);
491-
});
492-
493-
// Cannot use Page.reload on child targets → use Runtime.evaluate
494-
await session
495-
.send<Protocol.Runtime.EvaluateResponse>("Runtime.evaluate", {
496-
expression: "location.reload()",
497-
returnByValue: true,
498-
awaitPromise: false,
499-
})
500-
.catch(() => {});
501-
502-
// Wait up to ~3s for the child to finish loading (best effort)
503-
await Promise.race([
504-
loadWait,
505-
new Promise<void>((r) => setTimeout(r, 3000)),
506-
]);
507-
508-
// After reload, our addScriptToEvaluateOnNewDocument runs at doc-start.
509-
// We can optionally re-evaluate the piercer (idempotent), but not required.
510-
await this.ensurePiercer(session);
511-
} catch (e) {
512-
v3Logger({
513-
category: "ctx",
514-
message: "child reload attempt failed (continuing)",
515-
level: 2,
516-
auxiliary: {
517-
sessionId: { value: String(sessionId), type: "string" },
518-
err: { value: String(e), type: "string" },
519-
},
520-
});
521-
}
522-
}
523-
524-
// Resume only if Chrome actually paused (rare for iframes in your logs)
525-
if (waitingForDebugger) {
526-
await session.send("Runtime.runIfWaitingForDebugger").catch(() => {});
527-
}
528455
}
529456

530457
/**

packages/core/lib/v3/understudy/piercer.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@ import type { Protocol } from "devtools-protocol";
22
import { v3Logger } from "../logger";
33
import type { CDPSessionLike } from "./cdp";
44
import { v3ScriptContent } from "../dom/build/scriptV3Content";
5+
import { reRenderScriptContent } from "../dom/build/reRenderScriptContent";
56

67
export async function installV3PiercerIntoSession(
78
session: CDPSessionLike,
8-
): Promise<void> {
9-
await session.send("Page.enable").catch(() => {});
9+
): Promise<boolean> {
10+
const pageEnabled = await session
11+
.send("Page.enable")
12+
.then(() => true)
13+
.catch(() => false);
14+
if (!pageEnabled) return false;
15+
1016
await session.send("Runtime.enable").catch(() => {});
1117
try {
1218
await session.send<Protocol.Page.AddScriptToEvaluateOnNewDocumentResponse>(
@@ -15,9 +21,9 @@ export async function installV3PiercerIntoSession(
1521
);
1622
} catch (e) {
1723
const msg = String((e as Error)?.message ?? e ?? "");
18-
// If the session vanished during attach (common with shortlived OOPIFs),
19-
// swallow and return; re‑installs will happen on future sessions/loads.
20-
if (msg.includes("Session with given id not found")) return;
24+
// If the session vanished during attach (common with short-lived OOPIFs),
25+
// swallow and report failure so callers can early-return.
26+
if (msg.includes("Session with given id not found")) return false;
2127
// For other errors, keep going but don't throw — the next evaluate is idempotent.
2228
}
2329
await session
@@ -27,6 +33,18 @@ export async function installV3PiercerIntoSession(
2733
awaitPromise: true,
2834
})
2935
.catch(() => {});
36+
37+
// After the piercer is in place, re-render any custom elements whose
38+
// shadow roots were created before we patched attachShadow so their
39+
// closed roots are recreated under the hook.
40+
await session
41+
.send<Protocol.Runtime.EvaluateResponse>("Runtime.evaluate", {
42+
expression: reRenderScriptContent,
43+
returnByValue: true,
44+
awaitPromise: false,
45+
})
46+
.catch(() => {});
47+
return true;
3048
}
3149

3250
/** (Optional) stream patch logs in your node console during bring-up */

0 commit comments

Comments
 (0)