From 4943ff2d14152651eec57101a27e9120a726f0ba Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Thu, 16 Sep 2021 13:25:17 +0200 Subject: [PATCH 01/15] initial --- packages/gatsby-link/src/index.js | 13 ++-- packages/gatsby/cache-dir/app.js | 2 +- packages/gatsby/cache-dir/ensure-resources.js | 12 +++- packages/gatsby/cache-dir/find-path.js | 2 - packages/gatsby/cache-dir/loader.js | 62 +++++++++++-------- packages/gatsby/cache-dir/navigation.js | 2 +- packages/gatsby/cache-dir/production-app.js | 6 +- packages/gatsby/cache-dir/root.js | 6 +- .../src/utils/develop-preload-headers.ts | 2 +- 9 files changed, 64 insertions(+), 43 deletions(-) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index 47f6fc2e77767..c1abef043f2bb 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -117,17 +117,19 @@ class GatsbyLink extends React.Component { } _prefetch() { - let currentPath = window.location.pathname + let currentPath = window.location.pathname + window.location.search // reach router should have the correct state if (this.props._location && this.props._location.pathname) { - currentPath = this.props._location.pathname + currentPath = this.props._location.pathname + this.props._location.search } const rewrittenPath = rewriteLinkPath(this.props.to, currentPath) - const newPathName = parsePath(rewrittenPath).pathname + const parsed = parsePath(rewrittenPath) - // Prefech is used to speed up next navigations. When you use it on the current navigation, + const newPathName = parsed.pathname + parsed.search + + // Prefetch is used to speed up next navigations. When you use it on the current navigation, // there could be a race-condition where Chrome uses the stale data instead of waiting for the network to complete if (currentPath !== newPathName) { ___loader.enqueue(newPathName) @@ -224,7 +226,8 @@ class GatsbyLink extends React.Component { if (onMouseEnter) { onMouseEnter(e) } - ___loader.hovering(parsePath(prefixedTo).pathname) + const parsed = parsePath(prefixedTo) + ___loader.hovering(parsed.pathname + parsed.search) }} onClick={e => { if (onClick) { diff --git a/packages/gatsby/cache-dir/app.js b/packages/gatsby/cache-dir/app.js index dfd7cae909877..45f439e57bfd7 100644 --- a/packages/gatsby/cache-dir/app.js +++ b/packages/gatsby/cache-dir/app.js @@ -174,7 +174,7 @@ apiRunnerAsync(`onClientEntry`).then(() => { Promise.all([ loader.loadPage(`/dev-404-page/`), loader.loadPage(`/404.html`), - loader.loadPage(window.location.pathname), + loader.loadPage(window.location.pathname + window.location.search), ]).then(() => { navigationInit() diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 80bbb8d8dcdf9..4e37db710c16d 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -10,7 +10,9 @@ class EnsureResources extends React.Component { location: { ...location }, pageResources: pageResources || - loader.loadPageSync(location.pathname, { withErrorDetails: true }), + loader.loadPageSync(location.pathname + location.search, { + withErrorDetails: true, + }), } } @@ -48,7 +50,9 @@ class EnsureResources extends React.Component { shouldComponentUpdate(nextProps, nextState) { // Always return false if we're missing resources. if (!nextState.pageResources) { - this.loadResources(nextProps.location.pathname) + this.loadResources( + nextProps.location.pathname + nextProps.location.search + ) return false } @@ -56,7 +60,9 @@ class EnsureResources extends React.Component { process.env.BUILD_STAGE === `develop` && nextState.pageResources.stale ) { - this.loadResources(nextProps.location.pathname) + this.loadResources( + nextProps.location.pathname + nextProps.location.search + ) return false } diff --git a/packages/gatsby/cache-dir/find-path.js b/packages/gatsby/cache-dir/find-path.js index 969ce6a22765b..1b1d149d2051d 100644 --- a/packages/gatsby/cache-dir/find-path.js +++ b/packages/gatsby/cache-dir/find-path.js @@ -15,8 +15,6 @@ const trimPathname = rawPathname => { ) // Remove any hashfragment .split(`#`)[0] - // Remove search query - .split(`?`)[0] return trimmedPathname } diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index 3f08cabca0b56..671e7de173092 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -24,9 +24,12 @@ const stripSurroundingSlashes = s => { return s } -const createPageDataUrl = path => { +const createPageDataUrl = rawPath => { + const [path, maybeSearch] = rawPath.split(`?`) const fixedPath = path === `/` ? `index` : stripSurroundingSlashes(path) - return `${__PATH_PREFIX__}/page-data/${fixedPath}/page-data.json` + return `${__PATH_PREFIX__}/page-data/${fixedPath}/page-data.json${ + maybeSearch ? `?${maybeSearch}` : `` + }` } function doFetch(url, method = `GET`) { @@ -141,6 +144,11 @@ export class BaseLoader { throw new Error(`not a valid pageData response`) } + const maybeSearch = pagePath.split(`?`)[1] + if (maybeSearch) { + jsonPayload.path += `?${maybeSearch}` + } + return Object.assign(loadObj, { status: PageResourceStatus.Success, payload: jsonPayload, @@ -383,33 +391,34 @@ export class BaseLoader { } prefetch(pagePath) { - if (!this.shouldPrefetch(pagePath)) { - return false - } - - // Tell plugins with custom prefetching logic that they should start - // prefetching this path. - if (!this.prefetchTriggered.has(pagePath)) { - this.apiRunner(`onPrefetchPathname`, { pathname: pagePath }) - this.prefetchTriggered.add(pagePath) - } + return false + // if (!this.shouldPrefetch(pagePath)) { + // return false + // } - // If a plugin has disabled core prefetching, stop now. - if (this.prefetchDisabled) { - return false - } + // // Tell plugins with custom prefetching logic that they should start + // // prefetching this path. + // if (!this.prefetchTriggered.has(pagePath)) { + // this.apiRunner(`onPrefetchPathname`, { pathname: pagePath }) + // this.prefetchTriggered.add(pagePath) + // } - const realPath = findPath(pagePath) - // Todo make doPrefetch logic cacheable - // eslint-disable-next-line consistent-return - this.doPrefetch(realPath).then(() => { - if (!this.prefetchCompleted.has(pagePath)) { - this.apiRunner(`onPostPrefetchPathname`, { pathname: pagePath }) - this.prefetchCompleted.add(pagePath) - } - }) + // // If a plugin has disabled core prefetching, stop now. + // if (this.prefetchDisabled) { + // return false + // } - return true + // const realPath = findPath(pagePath) + // // Todo make doPrefetch logic cacheable + // // eslint-disable-next-line consistent-return + // this.doPrefetch(realPath).then(() => { + // if (!this.prefetchCompleted.has(pagePath)) { + // this.apiRunner(`onPostPrefetchPathname`, { pathname: pagePath }) + // this.prefetchCompleted.add(pagePath) + // } + // }) + + // return true } doPrefetch(pagePath) { @@ -571,6 +580,7 @@ export const publicLoader = { isPageNotFound: rawPath => instance.isPageNotFound(rawPath), hovering: rawPath => instance.hovering(rawPath), loadAppData: () => instance.loadAppData(), + instance: () => instance, } export default publicLoader diff --git a/packages/gatsby/cache-dir/navigation.js b/packages/gatsby/cache-dir/navigation.js index 6a7b1ae3f09c8..16e32736f1002 100644 --- a/packages/gatsby/cache-dir/navigation.js +++ b/packages/gatsby/cache-dir/navigation.js @@ -85,7 +85,7 @@ const navigate = (to, options = {}) => { }) }, 1000) - loader.loadPage(pathname).then(pageResources => { + loader.loadPage(pathname + search).then(pageResources => { // If no page resources, then refresh the page // Do this, rather than simply `window.location.reload()`, so that // pressing the back/forward buttons work - otherwise when pressing diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js index 5633e5863a706..6875e83554e71 100644 --- a/packages/gatsby/cache-dir/production-app.js +++ b/packages/gatsby/cache-dir/production-app.js @@ -107,8 +107,10 @@ apiRunnerAsync(`onClientEntry`).then(() => { pageResources.page.path === `/404.html` ? stripPrefix(location.pathname, __BASE_PATH__) : encodeURI( - pageResources.page.matchPath || + ( + pageResources.page.matchPath || pageResources.page.path + ).split(`?`)[0] ) } {...this.props} @@ -149,7 +151,7 @@ apiRunnerAsync(`onClientEntry`).then(() => { }) } - publicLoader.loadPage(browserLoc.pathname).then(page => { + publicLoader.loadPage(browserLoc.pathname + browserLoc.search).then(page => { if (!page || page.status === PageResourceStatus.Error) { const message = `page resources for ${browserLoc.pathname} not found. Not rendering React` diff --git a/packages/gatsby/cache-dir/root.js b/packages/gatsby/cache-dir/root.js index fc0b1625c3595..66c034d7641dd 100644 --- a/packages/gatsby/cache-dir/root.js +++ b/packages/gatsby/cache-dir/root.js @@ -32,7 +32,7 @@ class LocationHandler extends React.Component { render() { const { location } = this.props - if (!loader.isPageNotFound(location.pathname)) { + if (!loader.isPageNotFound(location.pathname + location.search)) { return ( {locationAndPageResources => ( @@ -48,8 +48,10 @@ class LocationHandler extends React.Component { > Date: Thu, 16 Sep 2021 20:00:59 +0200 Subject: [PATCH 02/15] tmp --- packages/gatsby/cache-dir/loader.js | 49 +++++++++---------- packages/gatsby/cache-dir/production-app.js | 4 +- .../gatsby/src/utils/page-ssr-module/entry.ts | 30 +++++++++--- 3 files changed, 48 insertions(+), 35 deletions(-) diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index 671e7de173092..ce3d40ecf92cf 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -391,34 +391,33 @@ export class BaseLoader { } prefetch(pagePath) { - return false - // if (!this.shouldPrefetch(pagePath)) { - // return false - // } + if (!this.shouldPrefetch(pagePath)) { + return false + } - // // Tell plugins with custom prefetching logic that they should start - // // prefetching this path. - // if (!this.prefetchTriggered.has(pagePath)) { - // this.apiRunner(`onPrefetchPathname`, { pathname: pagePath }) - // this.prefetchTriggered.add(pagePath) - // } + // Tell plugins with custom prefetching logic that they should start + // prefetching this path. + if (!this.prefetchTriggered.has(pagePath)) { + this.apiRunner(`onPrefetchPathname`, { pathname: pagePath }) + this.prefetchTriggered.add(pagePath) + } - // // If a plugin has disabled core prefetching, stop now. - // if (this.prefetchDisabled) { - // return false - // } + // If a plugin has disabled core prefetching, stop now. + if (this.prefetchDisabled) { + return false + } + + const realPath = findPath(pagePath) + // Todo make doPrefetch logic cacheable + // eslint-disable-next-line consistent-return + this.doPrefetch(realPath).then(() => { + if (!this.prefetchCompleted.has(pagePath)) { + this.apiRunner(`onPostPrefetchPathname`, { pathname: pagePath }) + this.prefetchCompleted.add(pagePath) + } + }) - // const realPath = findPath(pagePath) - // // Todo make doPrefetch logic cacheable - // // eslint-disable-next-line consistent-return - // this.doPrefetch(realPath).then(() => { - // if (!this.prefetchCompleted.has(pagePath)) { - // this.apiRunner(`onPostPrefetchPathname`, { pathname: pagePath }) - // this.prefetchCompleted.add(pagePath) - // } - // }) - - // return true + return true } doPrefetch(pagePath) { diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js index 6875e83554e71..4768225cf94c0 100644 --- a/packages/gatsby/cache-dir/production-app.js +++ b/packages/gatsby/cache-dir/production-app.js @@ -138,7 +138,7 @@ apiRunnerAsync(`onClientEntry`).then(() => { // - it's the offline plugin shell (/offline-plugin-app-shell-fallback/) if ( pagePath && - __BASE_PATH__ + pagePath !== browserLoc.pathname && + __BASE_PATH__ + pagePath !== browserLoc.pathname + browserLoc.search && !( loader.findMatchPath(stripPrefix(browserLoc.pathname, __BASE_PATH__)) || pagePath === `/404.html` || @@ -146,7 +146,7 @@ apiRunnerAsync(`onClientEntry`).then(() => { pagePath.match(/^\/offline-plugin-app-shell-fallback\/?$/) ) ) { - navigate(__BASE_PATH__ + pagePath + browserLoc.search + browserLoc.hash, { + navigate(__BASE_PATH__ + pagePath + browserLoc.hash, { replace: true, }) } diff --git a/packages/gatsby/src/utils/page-ssr-module/entry.ts b/packages/gatsby/src/utils/page-ssr-module/entry.ts index 221244265a970..b8c48eb85c603 100644 --- a/packages/gatsby/src/utils/page-ssr-module/entry.ts +++ b/packages/gatsby/src/utils/page-ssr-module/entry.ts @@ -29,6 +29,7 @@ export interface ISSRData { templateDetails: ITemplateDetails potentialPagePath: string serverDataHeaders?: Record + searchString: string } const pageTemplateDetailsMap: Record< @@ -101,15 +102,34 @@ export async function getData({ } results.pageContext = page.context + let searchString = `` + if (req?.query) { + const maybeQueryString = Object.entries(req.query) + .map(([k, v]) => `${k}=${v}`) + .join(`&`) + if (maybeQueryString) { + searchString = `?${maybeQueryString}` + } + } + return { results, page, templateDetails, potentialPagePath, serverDataHeaders: serverData?.headers, + searchString, } } +function getPath(data: ISSRData): string { + return ( + (data.page.mode !== `SSG` && data.page.matchPath + ? data.potentialPagePath + : data.page.path) + data.searchString + ) +} + export async function renderPageData({ data, }: { @@ -118,10 +138,7 @@ export async function renderPageData({ const results = await constructPageDataString( { componentChunkName: data.page.componentChunkName, - path: - data.page.mode !== `SSG` && data.page.matchPath - ? data.potentialPagePath - : data.page.path, + path: getPath(data), matchPath: data.page.matchPath, staticQueryHashes: data.templateDetails.staticQueryHashes, }, @@ -161,10 +178,7 @@ export async function renderHTML({ ) const results = await htmlComponentRenderer({ - pagePath: - data.page.mode !== `SSG` && data.page.matchPath - ? data.potentialPagePath - : data.page.path, + pagePath: getPath(data), pageData, staticQueryContext, ...data.templateDetails.assets, From 8fa1ca77a353a649277862696ce643b7ec14d626 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 22 Sep 2021 14:05:17 +0200 Subject: [PATCH 03/15] fix ensure-resources unit test --- packages/gatsby/cache-dir/__tests__/ensure-resources.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/gatsby/cache-dir/__tests__/ensure-resources.tsx b/packages/gatsby/cache-dir/__tests__/ensure-resources.tsx index 8d5841c614fbd..472617ee2b983 100644 --- a/packages/gatsby/cache-dir/__tests__/ensure-resources.tsx +++ b/packages/gatsby/cache-dir/__tests__/ensure-resources.tsx @@ -22,6 +22,7 @@ describe(`EnsureResources`, () => { it(`loads pages synchronously`, () => { const location = { pathname: `/`, + search: ``, } const { container } = render( From 14e0658f62fb4f47ca8559f9dc580ea437b3e8b5 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 27 Sep 2021 14:22:32 +0200 Subject: [PATCH 04/15] check search params only when pagepath contains them --- packages/gatsby/cache-dir/production-app.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js index 4768225cf94c0..225297cf018a2 100644 --- a/packages/gatsby/cache-dir/production-app.js +++ b/packages/gatsby/cache-dir/production-app.js @@ -130,15 +130,18 @@ apiRunnerAsync(`onClientEntry`).then(() => { const { pagePath, location: browserLoc } = window // Explicitly call navigate if the canonical path (window.pagePath) - // is different to the browser path (window.location.pathname). But - // only if NONE of the following conditions hold: + // is different to the browser path (window.location.pathname). SSR + // page paths might include search params, while SSG and DSG won't. + // If page path include search params we also compare query params. + // But only if NONE of the following conditions hold: // // - The url matches a client side route (page.matchPath) // - it's a 404 page // - it's the offline plugin shell (/offline-plugin-app-shell-fallback/) if ( pagePath && - __BASE_PATH__ + pagePath !== browserLoc.pathname + browserLoc.search && + __BASE_PATH__ + pagePath !== + browserLoc.pathname + (pagePath.includes(`?`) ? browserLoc.search : ``) && !( loader.findMatchPath(stripPrefix(browserLoc.pathname, __BASE_PATH__)) || pagePath === `/404.html` || From 6fea21c3cff54b1538d368c1898caabd10096df6 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 27 Sep 2021 15:35:06 +0200 Subject: [PATCH 05/15] strip and reattach search params when normalizing page path --- .../gatsby/cache-dir/normalize-page-path.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/gatsby/cache-dir/normalize-page-path.js b/packages/gatsby/cache-dir/normalize-page-path.js index e3aa70b2f45c5..51dd49a6b82b9 100644 --- a/packages/gatsby/cache-dir/normalize-page-path.js +++ b/packages/gatsby/cache-dir/normalize-page-path.js @@ -1,12 +1,17 @@ -export default path => { - if (path === undefined) { - return path +export default pathAndSearch => { + if (pathAndSearch === undefined) { + return pathAndSearch } + let [path, search = ``] = pathAndSearch.split(`?`) + if (search) { + search = `?` + search + } + if (path === `/`) { - return `/` + return `/` + search } if (path.charAt(path.length - 1) === `/`) { - return path.slice(0, -1) + return path.slice(0, -1) + search } - return path + return path + search } From f89ac169e5f1fbcc4d63448c2c0e4004eb349369 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 13:21:41 +0200 Subject: [PATCH 06/15] fix redirects race --- .../production-runtime/cypress/integration/redirects.js | 6 +++--- packages/gatsby/cache-dir/navigation.js | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/e2e-tests/production-runtime/cypress/integration/redirects.js b/e2e-tests/production-runtime/cypress/integration/redirects.js index 49229e27fa8dc..82e83350723ca 100644 --- a/e2e-tests/production-runtime/cypress/integration/redirects.js +++ b/e2e-tests/production-runtime/cypress/integration/redirects.js @@ -75,7 +75,7 @@ describe(`Redirects`, () => { failOnStatusCode: false, }).waitForRouteChange() - cy.location(`pathname`).should(`equal`, `/redirect-search-hash/`) + cy.location(`pathname`).should(`equal`, `/redirect-search-hash`) cy.location(`hash`).should(`equal`, `#anchor`) cy.location(`search`).should(`equal`, ``) }) @@ -96,7 +96,7 @@ describe(`Redirects`, () => { failOnStatusCode: false, }).waitForRouteChange() - cy.location(`pathname`).should(`equal`, `/redirect-search-hash/`) + cy.location(`pathname`).should(`equal`, `/redirect-search-hash`) cy.location(`hash`).should(`equal`, ``) cy.location(`search`).should(`equal`, `?query_param=hello`) }) @@ -117,7 +117,7 @@ describe(`Redirects`, () => { failOnStatusCode: false, }).waitForRouteChange() - cy.location(`pathname`).should(`equal`, `/redirect-search-hash/`) + cy.location(`pathname`).should(`equal`, `/redirect-search-hash`) cy.location(`hash`).should(`equal`, `#anchor`) cy.location(`search`).should(`equal`, `?query_param=hello`) }) diff --git a/packages/gatsby/cache-dir/navigation.js b/packages/gatsby/cache-dir/navigation.js index 16e32736f1002..14b43091ab251 100644 --- a/packages/gatsby/cache-dir/navigation.js +++ b/packages/gatsby/cache-dir/navigation.js @@ -172,9 +172,6 @@ function init() { window.___push = to => navigate(to, { replace: false }) window.___replace = to => navigate(to, { replace: true }) window.___navigate = (to, options) => navigate(to, options) - - // Check for initial page-load redirect - maybeRedirect(window.location.pathname) } class RouteAnnouncer extends React.Component { From 2b31f2b267f2a5bdf858620b853e98d549026478 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 14:15:23 +0200 Subject: [PATCH 07/15] barebones ssr e2e tests --- .../cypress/integration/ssr.js | 36 +++++++++++++++++++ .../src/pages/ssr/static-path.js | 22 ++++++++++++ packages/gatsby/cache-dir/ensure-resources.js | 9 +++-- packages/gatsby/cache-dir/loader.js | 2 +- 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 e2e-tests/production-runtime/cypress/integration/ssr.js create mode 100644 e2e-tests/production-runtime/src/pages/ssr/static-path.js diff --git a/e2e-tests/production-runtime/cypress/integration/ssr.js b/e2e-tests/production-runtime/cypress/integration/ssr.js new file mode 100644 index 0000000000000..165e6c82719f6 --- /dev/null +++ b/e2e-tests/production-runtime/cypress/integration/ssr.js @@ -0,0 +1,36 @@ +const staticPath = `/ssr/static-path/` + +describe(`Static path ('${staticPath}')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(staticPath).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(staticPath + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{}`) + }) + + it(`Client navigation to same path with different query params`, () => { + cy.visit(staticPath).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath + `?foo=baz`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"baz"}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + }) +}) diff --git a/e2e-tests/production-runtime/src/pages/ssr/static-path.js b/e2e-tests/production-runtime/src/pages/ssr/static-path.js new file mode 100644 index 0000000000000..ed3265bef789f --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/static-path.js @@ -0,0 +1,22 @@ +import React from "react" + +export default function StaticPath({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} diff --git a/packages/gatsby/cache-dir/ensure-resources.js b/packages/gatsby/cache-dir/ensure-resources.js index 4e37db710c16d..b1880c92b643b 100644 --- a/packages/gatsby/cache-dir/ensure-resources.js +++ b/packages/gatsby/cache-dir/ensure-resources.js @@ -18,9 +18,12 @@ class EnsureResources extends React.Component { static getDerivedStateFromProps({ location }, prevState) { if (prevState.location.href !== location.href) { - const pageResources = loader.loadPageSync(location.pathname, { - withErrorDetails: true, - }) + const pageResources = loader.loadPageSync( + location.pathname + location.search, + { + withErrorDetails: true, + } + ) return { pageResources, diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index ce3d40ecf92cf..47dcd4455cde6 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -145,7 +145,7 @@ export class BaseLoader { } const maybeSearch = pagePath.split(`?`)[1] - if (maybeSearch) { + if (maybeSearch && !jsonPayload.path.includes(maybeSearch)) { jsonPayload.path += `?${maybeSearch}` } From f8610e8be73dfe190c3fb4a0e055c4b440494166 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 14:42:51 +0200 Subject: [PATCH 08/15] add tests for dynamic paths --- .../cypress/integration/ssr.js | 72 +++++++++++++++++++ .../src/pages/ssr/param-path/[param].js | 22 ++++++ .../pages/ssr/wildcard-path/[...wildcard].js | 22 ++++++ 3 files changed, 116 insertions(+) create mode 100644 e2e-tests/production-runtime/src/pages/ssr/param-path/[param].js create mode 100644 e2e-tests/production-runtime/src/pages/ssr/wildcard-path/[...wildcard].js diff --git a/e2e-tests/production-runtime/cypress/integration/ssr.js b/e2e-tests/production-runtime/cypress/integration/ssr.js index 165e6c82719f6..6a28f0fef858b 100644 --- a/e2e-tests/production-runtime/cypress/integration/ssr.js +++ b/e2e-tests/production-runtime/cypress/integration/ssr.js @@ -1,4 +1,6 @@ const staticPath = `/ssr/static-path/` +const paramPath = `/ssr/param-path/` +const wildcardPath = `/ssr/wildcard-path/` describe(`Static path ('${staticPath}')`, () => { it(`Direct visit no query params`, () => { @@ -34,3 +36,73 @@ describe(`Static path ('${staticPath}')`, () => { cy.getTestElement(`params`).contains(`{}`) }) }) + +describe(`Param path ('${paramPath}:param')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(paramPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(paramPath + `foo/` + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + }) + + it(`Client navigation to same param path with different query params and url params`, () => { + cy.visit(paramPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + cy.window() + .then(win => win.___navigate(paramPath + `foo/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + cy.window() + .then(win => win.___navigate(paramPath + `baz/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"baz"}`) + cy.window() + .then(win => win.___navigate(paramPath + `baz/`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"baz"}`) + }) +}) + +describe.only(`Wildcard path ('${wildcardPath}*')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(wildcardPath + `foo/nested/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo/nested"}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(wildcardPath + `foo/nested/` + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo/nested"}`) + }) + + it(`Client navigation to same param path with different query params and url params`, () => { + cy.visit(wildcardPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `foo/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `baz/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"baz"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `baz/`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"baz"}`) + }) +}) diff --git a/e2e-tests/production-runtime/src/pages/ssr/param-path/[param].js b/e2e-tests/production-runtime/src/pages/ssr/param-path/[param].js new file mode 100644 index 0000000000000..af860ab139565 --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/param-path/[param].js @@ -0,0 +1,22 @@ +import React from "react" + +export default function Params({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} diff --git a/e2e-tests/production-runtime/src/pages/ssr/wildcard-path/[...wildcard].js b/e2e-tests/production-runtime/src/pages/ssr/wildcard-path/[...wildcard].js new file mode 100644 index 0000000000000..9af5b2c9e2f71 --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/wildcard-path/[...wildcard].js @@ -0,0 +1,22 @@ +import React from "react" + +export default function Wildcard({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} From a9cde80d0fef8843fff252b32e5b18ca53fd8d1c Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 15:30:15 +0200 Subject: [PATCH 09/15] fix(serve): let matchPath router handle path and don't 404 immediatelly --- packages/gatsby/src/commands/serve.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/gatsby/src/commands/serve.ts b/packages/gatsby/src/commands/serve.ts index b4e409bb8bff6..c8ef57fb61d37 100644 --- a/packages/gatsby/src/commands/serve.ts +++ b/packages/gatsby/src/commands/serve.ts @@ -295,8 +295,6 @@ module.exports = async (program: IServeProgram): Promise => { } return res.send(results) } - - return res.status(404).sendFile(`404.html`, { root }) } return next() }) From 25688033b157f0cd28ae5a536d769762aa6d7dd2 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 15:33:18 +0200 Subject: [PATCH 10/15] add same e2e tests to dev runtime tests --- .../cypress/integration/ssr.js | 108 ++++++++++++++++++ .../src/pages/ssr/param-path/[param].js | 22 ++++ .../src/pages/ssr/static-path.js | 22 ++++ .../pages/ssr/wildcard-path/[...wildcard].js | 22 ++++ 4 files changed, 174 insertions(+) create mode 100644 e2e-tests/development-runtime/cypress/integration/ssr.js create mode 100644 e2e-tests/development-runtime/src/pages/ssr/param-path/[param].js create mode 100644 e2e-tests/development-runtime/src/pages/ssr/static-path.js create mode 100644 e2e-tests/development-runtime/src/pages/ssr/wildcard-path/[...wildcard].js diff --git a/e2e-tests/development-runtime/cypress/integration/ssr.js b/e2e-tests/development-runtime/cypress/integration/ssr.js new file mode 100644 index 0000000000000..6a28f0fef858b --- /dev/null +++ b/e2e-tests/development-runtime/cypress/integration/ssr.js @@ -0,0 +1,108 @@ +const staticPath = `/ssr/static-path/` +const paramPath = `/ssr/param-path/` +const wildcardPath = `/ssr/wildcard-path/` + +describe(`Static path ('${staticPath}')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(staticPath).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(staticPath + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{}`) + }) + + it(`Client navigation to same path with different query params`, () => { + cy.visit(staticPath).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath + `?foo=baz`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"baz"}`) + cy.getTestElement(`params`).contains(`{}`) + cy.window() + .then(win => win.___navigate(staticPath)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{}`) + }) +}) + +describe(`Param path ('${paramPath}:param')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(paramPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(paramPath + `foo/` + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + }) + + it(`Client navigation to same param path with different query params and url params`, () => { + cy.visit(paramPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + cy.window() + .then(win => win.___navigate(paramPath + `foo/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"foo"}`) + cy.window() + .then(win => win.___navigate(paramPath + `baz/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"param":"baz"}`) + cy.window() + .then(win => win.___navigate(paramPath + `baz/`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"param":"baz"}`) + }) +}) + +describe.only(`Wildcard path ('${wildcardPath}*')`, () => { + it(`Direct visit no query params`, () => { + cy.visit(wildcardPath + `foo/nested/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo/nested"}`) + }) + + it(`Direct visit with query params`, () => { + cy.visit(wildcardPath + `foo/nested/` + `?foo=bar`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo/nested"}`) + }) + + it(`Client navigation to same param path with different query params and url params`, () => { + cy.visit(wildcardPath + `foo/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `foo/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"foo"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `baz/` + `?foo=bar`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{"foo":"bar"}`) + cy.getTestElement(`params`).contains(`{"wildcard":"baz"}`) + cy.window() + .then(win => win.___navigate(wildcardPath + `baz/`)) + .waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"wildcard":"baz"}`) + }) +}) diff --git a/e2e-tests/development-runtime/src/pages/ssr/param-path/[param].js b/e2e-tests/development-runtime/src/pages/ssr/param-path/[param].js new file mode 100644 index 0000000000000..af860ab139565 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/ssr/param-path/[param].js @@ -0,0 +1,22 @@ +import React from "react" + +export default function Params({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} diff --git a/e2e-tests/development-runtime/src/pages/ssr/static-path.js b/e2e-tests/development-runtime/src/pages/ssr/static-path.js new file mode 100644 index 0000000000000..ed3265bef789f --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/ssr/static-path.js @@ -0,0 +1,22 @@ +import React from "react" + +export default function StaticPath({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} diff --git a/e2e-tests/development-runtime/src/pages/ssr/wildcard-path/[...wildcard].js b/e2e-tests/development-runtime/src/pages/ssr/wildcard-path/[...wildcard].js new file mode 100644 index 0000000000000..9af5b2c9e2f71 --- /dev/null +++ b/e2e-tests/development-runtime/src/pages/ssr/wildcard-path/[...wildcard].js @@ -0,0 +1,22 @@ +import React from "react" + +export default function Wildcard({ serverData }) { + return ( +
+

Query

+ 
{JSON.stringify(serverData?.arg?.query)}
+

Params

+ 
{JSON.stringify(serverData?.arg?.params)}
+

Debug

+ 
{JSON.stringify({ serverData }, null, 2)}
+
+ ) +} + +export function getServerData(arg) { + return { + props: { + arg, + }, + } +} From 9a2e4db8f2289c1e8b97c7ff8ac0c3b1afcefb80 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 16:45:28 +0200 Subject: [PATCH 11/15] revert debugging code --- packages/gatsby/cache-dir/loader.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index 47dcd4455cde6..161205943d08f 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -579,7 +579,6 @@ export const publicLoader = { isPageNotFound: rawPath => instance.isPageNotFound(rawPath), hovering: rawPath => instance.hovering(rawPath), loadAppData: () => instance.loadAppData(), - instance: () => instance, } export default publicLoader From 0d4d98fcf15817c92f95196ecb062a4a8fa4af5a Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 16:47:27 +0200 Subject: [PATCH 12/15] drop no longer relevant comment --- packages/gatsby/cache-dir/navigation.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/gatsby/cache-dir/navigation.js b/packages/gatsby/cache-dir/navigation.js index 14b43091ab251..9bf8fbbb43679 100644 --- a/packages/gatsby/cache-dir/navigation.js +++ b/packages/gatsby/cache-dir/navigation.js @@ -102,10 +102,6 @@ const navigate = (to, options = {}) => { // If the loaded page has a different compilation hash to the // window, then a rebuild has occurred on the server. Reload. if (process.env.NODE_ENV === `production` && pageResources) { - // window.___webpackCompilationHash gets set in production-app.js after navigationInit() is called - // So on a direct visit of a page with a browser redirect this check is truthy and thus the codepath is hit - // While the resource actually exists, but only too late - // TODO: This should probably be fixed by setting ___webpackCompilationHash before navigationInit() is called if ( pageResources.page.webpackCompilationHash !== window.___webpackCompilationHash From ae0d15836fb3394f19c2c8e1422f37a38ec7bfec Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 18:47:25 +0200 Subject: [PATCH 13/15] fix(engine): only add search param to page path if it's SSR --- packages/gatsby/src/utils/page-ssr-module/entry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/page-ssr-module/entry.ts b/packages/gatsby/src/utils/page-ssr-module/entry.ts index b8c48eb85c603..fad7870043674 100644 --- a/packages/gatsby/src/utils/page-ssr-module/entry.ts +++ b/packages/gatsby/src/utils/page-ssr-module/entry.ts @@ -126,7 +126,7 @@ function getPath(data: ISSRData): string { return ( (data.page.mode !== `SSG` && data.page.matchPath ? data.potentialPagePath - : data.page.path) + data.searchString + : data.page.path) + (data.page.mode === `SSR` ? data.searchString : ``) ) } From 5a6855e441399492b0c0aa5d15fcbee79b913e1c Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 19:54:24 +0200 Subject: [PATCH 14/15] Update e2e-tests/production-runtime/cypress/integration/ssr.js Co-authored-by: Dustin Schau --- e2e-tests/production-runtime/cypress/integration/ssr.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/production-runtime/cypress/integration/ssr.js b/e2e-tests/production-runtime/cypress/integration/ssr.js index 6a28f0fef858b..00dc2236c4fd4 100644 --- a/e2e-tests/production-runtime/cypress/integration/ssr.js +++ b/e2e-tests/production-runtime/cypress/integration/ssr.js @@ -72,7 +72,7 @@ describe(`Param path ('${paramPath}:param')`, () => { }) }) -describe.only(`Wildcard path ('${wildcardPath}*')`, () => { +describe(`Wildcard path ('${wildcardPath}*')`, () => { it(`Direct visit no query params`, () => { cy.visit(wildcardPath + `foo/nested/`).waitForRouteChange() cy.getTestElement(`query`).contains(`{}`) From 8726ece2e61e3e62a872190f0af1578491e259b4 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 28 Sep 2021 19:56:42 +0200 Subject: [PATCH 15/15] un-only also in dev runtime variant --- e2e-tests/development-runtime/cypress/integration/ssr.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/development-runtime/cypress/integration/ssr.js b/e2e-tests/development-runtime/cypress/integration/ssr.js index 6a28f0fef858b..00dc2236c4fd4 100644 --- a/e2e-tests/development-runtime/cypress/integration/ssr.js +++ b/e2e-tests/development-runtime/cypress/integration/ssr.js @@ -72,7 +72,7 @@ describe(`Param path ('${paramPath}:param')`, () => { }) }) -describe.only(`Wildcard path ('${wildcardPath}*')`, () => { +describe(`Wildcard path ('${wildcardPath}*')`, () => { it(`Direct visit no query params`, () => { cy.visit(wildcardPath + `foo/nested/`).waitForRouteChange() cy.getTestElement(`query`).contains(`{}`)