fix: conditionally make calls to PerformanceObserver.takeRecords whic…

…h doesnt exist in Safari (#138)
Zizzamia · Aug 27, 2020 · 86edae4 · 86edae4
1 parent f9ec474
commit 86edae4
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 29 deletions.
diff --git a/__tests__/firstInput.spec.ts b/__tests__/firstInput.spec.ts
@@ -46,5 +46,13 @@ describe('firstInput', () => {
       expect(spy).toHaveBeenCalledWith(2, 'cls');
       expect(spy).toHaveBeenCalledWith(3, 'tbt');
     });
+
+    it('should conditionally call takeRecords', () => {
+      //Delete the takeRecords method which isn't available on Safari and make sure all the metrics are still logged
+      delete oi.perfObservers[3].takeRecords;
+      spy = jest.spyOn(log, 'logMetric');
+      initFirstInputDelay([]);
+      expect(spy.mock.calls.length).toEqual(3);
+    });
   });
 });
diff --git a/__tests__/observe.spec.ts b/__tests__/observe.spec.ts
@@ -21,11 +21,11 @@ describe('observe', () => {
     (window as any).PerformanceObserver = mock.PerformanceObserver;
     config.isResourceTiming = false;
     config.isElementTiming = false;
-    jest.spyOn(paint, 'initFirstPaint').mockImplementation(() => { });
-    jest.spyOn(firstInput, 'initFirstInputDelay').mockImplementation(() => { });
+    jest.spyOn(paint, 'initFirstPaint').mockImplementation(() => {});
+    jest.spyOn(firstInput, 'initFirstInputDelay').mockImplementation(() => {});
     jest
       .spyOn(paint, 'initLargestContentfulPaint')
-      .mockImplementation(() => { });
+      .mockImplementation(() => {});
   });
 
   afterEach(() => {
@@ -35,57 +35,63 @@ describe('observe', () => {
     }
   });
 
-  describe(".initPerformanceObserver()", () => {
+  describe('.initPerformanceObserver()', () => {
     describe.each`
       resourceTiming | elementTiming | calls
       ${false}       | ${false}      | ${4}
       ${true}        | ${false}      | ${5}
       ${false}       | ${true}       | ${5}
       ${true}        | ${true}       | ${6}
     `(
-      "when resourceTiming is $resourceTiming and elementTiming is $elementTiming",
+      'when resourceTiming is $resourceTiming and elementTiming is $elementTiming',
       ({ resourceTiming, elementTiming, calls }) => {
         beforeEach(() => {
           config.isResourceTiming = resourceTiming;
           config.isElementTiming = elementTiming;
 
-          spy = jest.spyOn(po, "po");
+          spy = jest.spyOn(po, 'po');
           initPerformanceObserver();
         });
 
         it(`should call po $calls times`, () => {
           expect(spy.mock.calls.length).toEqual(calls);
-          expect(spy).toHaveBeenCalledWith("paint", paint.initFirstPaint);
+          expect(spy).toHaveBeenCalledWith('paint', paint.initFirstPaint);
           expect(spy).toHaveBeenCalledWith(
-            "first-input",
-            firstInput.initFirstInputDelay
+            'first-input',
+            firstInput.initFirstInputDelay,
           );
           expect(spy).toHaveBeenCalledWith(
-            "largest-contentful-paint",
-            paint.initLargestContentfulPaint
+            'largest-contentful-paint',
+            paint.initLargestContentfulPaint,
           );
-          expect(spy).toHaveBeenCalledWith("layout-shift", initLayoutShift);
+          expect(spy).toHaveBeenCalledWith('layout-shift', initLayoutShift);
         });
 
-        it("should only call po for resource timing when is enabled", () => {
+        it('should only call po for resource timing when is enabled', () => {
           if (config.isResourceTiming) {
-            expect(spy).toHaveBeenCalledWith("resource", initResourceTiming);
+            expect(spy).toHaveBeenCalledWith('resource', initResourceTiming);
           } else {
-            expect(spy).not.toHaveBeenCalledWith("resource", initResourceTiming);
+            expect(spy).not.toHaveBeenCalledWith(
+              'resource',
+              initResourceTiming,
+            );
           }
         });
 
-        it("should only call po for element timing when is enabled", () => {
+        it('should only call po for element timing when is enabled', () => {
           if (config.isElementTiming) {
-            expect(spy).toHaveBeenCalledWith("element", paint.initElementTiming);
+            expect(spy).toHaveBeenCalledWith(
+              'element',
+              paint.initElementTiming,
+            );
           } else {
             expect(spy).not.toHaveBeenCalledWith(
-              "element",
-              paint.initElementTiming
+              'element',
+              paint.initElementTiming,
             );
           }
         });
-      }
+      },
     );
   });
 
@@ -98,5 +104,15 @@ describe('observe', () => {
       disconnectPerfObserversHidden();
       expect(spy.mock.calls.length).toEqual(3);
     });
+
+    it('should conditionally call takeRecords', () => {
+      perfObservers[2] = { disconnect: jest.fn() };
+      //Make sure all metrics are still logged even if we don't have the takeRecords method available
+      perfObservers[3] = { disconnect: jest.fn() };
+      perfObservers[4] = { disconnect: jest.fn() };
+      spy = jest.spyOn(log, 'logMetric');
+      disconnectPerfObserversHidden();
+      expect(spy.mock.calls.length).toEqual(3);
+    });
   });
 });
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/firstInput.ts b/src/firstInput.ts
@@ -1,7 +1,7 @@
 import { logData, logMetric } from './log';
 import { cls, lcp, rt, tbt } from './metrics';
-import { poDisconnect } from './performanceObserver';
 import { perfObservers } from './observeInstances';
+import { poDisconnect } from './performanceObserver';
 import { IPerformanceEntry } from './types';
 
 export const initFirstInputDelay = (
@@ -13,7 +13,7 @@ export const initFirstInputDelay = (
   }
   poDisconnect(1);
   logMetric(lcp.value, 'lcp');
-  if (perfObservers[3]) {
+  if (perfObservers[3] && typeof perfObservers[3].takeRecords === 'function') {
     perfObservers[3].takeRecords();
   }
   logMetric(cls.value, 'cls');

diff --git a/src/observe.ts b/src/observe.ts
@@ -4,7 +4,11 @@ import { initFirstInputDelay } from './firstInput';
 import { logMetric } from './log';
 import { cls, lcp, tbt } from './metrics';
 import { perfObservers } from './observeInstances';
-import { initFirstPaint, initLargestContentfulPaint, initElementTiming } from './paint';
+import {
+  initElementTiming,
+  initFirstPaint,
+  initLargestContentfulPaint,
+} from './paint';
 import { po, poDisconnect } from './performanceObserver';
 import { initResourceTiming } from './resourceTiming';
 
@@ -13,10 +17,7 @@ export const initPerformanceObserver = (): void => {
   // FID needs to be initialized as soon as Perfume is available
   // DataConsumption resolves after FID is triggered
   perfObservers[1] = po('first-input', initFirstInputDelay);
-  perfObservers[2] = po(
-    'largest-contentful-paint',
-    initLargestContentfulPaint,
-  );
+  perfObservers[2] = po('largest-contentful-paint', initLargestContentfulPaint);
   // Collects KB information related to resources on the page
   if (config.isResourceTiming) {
     po('resource', initResourceTiming);
@@ -33,7 +34,9 @@ export const disconnectPerfObserversHidden = (): void => {
     poDisconnect(2);
   }
   if (perfObservers[3]) {
-    perfObservers[3].takeRecords();
+    if (typeof perfObservers[3].takeRecords === 'function') {
+      perfObservers[3].takeRecords();
+    }
     logMetric(cls.value, `clsFinal`);
     poDisconnect(3);
   }