-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[feat]: add support for page.waitForSelector()
#1509
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
Conversation
🦋 Changeset detectedLatest commit: a238850 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 SummaryImplements Key changes:
Implementation highlights:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Page
participant resolveLocatorTarget
participant Frame
participant waitForSelector
participant MutationObserver
participant piercer as V3 Piercer
User->>Page: waitForSelector(selector, options)
Page->>resolveLocatorTarget: resolve iframe hops/XPath
alt has ">>" hop notation
resolveLocatorTarget->>Frame: navigate through iframes
Frame-->>resolveLocatorTarget: target frame
else deep XPath with iframe steps
resolveLocatorTarget->>Frame: parse XPath, resolve iframe steps
Frame-->>resolveLocatorTarget: target frame
else plain selector
resolveLocatorTarget-->>Page: use current frame
end
Page->>Frame: evaluate(waitForSelector script)
Frame->>waitForSelector: execute in page context
waitForSelector->>waitForSelector: check immediate condition
alt condition already met
waitForSelector-->>Frame: resolve(true)
else need to wait
waitForSelector->>MutationObserver: setup observers
waitForSelector->>piercer: setupShadowObservers (if pierceShadow)
piercer-->>waitForSelector: observe closed shadow roots
loop on mutations
MutationObserver->>waitForSelector: trigger check()
waitForSelector->>waitForSelector: findElement (CSS/XPath)
alt pierceShadow=true
waitForSelector->>piercer: query shadow DOM
piercer-->>waitForSelector: element or null
end
waitForSelector->>waitForSelector: checkState (visible/hidden/etc)
alt condition met
waitForSelector->>waitForSelector: set settled=true
waitForSelector->>waitForSelector: clearTimer & cleanup
waitForSelector-->>Frame: resolve(true)
end
end
alt timeout expires
waitForSelector->>waitForSelector: set settled=true
waitForSelector->>waitForSelector: cleanup observers
waitForSelector-->>Frame: reject(timeout error)
end
end
Frame-->>Page: return result
Page-->>User: return boolean
|
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.
5 files reviewed, 2 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.
5 files reviewed, 2 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 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/dom/locatorScripts/waitForSelector.ts">
<violation number="1" location="packages/core/lib/v3/dom/locatorScripts/waitForSelector.ts:442">
P2: Invalid state values are silently accepted and treated as `"visible"`. Consider validating the state parameter and throwing an error for invalid values to help users catch mistakes early.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const state = | ||
| (String(stateRaw ?? "visible") as WaitForSelectorState) || "visible"; |
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.
P2: Invalid state values are silently accepted and treated as "visible". Consider validating the state parameter and throwing an error for invalid values to help users catch mistakes early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/dom/locatorScripts/waitForSelector.ts, line 442:
<comment>Invalid state values are silently accepted and treated as `"visible"`. Consider validating the state parameter and throwing an error for invalid values to help users catch mistakes early.</comment>
<file context>
@@ -0,0 +1,549 @@
+ pierceShadowRaw?: boolean,
+): Promise<boolean> {
+ const selector = String(selectorRaw ?? "").trim();
+ const state =
+ (String(stateRaw ?? "visible") as WaitForSelectorState) || "visible";
+ const timeout =
</file context>
| const state = | |
| (String(stateRaw ?? "visible") as WaitForSelectorState) || "visible"; | |
| const validStates = ["attached", "detached", "visible", "hidden"]; | |
| const stateStr = String(stateRaw ?? "visible").trim() || "visible"; | |
| if (!validStates.includes(stateStr)) { | |
| throw new Error(`waitForSelector: Invalid state "${stateStr}". Must be one of: ${validStates.join(", ")}`); | |
| } | |
| const state = stateStr as WaitForSelectorState; |
why
page.waitForSelector(), which works with cross-iframe selectors & cross-shadow root selectorswhat changed
page.waitForSelector()which returnstruewhen the element that the selector points to resolves, and times out with an error when it does not resolvetest plan
Summary by cubic
Adds Page.waitForSelector() to wait for CSS or XPath targets to reach a state, piercing shadow DOM and traversing iframes. This makes waits reliable across complex trees and uses MutationObserver for efficiency.
New Features
Refactors
Written for commit a238850. Summary will update on new commits.