-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[fix]: make page.evaluate() use main world
#1331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix]: make page.evaluate() use main world
#1331
Conversation
🦋 Changeset detectedLatest commit: d4bf59d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Greptile OverviewGreptile SummaryChanged
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Context
participant Page
participant CDPSession
participant ExecutionContextRegistry
participant Browser
User->>Context: addInitScript(script)
Context->>Page: registerInitScript(source)
Page->>CDPSession: Page.addScriptToEvaluateOnNewDocument
CDPSession->>Browser: Install script in main world
User->>Page: goto(url)
Browser->>CDPSession: Runtime.executionContextCreated
CDPSession->>ExecutionContextRegistry: Register main world context
Note over ExecutionContextRegistry: Tracks frameId -> contextId mapping<br/>for main world (isDefault=true)
User->>Page: evaluate(function)
Page->>ExecutionContextRegistry: waitForMainWorld(frameId)
ExecutionContextRegistry-->>Page: Return main world contextId
Page->>CDPSession: Runtime.evaluate(contextId)
Note over CDPSession,Browser: Executes in main world<br/>where init scripts are available
Browser-->>Page: Return result
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/tests/context-addInitScript.spec.ts">
<violation number="1" location="packages/core/lib/v3/tests/context-addInitScript.spec.ts:127">
P2: This test uses an external URL (`https://example.com`) while all other tests in this file use the `toDataUrl()` helper to create data URLs. Using an external URL makes the test dependent on network connectivity and potentially flaky. Consider using a data URL for consistency and reliability.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
28dcdce to
d4bf59d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/tests/context-addInitScript.spec.ts">
<violation number="1" location="packages/core/lib/v3/tests/context-addInitScript.spec.ts:127">
P2: This test uses an external URL (`https://example.com`) while all other tests in this file use `toDataUrl()` for network-independent data URLs. This introduces unnecessary network dependency and potential test flakiness.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
why
page.evaluate()uses thev3-worldaddInitScript()what changed
page.evaluate()uses the main worldtest plan
page.evaluate()on a script added viacontext.addInitScript()Summary by cubic
Fixes a context mismatch: page.evaluate now runs in the main world so scripts injected via context.addInitScript are callable. Frame.evaluate was updated to use the main world too.
Why:
What:
Test Plan:
Written for commit d4bf59d. Summary will update automatically on new commits.