-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
feat(Console): Introduce ConsoleMessage type #909
feat(Console): Introduce ConsoleMessage type #909
Conversation
lib/Page.js
Outdated
@@ -313,7 +313,10 @@ class Page extends EventEmitter { | |||
return; | |||
} | |||
const values = await Promise.all(event.args.map(arg => helper.serializeRemoteObject(this._client, arg))); | |||
this.emit(Page.Events.Console, ...values); | |||
const text = values.length && typeof values[0] === 'string' ? values[0] : null; |
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.
Let's make text all the values.join(" ")
@@ -1093,6 +1097,21 @@ puppeteer.launch().then(async browser => { | |||
|
|||
Dialog's type, could be one of the `alert`, `beforeunload`, `confirm` and `prompt`. | |||
|
|||
### class: ConsoleMessage |
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.
Inline this into the console event?
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.
I think it's fine like this
It seems these fragments also need updating: |
This patch introduces ConsoleMessage type and starts dispatching
it for the
'console'
event.BREAKING CHANGE: this breaks the api of the
'console'
event.Fixes #744.