Skip to content

core(driver): add driver.wsEndpoint()#3864

Merged
paulirish merged 3 commits intomasterfrom
wsendpoint
Nov 21, 2017
Merged

core(driver): add driver.wsEndpoint()#3864
paulirish merged 3 commits intomasterfrom
wsendpoint

Conversation

@paulirish
Copy link
Member

for assistance when working with puppeteer

ref #3837

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm



/**
* @return {!Promise}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we still say !Promise<string> here just it'll always be rejected?

return this._connection.disconnect();
}

wsEndpoint() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy/paste jsdoc or hopefully tsc smart enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docs for which endpoint this is? And I can't imagine who would think otherwise, but probably worth noting that it's CRI-only

Copy link
Contributor

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % periods

}

/**
* Get the browser WebSocket endpoint for devtools protocol clients like Puppeteer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add some punctuation :P

@paulirish paulirish merged commit 2182513 into master Nov 21, 2017
@paulirish paulirish deleted the wsendpoint branch November 21, 2017 02:16
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 this pull request may close these issues.

3 participants