From f176e03bae0a221f2f633cbd7258ff5ce1a147dd Mon Sep 17 00:00:00 2001 From: Peter Hedenskog Date: Tue, 20 Jun 2023 16:08:26 +0200 Subject: [PATCH] Fix browserScript structure. (#3888) Oh, we had a really strange behaviour where we added all the Browsertime data inside the browserScript element (the data we collect with JS) and that made it so we pushed the HAR file inside that element. This fixes that and make sure we just cherry pick the exact data we pass on. --- .../browsertime/browsertimeAggregator.js | 141 +++++++++--------- lib/plugins/browsertime/index.js | 10 +- 2 files changed, 78 insertions(+), 73 deletions(-) diff --git a/lib/plugins/browsertime/browsertimeAggregator.js b/lib/plugins/browsertime/browsertimeAggregator.js index dbdf6eedd3..85ac91137f 100644 --- a/lib/plugins/browsertime/browsertimeAggregator.js +++ b/lib/plugins/browsertime/browsertimeAggregator.js @@ -43,98 +43,103 @@ export class BrowsertimeAggregator { } } - if (browsertimeRunData.timings.largestContentfulPaint) { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['timings', 'largestContentfulPaint'], - browsertimeRunData.timings.largestContentfulPaint.renderTime - ); - } - - if (browsertimeRunData.timings.interactionToNextPaint) { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['timings', 'interactionToNextPaint'], - browsertimeRunData.timings.interactionToNextPaint - ); - } - - if (browsertimeRunData.pageinfo.cumulativeLayoutShift) { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['pageinfo', 'cumulativeLayoutShift'], - browsertimeRunData.pageinfo.cumulativeLayoutShift - ); - } + if (browsertimeRunData.timings) { + if (browsertimeRunData.timings.largestContentfulPaint) { + pushGroupStats( + this.statsPerType, + this.groups[group], + ['timings', 'largestContentfulPaint'], + browsertimeRunData.timings.largestContentfulPaint.renderTime + ); + } - forEach(timings, timing => { - if (browsertimeRunData.timings[timing]) { + if (browsertimeRunData.timings.interactionToNextPaint) { pushGroupStats( this.statsPerType, this.groups[group], - timing, - browsertimeRunData.timings[timing] + ['timings', 'interactionToNextPaint'], + browsertimeRunData.timings.interactionToNextPaint ); } - }); - forEach(browsertimeRunData.timings.navigationTiming, (value, name) => { - if (value) { + forEach(timings, timing => { + if (browsertimeRunData.timings[timing]) { + pushGroupStats( + this.statsPerType, + this.groups[group], + timing, + browsertimeRunData.timings[timing] + ); + } + }); + + forEach(browsertimeRunData.timings.navigationTiming, (value, name) => { + if (value) { + pushGroupStats( + this.statsPerType, + this.groups[group], + ['navigationTiming', name], + value + ); + } + }); + + forEach(browsertimeRunData.timings.pageTimings, (value, name) => { pushGroupStats( this.statsPerType, this.groups[group], - ['navigationTiming', name], + ['pageTimings', name], value ); - } - }); + }); - // pick up one level of custom metrics - forEach(browsertimeRunData.custom, (value, name) => { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['custom', name], - value - ); - }); + forEach(browsertimeRunData.timings.paintTiming, (value, name) => { + pushGroupStats( + this.statsPerType, + this.groups[group], + ['paintTiming', name], + value + ); + }); - forEach(browsertimeRunData.timings.pageTimings, (value, name) => { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['pageTimings', name], - value - ); - }); + forEach(browsertimeRunData.timings.userTimings.marks, timing => { + pushGroupStats( + this.statsPerType, + this.groups[group], + ['userTimings', 'marks', timing.name], + timing.startTime + ); + }); - forEach(browsertimeRunData.timings.paintTiming, (value, name) => { - pushGroupStats( - this.statsPerType, - this.groups[group], - ['paintTiming', name], - value - ); - }); + forEach(browsertimeRunData.timings.userTimings.measures, timing => { + pushGroupStats( + this.statsPerType, + this.groups[group], + ['userTimings', 'measures', timing.name], + timing.duration + ); + }); + } - forEach(browsertimeRunData.timings.userTimings.marks, timing => { + if ( + browsertimeRunData.pageinfo && + browsertimeRunData.pageinfo.cumulativeLayoutShift + ) { pushGroupStats( this.statsPerType, this.groups[group], - ['userTimings', 'marks', timing.name], - timing.startTime + ['pageinfo', 'cumulativeLayoutShift'], + browsertimeRunData.pageinfo.cumulativeLayoutShift ); - }); + } - forEach(browsertimeRunData.timings.userTimings.measures, timing => { + // pick up one level of custom metrics + forEach(browsertimeRunData.custom, (value, name) => { pushGroupStats( this.statsPerType, this.groups[group], - ['userTimings', 'measures', timing.name], - timing.duration + ['custom', name], + value ); }); diff --git a/lib/plugins/browsertime/index.js b/lib/plugins/browsertime/index.js index 0a73ae4ff8..6c0741e2be 100644 --- a/lib/plugins/browsertime/index.js +++ b/lib/plugins/browsertime/index.js @@ -204,10 +204,10 @@ export default class BrowsertimePlugin extends SitespeedioPlugin { group = parse(url).hostname; } let runIndex = 0; - for (let run of result[resultIndex].browserScripts) { - // Kind of ugly way to add visualMetrics to a run - // it's outside of browserScripts today - // we could instead pass browsertime.visualMetrics maybe + for (let browserScriptsData of result[resultIndex].browserScripts) { + let run = {}; + Object.assign(run, browserScriptsData); + if (result[resultIndex].visualMetrics) { run.visualMetrics = result[resultIndex].visualMetrics[runIndex]; } @@ -461,7 +461,7 @@ export default class BrowsertimePlugin extends SitespeedioPlugin { // If the coach is turned on, collect the coach result if (options.coach) { - const coachAdvice = run.coach.coachAdvice; + const coachAdvice = browserScriptsData.coach.coachAdvice; // check if the coach has error(s) if (!isEmpty(coachAdvice.errors)) { log.error(