Skip to content

Commit

Permalink
deps: upgrade trace_engine and drop manually written types (#15810)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored Feb 15, 2024
1 parent 4a24adc commit 883fdb1
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 1,552 deletions.
7 changes: 5 additions & 2 deletions core/audits/layout-shifts.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,16 @@ class LayoutShifts extends Audit {

/** @type {Item[]} */
const items = [];
const layoutShiftEvents = clusters.flatMap(c => c.events);
const layoutShiftEvents =
/** @type {import('../lib/trace-engine.js').SaneSyntheticLayoutShift[]} */(
clusters.flatMap(c => c.events)
);
const topLayoutShiftEvents = layoutShiftEvents
.sort((a, b) => b.args.data.weighted_score_delta - a.args.data.weighted_score_delta)
.slice(0, MAX_LAYOUT_SHIFTS);
for (const event of topLayoutShiftEvents) {
const biggestImpactNodeId = TraceElements.getBiggestImpactNodeForShiftEvent(
event.args.data.impacted_nodes, impactByNodeId);
event.args.data.impacted_nodes || [], impactByNodeId);
const biggestImpactElement = traceElements.find(t => t.nodeId === biggestImpactNodeId);

// Turn root causes into sub-items.
Expand Down
8 changes: 7 additions & 1 deletion core/computed/metrics/cumulative-layout-shift.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,13 @@ class CumulativeLayoutShift {
LayoutShifts: TraceEngine.TraceHandlers.LayoutShifts,
Screenshots: TraceEngine.TraceHandlers.Screenshots,
});
await processor.parse(events);
// eslint-disable-next-line max-len
await processor.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ (
events
));
if (!processor.data) {
throw new Error('null trace engine result');
}
return processor.data.LayoutShifts.sessionMaxScore;
};
const cumulativeLayoutShift = await run(allFrameShiftEvents.map(e => e.event));
Expand Down
15 changes: 12 additions & 3 deletions core/computed/trace-engine-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ class TraceEngineResult {
Samples: TraceEngine.TraceHandlers.Samples,
Screenshots: TraceEngine.TraceHandlers.Screenshots,
});
await engine.parse(traceEvents);
return engine.data;
// eslint-disable-next-line max-len
await engine.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ (
traceEvents
));
// TODO: use TraceEngine.TraceProcessor.createWithAllHandlers above.
return /** @type {import('@paulirish/trace_engine').Handlers.Types.TraceParseData} */(
engine.data);
}

