From 97e942e28cccaf665fd94cfa8f3aed2c28237dbb Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Fri, 17 Dec 2021 16:18:51 +0100 Subject: [PATCH] fix(gatsby): handle case of html and data files mismatch (#34225) --- .../cypress/integration/compilation-hash.js | 191 ++++++++++++++---- .../cypress/plugins/compilation-hash.js | 37 ---- .../cypress/plugins/index.js | 3 +- .../gatsby-plugin-offline/src/gatsby-node.js | 9 +- .../gatsby-plugin-offline/src/sw-append.js | 18 ++ .../__snapshots__/static-entry.js.snap | 6 +- .../cache-dir/__tests__/static-entry.js | 9 +- packages/gatsby/cache-dir/production-app.js | 37 +++- packages/gatsby/cache-dir/static-entry.js | 3 +- packages/gatsby/src/commands/build-html.ts | 3 + packages/gatsby/src/commands/build.ts | 1 + .../utils/page-ssr-module/bundle-webpack.ts | 3 + .../gatsby/src/utils/page-ssr-module/entry.ts | 16 +- .../src/utils/worker/child/render-html.ts | 3 + 14 files changed, 245 insertions(+), 94 deletions(-) delete mode 100644 e2e-tests/production-runtime/cypress/plugins/compilation-hash.js diff --git a/e2e-tests/production-runtime/cypress/integration/compilation-hash.js b/e2e-tests/production-runtime/cypress/integration/compilation-hash.js index 29f169da4fb40..7dc8c72a31646 100644 --- a/e2e-tests/production-runtime/cypress/integration/compilation-hash.js +++ b/e2e-tests/production-runtime/cypress/integration/compilation-hash.js @@ -1,62 +1,179 @@ /* global cy */ +let spy +Cypress.on(`window:before:load`, win => { + spy = cy.spy(win.console, `error`).as(`errorMessage`) +}) + +Cypress.on(`uncaught:exception`, (err, runnable) => { + // returning false here prevents Cypress from + // failing the test + return false +}) + const getRandomInt = (min, max) => { min = Math.ceil(min) max = Math.floor(max) return Math.floor(Math.random() * (max - min)) + min } -const createMockCompilationHash = () => - [...Array(20)] +const createMockCompilationHash = () => { + const hash = [...Array(20)] .map(a => getRandomInt(0, 16)) .map(k => k.toString(16)) .join(``) + cy.log({ hash }) + return hash +} describe(`Webpack Compilation Hash tests`, () => { it(`should render properly`, () => { cy.visit(`/`).waitForRouteChange() }) - // This covers the case where a user loads a gatsby site and then - // the site is changed resulting in a webpack recompile and a - // redeploy. This could result in a mismatch between the page-data - // and the component. To protect against this, when gatsby loads a - // new page-data.json, it refreshes the page if it's webpack - // compilation hash differs from the one on on the window object - // (which was set on initial page load) - // - // Since initial page load results in all links being prefetched, we - // have to navigate to a non-prefetched page to test this. Thus the - // `deep-link-page`. - // - // We simulate a rebuild by updating all page-data.jsons and page - // htmls with the new hash. It's not pretty, but it's easier than - // figuring out how to perform an actual rebuild while cypress is - // running. See ../plugins/compilation-hash.js for the - // implementation - it.skip(`should reload page if build occurs in background`, () => { - cy.window().then(window => { - const oldHash = window.___webpackCompilationHash - expect(oldHash).to.not.eq(undefined) - + // Service worker is handling requests so this one is cached by previous runs + if (!Cypress.env(`TEST_PLUGIN_OFFLINE`)) { + // This covers the case where a user loads a gatsby site and then + // the site is changed resulting in a webpack recompile and a + // redeploy. This could result in a mismatch between the page-data + // and the component. To protect against this, when gatsby loads a + // new page-data.json, it refreshes the page if it's webpack + // compilation hash differs from the one on on the window object + // (which was set on initial page load) + // + // Since initial page load results in all links being prefetched, we + // have to navigate to a non-prefetched page to test this. Thus the + // `deep-link-page`. + // + // We simulate a rebuild by intercepting app-data request and responding with random hash + it(`should reload page on navigation if build occurs in background`, () => { const mockHash = createMockCompilationHash() - // Simulate a new webpack build - cy.task(`overwriteWebpackCompilationHash`, mockHash).then(() => { - cy.getTestElement(`compilation-hash`).click() - cy.waitForRouteChange() + cy.visit(`/`).waitForRouteChange() - // Navigate into a non-prefetched page - cy.getTestElement(`deep-link-page`).click() - cy.waitForRouteChange() + let didMock = false + cy.intercept("/app-data.json", req => { + if (!didMock) { + req.reply({ + webpackCompilationHash: mockHash, + }) + didMock = true + } + }).as(`appDataFetch`) - // If the window compilation hash has changed, we know the - // page was refreshed - cy.window().its(`___webpackCompilationHash`).should(`equal`, mockHash) + cy.window().then(window => { + // just setting some property on the window + // we will later assert that property to know wether + // browser reload happened or not. + window.notReloaded = true + window.___navigate(`/deep-link-page/`) }) - // Cleanup - cy.task(`overwriteWebpackCompilationHash`, oldHash) + cy.waitForRouteChange() + + // we expect reload to happen so our window property shouldn't be set anymore + cy.window().its(`notReloaded`).should(`not.equal`, true) + + // let's make sure we actually see the content + cy.contains( + `StaticQuery in wrapRootElement test (should show site title):Gatsby Default Starter` + ) }) - }) + + // This covers the case where user user loads "outdated" html from some kind of cache + // and our data files (page-data and app-data) are for newer built. + // We will mock both app-data (to change the hash) as well as example page-data + // to simulate changes to static query hashes between builds. + it(`should force reload page if on initial load the html is not matching newest app/page-data`, () => { + const mockHash = createMockCompilationHash() + + // trying to intercept just `/` seems to intercept all routes + // so intercepting same thing just with regex + cy.intercept(/^\/$/).as(`indexFetch`) + + // We will mock `app-data` and `page-data` json responses one time (for initial load) + let shouldMockAppDataRequests = true + let shouldMockPageDataRequests = true + cy.intercept("/app-data.json", req => { + if (shouldMockAppDataRequests) { + req.reply({ + webpackCompilationHash: mockHash, + }) + shouldMockAppDataRequests = false + } + }).as(`appDataFetch`) + + cy.readFile(`public/page-data/compilation-hash/page-data.json`).then( + originalPageData => { + cy.intercept("/page-data/index/page-data.json", req => { + if (shouldMockPageDataRequests) { + req.reply({ + ...originalPageData, + // setting this to empty array should break runtime with + // either placeholder "Loading (StaticQuery)" (for component) + // or thrown error "The result of this StaticQuery could not be fetched." (for `useStaticQuery` hook) + staticQueryHashes: [], + }) + shouldMockPageDataRequests = false + } + }).as(`pageDataFetch`) + } + ) + + cy.visit(`/`) + cy.wait(1500) + + // component case + cy.contains("Loading (StaticQuery)").should("not.exist") + + // useStaticQuery hook case + cy.get(`@errorMessage`).should(`not.called`) + + // let's make sure we actually see the content + cy.contains( + `StaticQuery in wrapRootElement test (should show site title):Gatsby Default Starter` + ) + + cy.get("@indexFetch.all").should("have.length", 2) + cy.get("@appDataFetch.all").should("have.length", 2) + cy.get("@pageDataFetch.all").should("have.length", 2) + }) + + it(`should not force reload indefinitely`, () => { + const mockHash = createMockCompilationHash() + + // trying to intercept just `/` seems to intercept all routes + // so intercepting same thing just with regex + cy.intercept(/^\/$/).as(`indexFetch`) + + // We will mock `app-data` and `page-data` json responses permanently + cy.intercept("/app-data.json", req => { + req.reply({ + webpackCompilationHash: mockHash, + }) + }).as(`appDataFetch`) + + cy.readFile(`public/page-data/index/page-data.json`).then( + originalPageData => { + cy.intercept("/page-data/index/page-data.json", req => { + req.reply({ + ...originalPageData, + // setting this to empty array should break runtime with + // either placeholder "Loading (StaticQuery)" (for component) + // or thrown error "The result of this StaticQuery could not be fetched." (for `useStaticQuery` hook) + staticQueryHashes: [], + }) + }).as(`pageDataFetch`) + } + ) + + cy.visit(`/`) + + cy.wait(1500) + + cy.get("@indexFetch.all").should("have.length", 2) + cy.get("@appDataFetch.all").should("have.length", 2) + cy.get("@pageDataFetch.all").should("have.length", 2) + }) + } }) diff --git a/e2e-tests/production-runtime/cypress/plugins/compilation-hash.js b/e2e-tests/production-runtime/cypress/plugins/compilation-hash.js deleted file mode 100644 index 57291aea22b60..0000000000000 --- a/e2e-tests/production-runtime/cypress/plugins/compilation-hash.js +++ /dev/null @@ -1,37 +0,0 @@ -const fs = require(`fs-extra`) -const path = require(`path`) -const glob = require(`glob`) - -const replaceHtmlCompilationHash = (filename, newHash) => { - const html = fs.readFileSync(filename, `utf-8`) - const regex = /window\.webpackCompilationHash="\w*"/ - const replace = `window.webpackCompilationHash="${newHash}"` - fs.writeFileSync(filename, html.replace(regex, replace), `utf-8`) -} - -const replacePageDataCompilationHash = (filename, newHash) => { - const pageData = JSON.parse(fs.readFileSync(filename, `utf-8`)) - pageData.webpackCompilationHash = newHash - fs.writeFileSync(filename, JSON.stringify(pageData), `utf-8`) -} - -const overwriteWebpackCompilationHash = newHash => { - glob - .sync(path.join(__dirname, `../../public/page-data/**/page-data.json`)) - .forEach(filename => replacePageDataCompilationHash(filename, newHash)) - glob - .sync(path.join(__dirname, `../../public/**/*.html`)) - .forEach(filename => { - // `page-data/404.html` matches the glob above but is a directory - // (containing `page-data.json`), so ignore this and similar dirs - if (!filename.match(/\/page-data\/[^/.]+\.html/)) - replaceHtmlCompilationHash(filename, newHash) - }) - - // cypress requires that null be returned instead of undefined - return null -} - -module.exports = { - overwriteWebpackCompilationHash, -} diff --git a/e2e-tests/production-runtime/cypress/plugins/index.js b/e2e-tests/production-runtime/cypress/plugins/index.js index d2c8c02c2dbdf..719823e977a3f 100644 --- a/e2e-tests/production-runtime/cypress/plugins/index.js +++ b/e2e-tests/production-runtime/cypress/plugins/index.js @@ -1,4 +1,3 @@ -const compilationHash = require(`./compilation-hash`) const blockResources = require(`./block-resources`) module.exports = (on, config) => { @@ -16,5 +15,5 @@ module.exports = (on, config) => { return args }) - on(`task`, Object.assign({}, compilationHash, blockResources)) + on(`task`, Object.assign({}, blockResources)) } diff --git a/packages/gatsby-plugin-offline/src/gatsby-node.js b/packages/gatsby-plugin-offline/src/gatsby-node.js index dacba3f556c75..a392d03412f0d 100644 --- a/packages/gatsby-plugin-offline/src/gatsby-node.js +++ b/packages/gatsby-plugin-offline/src/gatsby-node.js @@ -133,6 +133,13 @@ exports.onPostBuild = ( // since these files have unique URLs and their contents will never change dontCacheBustURLsMatching: /(\.js$|\.css$|static\/)/, runtimeCaching: [ + // ignore cypress endpoints (only for testing) + process.env.CYPRESS_SUPPORT + ? { + urlPattern: /\/__cypress\//, + handler: `NetworkOnly`, + } + : false, { // Use cacheFirst since these don't need to be revalidated (same RegExp // and same reason as above) @@ -156,7 +163,7 @@ exports.onPostBuild = ( urlPattern: /^https?:\/\/fonts\.googleapis\.com\/css/, handler: `StaleWhileRevalidate`, }, - ], + ].filter(Boolean), skipWaiting: true, clientsClaim: true, } diff --git a/packages/gatsby-plugin-offline/src/sw-append.js b/packages/gatsby-plugin-offline/src/sw-append.js index df7a37c4ab01e..3a8445304dff3 100644 --- a/packages/gatsby-plugin-offline/src/sw-append.js +++ b/packages/gatsby-plugin-offline/src/sw-append.js @@ -14,6 +14,24 @@ const MessageAPI = { clearPathResources: event => { event.waitUntil(idbKeyval.clear()) + + // We detected compilation hash mismatch + // we should clear runtime cache as data + // files might be out of sync and we should + // do fresh fetches for them + event.waitUntil( + caches.keys().then(function (keyList) { + return Promise.all( + keyList.map(function (key) { + if (key && key.includes(`runtime`)) { + return caches.delete(key) + } + + return Promise.resolve() + }) + ) + }) + ) }, enableOfflineShell: () => { diff --git a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap index 78706199ce5b9..68a7dab220352 100644 --- a/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap +++ b/packages/gatsby/cache-dir/__tests__/__snapshots__/static-entry.js.snap @@ -14,21 +14,21 @@ exports[`develop-static-entry onPreRenderHTML can be used to replace preBodyComp exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] = ` Object { - "html": "
", + "html": "
", "unsafeBuiltinsUsage": Array [], } `; exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = ` Object { - "html": "
div3
div2
div1
", + "html": "
div3
div2
div1
", "unsafeBuiltinsUsage": Array [], } `; exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = ` Object { - "html": "
div3
div2
div1
", + "html": "
div3
div2
div1
", "unsafeBuiltinsUsage": Array [], } `; diff --git a/packages/gatsby/cache-dir/__tests__/static-entry.js b/packages/gatsby/cache-dir/__tests__/static-entry.js index 0f0077bc8d54e..eee0027d7cc22 100644 --- a/packages/gatsby/cache-dir/__tests__/static-entry.js +++ b/packages/gatsby/cache-dir/__tests__/static-entry.js @@ -63,17 +63,18 @@ jest.mock( const pageDataMock = { componentChunkName: `page-component---src-pages-test-js`, path: `/about/`, - webpackCompilationHash: `1234567890abcdef1234`, staticQueryHashes: [], } +const webpackCompilationHash = `1234567890abcdef1234` + const MOCK_FILE_INFO = { [`${process.cwd()}/public/webpack.stats.json`]: `{}`, [`${process.cwd()}/public/chunk-map.json`]: `{}`, [join(process.cwd(), `/public/page-data/about/page-data.json`)]: JSON.stringify(pageDataMock), [join(process.cwd(), `/public/page-data/app-data.json`)]: JSON.stringify({ - webpackCompilationHash: `1234567890abcdef1234`, + webpackCompilationHash, }), } @@ -173,11 +174,10 @@ const SSR_DEV_MOCK_FILE_INFO = { [join(publicDir, `page-data/about/page-data.json`)]: JSON.stringify({ componentChunkName: `page-component---src-pages-about-js`, path: `/about/`, - webpackCompilationHash: `1234567890abcdef1234`, staticQueryHashes: [], }), [join(publicDir, `page-data/app-data.json`)]: JSON.stringify({ - webpackCompilationHash: `1234567890abcdef1234`, + webpackCompilationHash, }), } @@ -411,6 +411,7 @@ describe(`static-entry`, () => { styles: [], reversedStyles: [], reversedScripts: [], + webpackCompilationHash, } beforeEach(() => { diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js index 2dfe5aa6b4c7c..db2dcb4e491ad 100644 --- a/packages/gatsby/cache-dir/production-app.js +++ b/packages/gatsby/cache-dir/production-app.js @@ -35,6 +35,8 @@ window.___loader = publicLoader navigationInit() +const reloadStorageKey = `gatsby-reload-compilation-hash-match` + apiRunnerAsync(`onClientEntry`).then(() => { // Let plugins register a service worker. The plugin just needs // to return true. @@ -161,6 +163,39 @@ apiRunnerAsync(`onClientEntry`).then(() => { } publicLoader.loadPage(browserLoc.pathname + browserLoc.search).then(page => { + if (page.page.webpackCompilationHash !== window.___webpackCompilationHash) { + // Purge plugin-offline cache + if ( + `serviceWorker` in navigator && + navigator.serviceWorker.controller !== null && + navigator.serviceWorker.controller.state === `activated` + ) { + navigator.serviceWorker.controller.postMessage({ + gatsbyApi: `clearPathResources`, + }) + } + + // We have not matching html + js (inlined `window.___webpackCompilationHash`) + // with our data (coming from `app-data.json` file). This can cause issues such as + // errors trying to load static queries (as list of static queries is inside `page-data` + // which might not match to currently loaded `.js` scripts). + // We are making attempt to reload if hashes don't match, but we also have to handle case + // when reload doesn't fix it (possibly broken deploy) so we don't end up in infinite reload loop + if (sessionStorage) { + const isReloaded = sessionStorage.getItem(reloadStorageKey) === `1` + + if (!isReloaded) { + sessionStorage.setItem(reloadStorageKey, `1`) + window.location.reload(true) + return + } + } + } + + if (sessionStorage) { + sessionStorage.removeItem(reloadStorageKey) + } + if (!page || page.status === PageResourceStatus.Error) { const message = `page resources for ${browserLoc.pathname} not found. Not rendering React` @@ -174,8 +209,6 @@ apiRunnerAsync(`onClientEntry`).then(() => { throw new Error(message) } - window.___webpackCompilationHash = page.page.webpackCompilationHash - const SiteRoot = apiRunner( `wrapRootElement`, { element: }, diff --git a/packages/gatsby/cache-dir/static-entry.js b/packages/gatsby/cache-dir/static-entry.js index e1c05cb8cec46..7fc99f09d213d 100644 --- a/packages/gatsby/cache-dir/static-entry.js +++ b/packages/gatsby/cache-dir/static-entry.js @@ -111,6 +111,7 @@ export default async function staticPage({ reversedStyles, reversedScripts, inlinePageData = false, + webpackCompilationHash, }) { // for this to work we need this function to be sync or at least ensure there is single execution of it at a time global.unsafeBuiltinUsage = [] @@ -388,7 +389,7 @@ export default async function staticPage({ }) // Add page metadata for the current page - const windowPageData = `/**/` diff --git a/packages/gatsby/src/commands/build-html.ts b/packages/gatsby/src/commands/build-html.ts index c4d54b313e68d..86e06c755c95c 100644 --- a/packages/gatsby/src/commands/build-html.ts +++ b/packages/gatsby/src/commands/build-html.ts @@ -198,6 +198,8 @@ const renderHTMLQueue = async ( const sessionId = Date.now() + const { webpackCompilationHash } = store.getState() + const renderHTML = stage === `build-html` ? workerPool.single.renderHTMLProd @@ -212,6 +214,7 @@ const renderHTMLQueue = async ( htmlComponentRendererPath, paths: pageSegment, sessionId, + webpackCompilationHash, }) if (isPreview) { diff --git a/packages/gatsby/src/commands/build.ts b/packages/gatsby/src/commands/build.ts index 855a01f2d2cde..fb2772d039716 100644 --- a/packages/gatsby/src/commands/build.ts +++ b/packages/gatsby/src/commands/build.ts @@ -193,6 +193,7 @@ module.exports = async function build( rootDir: program.directory, components: state.components, staticQueriesByTemplate: state.staticQueriesByTemplate, + webpackCompilationHash: webpackCompilationHash as string, // we set webpackCompilationHash above reporter: report, isVerbose: program.verbose, }) diff --git a/packages/gatsby/src/utils/page-ssr-module/bundle-webpack.ts b/packages/gatsby/src/utils/page-ssr-module/bundle-webpack.ts index 473810ddb85d6..b00bd594cbca8 100644 --- a/packages/gatsby/src/utils/page-ssr-module/bundle-webpack.ts +++ b/packages/gatsby/src/utils/page-ssr-module/bundle-webpack.ts @@ -45,12 +45,14 @@ export async function createPageSSRBundle({ rootDir, components, staticQueriesByTemplate, + webpackCompilationHash, reporter, isVerbose = false, }: { rootDir: string components: IGatsbyState["components"] staticQueriesByTemplate: IGatsbyState["staticQueriesByTemplate"] + webpackCompilationHash: IGatsbyState["webpackCompilationHash"] reporter: Reporter isVerbose?: boolean }): Promise { @@ -150,6 +152,7 @@ export async function createPageSSRBundle({ plugins: [ new webpack.DefinePlugin({ INLINED_TEMPLATE_TO_DETAILS: JSON.stringify(toInline), + WEBPACK_COMPILATION_HASH: JSON.stringify(webpackCompilationHash), // eslint-disable-next-line @typescript-eslint/naming-convention "process.env.GATSBY_LOGGER": JSON.stringify(`yurnalist`), }), diff --git a/packages/gatsby/src/utils/page-ssr-module/entry.ts b/packages/gatsby/src/utils/page-ssr-module/entry.ts index de86c6cd1b7c0..d38aa07a6a20f 100644 --- a/packages/gatsby/src/utils/page-ssr-module/entry.ts +++ b/packages/gatsby/src/utils/page-ssr-module/entry.ts @@ -41,16 +41,17 @@ export interface ISSRData { searchString: string } +// just letting TypeScript know about injected data +// with DefinePlugin +declare global { + const INLINED_TEMPLATE_TO_DETAILS: Record + const WEBPACK_COMPILATION_HASH: string +} + const tracerReadyPromise = initTracer( process.env.GATSBY_OPEN_TRACING_CONFIG_FILE ?? `` ) -const pageTemplateDetailsMap: Record< - string, - ITemplateDetails - // @ts-ignore INLINED_TEMPLATE_TO_DETAILS is being "inlined" by bundler -> = INLINED_TEMPLATE_TO_DETAILS - type MaybePhantomActivity = | ReturnType | undefined @@ -106,7 +107,7 @@ export async function getData({ page = maybePage // 2. Lookup query used for a page (template) - templateDetails = pageTemplateDetailsMap[page.componentChunkName] + templateDetails = INLINED_TEMPLATE_TO_DETAILS[page.componentChunkName] if (!templateDetails) { throw new Error( `Page template details for "${page.componentChunkName}" not found` @@ -354,6 +355,7 @@ export async function renderHTML({ pagePath: getPath(data), pageData, staticQueryContext, + webpackCompilationHash: WEBPACK_COMPILATION_HASH, ...data.templateDetails.assets, inlinePageData: data.page.mode === `SSR` && data.results.serverData, }) diff --git a/packages/gatsby/src/utils/worker/child/render-html.ts b/packages/gatsby/src/utils/worker/child/render-html.ts index 3b0726a5e8d90..8ff193e6bae5e 100644 --- a/packages/gatsby/src/utils/worker/child/render-html.ts +++ b/packages/gatsby/src/utils/worker/child/render-html.ts @@ -116,11 +116,13 @@ export const renderHTMLProd = async ({ paths, envVars, sessionId, + webpackCompilationHash, }: { htmlComponentRendererPath: string paths: Array envVars: Array<[string, string | undefined]> sessionId: number + webpackCompilationHash: string }): Promise => { const publicDir = join(process.cwd(), `public`) const isPreview = process.env.GATSBY_IS_PREVIEW === `true` @@ -160,6 +162,7 @@ export const renderHTMLProd = async ({ await htmlComponentRenderer.default({ pagePath, pageData, + webpackCompilationHash, ...resourcesForTemplate, })