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

core: add hidden audits for each insight #16312

Merged
merged 34 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
22b7291
wip add viewport-insight
connorjclark Jan 22, 2025
8ba0676
license
connorjclark Jan 22, 2025
f32b4e5
resolve trace engine result nodes in TraceElements
connorjclark Jan 29, 2025
9369fb6
update
connorjclark Jan 29, 2025
63efe6a
rendering
connorjclark Jan 29, 2025
46e62aa
dom-size
connorjclark Jan 30, 2025
ae78390
update package, got all the strings now
connorjclark Feb 4, 2025
8f474ba
generate skeletons
connorjclark Feb 4, 2025
c280ae5
lints
connorjclark Feb 4, 2025
59fa89d
update sample json
connorjclark Feb 4, 2025
e3a2608
mv
connorjclark Feb 4, 2025
af806a1
oops
connorjclark Feb 4, 2025
f9afe92
recursiveObjectEnumerate for node ids
connorjclark Feb 4, 2025
98896d1
prevent cycle
connorjclark Feb 4, 2025
91cf402
fix
connorjclark Feb 4, 2025
b4f51d4
snaps
connorjclark Feb 5, 2025
49b7a07
hide
connorjclark Feb 5, 2025
86ab3dc
navid
connorjclark Feb 5, 2025
d7b3f97
dom-size-insghts
connorjclark Feb 5, 2025
12f9669
Merge remote-tracking branch 'origin/main' into insight-audit
connorjclark Feb 5, 2025
a826ce5
fix tests, merge
connorjclark Feb 5, 2025
8b518f4
fix insight set resolver
connorjclark Feb 5, 2025
0e299f1
fix getAuditList
connorjclark Feb 5, 2025
a4d240c
graceful failure
connorjclark Feb 5, 2025
52cbf14
oops
connorjclark Feb 5, 2025
eafd228
maybe fix smoke
connorjclark Feb 5, 2025
d011c22
e2e
connorjclark Feb 5, 2025
058e947
update trace engine
connorjclark Feb 5, 2025
1b7a488
guidance
connorjclark Feb 5, 2025
b7d2e3f
render-blocking-insight
connorjclark Feb 6, 2025
adb1f1e
NOT_APPLICABLE
connorjclark Feb 6, 2025
0bf466e
handle empty tables
connorjclark Feb 6, 2025
70028c4
flaky
connorjclark Feb 6, 2025
7b23538
comment
connorjclark Feb 6, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/build-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function buildStandaloneReport() {
outfile: 'dist/report/standalone.js',
format: 'iife',
bundle: true,
minify: true,
Copy link
Collaborator Author

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.

minify: !process.env.DEBUG,
});
}

Expand Down
76 changes: 76 additions & 0 deletions core/audits/insights/dom-size-insight.js
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) {
Copy link
Member

Choose a reason for hiding this comment

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

Technically we will still have a valid totalElements value even if these are missing. In practice though, this will only happen if the document element is removed so it's an edge case that really doesn't matter.

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;
54 changes: 54 additions & 0 deletions core/audits/insights/insight-audit.js
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,
};
61 changes: 61 additions & 0 deletions core/audits/insights/viewport-insight.js
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;
10 changes: 10 additions & 0 deletions core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const UIStrings = {
performanceCategoryTitle: 'Performance',
/** Title of the speed metrics section of the Performance category. Within this section are various speed metrics which quantify the pageload performance into values presented in seconds and milliseconds. */
metricGroupTitle: 'Metrics',
/** Title of the insights section of the Performance category. Within this section are various insights to give developers tips on how to improve the performance of their page. */
insightGroupTitle: 'Insights',
/** Title of an opportunity sub-section of the Performance category. Within this section are audits with imperative titles that suggest actions the user can take to improve the time of the first initial render of the webpage. */
firstPaintImprovementsGroupTitle: 'First Paint Improvements',
/** Description of an opportunity sub-section of the Performance category. Within this section are audits with imperative titles that suggest actions the user can take to improve the time of the first initial render of the webpage. */
Expand Down Expand Up @@ -308,11 +310,16 @@ const defaultConfig = {
'seo/manual/structured-data',
'work-during-interaction',
'bf-cache',
'insights/dom-size-insight',
'insights/viewport-insight',
],
groups: {
'metrics': {
title: str_(UIStrings.metricGroupTitle),
},
'insights': {
title: str_(UIStrings.insightGroupTitle),
},
'diagnostics': {
title: str_(UIStrings.diagnosticsGroupTitle),
description: str_(UIStrings.diagnosticsGroupDescription),
Expand Down Expand Up @@ -388,6 +395,9 @@ const defaultConfig = {
{id: 'speed-index', weight: 10, group: 'metrics', acronym: 'SI'},
{id: 'interaction-to-next-paint', weight: 0, group: 'metrics', acronym: 'INP'},

{id: 'dom-size-insight', weight: 0, group: 'insights'},
{id: 'viewport-insight', weight: 0, group: 'insights'},

// These are our "invisible" metrics. Not displayed, but still in the LHR.
{id: 'interactive', weight: 0, group: 'hidden', acronym: 'TTI'},
{id: 'max-potential-fid', weight: 0, group: 'hidden'},
Expand Down
30 changes: 30 additions & 0 deletions core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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).
Expand Down Expand Up @@ -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);
Expand All @@ -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],
Expand All @@ -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++) {
Expand Down
1 change: 1 addition & 0 deletions core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ vs
...fs.readdirSync(path.join(moduleDir, './audits/byte-efficiency'))
.map(f => `byte-efficiency/${f}`),
...fs.readdirSync(path.join(moduleDir, './audits/manual')).map(f => `manual/${f}`),
...fs.readdirSync(path.join(moduleDir, './audits/insights')).map(f => `insights/${f}`),
];
return fileList.filter(f => {
return /\.js$/.test(f) && !ignoredFiles.includes(f);
Expand Down
Loading
Loading