/**
Expand Down Expand Up @@ -61,7 +66,11 @@ class TraceEngineResult {
}
}

return TraceEngineResult.runTraceEngine(traceEvents);
const result = await TraceEngineResult.runTraceEngine(traceEvents);
if (!result) {
throw new Error('null trace engine result');
}
return result;
}
}

Expand Down
31 changes: 22 additions & 9 deletions core/gather/gatherers/root-causes.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,33 @@ class RootCauses extends BaseGatherer {
// nodeIds if the DOM domain was enabled before this gatherer, invoke it to be safe.
await driver.defaultSession.sendCommand('DOM.getDocument', {depth: -1, pierce: true});

/** @type {import('@paulirish/trace_engine').RootCauses.RootCauses.RootCauseProtocolInterface} */
const protocolInterface = {
/** @param {string} url */
// eslint-disable-next-line no-unused-vars
getInitiatorForRequest(url) {
return null;
},
/** @param {number[]} backendNodeIds */
/** @param {import('@paulirish/trace_engine/generated/protocol.js').DOM.BackendNodeId[]} backendNodeIds */
async pushNodesByBackendIdsToFrontend(backendNodeIds) {
const response = await driver.defaultSession.sendCommand(
'DOM.pushNodesByBackendIdsToFrontend', {backendNodeIds});
return response.nodeIds;
const nodeIds =
/** @type {import('@paulirish/trace_engine/generated/protocol.js').DOM.NodeId[]} */(
response.nodeIds);
return nodeIds;
},
/** @param {number} nodeId */
/** @param {import('@paulirish/trace_engine/generated/protocol.js').DOM.NodeId} nodeId */
async getNode(nodeId) {
try {
const response = await driver.defaultSession.sendCommand('DOM.describeNode', {nodeId});
// This always zero, so let's fix it here.
// https://bugs.chromium.org/p/chromium/issues/detail?id=1515175
response.node.nodeId = nodeId;
return response.node;
const node =
/** @type {import('@paulirish/trace_engine/generated/protocol.js').DOM.Node} */(
response.node);
return node;
} catch (err) {
if (err.message.includes('Could not find node with given id')) {
// TODO: when injecting an iframe, the engine gets the node of that frame's document element.
Expand All @@ -64,7 +71,8 @@ class RootCauses extends BaseGatherer {
// When this is fixed, remove this try/catch.
// Note: this could be buggy by giving the wrong node detail if a node id meant for a non-main frame
// happens to match one from the main frame ... which is pretty likely...
return null;
// TODO: fix trace engine type to allow returning null.
return /** @type {any} */(null);
}
throw err;
}
Expand All @@ -79,17 +87,20 @@ class RootCauses extends BaseGatherer {
return [];
}
},
/** @param {number} nodeId */
/** @param {import('@paulirish/trace_engine/generated/protocol.js').DOM.NodeId} nodeId */
async getMatchedStylesForNode(nodeId) {
try {
const response = await driver.defaultSession.sendCommand(
'CSS.getMatchedStylesForNode', {nodeId});
return response;
} catch {
return [];
return {...response, getError() {}};
} catch (err) {
return /** @type {any} */({getError() {
return err.toString();
}});
}
},
/** @param {string} url */
// @ts-expect-error not using, dont care about type error.
// eslint-disable-next-line no-unused-vars
async fontFaceForSource(url) {
return null;
Expand All @@ -104,6 +115,8 @@ class RootCauses extends BaseGatherer {
const layoutShiftEvents = traceEngineResult.LayoutShifts.clusters.flatMap(c => c.events);
for (const event of layoutShiftEvents) {
const r = await rootCausesEngine.layoutShifts.rootCausesForEvent(traceEngineResult, event);
if (!r) continue;

for (const cause of r.fontChanges) {
// TODO: why isn't trace engine unwrapping this promise ...
cause.fontFace = await cause.fontFace;
Expand Down
8 changes: 6 additions & 2 deletions core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,19 @@ class TraceElements extends BaseGatherer {
static async getTopLayoutShifts(trace, traceEngineResult, rootCauses, context) {
const {impactByNodeId} = await CumulativeLayoutShift.request(trace, context);
const clusters = traceEngineResult.LayoutShifts.clusters ?? [];
const layoutShiftEvents = clusters.flatMap(c => c.events);
const layoutShiftEvents =
/** @type {import('../../lib/trace-engine.js').SaneSyntheticLayoutShift[]} */(
clusters.flatMap(c => c.events)
);

return layoutShiftEvents
.sort((a, b) => b.args.data.weighted_score_delta - a.args.data.weighted_score_delta)
.slice(0, MAX_LAYOUT_SHIFTS)
.flatMap(event => {
const nodeIds = [];
const impactedNodes = event.args.data.impacted_nodes || [];
const biggestImpactedNodeId =
this.getBiggestImpactNodeForShiftEvent(event.args.data.impacted_nodes, impactByNodeId);
this.getBiggestImpactNodeForShiftEvent(impactedNodes, impactByNodeId);
if (biggestImpactedNodeId !== undefined) {
nodeIds.push(biggestImpactedNodeId);
}
Expand Down
7 changes: 3 additions & 4 deletions core/lib/trace-engine.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
// @ts-expect-error missing types
import * as TraceEngine from '@paulirish/trace_engine';

import {polyfillDOMRect} from './polyfill-dom-rect.js';

/** @typedef {import('@paulirish/trace_engine').Types.TraceEvents.SyntheticLayoutShift} SyntheticLayoutShift */
/** @typedef {SyntheticLayoutShift & {args: {data: NonNullable<SyntheticLayoutShift['args']['data']>}}} SaneSyntheticLayoutShift */

polyfillDOMRect();

/** @type {import('../../types/trace-engine.js').TraceProcessor & typeof import('../../types/trace-engine.js').TraceProcessor} */
const TraceProcessor = TraceEngine.Processor.TraceProcessor;
/** @type {import('../../types/trace-engine.js').TraceHandlers} */
const TraceHandlers = TraceEngine.Handlers.ModelHandlers;
/** @type {import('../../types/trace-engine.js').RootCauses & typeof import('../../types/trace-engine.js').RootCauses} */
const RootCauses = TraceEngine.RootCauses.RootCauses.RootCauses;

export {
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,13 @@
"webtreemap-cdt": "^3.2.1"
},
"dependencies": {
"@paulirish/trace_engine": "^0.0.7",
"@paulirish/trace_engine": "^0.0.12",
"@sentry/node": "^6.17.4",
"axe-core": "^4.8.4",
"chrome-launcher": "^1.1.0",
"configstore": "^5.0.1",
"csp_evaluator": "1.1.1",
"devtools-protocol": "0.0.1211954",
"devtools-protocol": "0.0.1232444",
"enquirer": "^2.3.6",
"http-link-header": "^1.1.1",
"intl-messageformat": "^10.5.3",
Expand All @@ -211,8 +211,8 @@
"yargs-parser": "^21.0.0"
},
"resolutions": {
"puppeteer/**/devtools-protocol": "0.0.1211954",
"puppeteer-core/**/devtools-protocol": "0.0.1211954"
"puppeteer/**/devtools-protocol": "0.0.1232444",
"puppeteer-core/**/devtools-protocol": "0.0.1232444"
},
"repository": "GoogleChrome/lighthouse",
"keywords": [
Expand Down
7 changes: 4 additions & 3 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/

import {Protocol as Crdp} from 'devtools-protocol/types/protocol.js';
import * as TraceEngine from '@paulirish/trace_engine';
import {LayoutShiftRootCausesData} from '@paulirish/trace_engine/models/trace/root-causes/LayoutShift.js';

import {parseManifest} from '../core/lib/manifest-parser.js';
import {Simulator} from '../core/lib/dependency-graph/simulator/simulator.js';
Expand All @@ -23,7 +25,6 @@ import LHResult from './lhr/lhr.js'
import Protocol from './protocol.js';
import Util from './utility-types.js';
import Audit from './audit.js';
import {TraceProcessor, LayoutShiftRootCauses} from './trace-engine.js';

export type Artifacts = BaseArtifacts & GathererArtifacts;

Expand Down Expand Up @@ -569,10 +570,10 @@ declare module Artifacts {
type?: string;
}

type TraceEngineResult = TraceProcessor['data'];
type TraceEngineResult = TraceEngine.Handlers.Types.TraceParseData;

interface TraceEngineRootCauses {
layoutShifts: Record<number, LayoutShiftRootCauses>;
layoutShifts: Record<number, LayoutShiftRootCausesData>;
}

interface ViewportDimensions {
Expand Down
Loading

0 comments on commit 883fdb1

Please sign in to comment.