diff --git a/e2e-tests/production-runtime/cypress/integration/ssr.js b/e2e-tests/production-runtime/cypress/integration/ssr.js index f191a0089cce9..89a305543dd86 100644 --- a/e2e-tests/production-runtime/cypress/integration/ssr.js +++ b/e2e-tests/production-runtime/cypress/integration/ssr.js @@ -1,6 +1,7 @@ const staticPath = `/ssr/static-path/` const paramPath = `/ssr/param-path/` const wildcardPath = `/ssr/wildcard-path/` +const pathRaking = `/ssr/path-ranking/` describe(`Static path ('${staticPath}')`, () => { it(`Direct visit no query params`, () => { @@ -72,6 +73,32 @@ describe(`Param path ('${paramPath}:param')`, () => { }) }) +describe(`Path ranking resolution ('${pathRaking}*')`, () => { + it(`Resolves to [...].js template at ${pathRaking}p1`, () => { + cy.visit(pathRaking + `p1/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"*":"p1"}`) + }) + + it(`Resolves to [p1]/[p2].js template at ${pathRaking}p1/p2`, () => { + cy.visit(pathRaking + `p1/p2/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"p1":"p1","p2":"p2"}`) + }) + + it(`Resolves to [p1]/page.js template at ${pathRaking}p1/page`, () => { + cy.visit(pathRaking + `p1/page/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"p1":"p1"}`) + }) + + it(`Resolves to [...].js template at ${pathRaking}p1/p2/p3`, () => { + cy.visit(pathRaking + `p1/p2/p3/`).waitForRouteChange() + cy.getTestElement(`query`).contains(`{}`) + cy.getTestElement(`params`).contains(`{"*":"p1/p2/p3"}`) + }) +}) + describe(`Wildcard path ('${wildcardPath}*')`, () => { it(`Direct visit no query params`, () => { cy.visit(wildcardPath + `foo/nested/`).waitForRouteChange() diff --git a/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[...].js b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[...].js new file mode 100644 index 0000000000000..ed3265bef789f --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[...].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/production-runtime/src/pages/ssr/path-ranking/[p1]/[p2].js b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[p1]/[p2].js new file mode 100644 index 0000000000000..ed3265bef789f --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[p1]/[p2].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/production-runtime/src/pages/ssr/path-ranking/[p1]/page.js b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[p1]/page.js new file mode 100644 index 0000000000000..ed3265bef789f --- /dev/null +++ b/e2e-tests/production-runtime/src/pages/ssr/path-ranking/[p1]/page.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/src/utils/__tests__/find-page-by-path.ts b/packages/gatsby/src/utils/__tests__/find-page-by-path.ts index ca3fbf10f630a..56175cd071946 100644 --- a/packages/gatsby/src/utils/__tests__/find-page-by-path.ts +++ b/packages/gatsby/src/utils/__tests__/find-page-by-path.ts @@ -41,7 +41,22 @@ const commonPages = [ path: `/app/`, matchPath: `/app/*`, }, + { + path: `/app/p1/page/`, + matchPath: `/app/:p1/page`, + }, + { + path: `/app/p1/p2/`, + matchPath: `/app/:p1/:p2`, + }, + { + path: `/app/p1/page2/`, + // this is very similar to `/app/:p1/page`, point of adding 2 of those is to make sure order of pages in state + // doesn't impact deterministic page selection + matchPath: `/app/:p1/page2`, + }, `/app/static/`, + `/app/static/page/`, ] const state = generatePagesState([...commonPages]) @@ -121,6 +136,38 @@ describe(`findPageByPath`, () => { expect(page?.path).toEqual(`/app/`) }) + it(`Picks most specific matchPath`, () => { + { + const page = findPageByPath(state, `/app/foo`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/`) + } + + { + const page = findPageByPath(state, `/app/foo/bar/baz`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/`) + } + + { + const page = findPageByPath(state, `/app/foo/bar`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/p1/p2/`) + } + + { + const page = findPageByPath(state, `/app/foo/page`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/p1/page/`) + } + + { + const page = findPageByPath(state, `/app/foo/page2`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/p1/page2/`) + } + }) + it(`Can match client-only path by static`, () => { const page = findPageByPath(state, `/app`) expect(page).toBeDefined() @@ -128,9 +175,17 @@ describe(`findPageByPath`, () => { }) it(`Will prefer static page over client-only in case both match`, () => { - const page = findPageByPath(state, `/app/static`) - expect(page).toBeDefined() - expect(page?.path).toEqual(`/app/static/`) + { + const page = findPageByPath(state, `/app/static`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/static/`) + } + + { + const page = findPageByPath(state, `/app/static/page`) + expect(page).toBeDefined() + expect(page?.path).toEqual(`/app/static/page/`) + } }) }) diff --git a/packages/gatsby/src/utils/find-page-by-path.ts b/packages/gatsby/src/utils/find-page-by-path.ts index ceeac70bd4d0d..c9e7f456eeca9 100644 --- a/packages/gatsby/src/utils/find-page-by-path.ts +++ b/packages/gatsby/src/utils/find-page-by-path.ts @@ -1,5 +1,44 @@ import { IGatsbyPage, IGatsbyState } from "../redux/types" -import { match } from "@gatsbyjs/reach-router/lib/utils" +import { pick } from "@gatsbyjs/reach-router/lib/utils" + +// Ranks and picks the best page to match. Each segment gets the highest +// amount of points, then the type of segment gets an additional amount of +// points where +// +// static > dynamic > splat > root +// +// This way we don't have to worry about the order of our pages, let the +// computers do it. +// +// In the future, we could move this pagesByMatchPath computation outside this +// function and save some processing power +const findBestMatchingPage = ( + pages: Map, + path: string +): IGatsbyPage | null => { + // Pick only routes with matchPath for better performance. + // Exact match should have already been checked + const pagesByMatchPath: Record = {} + for (const page of pages.values()) { + const matchPath = page.matchPath + if (matchPath) { + pagesByMatchPath[matchPath] = page + } + } + + const routes = Object.keys(pagesByMatchPath).map(path => { + return { path } + }) + + // picks best matching route with reach router's algorithm + const picked = pick(routes, path) + + if (picked) { + return pagesByMatchPath[picked.route.path] + } + + return null +} export function findPageByPath( state: IGatsbyState, @@ -46,10 +85,14 @@ export function findPageByPath( } // we didn't find exact static page, time to check matchPaths - for (const page of pages.values()) { - if (page.matchPath && match(page.matchPath, path)) { - return page - } + // TODO: consider using `match-paths.json` generated by `requires-writer` + // to avoid looping through all pages again. Ideally generate smaller `match-paths.json` + // variant that doesn't including overlapping static pages in `requires-writer` as well + // as this function already checked static paths at this point + const matchingPage = findBestMatchingPage(pages, path) + + if (matchingPage) { + return matchingPage } if (fallbackTo404) {