diff --git a/package.json b/package.json index 5e4b9b37..a14abf95 100644 --- a/package.json +++ b/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", diff --git a/src/api/addVersion.js b/src/api/addVersion.js new file mode 100644 index 00000000..2a3e14d9 --- /dev/null +++ b/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 +} diff --git a/src/api/getAPIURL.js b/src/api/getAPIURL.js index b6f52729..e6a92b54 100644 --- a/src/api/getAPIURL.js +++ b/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 } diff --git a/src/fetch.js b/src/fetch.js index 7122e9b0..8c14afe3 100644 --- a/src/fetch.js +++ b/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') diff --git a/src/plugins/withReactStorefront.js b/src/plugins/withReactStorefront.js index c6177f87..714078e8 100644 --- a/src/plugins/withReactStorefront.js +++ b/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), }), ) diff --git a/src/serviceWorker.js b/src/serviceWorker.js index ca8e361a..49f061c3 100644 --- a/src/serviceWorker.js +++ b/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 } diff --git a/test/api/addVersion.test.js b/test/api/addVersion.test.js new file mode 100644 index 00000000..4626c3f8 --- /dev/null +++ b/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') + }) +}) diff --git a/test/fetch.test.js b/test/fetch.test.js index 6014c1c1..af999bfe 100644 --- a/test/fetch.test.js +++ b/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') + }) }) diff --git a/test/plp/SearchResultsProvider.test.js b/test/plp/SearchResultsProvider.test.js index e5fb3263..259337ae 100644 --- a/test/plp/SearchResultsProvider.test.js +++ b/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() }) diff --git a/test/serviceWorker.test.js b/test/serviceWorker.test.js index aa8e6d2a..6639426d 100644 --- a/test/serviceWorker.test.js +++ b/test/serviceWorker.test.js @@ -5,88 +5,116 @@ import { resetPrefetches, } from '../src/serviceWorker' -describe('serviceWorker', () => { - let postMessage, addEventListener - +describe('all', () => { beforeEach(() => { - postMessage = jest.fn() - addEventListener = jest.fn((event, cb) => { - setImmediate(cb) - }) - navigator.serviceWorker = { - ready: Promise.resolve(), - controller: { - postMessage, - }, - addEventListener, - } + process.env.SERVICE_WORKER = 'true' }) - describe('waitForServiceWorker', () => { - it('should return true if navigator.serviceWorker.controller is defined', async () => { - expect(await waitForServiceWorker()).toBe(true) - }) + afterEach(() => { + delete process.env.SERVICE_WORKER + }) - it('should wait for controllerchange if controller is not defined', async () => { - delete navigator.serviceWorker.controller - await waitForServiceWorker() - expect(addEventListener).toHaveBeenCalledWith('controllerchange', expect.any(Function)) - }) + describe('serviceWorker', () => { + let postMessage, addEventListener - it('should return false if navigator.serviceWorker is undefined', async () => { - delete navigator.serviceWorker - expect(await waitForServiceWorker()).toBe(false) + beforeEach(() => { + postMessage = jest.fn() + addEventListener = jest.fn((event, cb) => { + setImmediate(cb) + }) + navigator.serviceWorker = { + ready: Promise.resolve(), + controller: { + postMessage, + }, + addEventListener, + } }) - it('should return false if navigator.serviceWorker.ready is undefined', async () => { - delete navigator.serviceWorker.ready - expect(await waitForServiceWorker()).toBe(false) - }) - }) -}) + describe('waitForServiceWorker', () => { + it('should return true if navigator.serviceWorker.controller is defined', async () => { + expect(await waitForServiceWorker()).toBe(true) + }) -describe('prefetch', () => { - beforeEach(() => { - process.env.RSF_PREFETCH_QUERY_PARAM = '__prefetch__' - document.head.innerHTML = '' - resetPrefetches() - }) + it('should wait for controllerchange if controller is not defined', async () => { + delete navigator.serviceWorker.controller + await waitForServiceWorker() + expect(addEventListener).toHaveBeenCalledWith('controllerchange', expect.any(Function)) + }) - afterEach(() => { - delete window.RSF_PREFETCH_QUERY_PARAM + it('should return false if navigator.serviceWorker is undefined', async () => { + delete navigator.serviceWorker + expect(await waitForServiceWorker()).toBe(false) + }) + + it('should return false if navigator.serviceWorker.ready is undefined', async () => { + delete navigator.serviceWorker.ready + expect(await waitForServiceWorker()).toBe(false) + }) + }) }) describe('prefetch', () => { - it('should append a single link tag per url', async () => { - await prefetch('/foo') - await prefetch('/foo') - expect( - document.head.querySelectorAll('link[href="http://localhost/foo?__prefetch__=1"]'), - ).toHaveLength(1) + beforeEach(() => { + process.env.RSF_PREFETCH_QUERY_PARAM = '__prefetch__' + document.head.innerHTML = '' + window.__NEXT_DATA__ = { + buildId: 'development', + } + resetPrefetches() }) - it('should not require window.RSF_PREFETCH_QUERY_PARAM to be defined', async () => { - delete process.env.RSF_PREFETCH_QUERY_PARAM - await prefetch('/foo') - expect(document.head.querySelectorAll('link[href="/foo"]')).toHaveLength(1) + afterEach(() => { + delete window.__NEXT_DATA__ + delete window.RSF_PREFETCH_QUERY_PARAM }) - it('should not add RSF_PREFETCH_QUERY_PARAM when fetching from a 3rd party', async () => { - await prefetch('https://www.thirdparty.com/foo') + describe('prefetch', () => { + it('should append a single link tag per url', async () => { + await prefetch('/foo') + await prefetch('/foo') + expect( + document.head.querySelectorAll( + 'link[href="http://localhost/foo?__v__=development&__prefetch__=1"]', + ), + ).toHaveLength(1) + }) - expect( - document.head.querySelectorAll('link[href="https://www.thirdparty.com/foo"]'), - ).toHaveLength(1) + it('should not require window.RSF_PREFETCH_QUERY_PARAM to be defined', async () => { + delete process.env.RSF_PREFETCH_QUERY_PARAM + await prefetch('/foo') + expect( + document.head.querySelectorAll('link[href="http://localhost/foo?__v__=development"]'), + ).toHaveLength(1) + }) + + it('should not add RSF_PREFETCH_QUERY_PARAM when fetching from a 3rd party', async () => { + await prefetch('https://www.thirdparty.com/foo') + + expect( + document.head.querySelectorAll( + 'link[href="https://www.thirdparty.com/foo?__v__=development"]', + ), + ).toHaveLength(1) + }) }) - }) - describe('prefetchJsonFor', () => { - it('should append the api prefix', async () => { - await prefetchJsonFor('/foo') + describe('prefetchJsonFor', () => { + it('should append the api prefix', async () => { + await prefetchJsonFor('/foo') + + expect( + document.head.querySelectorAll( + 'link[href="http://localhost/api/foo?__v__=development&__prefetch__=1"]', + ), + ).toHaveLength(1) + }) + it('should not prefetch if process.env.SERVICE_WORKER is not defined', async () => { + delete process.env.SERVICE_WORKER + await prefetchJsonFor('/foo') - expect( - document.head.querySelectorAll('link[href="http://localhost/api/foo?__prefetch__=1"]'), - ).toHaveLength(1) + expect(document.head.querySelectorAll('link')).toHaveLength(0) + }) }) }) })