Merge pull request #86 from GoogleChrome/remove-is-final

Remove the isFinal flag from the Metric interface

src/getCLS.ts

@@ -43,14 +43,10 @@ export const getCLS = (onReport: ReportHandler, reportAllChanges = false) => {
 
   const po = observe('layout-shift', entryHandler as PerformanceEntryHandler);
   if (po) {
-    report = bindReporter(onReport, metric, po, reportAllChanges);
+    report = bindReporter(onReport, metric, reportAllChanges);
 
-    onHidden(({isUnloading}) => {
+    onHidden(() => {
       po.takeRecords().map(entryHandler as PerformanceEntryHandler);
-
-      if (isUnloading) {
-        metric.isFinal = true;
-      }
       report();
     });
   }

src/getFCP.ts

@@ -15,6 +15,7 @@
  */
 
 import {bindReporter} from './lib/bindReporter.js';
+import {finalMetrics} from './lib/finalMetrics.js';
 import {getFirstHidden} from './lib/getFirstHidden.js';
 import {initMetric} from './lib/initMetric.js';
 import {observe} from './lib/observe.js';
@@ -29,18 +30,22 @@ export const getFCP = (onReport: ReportHandler) => {
 
   const entryHandler = (entry: PerformanceEntry) => {
     if (entry.name === 'first-contentful-paint') {
+      if (po) {
+        po.disconnect();
+      }
+
       // Only report if the page wasn't hidden prior to the first paint.
       if (entry.startTime < firstHidden.timeStamp) {
         metric.value = entry.startTime;
-        metric.isFinal = true;
         metric.entries.push(entry);
+        finalMetrics.add(metric);
         report();
       }
     }
   };
 
   const po = observe('paint', entryHandler);
   if (po) {
-    report = bindReporter(onReport, metric, po);
+    report = bindReporter(onReport, metric);
   }
 };

src/getFID.ts

@@ -15,6 +15,7 @@
  */
 
 import {bindReporter} from './lib/bindReporter.js';
+import {finalMetrics} from './lib/finalMetrics.js';
 import {getFirstHidden} from './lib/getFirstHidden.js';
 import {initMetric} from './lib/initMetric.js';
 import {observe, PerformanceEntryHandler} from './lib/observe.js';
@@ -52,13 +53,13 @@ export const getFID = (onReport: ReportHandler) => {
     if (entry.startTime < firstHidden.timeStamp) {
       metric.value = entry.processingStart - entry.startTime;
       metric.entries.push(entry);
-      metric.isFinal = true;
+      finalMetrics.add(metric);
       report();
     }
   };
 
   const po = observe('first-input', entryHandler as PerformanceEntryHandler);
-  const report = bindReporter(onReport, metric, po);
+  const report = bindReporter(onReport, metric);
 
   if (po) {
     onHidden(() => {
@@ -71,7 +72,6 @@ export const getFID = (onReport: ReportHandler) => {
         // Only report if the page wasn't hidden prior to the first input.
         if (event.timeStamp < firstHidden.timeStamp) {
           metric.value = value;
-          metric.isFinal = true;
           metric.entries = [{
             entryType: 'first-input',
             name: event.type,
@@ -80,6 +80,7 @@ export const getFID = (onReport: ReportHandler) => {
             startTime: event.timeStamp,
             processingStart: event.timeStamp + value,
           } as PerformanceEventTiming];
+          finalMetrics.add(metric);
           report();
         }
       });

src/getLCP.ts

@@ -15,6 +15,7 @@
  */
 
 import {bindReporter} from './lib/bindReporter.js';
+import {finalMetrics} from './lib/finalMetrics.js';
 import {getFirstHidden} from './lib/getFirstHidden.js';
 import {initMetric} from './lib/initMetric.js';
 import {observe, PerformanceEntryHandler} from './lib/observe.js';
@@ -38,8 +39,6 @@ export const getLCP = (onReport: ReportHandler, reportAllChanges = false) => {
     if (value < firstHidden.timeStamp) {
       metric.value = value;
       metric.entries.push(entry);
-    } else {
-      metric.isFinal = true;
     }
 
     report();
@@ -48,12 +47,13 @@ export const getLCP = (onReport: ReportHandler, reportAllChanges = false) => {
   const po = observe('largest-contentful-paint', entryHandler);
 
   if (po) {
-    report = bindReporter(onReport, metric, po, reportAllChanges);
+    report = bindReporter(onReport, metric, reportAllChanges);
 
     const stopListening = () => {
-      if (!metric.isFinal) {
+      if (!finalMetrics.has(metric)) {
         po.takeRecords().map(entryHandler as PerformanceEntryHandler);
-        metric.isFinal = true;
+        po.disconnect();
+        finalMetrics.add(metric);
         report();
       }
     }

src/getTTFB.ts

@@ -107,7 +107,6 @@ export const getTTFB = (onReport: ReportHandler) => {
           (navigationEntry as PerformanceNavigationTiming).responseStart;
 
       metric.entries = [navigationEntry];
-      metric.isFinal = true;
 
       onReport(metric);
     } catch (error) {

src/lib/bindReporter.ts

@@ -14,33 +14,30 @@
  * limitations under the License.
  */
 
+import {finalMetrics} from './finalMetrics.js';
 import {Metric, ReportHandler} from '../types.js';
 
 
 export const bindReporter = (
   callback: ReportHandler,
   metric: Metric,
-  po: PerformanceObserver | undefined,
   observeAllUpdates?: boolean,
 ) => {
   let prevValue: number;
   return () => {
-    if (po && metric.isFinal) {
-      po.disconnect();
-    }
     if (metric.value >= 0) {
       if (observeAllUpdates ||
-          metric.isFinal ||
+          finalMetrics.has(metric) ||
           document.visibilityState === 'hidden') {
         metric.delta = metric.value - (prevValue || 0);
 
         // Report the metric if there's a non-zero delta, if the metric is
         // final, or if no previous value exists (which can happen in the case
         // of the document becoming hidden when the metric value is 0).
         // See: https://github.com/GoogleChrome/web-vitals/issues/14
-        if (metric.delta || metric.isFinal || prevValue === undefined) {
-          callback(metric);
+        if (metric.delta || prevValue === undefined) {
           prevValue = metric.value;
+          callback(metric);
         }
       }
     }

src/lib/finalMetrics.ts

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {Metric} from '../types.js';
+
+
+export const finalMetrics: WeakSet<Metric> = new WeakSet();

src/lib/initMetric.ts

@@ -24,7 +24,6 @@ export const initMetric = (name: Metric['name'], value = -1): Metric => {
     value,
     delta: 0,
     entries: [],
-    id: generateUniqueID(),
-    isFinal: false
+    id: generateUniqueID()
   };
 };

src/lib/onHidden.ts

@@ -16,35 +16,27 @@
 
 
 export interface OnHiddenCallback {
-  // TODO(philipwalton): add `isPersisted` if needed for bfcache.
-  ({timeStamp, isUnloading}: {timeStamp: number; isUnloading: boolean}): void;
+  (event: Event): void;
 }
 
-let isUnloading = false;
-let listenersAdded = false;
-
-const onPageHide = (event: PageTransitionEvent) => {
-  isUnloading = !event.persisted;
-};
-
-const addListeners = () => {
-  addEventListener('pagehide', onPageHide);
-
-  // `beforeunload` is needed to fix this bug:
-  // https://bugs.chromium.org/p/chromium/issues/detail?id=987409
-  // eslint-disable-next-line @typescript-eslint/no-empty-function
-  addEventListener('beforeunload', () => {});
-}
+let beforeUnloadFixAdded = false;
 
 export const onHidden = (cb: OnHiddenCallback, once = false) => {
-  if (!listenersAdded) {
-    addListeners();
-    listenersAdded = true;
+  // Adding a `beforeunload` listener is needed to fix this bug:
+  // https://bugs.chromium.org/p/chromium/issues/detail?id=987409
+  if (!beforeUnloadFixAdded) {
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    addEventListener('beforeunload', () => {});
+    beforeUnloadFixAdded = true;
   }
 
-  addEventListener('visibilitychange', ({timeStamp}) => {
+  const onVisibilityChange = (event: Event) => {
     if (document.visibilityState === 'hidden') {
-      cb({timeStamp, isUnloading});
+      cb(event);
+      if (once) {
+        removeEventListener('visibilitychange', onVisibilityChange, true);
+      }
     }
-  }, {capture: true, once});
+  }
+  addEventListener('visibilitychange', onVisibilityChange, true);
 };

src/types.ts

@@ -31,10 +31,6 @@ export interface Metric {
   // together and calculate a total.
   id: string;
 
-  // `false` if the value of the metric may change in the future,
-  // for the current page.
-  isFinal: boolean;
-
   // Any performance entries used in the metric value calculation.
   // Note, entries will be added to the array as the value changes.
   entries: PerformanceEntry[];