fix(core): respect v3 piercer debug option#2083
fix(core): respect v3 piercer debug option#2083EfeDurmaz16 wants to merge 1 commit intobrowserbase:mainfrom
Conversation
|
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16feb5b8b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function installV3ShadowPiercer(opts: V3ShadowPatchOptions = {}): void { | ||
| // hardcoded debug (remove later if desired) | ||
| const DEBUG = true; | ||
| const DEBUG = opts.debug ?? false; |
There was a problem hiding this comment.
Stop enabling piercer debug in the injected entry
Changing this default does not silence the runtime path used by normal V3 sessions: scripts/build-dom-scripts.ts bundles lib/v3/dom/piercer.entry.ts into v3ScriptContent, understudy/piercer.ts injects/evaluates that script, and the entry still calls installV3ShadowPiercer({ debug: true, tagExisting: false }). In those sessions the new default is bypassed, so [v3-piercer] console logs continue even when debug is not requested; the added unit test only covers direct calls to installV3ShadowPiercer().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Client as Client Page
participant V3Piercer as V3 Shadow Piercer
participant Console as console.info
participant Test as Test Suite
Note over Client,Test: V3 Shadow Piercer Debug Logging Flow
Client->>V3Piercer: installV3ShadowPiercer(opts)
V3Piercer->>V3Piercer: opts.debug ?? false -> DEBUG
alt DEBUG = false (default)
Client->>V3Piercer: element.attachShadow({ mode: "closed" })
V3Piercer->>V3Piercer: Patch attachShadow, no logging
V3Piercer-->>Client: ShadowRoot
Note over V3Piercer,Console: No console.info call
else DEBUG = true
Client->>V3Piercer: element.attachShadow({ mode: "closed" })
V3Piercer->>V3Piercer: Patch attachShadow, log info
V3Piercer->>Console: info("v3 piercer patched shadow...")
Console-->>V3Piercer: (logged)
V3Piercer-->>Client: ShadowRoot
end
Note over Test: Regression Tests
Test->>Test: installDomGlobals()
Test->>V3Piercer: installV3ShadowPiercer() (no opts)
Test->>Test: new TestElement().attachShadow(...)
Test->>Test: expect(console.info).not.toHaveBeenCalled()
Test->>Test: installDomGlobals()
Test->>V3Piercer: installV3ShadowPiercer({ debug: true })
Test->>Test: new TestElement().attachShadow(...)
Test->>Test: expect(console.info).toHaveBeenCalled()
Summary
opts.debug ?? falseinstead of always enabling V3 piercer debug loggingFixes #1996.
Verification
Summary by cubic
Fixes v3 shadow piercer logging to respect the
debugoption. Default behavior is now silent; logs only whendebug: trueis passed.opts.debug ?? falseininstallV3ShadowPiercer.Written for commit 16feb5b. Summary will update on new commits.