Skip to content
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

consider replacing driver.evaluateAsync with new async debugger evaluate #580

Closed
brendankenny opened this issue Aug 11, 2016 · 5 comments · Fixed by #793
Closed

consider replacing driver.evaluateAsync with new async debugger evaluate #580

brendankenny opened this issue Aug 11, 2016 · 5 comments · Fixed by #793

Comments

@brendankenny
Copy link
Member

Should simplify it a good bit. Will require all client-side scripts to evaluate to a promise (instead of calling __returnResults)

https://chromedevtools.github.io/debugger-protocol-viewer/tot/Runtime/#method-evaluate

@paulirish
Copy link
Member

the awaitPromise param in particular,, yeah.
FWIW this landed in https://codereview.chromium.org/2196003002/ which makes it m54. We may want to wait until Chrome stable hits 54 (around Oct 18th) before moving to this.

@wardpeet
Copy link
Collaborator

Would it be a lot of work to support both? With a feature check? (not sure how __returnResults is used atm)

@paulirish
Copy link
Member

We could "feature-detect" the protocol to see if there's support for it, but I'd rather do the switch in a few months at once, rather than support two codepaths simultaneously.

(TBH, our evaluateAsync has been totally stable, so i'm not particularly concerned about maintenance, but we're not in any rush either.)

@wardpeet
Copy link
Collaborator

That's true :) perhaps add a milestone or a label to it.

@paulirish
Copy link
Member

cc @ak239

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants