From 808e080120d1462ec14e6feab6df7a017325ef0c Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 24 Feb 2021 14:16:01 -0800 Subject: [PATCH 1/4] fix(gatsby): Fix various small DEV_SSR bugs exposed in development_runtime tests in v2 --- .../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 +- packages/gatsby/cache-dir/app.js | 2 +- .../gatsby/src/utils/dev-ssr/develop-html-route.ts | 6 +++--- .../gatsby/src/utils/dev-ssr/render-dev-html.ts | 7 ++++++- .../gatsby/src/utils/develop-preload-headers.ts | 2 +- 10 files changed, 31 insertions(+), 16 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 450d61d9fa8ae..808bd31c49cfe 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`) 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) diff --git a/packages/gatsby/cache-dir/app.js b/packages/gatsby/cache-dir/app.js index 0927b5999202f..3eb530b5d90a8 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -121,7 +121,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] 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.

error

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 16004b36bbefb..60e1c511cac5d 100644 --- a/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts +++ b/packages/gatsby/src/utils/dev-ssr/render-dev-html.ts @@ -58,8 +58,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) { 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 c6f66434a6699143b879cd176611dda2d4d18384 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Wed, 24 Feb 2021 15:32:16 -0800 Subject: [PATCH 2/4] Revert changes to tests for full 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 808bd31c49cfe..450d61d9fa8ae 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 04af7489f28378db4f9cb00b6fc702712142ef28 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Thu, 25 Feb 2021 16:35:29 -0800 Subject: [PATCH 3/4] Will this fix things?? --- 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 26464fcab2dd9..d5a7416608ad6 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -122,7 +122,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-focus-wrapper`).children.length === 0 + document.getElementById(`___gatsby`).children.length === 0 ? ReactDOM.render : ReactDOM.hydrate )[0] From 0e9399a0b7b05ae841102259905fb140707e763d Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Fri, 26 Feb 2021 11:34:22 -0800 Subject: [PATCH 4/4] Better check for SSR --- 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 d5a7416608ad6..0d2e6cedd95e2 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -117,14 +117,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`).children.length === 0 - ? ReactDOM.render - : ReactDOM.hydrate + focusEl && focusEl.children.length > 0 ? ReactDOM.hydrate : ReactDOM.render )[0] let dismissLoadingIndicator