fix(gatsby): fix root resolving of resources (#29577)

* fix(gatsby): fix root resolving of resources * add tests * revert wrong changes * fix e2e * update timeout * fix resource blocking test * fix test * revert change * fix offline test * add test to test block-resources * update absolute url tests * Update packages/gatsby/src/utils/webpack-utils.ts Co-authored-by: Ward Peeters <[email protected]> * actually test absolute url cases Co-authored-by: Michal Piechowiak <[email protected]>
gatsbyjs · Feb 24, 2021 · affb1d6 · affb1d6
1 parent 4d8835d
commit affb1d6
Show file tree

Hide file tree

Showing 12 changed files with 185 additions and 120 deletions.
diff --git a/e2e-tests/production-runtime/cypress/integration/1-production.js b/e2e-tests/production-runtime/cypress/integration/1-production.js
@@ -132,7 +132,7 @@ describe(`Production build tests`, () => {
 
   describe(`Supports unicode characters in urls`, () => {
     it(`Can navigate directly`, () => {
-      cy.visit(`/안녕/`, {
+      cy.visit(encodeURI(`/안녕/`), {
         // Cypress seems to think it's 404
         // even if it's not. 404 page doesn't have
         // `page-2-message` element so the test will fail on
@@ -156,7 +156,7 @@ describe(`Production build tests`, () => {
     })
 
     it(`should show 404 page when url with unicode characters point to a non-existent page route when navigating directly`, () => {
-      cy.visit(`/안녕404/`, {
+      cy.visit(encodeURI(`/안녕404/`), {
         failOnStatusCode: false,
       }).waitForRouteChange()
 

diff --git a/e2e-tests/production-runtime/cypress/integration/global-styles.js b/e2e-tests/production-runtime/cypress/integration/global-styles.js
@@ -2,7 +2,15 @@ const zIndex = `9001`
 
 describe(`Global style from gatsby-browser.js`, () => {
   beforeEach(() => {
-    cy.visit(`/global-style`).waitForRouteChange()
+    cy.intercept("/dog-thumbnail.jpg").as("thumbnail")
+    cy.intercept("/static/merriweather-latin**.woff2").as("font")
+    cy.intercept("localhost:9000/dog-thumbnail-flip.jpg").as(
+      "absolute-url-without-protocol"
+    )
+    cy.intercept("localhost:9000/dog-thumbnail-dither.jpg").as(
+      "absolute-url-with-protocol"
+    )
+    cy.visit(`/global-style/`).waitForRouteChange()
   })
 
   it(`should apply any styles in root gatsby-browser.js`, () => {
@@ -16,4 +24,48 @@ describe(`Global style from gatsby-browser.js`, () => {
       zIndex
     )
   })
+
+  describe(`should resolve absolute path in url()`, () => {
+    it(`url without host`, () => {
+      cy.wait("@thumbnail").should(req => {
+        expect(req.response.statusCode).to.be.gte(200).and.lt(400)
+      })
+      cy.getTestElement(`global-style-background`).should(
+        `have.css`,
+        `background-image`,
+        `url("http://localhost:9000/dog-thumbnail.jpg")`
+      )
+    })
+
+    it(`url with host and without protocol`, () => {
+      cy.wait("@absolute-url-without-protocol").should(req => {
+        expect(req.response.statusCode).to.be.gte(200).and.lt(400)
+      })
+      cy.getTestElement(`global-style-urlwithoutprotocol`).should(
+        `have.css`,
+        `background-image`,
+        `url("http://localhost:9000/dog-thumbnail-flip.jpg")`
+      )
+    })
+
+    it(`url with host and with protocol`, () => {
+      cy.wait("@absolute-url-with-protocol").should(req => {
+        expect(req.response.statusCode).to.be.gte(200).and.lt(400)
+      })
+      cy.getTestElement(`global-style-fullurl`).should(
+        `have.css`,
+        `background-image`,
+        `url("http://localhost:9000/dog-thumbnail-dither.jpg")`
+      )
+    })
+  })
+
+  // Service worker is handling requests so this one is cached by previous runs
+  if (!Cypress.env(`TEST_PLUGIN_OFFLINE`)) {
+    it(`should resolve relative path in url()`, () => {
+      cy.wait("@font").should(req => {
+        expect(req.response.statusCode).to.be.gte(200).and.lt(400)
+      })
+    })
+  }
 })
diff --git a/e2e-tests/production-runtime/cypress/integration/resource-loading-resilience.js b/e2e-tests/production-runtime/cypress/integration/resource-loading-resilience.js
@@ -6,7 +6,7 @@ Cypress.on(`uncaught:exception`, (err, runnable) => {
 })
 
 const waitForAPIOptions = {
-  timeout: 3000,
+  timeout: 10000,
 }
 
 function assertOnNavigate(
@@ -24,7 +24,7 @@ function assertOnNavigate(
   cy.getTestElement(`dom-marker`).contains(assertShouldBe)
 }
 
-const runTests = (testNameSuffix = `Unknown scenario`) => {
+function runTests(testNameSuffix) {
   it(`Loads index - ${testNameSuffix}`, () => {
     cy.visit(`/`).waitForAPIorTimeout(`onRouteUpdate`, waitForAPIOptions)
     cy.getTestElement(`dom-marker`).contains(`index`)
@@ -63,52 +63,60 @@ const runTests = (testNameSuffix = `Unknown scenario`) => {
   })
 }
 
-const getTestNameSuffix = (scenario, args) => {
-  if (scenario === `blockAssetsForChunk` && args.chunk === `app`) {
-    return `Blocked "app" chunk`
-  } else if (scenario === `blockAssetsForPage`) {
-    return `Blocked "${args.filter}" for "${args.pagePath}"`
-  }
-
-  return undefined
-}
-
-const runBlockedScenario = (scenario, args) => {
-  describe(`Block resources`, () => {
-    before(done => {
-      cy.task(`restoreAllBlockedResources`).then(() => {
-        cy.task(scenario, args).then(() => {
-          done()
+const runBlockedScenario = args => {
+  beforeEach(() => {
+    cy.task("getAssetsForPage", {
+      pagePath: args.pagePath,
+      filter: args.filter,
+    }).then(urls => {
+      for (const url of urls) {
+        cy.intercept(url, {
+          statusCode: 404,
+          body: "",
         })
-      })
+        cy.log(`intercept ${url}`)
+      }
     })
+  })
 
-    runTests(getTestNameSuffix(scenario, args))
+  afterEach(() => {
+    // check if assets are actually stubbed
+    cy.task("getAssetsForPage", {
+      pagePath: args.pagePath,
+      filter: args.filter,
+    }).then(urls => {
+      expect(Object.keys(cy.state("routes")).length).to.equal(urls.length)
+    })
+  })
+
+  runTests(`Blocked "${args.filter}" for "${args.pagePath}"`, {
+    ...args,
+    task: "getAssetsForPage",
   })
 }
 
 const runSuiteForPage = (label, pagePath) => {
   describe(`Missing "${label}" resources`, () => {
     describe(`Missing "${label}" page query results`, () => {
-      runBlockedScenario(`blockAssetsForPage`, {
+      runBlockedScenario({
         pagePath,
         filter: `page-data`,
       })
     })
     describe(`Missing "${label}" page page-template asset`, () => {
-      runBlockedScenario(`blockAssetsForPage`, {
+      runBlockedScenario({
         pagePath,
         filter: `page-template`,
       })
     })
     describe(`Missing "${label}" page extra assets`, () => {
-      runBlockedScenario(`blockAssetsForPage`, {
+      runBlockedScenario({
         pagePath,
         filter: `extra`,
       })
     })
     describe(`Missing all "${label}" page assets`, () => {
-      runBlockedScenario(`blockAssetsForPage`, {
+      runBlockedScenario({
         pagePath,
         filter: `all`,
       })
@@ -117,24 +125,38 @@ const runSuiteForPage = (label, pagePath) => {
 }
 
 describe(`Every resources available`, () => {
-  it(`Restore resources`, () => {
-    cy.task(`restoreAllBlockedResources`)
-  })
   runTests(`Every resource available`)
 })
 
 describe(`Missing top level resources`, () => {
   describe(`Deleted app chunk assets`, () => {
-    runBlockedScenario(`blockAssetsForChunk`, { chunk: `app` })
+    beforeEach(() => {
+      cy.task("getAssetsForChunk", {
+        filter: "app",
+      }).then(urls => {
+        for (const url of urls) {
+          cy.intercept(url, {
+            statusCode: 404,
+            body: "",
+          })
+          cy.log(`intercept ${url}`)
+        }
+      })
+    })
+
+    afterEach(() => {
+      // check if assets are actually stubbed
+      cy.task("getAssetsForChunk", {
+        filter: "app",
+      }).then(urls => {
+        expect(Object.keys(cy.state("routes")).length).to.equal(urls.length)
+      })
+    })
+
+    runTests(`Blocked "app" chunk`)
   })
 })
 
 runSuiteForPage(`Index`, `/`)
 runSuiteForPage(`Page-2`, `/page-2/`)
 runSuiteForPage(`404`, `/404.html`)
-
-describe(`Cleanup`, () => {
-  it(`Restore resources`, () => {
-    cy.task(`restoreAllBlockedResources`)
-  })
-})
diff --git a/e2e-tests/production-runtime/cypress/plugins/block-resources.js b/e2e-tests/production-runtime/cypress/plugins/block-resources.js
@@ -1,69 +1,20 @@
 const fs = require(`fs-extra`)
 const path = require(`path`)
-const glob = require(`glob`)
 
 const publicDir = path.join(__dirname, `..`, `..`, `public`)
 
-const getAssetManifest = () => {
+function getAssetManifest() {
   const { assetsByChunkName } = require(`${publicDir}/webpack.stats.json`)
   return assetsByChunkName
 }
 
-const moveAsset = (from, to) => {
-  const fromExists = fs.existsSync(from)
-  const toExists = fs.existsSync(to)
-
-  if (fromExists && !toExists) {
-    fs.moveSync(from, to, {
-      overwrite: true,
-    })
-  }
-}
-
-const getAssetPath = assetFileName => path.join(publicDir, assetFileName)
-const getHiddenAssetPath = assetFileName => getAssetPath(`_${assetFileName}`)
-
-const restoreAsset = assetFileName => {
-  moveAsset(getHiddenAssetPath(assetFileName), getAssetPath(assetFileName))
-}
-
-const blockAsset = assetFileName => {
-  moveAsset(getAssetPath(assetFileName), getHiddenAssetPath(assetFileName))
-}
-
-const blockAssetsForChunk = ({ chunk, filter }) => {
-  const assetManifest = getAssetManifest()
-  assetManifest[chunk].forEach(blockAsset)
-  console.log(`Blocked assets for chunk "${chunk}"`)
-  return null
-}
-
-const restorePageData = hiddenPath => {
-  if (path.basename(hiddenPath).charAt(0) !== `_`) {
-    throw new Error(`hiddenPath should have _ prefix`)
-  }
-  const restoredPath = path.join(
-    path.dirname(hiddenPath),
-    path.basename(hiddenPath).slice(1)
-  )
-  moveAsset(hiddenPath, restoredPath)
-}
-
-const getPageDataPath = pagePath => {
+function getPageDataPath(pagePath) {
   const fixedPagePath = pagePath === `/` ? `index` : pagePath
-  return path.join(publicDir, `page-data`, fixedPagePath, `page-data.json`)
+  return path.posix.join(`page-data`, fixedPagePath, `page-data.json`)
 }
 
-const getHiddenPageDataPath = pagePath => {
-  const fixedPagePath = pagePath === `/` ? `index` : pagePath
-  return path.join(publicDir, `page-data`, fixedPagePath, `_page-data.json`)
-}
-
-const blockPageData = pagePath =>
-  moveAsset(getPageDataPath(pagePath), getHiddenPageDataPath(pagePath))
-
-const filterAssets = (assetsForPath, filter) =>
-  assetsForPath.filter(asset => {
+const filterAssets = (assetsForPath, filter) => {
+  return assetsForPath.filter(asset => {
     if (filter === `all`) {
       return true
     } else if (filter === `page-data`) {
@@ -78,43 +29,36 @@ const filterAssets = (assetsForPath, filter) =>
     }
     return false
   })
+}
+
+function getAssetsForChunk({ filter }) {
+  const assetManifest = getAssetManifest()
+
+  return assetManifest[filter].map(asset => `/${asset}`)
+}
 
-const blockAssetsForPage = ({ pagePath, filter }) => {
+function getAssetsForPage({ pagePath, filter }) {
   const assetManifest = getAssetManifest()
 
-  const pageData = JSON.parse(fs.readFileSync(getPageDataPath(pagePath)))
+  const pageDataUrl = getPageDataPath(pagePath)
+  const pageData = JSON.parse(
+    fs.readFileSync(path.join(publicDir, pageDataUrl))
+  )
   const { componentChunkName } = pageData
   const assetsForPath = assetManifest[componentChunkName]
 
-  const assets = filterAssets(assetsForPath, filter)
-  assets.forEach(blockAsset)
+  const assets = filterAssets(assetsForPath, filter).map(
+    assetFileName => `/${assetFileName}`
+  )
 
   if (filter === `all` || filter === `page-data`) {
-    blockPageData(pagePath)
+    assets.push(`/${pageDataUrl}`)
   }
 
-  console.log(`Blocked assets for path "${pagePath}" [${filter}]`)
-  return null
-}
-
-const restore = () => {
-  const allAssets = Object.values(getAssetManifest()).reduce((acc, assets) => {
-    assets.forEach(asset => acc.add(asset))
-    return acc
-  }, new Set())
-
-  allAssets.forEach(restoreAsset)
-
-  const globPattern = path.join(publicDir, `/page-data/**`, `_page-data.json`)
-  const hiddenPageDatas = glob.sync(globPattern)
-  hiddenPageDatas.forEach(restorePageData)
-
-  console.log(`Restored resources`)
-  return null
+  return assets
 }
 
 module.exports = {
-  restoreAllBlockedResources: restore,
-  blockAssetsForChunk,
-  blockAssetsForPage,
+  getAssetsForPage,
+  getAssetsForChunk,
 }
diff --git a/e2e-tests/production-runtime/gatsby-browser.js b/e2e-tests/production-runtime/gatsby-browser.js
@@ -1,5 +1,6 @@
 const Wrapper = require(`./src/wrap-root-element`).default
 require(`./src/index.css`)
+require(`typeface-merriweather`)
 
 if (typeof window !== `undefined`) {
   window.___PageComponentLifecycleCallsLog = []

diff --git a/e2e-tests/production-runtime/package.json b/e2e-tests/production-runtime/package.json
@@ -4,7 +4,7 @@
   "version": "1.0.0",
   "author": "Kyle Mathews <[email protected]>",
   "dependencies": {
-    "cypress": "3.4.1",
+    "cypress": "^6.5.0",
     "gatsby": "^3.0.0-next.6",
     "gatsby-plugin-image": "^1.0.0-next.5",
     "gatsby-plugin-manifest": "^3.0.0-next.0",
@@ -15,7 +15,8 @@
     "glob": "^7.1.3",
     "react": "^16.8.0",
     "react-dom": "^16.8.0",
-    "react-helmet": "^5.2.0"
+    "react-helmet": "^5.2.0",
+    "typeface-merriweather": "^1.1.13"
   },
   "keywords": [
     "gatsby"