core(console-messages): use source-location #11899
Conversation
| // JS deprecations will have a stack trace. | ||
| // CSS deprecations only expose a line number. | ||
| const topCallFrame = log.stackTrace && log.stackTrace.callFrames[0]; | ||
| const line = log.lineNumber || 0; |
There was a problem hiding this comment.
the call frames are now used in ConsoleMessages to set line/col, so we can just defer to that here now.
| const consoleRows = artifacts.ConsoleMessages | ||
| .filter(item => item.level === 'error') | ||
| .map(item => { | ||
| return { | ||
| source: item.source, | ||
| description: item.text, | ||
| url: item.url, | ||
| sourceLocation: ConsoleMessages.createSourceLocation(item), |
There was a problem hiding this comment.
It's unfortunate that source can't be used here, like every other audit.
| @@ -81,14 +81,15 @@ class ErrorLogs extends Audit { | |||
| /** @type {AuditOptions} */ | |||
| const auditOptions = context.options; | |||
|
|
|||
| /** @type {Array<{source: string, description: string|undefined, url: string|undefined}>} */ | |||
| /** @type {Array<{source: string, description: string|undefined, sourceLocation: LH.Audit.Details.SourceLocationValue|undefined}>} */ | |||
| const consoleRows = artifacts.ConsoleMessages | |||
| .filter(item => item.level === 'error') | |||
| .map(item => { | |||
| return { | |||
| source: item.source, | |||
| description: item.text, | |||
| url: item.url, | |||
There was a problem hiding this comment.
wondering if url should stick around and be removed in v8 (ditto for all these other audits), or are we good with removing the old properties immediately?
There was a problem hiding this comment.
I don't have a strong opinion on whether we should keep the url field around until v8. However, if you decide to leave it for now please add a TODO to remove it later.
There was a problem hiding this comment.
I like leaving it here too 👍
| "eventType": "protocolLog", | ||
| "source": "deprecation", | ||
| "level": "warning", | ||
| "text": "/deep/ combinator is no longer supported in CSS dynamic profile. It is now effectively no-op, acting as if it were a descendant combinator. /deep/ combinator will be removed, and will be invalid at M65. You should remove it. See https://www.chromestatus.com/features/4964279606312960 for more details.", |
There was a problem hiding this comment.
I suppose this deprecation noticed has been since deleted in Chromium.
| @@ -81,14 +81,15 @@ class ErrorLogs extends Audit { | |||
| /** @type {AuditOptions} */ | |||
| const auditOptions = context.options; | |||
|
|
|||
| /** @type {Array<{source: string, description: string|undefined, url: string|undefined}>} */ | |||
| /** @type {Array<{source: string, description: string|undefined, sourceLocation: LH.Audit.Details.SourceLocationValue|undefined}>} */ | |||
| const consoleRows = artifacts.ConsoleMessages | |||
| .filter(item => item.level === 'error') | |||
| .map(item => { | |||
| return { | |||
| source: item.source, | |||
| description: item.text, | |||
| url: item.url, | |||
There was a problem hiding this comment.
I like leaving it here too 👍
| "url": "http://localhost:10200/dobetterweb/dbw_tester.js", | ||
| "lineNumber": 13 | ||
| "lineNumber": 13, | ||
| "columnNumber": 9 |
Change all audits that rely on
ConsoleMessagesto produce asource-locationcolumn, instead ofurl+ "location" columns.I also tweaked all the
source-locationcolumns to use theUIStrings.columnSourcelabel.Audits now using
source-locationand previously weren't:deprecationsgeolocation-on-startno-document-writenotification-on-startuses-passive-event-listenerserrors-in-consoleAudits now using
UIStrings.columnSourceforsource-locationand previously weren't:deprecationsno-unload-listenersbefore
after
fixes #11761
(related #10930–this PR is low impact, only fixing the off-by-one line number + adding column, but once this other PR lands we'll also supercharge the URL here by using a source map for JS events)