don't depend on window.ga till it's actually needed

Fixes mdn#5986
peterbe · Oct 29, 2019 · 9f9c656 · 9f9c656
1 parent ea715ff
commit 9f9c656
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 47 deletions.
diff --git a/kuma/javascript/src/ga-provider.jsx b/kuma/javascript/src/ga-provider.jsx
@@ -4,9 +4,13 @@ import { useContext, useEffect, useState } from 'react';
 
 export type GAFunction = (...any) => void;
 
-const noop: GAFunction = () => {};
+function ga(...args) {
+    if (typeof window === 'object' && typeof window.ga === 'function') {
+        window.ga(...args);
+    }
+}
 
-const context = React.createContext<GAFunction>(noop);
+const context = React.createContext<GAFunction>(ga);
 
 /**
  * If we're running in the browser (not being server-side rendered)
@@ -24,16 +28,6 @@ const context = React.createContext<GAFunction>(noop);
 export default function GAProvider(props: {
     children: React.Node
 }): React.Node {
-    let ga: GAFunction;
-
-    // If there is a window object that defines a ga() function, then
-    // that ga function is the value we will provide. Otherwise we just
-    // provide a dummy function that does nothing.
-    if (typeof window === 'object' && typeof window.ga === 'function') {
-        ga = window.ga;
-    } else {
-        ga = noop;
-    }
     return <context.Provider value={ga}>{props.children}</context.Provider>;
 }
 

diff --git a/kuma/javascript/src/ga-provider.test.js b/kuma/javascript/src/ga-provider.test.js
@@ -12,7 +12,7 @@ describe('GAProvider', () => {
     test('Provides the window.ga() function', () => {
         const Consumer = GAProvider.context.Consumer;
         const contextConsumer = jest.fn();
-        const dummyGAFunc = () => {};
+        const dummyGAFunc = jest.fn();
         window.ga = dummyGAFunc;
 
         create(
@@ -21,11 +21,12 @@ describe('GAProvider', () => {
             </GAProvider>
         );
 
-        // We expect GAProvider to set window.ga as its value and we
-        // expect the Consumer to get that value from the provider and
-        // pass it to the contextConsumer function.
-        expect(contextConsumer.mock.calls.length).toBe(1);
-        expect(contextConsumer.mock.calls[0][0]).toEqual(dummyGAFunc);
+        expect(dummyGAFunc).not.toHaveBeenCalled();
+        // // We expect GAProvider to set window.ga as its value and we
+        // // expect the Consumer to get that value from the provider and
+        // // pass it to the contextConsumer function.
+        // expect(contextConsumer.mock.calls.length).toBe(1);
+        // expect(contextConsumer.mock.calls[0][0]).toEqual(dummyGAFunc);
     });
 
     test('Provides a dummy if no window.ga function', () => {
@@ -52,21 +53,7 @@ describe('GAProvider.useClientId', () => {
         delete window.ga;
     });
 
-    test('hook returns "" if there is no ga function', () => {
-        function Test() {
-            const clientId = GAProvider.useClientId();
-            expect(clientId).toBe('');
-            return null;
-        }
-
-        create(
-            <GAProvider>
-                <Test />
-            </GAProvider>
-        );
-    });
-
-    test('hook works if there is a ga function', done => {
+    test('hook works if there is a ga function', () => {
         const mockTrackerObject = {
             get(p) {
                 return p === 'clientId' ? 'mockClientId' : '';
@@ -80,27 +67,17 @@ describe('GAProvider.useClientId', () => {
         }
         window.ga = mockGA;
 
-        let numCalls = 0;
-
         function Test() {
-            const clientId = GAProvider.useClientId();
-            switch (numCalls) {
-                case 0:
-                    expect(clientId).toBe('');
-                    numCalls++;
-                    break;
-                case 1:
-                    expect(clientId).toBe('mockClientId');
-                    done();
-            }
-            return null;
+            return GAProvider.useClientId();
         }
 
-        create(
+        const renderer = create(
             <GAProvider>
                 <Test />
             </GAProvider>
         );
+        expect(renderer.toJSON()).toBe('');
         act(() => {});
+        expect(renderer.toJSON()).toBe('mockClientId');
     });
 });