Only prefetch in production and in dev with process.env.SERVICE_WORKE… (#83)

* Only prefetch in production and in dev with process.env.SERVICE_WORKER set to true.

* bump version

* Always send version when fetching

* Support Request as the first parameter to patched fetch.

Author: markbrocato

package.json

@@ -1,6 +1,6 @@
 {
   "name": "react-storefront",
-  "version": "8.2.0",
+  "version": "8.3.0",
   "description": "Build and deploy e-commerce progressive web apps (PWAs) in record time.",
   "module": "./index.js",
   "license": "Apache-2.0",

src/api/addVersion.js

@@ -0,0 +1,24 @@
+/**
+ * The query param that is added to fetch alls to /api routes to ensure that
+ * cached results from previous versions of the app are not served to new versions
+ * of the app.
+ */
+export const VERSION_PARAM = '__v__'
+
+/**
+ * Adds the next build id as the __v__ query parameter to the given url
+ * @param {String} url Any URL
+ * @return {URL}
+ */
+export default function addVersion(url) {
+  /* istanbul ignore next */
+  if (typeof window === 'undefined') return new URL(url, 'http://throwaway.api')
+
+  const parsed = new URL(url, window.location.href)
+
+  if (!parsed.searchParams.has(VERSION_PARAM) && typeof __NEXT_DATA__ !== 'undefined') {
+    parsed.searchParams.append(VERSION_PARAM, __NEXT_DATA__.buildId)
+  }
+
+  return parsed
+}

src/api/getAPIURL.js

@@ -1,14 +1,11 @@
+import addVersion from './addVersion'
+
 /**
  * Returns the API URL for the given page
  * @param {String} pageURI The page URI
  * @return {String}
  */
 export default function getAPIURL(pageURI) {
-  const parsed = new URL(pageURI, 'http://throwaway.com')
-
-  if (typeof __NEXT_DATA__ !== 'undefined') {
-    parsed.searchParams.append('__v__', __NEXT_DATA__.buildId)
-  }
-
+  const parsed = addVersion(pageURI)
   return '/api' + parsed.pathname.replace(/\/$/, '') + parsed.search
 }

src/fetch.js

@@ -1,6 +1,35 @@
-let fetch = require('isomorphic-unfetch')
+import addVersion from './api/addVersion'
+
+// Here we patch fetch and XMLHttpRequest to always add version parameter to api calls so that cached results
+// from previous versions of the app aren't served to new versions.
+/* istanbul ignore else */
+if (typeof window !== 'undefined') {
+  const originalFetch = window.fetch
+
+  window.fetch = function rsfVersionedFetch(url, init) {
+    if (url.url) {
+      // the first param can be a request object
+      url = new Request(addVersion(url.url).toString(), url)
+    } else {
+      url = addVersion(url).toString()
+    }
+
+    return originalFetch(url, init)
+  }
+}
+
+/* istanbul ignore else */
+if (typeof XMLHttpRequest !== 'undefined') {
+  const originalOpen = XMLHttpRequest.prototype.open
+
+  XMLHttpRequest.prototype.open = function rsfVersionedOpen(method, url, ...others) {
+    return originalOpen.call(this, method, addVersion(url).toString(), ...others)
+  }
+}
 
 /**
  * An isomorphic implementation of the fetch API. You should always use this to fetch data on both the client and server.
+ * When making requests to /api routes, ?__v__={next_build_id} will always be added to ensure that cached results
+ * from previous versions of the app aren't served to new versions.
  */
-export default fetch
+export default require('isomorphic-unfetch')

src/plugins/withReactStorefront.js

@@ -30,6 +30,7 @@ module.exports = ({ prefetchQueryParam, ...nextConfig } = {}) => {
       config.plugins.push(
         new webpack.DefinePlugin({
           'process.env.RSF_PREFETCH_QUERY_PARAM': JSON.stringify(prefetchQueryParam),
+          'process.env.SERVICE_WORKER': JSON.stringify(process.env.SERVICE_WORKER),
         }),
       )
 

src/serviceWorker.js

@@ -1,4 +1,6 @@
 import getAPIURL from './api/getAPIURL'
+import addVersion from './api/addVersion'
+
 const prefetched = new Set()
 
 /**
@@ -28,6 +30,16 @@ export function waitForServiceWorker() {
  * @param {String} url The URL to prefetch
  */
 export async function prefetch(url) {
+  if (process.env.NODE_ENV !== 'production' && process.env.SERVICE_WORKER !== 'true') {
+    // note that even though we wait for the service worker to be available, during local
+    // development it is still possible for a service worker to be around from a previous
+    // build of the app, so we disable prefetching in development unless process.env.SERVICE_WORKER = true
+    // so that prefetching does not slow bog down the local node server and slow down development
+    return
+  }
+
+  url = addVersion(url).toString()
+
   if (prefetched.has(url)) {
     return
   }

test/api/addVersion.test.js

@@ -0,0 +1,25 @@
+import addVersion from '../../src/api/addVersion'
+
+describe('addVersion', () => {
+  beforeEach(() => {
+    window.__NEXT_DATA__ = {
+      buildId: 'development',
+    }
+  })
+
+  afterEach(() => {
+    delete window.__NEXT_DATA__
+  })
+
+  it('should add the version query param', () => {
+    expect(addVersion('/foo').toString()).toBe('http://localhost/foo?__v__=development')
+  })
+
+  it('should not duplicate the version query param', () => {
+    expect(addVersion('/foo?__v__=1').toString()).toBe('http://localhost/foo?__v__=1')
+  })
+
+  it('should leave the original hostname intact', () => {
+    expect(addVersion('http://foo.com/foo').toString()).toBe('http://foo.com/foo?__v__=development')
+  })
+})

test/fetch.test.js

@@ -1,14 +1,41 @@
 describe('fetch', () => {
-  let f, fetch
+  let f, fetch, originalOpen, originalFetch
 
   beforeEach(() => {
-    f = jest.fn()
-    jest.doMock('isomorphic-unfetch', () => f)
-    fetch = require('../src/fetch').default
+    jest.isolateModules(() => {
+      f = jest.fn()
+      jest.doMock('isomorphic-unfetch', () => f)
+      originalOpen = jest.spyOn(XMLHttpRequest.prototype, 'open')
+      originalFetch = jest.spyOn(window, 'fetch').mockImplementation()
+      fetch = require('../src/fetch').default
+      window.__NEXT_DATA__ = {
+        buildId: 'development',
+      }
+    })
+  })
+
+  afterEach(() => {
+    delete window.__NEXT_DATA__
   })
 
   it('should export isomorphic-unfetch for backwards compatibility', () => {
     fetch('/foo')
     expect(f).toHaveBeenCalledWith('/foo')
   })
+
+  it('should patch window.fetch', () => {
+    window.fetch('/foo')
+    expect(originalFetch).toHaveBeenCalledWith('http://localhost/foo?__v__=development', undefined)
+  })
+
+  it('should accept a request object as the first parameter', () => {
+    window.fetch(new Request('/foo'))
+    expect(originalFetch.mock.calls[0][0].url).toBe('http://localhost/foo?__v__=development')
+  })
+
+  it('should add the version to XMLHttpRequest.open', () => {
+    const req = new XMLHttpRequest()
+    req.open('GET', '/foo')
+    expect(originalOpen).toHaveBeenCalledWith('GET', 'http://localhost/foo?__v__=development')
+  })
 })

test/plp/SearchResultsProvider.test.js

@@ -16,10 +16,17 @@ describe('SearchResultsProvider', () => {
   }
   let wrapper, context, getStore
 
+  beforeEach(() => {
+    window.__NEXT_DATA__ = {
+      buildId: 'development',
+    }
+  })
+
   afterEach(() => {
     wrapper.unmount()
     context = undefined
     getStore = undefined
+    delete window.__NEXT_DATA__
   })
 
   const Test = () => {
@@ -147,9 +154,12 @@ describe('SearchResultsProvider', () => {
     })
 
     it('refresh - should always remove "more" query param if sees it', async () => {
-      const windowSpy = jest
-        .spyOn(global.window, 'location', 'get')
-        .mockReturnValue({ search: '?more=1', hash: '', pathname: '/test' })
+      const windowSpy = jest.spyOn(global.window, 'location', 'get').mockReturnValue({
+        search: '?more=1',
+        hash: '',
+        pathname: '/test',
+        href: 'http://localhost',
+      })
       fetchMock.mockResponseOnce(
         JSON.stringify({
           pageData: {
@@ -164,7 +174,7 @@ describe('SearchResultsProvider', () => {
         await wrapper.update()
       })
 
-      expect(fetch).toHaveBeenCalledWith('/api/test')
+      expect(fetch).toHaveBeenCalledWith('/api/test?__v__=development')
 
       windowSpy.mockRestore()
     })