From 1ab0f7cfc5e239b4e54f2a868a88dfa6b659ea34 Mon Sep 17 00:00:00 2001 From: xMokAx Date: Sat, 12 Jan 2019 20:27:29 +0200 Subject: [PATCH 1/4] support gtm.js runtime caching --- .../workbox-google-analytics/initialize.mjs | 18 ++++++++++++++++++ .../utils/constants.mjs | 1 + 2 files changed, 19 insertions(+) diff --git a/packages/workbox-google-analytics/initialize.mjs b/packages/workbox-google-analytics/initialize.mjs index 1b16570f2..df2de2fbf 100644 --- a/packages/workbox-google-analytics/initialize.mjs +++ b/packages/workbox-google-analytics/initialize.mjs @@ -21,6 +21,7 @@ import { GTM_HOST, ANALYTICS_JS_PATH, GTAG_JS_PATH, + GTM_JS_PATH, COLLECT_PATHS_REGEX, } from './utils/constants.mjs'; import './_version.mjs'; @@ -157,6 +158,22 @@ const createGtagJsRoute = (cacheName) => { return new Route(match, handler, 'GET'); }; +/** + * Creates a route with a network first strategy for the gtm.js script. + * + * @param {string} cacheName + * @return {Route} The created route. + * + * @private + */ +const createGtmJsRoute = (cacheName) => { + const match = ({url}) => url.hostname === GTM_HOST && + url.pathname === GTM_JS_PATH; + const handler = new NetworkFirst({cacheName}); + + return new Route(match, handler, 'GET'); +}; + /** * @param {Object=} [options] * @param {Object} [options.cacheName] The cache name to store and retrieve @@ -182,6 +199,7 @@ const initialize = (options = {}) => { }); const routes = [ + createGtmJsRoute(cacheName), createAnalyticsJsRoute(cacheName), createGtagJsRoute(cacheName), ...createCollectRoutes(queuePlugin), diff --git a/packages/workbox-google-analytics/utils/constants.mjs b/packages/workbox-google-analytics/utils/constants.mjs index 68d5627dc..4dd3cc650 100644 --- a/packages/workbox-google-analytics/utils/constants.mjs +++ b/packages/workbox-google-analytics/utils/constants.mjs @@ -14,6 +14,7 @@ export const GOOGLE_ANALYTICS_HOST = 'www.google-analytics.com'; export const GTM_HOST = 'www.googletagmanager.com'; export const ANALYTICS_JS_PATH = '/analytics.js'; export const GTAG_JS_PATH = '/gtag/js'; +export const GTM_JS_PATH = '/gtm.js'; export const COLLECT_DEFAULT_PATH = '/collect'; // This RegExp matches all known Measurement Protocol single-hit collect From 6c23241a04aaff0288cac8006ae4ee13c1d63f07 Mon Sep 17 00:00:00 2001 From: xMokAx Date: Thu, 24 Jan 2019 21:06:43 +0200 Subject: [PATCH 2/4] Adding unit test to ensure gtm.js runtime caching is working as expected --- .../workbox-google-analytics/node/test-index.mjs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/workbox-google-analytics/node/test-index.mjs b/test/workbox-google-analytics/node/test-index.mjs index db1c6a622..2204d8011 100644 --- a/test/workbox-google-analytics/node/test-index.mjs +++ b/test/workbox-google-analytics/node/test-index.mjs @@ -19,6 +19,7 @@ import { GTM_HOST, ANALYTICS_JS_PATH, GTAG_JS_PATH, + GTM_JS_PATH, } from '../../../packages/workbox-google-analytics/utils/constants.mjs'; const PAYLOAD = 'v=1&t=pageview&tid=UA-12345-1&cid=1&dp=%2F'; @@ -73,6 +74,21 @@ describe(`[workbox-google-analytics] initialize`, function() { expect(NetworkFirst.prototype.handle.calledOnce).to.be.true; }); + it(`should register a handler to cache the gtm.js script`, function() { + sandbox.spy(NetworkFirst.prototype, 'handle'); + + googleAnalytics.initialize(); + + self.dispatchEvent(new FetchEvent('fetch', { + request: new Request( + `https://${GTM_HOST}${GTM_JS_PATH}?id=GTM-XXXX`, { + mode: 'no-cors', + }), + })); + + expect(NetworkFirst.prototype.handle.calledOnce).to.be.true; + }); + it(`should accept an optional cache name`, async function() { initialize({cacheName: 'foobar'}); From 9c77fbfc882474204cdb7f0606fa452a4e781dad Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Wed, 30 Jan 2019 10:14:52 -0800 Subject: [PATCH 3/4] Fix lint errors --- test/workbox-google-analytics/node/test-index.mjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/workbox-google-analytics/node/test-index.mjs b/test/workbox-google-analytics/node/test-index.mjs index 2204d8011..1c855623d 100644 --- a/test/workbox-google-analytics/node/test-index.mjs +++ b/test/workbox-google-analytics/node/test-index.mjs @@ -77,13 +77,13 @@ describe(`[workbox-google-analytics] initialize`, function() { it(`should register a handler to cache the gtm.js script`, function() { sandbox.spy(NetworkFirst.prototype, 'handle'); - googleAnalytics.initialize(); + initialize(); self.dispatchEvent(new FetchEvent('fetch', { request: new Request( `https://${GTM_HOST}${GTM_JS_PATH}?id=GTM-XXXX`, { - mode: 'no-cors', - }), + mode: 'no-cors', + }), })); expect(NetworkFirst.prototype.handle.calledOnce).to.be.true; From 5b90f25ad676299ebaa734d547d051e522d8342f Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Wed, 30 Jan 2019 14:03:55 -0800 Subject: [PATCH 4/4] Fix flakey integration tests --- .../integration/basic-example.js | 22 ++++++++++-------- .../static/basic-example/sw.js | 23 +++++++------------ 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/test/workbox-google-analytics/integration/basic-example.js b/test/workbox-google-analytics/integration/basic-example.js index c3970c06c..212789a88 100644 --- a/test/workbox-google-analytics/integration/basic-example.js +++ b/test/workbox-google-analytics/integration/basic-example.js @@ -10,6 +10,8 @@ const expect = require('chai').expect; const qs = require('qs'); const {By} = require('selenium-webdriver'); const activateAndControlSW = require('../../../infra/testing/activate-and-control'); +const waitUntil = require('../../../infra/testing/wait-until'); + describe(`[workbox-google-analytics] Load and use Google Analytics`, function() { const driver = global.__workbox.webdriver; @@ -101,6 +103,7 @@ describe(`[workbox-google-analytics] Load and use Google Analytics`, function() event_callback: () => done(), }); }); + // This request should not match GA routes, so it shouldn't be replayed. await driver.executeAsyncScript((done) => { fetch('https://httpbin.org/get').then(() => done()); }); @@ -112,17 +115,17 @@ describe(`[workbox-google-analytics] Load and use Google Analytics`, function() }); expect(requests).to.have.lengthOf(0); - // Uncheck the "simulate offline" checkbox and then trigger a sync. + // Uncheck the "simulate offline" checkbox, which should let queued + // requests start to replay successfully. await simulateOfflineEl.click(); - await driver.executeAsyncScript(messageSW, { - action: 'dispatch-sync-event', - }); - // Ensure only 2 requests have replayed, since only 2 of them were to GA. - requests = await driver.executeAsyncScript(messageSW, { - action: 'get-spied-requests', - }); - expect(requests).to.have.lengthOf(2); + // Wait until all expected requests have replayed. + await waitUntil(async () => { + requests = await driver.executeAsyncScript(messageSW, { + action: 'get-spied-requests', + }); + return requests.length === 2; + }, 25, 200); // Parse the request bodies to set the params as an object and convert the // qt param to a number. @@ -132,7 +135,6 @@ describe(`[workbox-google-analytics] Load and use Google Analytics`, function() request.originalTime = request.timestamp - request.params.qt; } - expect(requests[0].params.ea).to.equal('beacon'); expect(requests[1].params.ea).to.equal('pixel'); diff --git a/test/workbox-google-analytics/static/basic-example/sw.js b/test/workbox-google-analytics/static/basic-example/sw.js index d19113b97..4276b97f6 100644 --- a/test/workbox-google-analytics/static/basic-example/sw.js +++ b/test/workbox-google-analytics/static/basic-example/sw.js @@ -11,6 +11,10 @@ importScripts('/infra/testing/comlink/sw-interface.js'); workbox.setConfig({modulePathPrefix: '/__WORKBOX/buildFile/'}); +const sleep = async (ms) => { + return new Promise((resolve) => setTimeout(resolve, ms)); +}; + // Spy on .fetch() calls from inside the service worker. // If `simulateOffline` is set to true, throw a network error. let simulateOffline = false; @@ -18,7 +22,10 @@ let spiedRequests = []; const originalFetch = self.fetch; self.fetch = async (...args) => { if (simulateOffline) { - throw Response.error(); + // Simulate network latency, so the sync event doesn't just fire + // in an endless loop. + await sleep(100); + throw new Error('Simulated network error'); } const clone = args[0] instanceof Request ? args[0].clone() : new Request(args[0]); @@ -45,20 +52,6 @@ self.addEventListener('message', (evt) => { case 'get-spied-requests': evt.ports[0].postMessage(spiedRequests); break; - case 'dispatch-sync-event': - { - // Override `.waitUntil` so we can signal when the sync is done. - const originalSyncEventWaitUntil = SyncEvent.prototype.waitUntil; - SyncEvent.prototype.waitUntil = (promise) => { - return promise.then(() => evt.ports[0].postMessage(null)); - }; - - self.dispatchEvent(new SyncEvent('sync', { - tag: 'workbox-background-sync:workbox-google-analytics', - })); - SyncEvent.prototype.waitUntil = originalSyncEventWaitUntil; - } - break; } });