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.
sitespeedio · Jun 20, 2023 · f176e03 · f176e03
1 parent bbeeb91
commit f176e03
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 73 deletions.
diff --git 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
@@ -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(