From 009d4f13a7be1ee92d31c22638d53f093cce5172 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:25:01 -0800 Subject: [PATCH 1/9] the dev server now returns 404s which would otherwise fail the tests --- .../cypress/integration/functionality/hooks.js | 2 +- .../integration/functionality/query-data-caches.js | 4 +++- .../cypress/integration/navigation/linking.js | 6 ++++-- .../cypress/integration/navigation/redirect.js | 14 ++++++++++---- .../cypress/integration/page-not-found/404.js | 2 +- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js b/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js index 75595ad6c3c38..265cbe2c2f83e 100644 --- a/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js +++ b/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js @@ -2,7 +2,7 @@ const COUNT_ID = `count` describe(`hooks`, () => { beforeEach(() => { - cy.visit(`/hooks`).waitForRouteChange() + cy.visit(`/hooks`, { failOnStatusCode: false }).waitForRouteChange() }) it(`displays initial state`, () => { diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js b/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js index c6b9148e77a9a..8cea7334f6e32 100644 --- a/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js +++ b/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js @@ -110,7 +110,9 @@ function pageTitleAndDataAssertion(config) { function runTests(config) { preTestSetup(config) - cy.visit(`/query-data-caches/${config.slug}/page-A/`).waitForRouteChange() + cy.visit(`/query-data-caches/${config.slug}/page-A/`, { + failOnStatusCode: false, + }).waitForRouteChange() setupForAssertingNotReloading() diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js index 46e7d7763505b..7347635d66815 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js @@ -79,7 +79,9 @@ describe(`navigation`, () => { describe(`non-existent route`, () => { beforeEach(() => { - cy.getTestElement(`broken-link`).click().waitForRouteChange() + cy.getTestElement(`broken-link`) + .click({ failOnStatusCode: false }) + .waitForRouteChange() }) it(`displays 404 page on broken link`, () => { @@ -134,7 +136,7 @@ describe(`navigation`, () => { }) it(`should show 404 page when url with unicode characters point to a non-existent page route when navigating on client`, () => { - cy.visit(`/`).waitForRouteChange() + cy.visit(`/`, { failOnStatusCode: false }).waitForRouteChange() cy.window() .then(win => win.___navigate(`/안녕404/`)) .waitForRouteChange() diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js index 293930eda4c16..bde2ab94fc8f1 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js @@ -5,7 +5,9 @@ Cypress.on(`window:before:load`, win => { const runTests = () => { it(`should redirect page to index page when there is no such page`, () => { - cy.visit(`/redirect-without-page`).waitForRouteChange() + cy.visit(`/redirect-without-page`, { + failOnStatusCode: false, + }).waitForRouteChange() cy.location(`pathname`).should(`equal`, `/`) cy.then(() => { @@ -42,7 +44,7 @@ const runTests = () => { }) it(`should redirect to a dynamically-created replacement page`, () => { - cy.visit(`/redirect-me/`).waitForRouteChange() + cy.visit(`/redirect-me/`, { failOnStatusCode: false }).waitForRouteChange() cy.location(`pathname`).should(`equal`, `/pt/redirect-me/`) cy.then(() => { @@ -65,7 +67,9 @@ describe(`redirect`, () => { // this is sanity check for this group it(`make sure 404 is present`, () => { - cy.visit(`/______not_existing_page`).waitForRouteChange() + cy.visit(`/______not_existing_page`, { + failOnStatusCode: false, + }).waitForRouteChange() cy.findByText("Preview custom 404 page").click() cy.findByText("A custom 404 page wasn't detected", { exact: false, @@ -100,7 +104,9 @@ describe(`redirect`, () => { }) it(`make sure 404 is NOT present`, () => { - cy.visit(`/______not_existing_page`).waitForRouteChange() + cy.visit(`/______not_existing_page`, { + failOnStatusCode: false, + }).waitForRouteChange() cy.findByText("Preview custom 404 page").click() cy.findByText("A custom 404 page wasn't detected", { exact: false, diff --git a/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js b/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js index 400e7c13a27e1..a7573caae6396 100644 --- a/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js +++ b/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js @@ -1,6 +1,6 @@ describe(`page not found`, () => { beforeEach(() => { - cy.visit(`/__404__`) + cy.visit(`/__404__`, { failOnStatusCode: false }) }) it(`should display message `, () => { cy.get(`h1`).invoke(`text`).should(`eq`, `Gatsby.js development 404 page`) From 45413057e9ab99fa40fccde524567c25128e41b2 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:25:53 -0800 Subject: [PATCH 2/9] Correctly detect if there's any SSRed HTML as ___gatsby always has the focus wrapper child --- packages/gatsby/cache-dir/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/cache-dir/app.js b/packages/gatsby/cache-dir/app.js index 3676dd72be5e0..76624f6e13119 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -126,7 +126,7 @@ apiRunnerAsync(`onClientEntry`).then(() => { undefined, // Client only pages have any empty body so we just do a normal // render to avoid React complaining about hydration mis-matches. - document.getElementById(`___gatsby`).children.length === 0 + document.getElementById(`gatsby-focus-wrapper`).children.length === 0 ? ReactDOM.render : ReactDOM.hydrate )[0] From b3a711372935fcb1c51a3247ca34483811b6f5b5 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:26:31 -0800 Subject: [PATCH 3/9] =?UTF-8?q?Handle=20unicode=20paths=20like=20/?= =?UTF-8?q?=EC=95=88=EB=85=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/gatsby/src/utils/dev-ssr/develop-html-route.ts | 6 +++--- packages/gatsby/src/utils/develop-preload-headers.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/gatsby/src/utils/dev-ssr/develop-html-route.ts b/packages/gatsby/src/utils/dev-ssr/develop-html-route.ts index 9953f1627892e..4a3b8f2596673 100644 --- a/packages/gatsby/src/utils/dev-ssr/develop-html-route.ts +++ b/packages/gatsby/src/utils/dev-ssr/develop-html-route.ts @@ -10,13 +10,13 @@ export const route = ({ app, program, store }): any => app.get(`*`, async (req, res, next) => { trackFeatureIsUsed(`GATSBY_EXPERIMENTAL_DEV_SSR`) - const pathObj = findPageByPath(store.getState(), req.path) + const pathObj = findPageByPath(store.getState(), decodeURI(req.path)) if (!pathObj) { return next() } - await appendPreloadHeaders(req.path, res) + await appendPreloadHeaders(pathObj.path, res) const htmlActivity = report.phantomActivity(`building HTML for path`, {}) htmlActivity.start() @@ -152,7 +152,7 @@ export const route = ({ app, program, store }): any => node.js, it errored.

    -
  • URL path: ${req.path}
    • +
  • URL path: ${pathObj.path}
  • File path: ${error.filename}

error

diff --git a/packages/gatsby/src/utils/develop-preload-headers.ts b/packages/gatsby/src/utils/develop-preload-headers.ts index 4673eee215612..24682100a78bb 100644 --- a/packages/gatsby/src/utils/develop-preload-headers.ts +++ b/packages/gatsby/src/utils/develop-preload-headers.ts @@ -43,7 +43,7 @@ export async function appendPreloadHeaders( `Link`, `; rel=preload; as=fetch ; crossorigin` ) From 9b73a27d65ee3e3ff476654bfd2a6a80f3cba4a2 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:26:54 -0800 Subject: [PATCH 4/9] Properly escape paths with special characters in it --- packages/gatsby/src/utils/dev-ssr/render-dev-html.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts b/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts index 5621e1ac5f5d0..28aef6ee541fa 100644 --- a/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts +++ b/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts @@ -59,8 +59,13 @@ export const restartWorker = (htmlComponentRendererPath): void => { const searchFileForString = (substring, filePath): Promise => new Promise(resolve => { + const escapedSubString = substring.replace(/[.*+?^${}()|[\]\\]/g, `\\$&`) + // See if the chunk is in the newComponents array (not the notVisited). - const chunkRegex = RegExp(`exports.ssrComponents.*${substring}.*}`, `gs`) + const chunkRegex = RegExp( + `exports.ssrComponents.*${escapedSubString}.*}`, + `gs` + ) const stream = fs.createReadStream(filePath) let found = false stream.on(`data`, function (d) { From dee3f580bf4a8062ae6dfd8dfceee33c819d9157 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:27:10 -0800 Subject: [PATCH 5/9] Enable DEV_SSR for everyone --- packages/gatsby/src/utils/flags.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index c295a5a824207..ab69eabca0dd5 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -1,8 +1,6 @@ import _ from "lodash" import semver from "semver" -import sampleSiteForExperiment from "./sample-site-for-experiment" - // Does this experiment run for only builds type executingCommand = "build" | "develop" | "all" @@ -94,13 +92,7 @@ const activeFlags: Array = [ experimental: false, description: `Server Side Render (SSR) pages on full reloads during develop. Helps you detect SSR bugs and fix them without needing to do full builds.`, umbrellaIssue: `https://gatsby.dev/dev-ssr-feedback`, - testFitness: (): fitnessEnum => { - if (sampleSiteForExperiment(`DEV_SSR`, 20)) { - return `OPT_IN` - } else { - return true - } - }, + testFitness: (): fitnessEnum => `LOCKED_IN`, }, { name: `QUERY_ON_DEMAND`, From 83b63d0de47a0ceb15362c73081ea577adb6dd7f Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 23 Feb 2021 15:55:30 -0800 Subject: [PATCH 6/9] Fix status in test --- .../gatsby-pipeline/__tests__/lazy-image-build/develop.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js b/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js index b4f1abb947868..0231c8f831f43 100644 --- a/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js +++ b/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js @@ -29,7 +29,7 @@ describe(`Lazy images`, () => { await kill() - expect(response.status).toBe(200) + expect(response.status).toBe(404) const images = glob.sync(`${basePath}/public/**/*.png`) expect(images.length).toBe(6) From aa651e52f8bd504a046a0879bfa6ea10bad7e9b9 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 24 Feb 2021 14:11:15 -0800 Subject: [PATCH 7/9] Revert to 20% rollout --- packages/gatsby/src/utils/flags.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index ab69eabca0dd5..c295a5a824207 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -1,6 +1,8 @@ import _ from "lodash" import semver from "semver" +import sampleSiteForExperiment from "./sample-site-for-experiment" + // Does this experiment run for only builds type executingCommand = "build" | "develop" | "all" @@ -92,7 +94,13 @@ const activeFlags: Array = [ experimental: false, description: `Server Side Render (SSR) pages on full reloads during develop. Helps you detect SSR bugs and fix them without needing to do full builds.`, umbrellaIssue: `https://gatsby.dev/dev-ssr-feedback`, - testFitness: (): fitnessEnum => `LOCKED_IN`, + testFitness: (): fitnessEnum => { + if (sampleSiteForExperiment(`DEV_SSR`, 20)) { + return `OPT_IN` + } else { + return true + } + }, }, { name: `QUERY_ON_DEMAND`, From 63e3f3d5a3e55d8ca7dd044973ddd2df62b0171b Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 24 Feb 2021 15:34:17 -0800 Subject: [PATCH 8/9] revert changes for 100% rollout --- .../cypress/integration/functionality/hooks.js | 2 +- .../integration/functionality/query-data-caches.js | 4 +--- .../cypress/integration/navigation/linking.js | 6 ++---- .../cypress/integration/navigation/redirect.js | 14 ++++---------- .../cypress/integration/page-not-found/404.js | 2 +- .../__tests__/lazy-image-build/develop.js | 2 +- 6 files changed, 10 insertions(+), 20 deletions(-) diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js b/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js index 265cbe2c2f83e..75595ad6c3c38 100644 --- a/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js +++ b/e2e-tests/development-runtime/cypress/integration/functionality/hooks.js @@ -2,7 +2,7 @@ const COUNT_ID = `count` describe(`hooks`, () => { beforeEach(() => { - cy.visit(`/hooks`, { failOnStatusCode: false }).waitForRouteChange() + cy.visit(`/hooks`).waitForRouteChange() }) it(`displays initial state`, () => { diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js b/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js index 8cea7334f6e32..c6b9148e77a9a 100644 --- a/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js +++ b/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js @@ -110,9 +110,7 @@ function pageTitleAndDataAssertion(config) { function runTests(config) { preTestSetup(config) - cy.visit(`/query-data-caches/${config.slug}/page-A/`, { - failOnStatusCode: false, - }).waitForRouteChange() + cy.visit(`/query-data-caches/${config.slug}/page-A/`).waitForRouteChange() setupForAssertingNotReloading() diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js index 7347635d66815..46e7d7763505b 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js @@ -79,9 +79,7 @@ describe(`navigation`, () => { describe(`non-existent route`, () => { beforeEach(() => { - cy.getTestElement(`broken-link`) - .click({ failOnStatusCode: false }) - .waitForRouteChange() + cy.getTestElement(`broken-link`).click().waitForRouteChange() }) it(`displays 404 page on broken link`, () => { @@ -136,7 +134,7 @@ describe(`navigation`, () => { }) it(`should show 404 page when url with unicode characters point to a non-existent page route when navigating on client`, () => { - cy.visit(`/`, { failOnStatusCode: false }).waitForRouteChange() + cy.visit(`/`).waitForRouteChange() cy.window() .then(win => win.___navigate(`/안녕404/`)) .waitForRouteChange() diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js index bde2ab94fc8f1..293930eda4c16 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js @@ -5,9 +5,7 @@ Cypress.on(`window:before:load`, win => { const runTests = () => { it(`should redirect page to index page when there is no such page`, () => { - cy.visit(`/redirect-without-page`, { - failOnStatusCode: false, - }).waitForRouteChange() + cy.visit(`/redirect-without-page`).waitForRouteChange() cy.location(`pathname`).should(`equal`, `/`) cy.then(() => { @@ -44,7 +42,7 @@ const runTests = () => { }) it(`should redirect to a dynamically-created replacement page`, () => { - cy.visit(`/redirect-me/`, { failOnStatusCode: false }).waitForRouteChange() + cy.visit(`/redirect-me/`).waitForRouteChange() cy.location(`pathname`).should(`equal`, `/pt/redirect-me/`) cy.then(() => { @@ -67,9 +65,7 @@ describe(`redirect`, () => { // this is sanity check for this group it(`make sure 404 is present`, () => { - cy.visit(`/______not_existing_page`, { - failOnStatusCode: false, - }).waitForRouteChange() + cy.visit(`/______not_existing_page`).waitForRouteChange() cy.findByText("Preview custom 404 page").click() cy.findByText("A custom 404 page wasn't detected", { exact: false, @@ -104,9 +100,7 @@ describe(`redirect`, () => { }) it(`make sure 404 is NOT present`, () => { - cy.visit(`/______not_existing_page`, { - failOnStatusCode: false, - }).waitForRouteChange() + cy.visit(`/______not_existing_page`).waitForRouteChange() cy.findByText("Preview custom 404 page").click() cy.findByText("A custom 404 page wasn't detected", { exact: false, diff --git a/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js b/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js index a7573caae6396..400e7c13a27e1 100644 --- a/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js +++ b/e2e-tests/development-runtime/cypress/integration/page-not-found/404.js @@ -1,6 +1,6 @@ describe(`page not found`, () => { beforeEach(() => { - cy.visit(`/__404__`, { failOnStatusCode: false }) + cy.visit(`/__404__`) }) it(`should display message `, () => { cy.get(`h1`).invoke(`text`).should(`eq`, `Gatsby.js development 404 page`) diff --git a/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js b/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js index 0231c8f831f43..b4f1abb947868 100644 --- a/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js +++ b/integration-tests/gatsby-pipeline/__tests__/lazy-image-build/develop.js @@ -29,7 +29,7 @@ describe(`Lazy images`, () => { await kill() - expect(response.status).toBe(404) + expect(response.status).toBe(200) const images = glob.sync(`${basePath}/public/**/*.png`) expect(images.length).toBe(6) From 0e95ada6391c6c3bdc827279b8e3c21f9b2920ab Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 25 Feb 2021 23:04:15 -0800 Subject: [PATCH 9/9] test this --- packages/gatsby/cache-dir/app.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/gatsby/cache-dir/app.js b/packages/gatsby/cache-dir/app.js index 76624f6e13119..312b5e641bdff 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -121,14 +121,13 @@ apiRunnerAsync(`onClientEntry`).then(() => { const rootElement = document.getElementById(`___gatsby`) + const focusEl = document.getElementById(`gatsby-focus-wrapper`) const renderer = apiRunner( `replaceHydrateFunction`, undefined, // Client only pages have any empty body so we just do a normal // render to avoid React complaining about hydration mis-matches. - document.getElementById(`gatsby-focus-wrapper`).children.length === 0 - ? ReactDOM.render - : ReactDOM.hydrate + focusEl && focusEl.children.length > 0 ? ReactDOM.hydrate : ReactDOM.render )[0] let dismissLoadingIndicator