-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core: add hidden audits for each insight #16312
Changes from 6 commits
22b7291
8ba0676
f32b4e5
9369fb6
63efe6a
46e62aa
ae78390
8f474ba
c280ae5
59fa89d
e3a2608
af806a1
f9afe92
98896d1
91cf402
b4f51d4
49b7a07
86ab3dc
d7b3f97
12f9669
a826ce5
8b518f4
0e299f1
a4d240c
52cbf14
eafd228
d011c22
058e947
1b7a488
b7d2e3f
adb1f1e
0bf466e
70028c4
7b23538
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/** | ||
* @license | ||
* Copyright 2025 Google LLC | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import {UIStrings} from '@paulirish/trace_engine/models/trace/insights/DOMSize.js'; | ||
|
||
import {Audit} from '../audit.js'; | ||
import * as i18n from '../../lib/i18n/i18n.js'; | ||
import {adaptInsightToAuditProduct, makeNodeItemForNodeId} from './insight-audit.js'; | ||
|
||
// eslint-disable-next-line max-len | ||
const str_ = i18n.createIcuMessageFn('node_modules/@paulirish/trace_engine/models/trace/insights/DOMSize.js', UIStrings); | ||
|
||
class DOMSizeInsight extends Audit { | ||
/** | ||
* @return {LH.Audit.Meta} | ||
*/ | ||
static get meta() { | ||
return { | ||
id: 'dom-size-insight', | ||
title: str_(UIStrings.title), | ||
description: str_(UIStrings.description), | ||
guidanceLevel: 3, | ||
requiredArtifacts: ['traces', 'TraceElements'], | ||
scoreDisplayMode: Audit.SCORING_MODES.METRIC_SAVINGS, | ||
}; | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts} artifacts | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Audit.Product>} | ||
*/ | ||
static async audit(artifacts, context) { | ||
return adaptInsightToAuditProduct(artifacts, context, 'DOMSize', (insight) => { | ||
if (!insight.maxDOMStats?.args.data.maxChildren || !insight.maxDOMStats?.args.data.maxDepth) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically we will still have a valid |
||
return; | ||
} | ||
|
||
const {maxChildren, maxDepth} = insight.maxDOMStats.args.data; | ||
|
||
/** @type {LH.Audit.Details.Table['headings']} */ | ||
const headings = [ | ||
{key: 'statistic', valueType: 'text', label: 'Statistic'}, // TODO ! | ||
{key: 'node', valueType: 'node', label: str_(i18n.UIStrings.columnElement)}, | ||
{key: 'value', valueType: 'numeric', label: 'Value'}, // TODO ! | ||
]; | ||
/** @type {LH.Audit.Details.Table['items']} */ | ||
const items = [ | ||
{ | ||
connorjclark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
statistic: 'Most children', | ||
node: makeNodeItemForNodeId(artifacts.TraceElements, maxChildren.nodeId), | ||
value: { | ||
type: 'numeric', | ||
granularity: 1, | ||
value: maxChildren.numChildren, | ||
}, | ||
}, | ||
{ | ||
statistic: 'DOM depth', | ||
node: makeNodeItemForNodeId(artifacts.TraceElements, maxDepth.nodeId), | ||
value: { | ||
type: 'numeric', | ||
granularity: 1, | ||
value: maxDepth.depth, | ||
}, | ||
}, | ||
]; | ||
return Audit.makeTableDetails(headings, items); | ||
}); | ||
} | ||
} | ||
|
||
export default DOMSizeInsight; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/** | ||
* @license | ||
* Copyright 2025 Google LLC | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import {NavigationInsights} from '../../computed/navigation-insights.js'; | ||
import {Audit} from '../audit.js'; | ||
|
||
/** | ||
* @param {LH.Artifacts} artifacts | ||
* @param {LH.Audit.Context} context | ||
* @param {T} insightName | ||
* @param {(insight: import('@paulirish/trace_engine/models/trace/insights/types.js').InsightModels[T]) => LH.Audit.Details|undefined} createDetails | ||
* @template {keyof import('@paulirish/trace_engine/models/trace/insights/types.js').InsightModelsType} T | ||
* @return {Promise<LH.Audit.Product>} | ||
*/ | ||
async function adaptInsightToAuditProduct(artifacts, context, insightName, createDetails) { | ||
const trace = artifacts.traces[Audit.DEFAULT_PASS]; | ||
const navInsights = await NavigationInsights.request(trace, context); | ||
const insight = navInsights.model[insightName]; | ||
const details = createDetails(insight); | ||
return { | ||
score: insight.shouldShow ? 0 : 1, | ||
metricSavings: insight.metricSavings, | ||
warnings: insight.warnings, | ||
details, | ||
}; | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts.TraceElement[]} traceElements | ||
* @param {number|null|undefined} nodeId | ||
* @return {LH.Audit.Details.NodeValue|undefined} | ||
*/ | ||
function makeNodeItemForNodeId(traceElements, nodeId) { | ||
if (typeof nodeId !== 'number') { | ||
return; | ||
} | ||
|
||
const traceElement = | ||
traceElements.find(te => te.traceEventType === 'trace-engine' && te.nodeId === nodeId); | ||
const node = traceElement?.node; | ||
if (!node) { | ||
return; | ||
} | ||
|
||
return Audit.makeNodeItem(node); | ||
} | ||
|
||
export { | ||
adaptInsightToAuditProduct, | ||
makeNodeItemForNodeId, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/** | ||
* @license | ||
* Copyright 2025 Google LLC | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import {UIStrings} from '@paulirish/trace_engine/models/trace/insights/Viewport.js'; | ||
|
||
import {Audit} from '../audit.js'; | ||
import * as i18n from '../../lib/i18n/i18n.js'; | ||
import {adaptInsightToAuditProduct, makeNodeItemForNodeId} from './insight-audit.js'; | ||
|
||
// eslint-disable-next-line max-len | ||
const str_ = i18n.createIcuMessageFn('node_modules/@paulirish/trace_engine/models/trace/insights/Viewport.js', UIStrings); | ||
|
||
class ViewportInsight extends Audit { | ||
/** | ||
* @return {LH.Audit.Meta} | ||
*/ | ||
static get meta() { | ||
return { | ||
id: 'viewport-insight', | ||
title: str_(UIStrings.title), | ||
description: str_(UIStrings.description), | ||
guidanceLevel: 3, | ||
requiredArtifacts: ['traces', 'TraceElements'], | ||
scoreDisplayMode: Audit.SCORING_MODES.METRIC_SAVINGS, | ||
}; | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts} artifacts | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Audit.Product>} | ||
*/ | ||
static async audit(artifacts, context) { | ||
return adaptInsightToAuditProduct(artifacts, context, 'Viewport', (insight) => { | ||
const nodeId = insight.viewportEvent?.args.data.node_id; | ||
const htmlSnippet = insight.viewportEvent ? | ||
`<meta name=viewport content="${insight.viewportEvent.args.data.content}">` : | ||
null; | ||
|
||
/** @type {LH.Audit.Details.Table['headings']} */ | ||
const headings = [ | ||
{key: 'node', valueType: 'node', label: ''}, | ||
]; | ||
/** @type {LH.Audit.Details.Table['items']} */ | ||
const items = [ | ||
{node: makeNodeItemForNodeId(artifacts.TraceElements, nodeId)}, | ||
]; | ||
const table = Audit.makeTableDetails(headings, items); | ||
|
||
return Audit.makeListDetails([ | ||
table, | ||
{type: 'debugdata', items: [htmlSnippet]}, | ||
]); | ||
}); | ||
} | ||
} | ||
|
||
export default ViewportInsight; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,29 @@ class TraceElements extends BaseGatherer { | |
if (name) this.animationIdToName.set(id, name); | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts.TraceEngineResult} traceEngineResult | ||
* @param {string} mainFrameId | ||
* @return {Promise<Array<{nodeId: number}>>} | ||
*/ | ||
static async getTraceEngineElements(traceEngineResult, mainFrameId) { | ||
// Can only resolve elements for the latest insight set, which should correspond | ||
// to the current frame id. Can't resolve elements for pages that are gone. | ||
const insightSet = [...traceEngineResult.insights.values()] | ||
.reverse().find(insightSet => insightSet.frameId === mainFrameId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty sure this will always be the last insight set in the list. The main frame ID does not change across navigations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should actually just grab the last one, and then check if it is valid by looking at navigation id. updated. |
||
if (!insightSet) { | ||
return []; | ||
} | ||
|
||
const nodeIds = [ | ||
insightSet.model.DOMSize.maxDOMStats?.args.data.maxChildren?.nodeId, | ||
insightSet.model.DOMSize.maxDOMStats?.args.data.maxDepth?.nodeId, | ||
insightSet.model.Viewport.viewportEvent?.args.data.node_id, | ||
]; | ||
|
||
return nodeIds.filter(id => id !== undefined).map(id => ({nodeId: id})); | ||
} | ||
|
||
/** | ||
* We want to a single representative node to represent the shift, so let's pick | ||
* the one with the largest impact (size x distance moved). | ||
|
@@ -320,6 +343,11 @@ class TraceElements extends BaseGatherer { | |
const processedTrace = await ProcessedTrace.request(trace, context); | ||
const {mainThreadEvents} = processedTrace; | ||
|
||
const frameTreeResponse = await session.sendCommand('Page.getFrameTree'); | ||
const mainFrameId = frameTreeResponse.frameTree.frame.id; | ||
|
||
const traceEngineData = await TraceElements.getTraceEngineElements( | ||
traceEngineResult, mainFrameId); | ||
const lcpNodeData = await TraceElements.getLcpElement(trace, context); | ||
const shiftsData = await TraceElements.getTopLayoutShifts( | ||
trace, traceEngineResult.data, rootCauses, context); | ||
|
@@ -328,6 +356,7 @@ class TraceElements extends BaseGatherer { | |
|
||
/** @type {Map<string, TraceElementData[]>} */ | ||
const backendNodeDataMap = new Map([ | ||
['trace-engine', traceEngineData], | ||
['largest-contentful-paint', lcpNodeData ? [lcpNodeData] : []], | ||
['layout-shift', shiftsData], | ||
['animation', animatedElementData], | ||
|
@@ -336,6 +365,7 @@ class TraceElements extends BaseGatherer { | |
|
||
/** @type {Map<number, LH.Crdp.Runtime.CallFunctionOnResponse | null>} */ | ||
const callFunctionOnCache = new Map(); | ||
/** @type {LH.Artifacts.TraceElement[]} */ | ||
const traceElements = []; | ||
for (const [traceEventType, backendNodeData] of backendNodeDataMap) { | ||
for (let i = 0; i < backendNodeData.length; i++) { | ||
|
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.
drive by. this is very useful. surprised we didn't already do this.