From c10bf51125f6c601902d62f5336b32a1a4c05044 Mon Sep 17 00:00:00 2001 From: Lennart Date: Thu, 20 Oct 2022 14:29:20 +0200 Subject: [PATCH] fix(gatsby): Set trailingSlash default to "always" (#36798) --- .../functionality/client-only-paths.js | 12 +- .../functionality/query-data-caches.js | 2 +- .../cypress/integration/navigation/linking.js | 8 +- .../integration/navigation/redirect.js | 22 +- e2e-tests/development-runtime/gatsby-node.js | 8 +- .../development-runtime/src/pages/index.js | 6 +- .../cypress/integration/navigate.js | 8 +- .../cypress/integration/1-production.js | 14 +- .../cypress/integration/redirects.js | 24 +- e2e-tests/production-runtime/gatsby-node.ts | 12 +- .../production-runtime/src/pages/index.js | 6 +- e2e-tests/trailing-slash/cypress-legacy.json | 5 - .../cypress/integration/always.js | 6 + .../cypress/integration/ignore.js | 18 +- .../cypress/integration/legacy.js | 274 ------------------ .../cypress/integration/never.js | 6 + e2e-tests/trailing-slash/gatsby-config.js | 2 +- e2e-tests/trailing-slash/package.json | 7 +- .../artifacts/__tests__/index.js | 6 +- .../__tests__/create-node-manifest.test.js | 8 +- .../src/__tests__/rewrite-link-path.js | 33 ++- packages/gatsby-link/src/rewrite-link-path.js | 14 +- .../__tests__/apply-trailing-slash-option.ts | 6 - .../src/__tests__/create-path.ts | 12 +- .../src/apply-trailing-slash-option.ts | 5 +- packages/gatsby-page-utils/src/create-path.ts | 3 +- .../src/gatsby-node.ts | 23 +- packages/gatsby/index.d.ts | 4 +- .../bootstrap/__tests__/redirects-writer.ts | 2 +- .../gatsby/src/joi-schemas/__tests__/joi.ts | 8 +- packages/gatsby/src/joi-schemas/joi.ts | 4 +- .../src/query/__tests__/data-tracking.js | 74 ++--- .../src/redux/actions/__tests__/internal.ts | 4 +- .../src/services/__tests__/create-pages.ts | 30 +- .../utils/__tests__/gatsby-cloud-config.ts | 2 +- .../gatsby/src/utils/gatsby-cloud-config.ts | 2 +- 36 files changed, 207 insertions(+), 473 deletions(-) delete mode 100644 e2e-tests/trailing-slash/cypress-legacy.json delete mode 100644 e2e-tests/trailing-slash/cypress/integration/legacy.js diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/client-only-paths.js b/e2e-tests/development-runtime/cypress/integration/functionality/client-only-paths.js index 72b7dc1a4f746..c2a5240d14dd5 100644 --- a/e2e-tests/development-runtime/cypress/integration/functionality/client-only-paths.js +++ b/e2e-tests/development-runtime/cypress/integration/functionality/client-only-paths.js @@ -1,32 +1,32 @@ describe(`Client only paths`, () => { const routes = [ { - path: `/client-only-paths`, + path: `/client-only-paths/`, marker: `index`, label: `Index route`, }, { - path: `/client-only-paths/page/profile`, + path: `/client-only-paths/page/profile/`, marker: `profile`, label: `Dynamic route`, }, { - path: `/client-only-paths/not-found`, + path: `/client-only-paths/not-found/`, marker: `NotFound`, label: `Default route (not found)`, }, { - path: `/client-only-paths/nested`, + path: `/client-only-paths/nested/`, marker: `nested-page/index`, label: `Index route inside nested router`, }, { - path: `/client-only-paths/nested/foo`, + path: `/client-only-paths/nested/foo/`, marker: `nested-page/foo`, label: `Dynamic route inside nested router`, }, { - path: `/client-only-paths/static`, + path: `/client-only-paths/static/`, marker: `static-sibling`, label: `Static route that is a sibling to client only path`, }, 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 e8a999aa5ac0a..182c8ece8bf62 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 @@ -73,7 +73,7 @@ function getExpectedCanonicalPath(config) { return `/query-data-caches/${config.slug}/${ config.page === `client-only` ? `:client-only` - : `page-${config.page}${config.trailingSlash === false ? `` : `/`}` + : `page-${config.page}/` }` } diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js index 44cbf52dd3b34..c53d5d36920da 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js @@ -46,7 +46,7 @@ describe(`navigation`, () => { cy.getTestElement(`subdir-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-1`) + .should(`eq`, `/subdirectory/page-1/`) }) it(`can navigate to a sibling page`, () => { @@ -55,7 +55,7 @@ describe(`navigation`, () => { .getTestElement(`page-2-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-2`) + .should(`eq`, `/subdirectory/page-2/`) }) it(`can navigate to a parent page`, () => { @@ -64,7 +64,7 @@ describe(`navigation`, () => { .getTestElement(`page-parent-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory`) + .should(`eq`, `/subdirectory/`) }) it(`can navigate to a sibling page programatically`, () => { @@ -73,7 +73,7 @@ describe(`navigation`, () => { .getTestElement(`page-2-button-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-2`) + .should(`eq`, `/subdirectory/page-2/`) }) }) diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js index 859921b3230bb..14e0ef76cbee0 100644 --- a/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js +++ b/e2e-tests/development-runtime/cypress/integration/navigation/redirect.js @@ -1,6 +1,6 @@ const runTests = () => { it(`should redirect page to index page when there is no such page`, () => { - cy.visit(`/redirect-without-page`, { + cy.visit(`/redirect-without-page/`, { failOnStatusCode: false, }).waitForRouteChange() @@ -8,7 +8,7 @@ const runTests = () => { }) it(`should redirect page to index page even there is a such page`, () => { - cy.visit(`/redirect`, { + cy.visit(`/redirect/`, { failOnStatusCode: false, }).waitForRouteChange() @@ -29,17 +29,17 @@ const runTests = () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-anchor`).click().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`, ``) }) it(`should support hash parameter on direct visit`, () => { - cy.visit(`/redirect-two#anchor`, { + cy.visit(`/redirect-two/#anchor`, { 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`, ``) }) @@ -50,17 +50,17 @@ const runTests = () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-search`).click().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`) }) it(`should support search parameter on direct visit`, () => { - cy.visit(`/redirect-two?query_param=hello`, { + cy.visit(`/redirect-two/?query_param=hello`, { 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`) }) @@ -71,17 +71,17 @@ const runTests = () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-search-anchor`).click().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`) }) it(`should support search & hash parameter on direct visit`, () => { - cy.visit(`/redirect-two?query_param=hello#anchor`, { + cy.visit(`/redirect-two/?query_param=hello#anchor`, { 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/e2e-tests/development-runtime/gatsby-node.js b/e2e-tests/development-runtime/gatsby-node.js index df74a650d67d9..d4fc084389919 100644 --- a/e2e-tests/development-runtime/gatsby-node.js +++ b/e2e-tests/development-runtime/gatsby-node.js @@ -196,22 +196,22 @@ exports.createPages = async function createPages({ }) createRedirect({ - fromPath: `/redirect-without-page`, + fromPath: `/redirect-without-page/`, toPath: `/`, isPermanent: true, redirectInBrowser: true, }) createRedirect({ - fromPath: `/redirect`, + fromPath: `/redirect/`, toPath: `/`, isPermanent: true, redirectInBrowser: true, }) createRedirect({ - fromPath: `/redirect-two`, - toPath: `/redirect-search-hash`, + fromPath: `/redirect-two/`, + toPath: `/redirect-search-hash/`, isPermanent: true, redirectInBrowser: true, }) diff --git a/e2e-tests/development-runtime/src/pages/index.js b/e2e-tests/development-runtime/src/pages/index.js index 622f2d7e43ca6..993542f4e5c30 100644 --- a/e2e-tests/development-runtime/src/pages/index.js +++ b/e2e-tests/development-runtime/src/pages/index.js @@ -46,17 +46,17 @@ const IndexPage = ({ data }) => ( Created by hot-reloading/new-file.js - + Go to redirect with hash Go to redirect with query param Go to redirect with query param and hash diff --git a/e2e-tests/path-prefix/cypress/integration/navigate.js b/e2e-tests/path-prefix/cypress/integration/navigate.js index 92f32673c1e72..988700d03aab8 100644 --- a/e2e-tests/path-prefix/cypress/integration/navigate.js +++ b/e2e-tests/path-prefix/cypress/integration/navigate.js @@ -29,7 +29,7 @@ describe(`navigate`, () => { cy.getTestElement(`subdir-link`) .click() .location(`pathname`) - .should(`eq`, `${pathPrefix}/subdirectory/page-1`) + .should(`eq`, withTrailingSlash(`${pathPrefix}/subdirectory/page-1`)) }) it(`can navigate to a sibling page`, () => { @@ -38,7 +38,7 @@ describe(`navigate`, () => { .getTestElement(`page-2-link`) .click() .location(`pathname`) - .should(`eq`, `${pathPrefix}/subdirectory/page-2`) + .should(`eq`, withTrailingSlash(`${pathPrefix}/subdirectory/page-2`)) }) it(`can navigate to a parent page`, () => { @@ -47,7 +47,7 @@ describe(`navigate`, () => { .getTestElement(`page-parent-link`) .click() .location(`pathname`) - .should(`eq`, `${pathPrefix}/subdirectory`) + .should(`eq`, withTrailingSlash(`${pathPrefix}/subdirectory`)) }) it(`can navigate to a sibling page programatically`, () => { @@ -56,7 +56,7 @@ describe(`navigate`, () => { .getTestElement(`page-2-button-link`) .click() .location(`pathname`) - .should(`eq`, `${pathPrefix}/subdirectory/page-2`) + .should(`eq`, withTrailingSlash(`${pathPrefix}/subdirectory/page-2`)) }) it(`can navigate to SSR page`, () => { diff --git a/e2e-tests/production-runtime/cypress/integration/1-production.js b/e2e-tests/production-runtime/cypress/integration/1-production.js index 47fb8483473ff..b1cf53867b6a7 100644 --- a/e2e-tests/production-runtime/cypress/integration/1-production.js +++ b/e2e-tests/production-runtime/cypress/integration/1-production.js @@ -50,7 +50,7 @@ describe(`Production build tests`, () => { .getTestElement(`subdir-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-1`) + .should(`eq`, `/subdirectory/page-1/`) }) it(`can navigate to a sibling page`, () => { @@ -59,7 +59,7 @@ describe(`Production build tests`, () => { .getTestElement(`page-2-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-2`) + .should(`eq`, `/subdirectory/page-2/`) }) it(`can navigate to a parent page`, () => { @@ -68,7 +68,7 @@ describe(`Production build tests`, () => { .getTestElement(`page-parent-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory`) + .should(`eq`, `/subdirectory/`) }) it(`can navigate to a sibling page programatically`, () => { @@ -77,7 +77,7 @@ describe(`Production build tests`, () => { .getTestElement(`page-2-button-link`) .click() .location(`pathname`) - .should(`eq`, `/subdirectory/page-2`) + .should(`eq`, `/subdirectory/page-2/`) }) }) @@ -224,7 +224,7 @@ describe(`Production build tests`, () => { }) describe(`Keeps search query`, () => { - describe(`No trailing slash canonical path (/slashes/no-trailing)`, () => { + describe(`Converts no trailing slash canonical path to trailing (/slashes/no-trailing)`, () => { it(`/slashes/no-trailing?param=value`, () => { cy.visit(`/slashes/no-trailing?param=value`).waitForRouteChange() @@ -232,7 +232,7 @@ describe(`Production build tests`, () => { .invoke(`text`) .should(`equal`, `?param=value`) - cy.location(`pathname`).should(`equal`, `/slashes/no-trailing`) + cy.location(`pathname`).should(`equal`, `/slashes/no-trailing/`) cy.location(`search`).should(`equal`, `?param=value`) }) @@ -243,7 +243,7 @@ describe(`Production build tests`, () => { .invoke(`text`) .should(`equal`, `?param=value`) - cy.location(`pathname`).should(`equal`, `/slashes/no-trailing`) + cy.location(`pathname`).should(`equal`, `/slashes/no-trailing/`) cy.location(`search`).should(`equal`, `?param=value`) }) }) diff --git a/e2e-tests/production-runtime/cypress/integration/redirects.js b/e2e-tests/production-runtime/cypress/integration/redirects.js index f5b8ab4c2aa04..65a0564c35a4a 100644 --- a/e2e-tests/production-runtime/cypress/integration/redirects.js +++ b/e2e-tests/production-runtime/cypress/integration/redirects.js @@ -14,7 +14,7 @@ describe(`Redirects`, () => { // this DOES happen locally, but it's quite difficult to understand // we are getting hydration failures right now it.skip(`are case insensitive when ignoreCase is set to true`, () => { - cy.visit(`/Longue-PAGE`, { + cy.visit(`/Longue-PAGE/`, { failOnStatusCode: false, }).waitForRouteChange() @@ -22,7 +22,7 @@ describe(`Redirects`, () => { }) it(`are case sensitive when ignoreCase is set to false`, () => { - cy.visit(`/PAGINA-larga`, { + cy.visit(`/PAGINA-larga/`, { failOnStatusCode: false, }).waitForRouteChange() @@ -37,7 +37,7 @@ describe(`Redirects`, () => { }, }, () => { - const expectedLinks = [`/Longue-PAGE`, `/pagina-larga`] + const expectedLinks = [`/Longue-PAGE/`, `/pagina-larga/`] // we should not hit those routes cy.intercept("GET", "/page-data/Longue-PAGE/page-data.json").as( @@ -86,7 +86,7 @@ describe(`Redirects`, () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-anchor`).click().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`, ``) }) @@ -95,11 +95,11 @@ describe(`Redirects`, () => { // this DOES happen locally, but it's quite difficult to understand // we are getting hydration failures right now it.skip(`should support hash parameter on direct visit`, () => { - cy.visit(`/redirect-two#anchor`, { + cy.visit(`/redirect-two/#anchor`, { 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`, ``) }) @@ -110,17 +110,17 @@ describe(`Redirects`, () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-search`).click().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`) }) it(`should support search parameter on direct visit`, () => { - cy.visit(`/redirect-two?query_param=hello`, { + cy.visit(`/redirect-two/?query_param=hello`, { 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`) }) @@ -131,17 +131,17 @@ describe(`Redirects`, () => { }).waitForRouteChange() cy.getTestElement(`redirect-two-search-anchor`).click().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`) }) it(`should support search & hash parameter on direct visit`, () => { - cy.visit(`/redirect-two?query_param=hello#anchor`, { + cy.visit(`/redirect-two/?query_param=hello#anchor`, { 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/e2e-tests/production-runtime/gatsby-node.ts b/e2e-tests/production-runtime/gatsby-node.ts index 775be229ff9b5..3d14eaea677df 100644 --- a/e2e-tests/production-runtime/gatsby-node.ts +++ b/e2e-tests/production-runtime/gatsby-node.ts @@ -242,24 +242,24 @@ export const createPages: GatsbyNode["createPages"] = ({ } createRedirect({ - fromPath: "/pagina-larga", - toPath: "/long-page", + fromPath: "/pagina-larga/", + toPath: "/long-page/", isPermanent: true, redirectInBrowser: true, ignoreCase: false, }) createRedirect({ - fromPath: "/Longue-Page", - toPath: "/long-page", + fromPath: "/Longue-Page/", + toPath: "/long-page/", isPermanent: true, redirectInBrowser: true, ignoreCase: true, }) createRedirect({ - fromPath: `/redirect-two`, - toPath: `/redirect-search-hash`, + fromPath: `/redirect-two/`, + toPath: `/redirect-search-hash/`, isPermanent: true, redirectInBrowser: true, }) diff --git a/e2e-tests/production-runtime/src/pages/index.js b/e2e-tests/production-runtime/src/pages/index.js index c346529520d4a..33a654f2f25f2 100644 --- a/e2e-tests/production-runtime/src/pages/index.js +++ b/e2e-tests/production-runtime/src/pages/index.js @@ -99,13 +99,13 @@ const IndexPage = ({ pageContext }) => (
  • - + Go to redirect with hash
  • Go to redirect with query param @@ -113,7 +113,7 @@ const IndexPage = ({ pageContext }) => (
  • Go to redirect with query param and hash diff --git a/e2e-tests/trailing-slash/cypress-legacy.json b/e2e-tests/trailing-slash/cypress-legacy.json deleted file mode 100644 index d4d72f3ae04e6..0000000000000 --- a/e2e-tests/trailing-slash/cypress-legacy.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "videoUploadOnPasses": false, - "chromeWebSecurity": false, - "testFiles": ["legacy.js", "functions.js", "static.js"] -} diff --git a/e2e-tests/trailing-slash/cypress/integration/always.js b/e2e-tests/trailing-slash/cypress/integration/always.js index 9b866f532a48d..13e4f4c33ac32 100644 --- a/e2e-tests/trailing-slash/cypress/integration/always.js +++ b/e2e-tests/trailing-slash/cypress/integration/always.js @@ -1,5 +1,11 @@ import { assertPageVisits } from "../support/utils/trailing-slash" +Cypress.on('uncaught:exception', (err) => { + if (err.message.includes('Minified React error #418') || err.message.includes('Minified React error #423') || ('Minified React error #425')) { + return false + } +}) + describe(`always`, () => { beforeEach(() => { cy.visit(`/`).waitForRouteChange() diff --git a/e2e-tests/trailing-slash/cypress/integration/ignore.js b/e2e-tests/trailing-slash/cypress/integration/ignore.js index 7a9497a23efb6..2db16b574d098 100644 --- a/e2e-tests/trailing-slash/cypress/integration/ignore.js +++ b/e2e-tests/trailing-slash/cypress/integration/ignore.js @@ -1,5 +1,11 @@ import { assertPageVisits } from "../support/utils/trailing-slash" +Cypress.on('uncaught:exception', (err) => { + if (err.message.includes('Minified React error #418') || err.message.includes('Minified React error #423') || ('Minified React error #425')) { + return false + } +}) + describe(`ignore`, () => { beforeEach(() => { cy.visit(`/`).waitForRouteChange() @@ -103,7 +109,6 @@ describe(`ignore (direct visits)`, () => { cy.visit(`/`).waitForRouteChange() }) - //Fix it(`page-creator`, () => { assertPageVisits([ { @@ -115,8 +120,7 @@ describe(`ignore (direct visits)`, () => { cy.visit(`/page-2`) .waitForRouteChange() - // TODO(v5): Should behave like "always" - .assertRoute(IS_BUILD ? `/page-2/` : `/page-2`) + .assertRoute(`/page-2`) }) it(`create-page with`, () => { @@ -200,7 +204,7 @@ describe(`ignore (direct visits)`, () => { cy.visit(`/page-2/?query_param=hello#anchor`) .waitForRouteChange() - .assertRoute(`/page-2/?query_param=hello#anchor`) + .assertRoute(IS_BUILD ? `/page-2?query_param=hello#anchor` : `/page-2/?query_param=hello#anchor`) }) it(`query-param-hash without`, () => { assertPageVisits([ @@ -215,10 +219,6 @@ describe(`ignore (direct visits)`, () => { cy.visit(`/page-2?query_param=hello#anchor`) .waitForRouteChange() - .assertRoute( - IS_BUILD - ? `/page-2/?query_param=hello#anchor` - : `/page-2?query_param=hello#anchor` - ) + .assertRoute(`/page-2?query_param=hello#anchor`) }) }) diff --git a/e2e-tests/trailing-slash/cypress/integration/legacy.js b/e2e-tests/trailing-slash/cypress/integration/legacy.js deleted file mode 100644 index 1a9090fc43af4..0000000000000 --- a/e2e-tests/trailing-slash/cypress/integration/legacy.js +++ /dev/null @@ -1,274 +0,0 @@ -import { assertPageVisits } from "../support/utils/trailing-slash" - -describe(`legacy`, () => { - beforeEach(() => { - cy.visit(`/`).waitForRouteChange() - }) - it(`page-creator without slash`, () => { - cy.getTestElement(`page-creator-without`).click() - cy.waitForRouteChange().assertRoute(`/page-2`) - }) - it(`page-creator with slash`, () => { - cy.getTestElement(`page-creator-with`).click() - cy.waitForRouteChange().assertRoute(`/page-2/`) - }) - it(`create-page with slash`, () => { - cy.getTestElement(`create-page-with`).click() - cy.waitForRouteChange().assertRoute(`/create-page/with/`) - }) - it(`create-page without slash`, () => { - cy.getTestElement(`create-page-without`).click() - cy.waitForRouteChange().assertRoute(`/create-page/without`) - }) - it(`fs-api with slash`, () => { - cy.getTestElement(`fs-api-with`).click() - cy.waitForRouteChange().assertRoute(`/fs-api/with/`) - }) - it(`fs-api without slash`, () => { - cy.getTestElement(`fs-api-without`).click() - cy.waitForRouteChange().assertRoute(`/fs-api/without`) - }) - it(`fs-api client only splat without slash`, () => { - cy.getTestElement(`fs-api-client-only-without`).click() - cy.waitForRouteChange().assertRoute(`/fs-api/without/without`) - cy.getTestElement(`title`).should(`have.text`, `without`) - }) - it(`fs-api client only splat with slash`, () => { - cy.getTestElement(`fs-api-client-only-with`).click() - cy.waitForRouteChange().assertRoute(`/fs-api/with/with/`) - cy.getTestElement(`title`).should(`have.text`, `with`) - }) - it(`fs-api-simple with slash`, () => { - cy.getTestElement(`fs-api-simple-with`).click() - cy.waitForRouteChange().assertRoute(`/fs-api-simple/with/`) - }) - it(`fs-api-simple without slash`, () => { - cy.getTestElement(`fs-api-simple-without`).click() - cy.waitForRouteChange().assertRoute(`/fs-api-simple/without`) - }) - it(`gatsbyPath works`, () => { - cy.getTestElement(`gatsby-path-1`).should( - "have.attr", - "href", - "/fs-api-simple/with/" - ) - cy.getTestElement(`gatsby-path-2`).should( - "have.attr", - "href", - "/fs-api-simple/without/" - ) - }) - it(`hash`, () => { - cy.getTestElement(`hash`).click() - cy.waitForRouteChange().assertRoute(`/page-2#anchor`) - }) - it(`hash trailing`, () => { - cy.getTestElement(`hash-trailing`).click() - cy.waitForRouteChange().assertRoute(`/page-2/#anchor`) - }) - it(`query-param`, () => { - cy.getTestElement(`query-param`).click() - cy.waitForRouteChange().assertRoute(`/page-2?query_param=hello`) - }) - it(`query-param-hash`, () => { - cy.getTestElement(`query-param-hash`).click() - cy.waitForRouteChange().assertRoute(`/page-2?query_param=hello#anchor`) - }) - it(`client-only without slash`, () => { - cy.getTestElement(`client-only-simple-without`).click() - cy.waitForRouteChange().assertRoute(`/client-only/without`) - cy.getTestElement(`title`).should(`have.text`, `without`) - }) - it(`client-only with slash`, () => { - cy.getTestElement(`client-only-simple-with`).click() - cy.waitForRouteChange().assertRoute(`/client-only/with/`) - cy.getTestElement(`title`).should(`have.text`, `with`) - }) - it(`client-only-splat without slash`, () => { - cy.getTestElement(`client-only-splat-without`).click() - cy.waitForRouteChange().assertRoute(`/client-only-splat/without/without`) - cy.getTestElement(`title`).should(`have.text`, `without/without`) - }) - it(`client-only-splat with slash`, () => { - cy.getTestElement(`client-only-splat-with`).click() - cy.waitForRouteChange().assertRoute(`/client-only-splat/with/with/`) - cy.getTestElement(`title`).should(`have.text`, `with/with`) - }) -}) - -const IS_BUILD = Cypress.env(`IS_BUILD`) - -describe(`legacy (direct visits)`, () => { - beforeEach(() => { - cy.visit(`/`).waitForRouteChange() - }) - - it(`page-creator`, () => { - assertPageVisits([ - { - path: "/page-2/", - destinationPath: IS_BUILD ? false : `/page-2`, - status: IS_BUILD ? 200 : 301, - }, - ]) - - cy.visit(`/page-2`) - .waitForRouteChange() - .assertRoute(IS_BUILD ? `/page-2/` : `/page-2`) - }) - - it(`create-page with`, () => { - assertPageVisits([ - { - path: "/create-page/with/", - status: 200, - }, - ]) - - cy.visit(`/create-page/with/`) - .waitForRouteChange() - .assertRoute(`/create-page/with/`) - }) - - it(`create-page without`, () => { - assertPageVisits([ - { - path: "/create-page/without", - status: 200, - }, - ]) - - cy.visit(`/create-page/without`) - .waitForRouteChange() - .assertRoute(`/create-page/without`) - }) - it(`fs-api-simple with`, () => { - assertPageVisits([ - { - path: "/fs-api-simple/with/", - status: 200, - }, - ]) - - cy.visit(`/fs-api-simple/with/`) - .waitForRouteChange() - .assertRoute(`/fs-api-simple/with/`) - }) - it(`fs-api-simple without`, () => { - assertPageVisits([ - { - path: "/fs-api-simple/without", - status: IS_BUILD ? 301 : 200, - destinationPath: IS_BUILD ? `/fs-api-simple/without/` : false, - }, - ]) - - cy.visit(`/fs-api-simple/without`) - .waitForRouteChange() - .assertRoute( - IS_BUILD ? `/fs-api-simple/without/` : `/fs-api-simple/without` - ) - }) - it(`fs-api client only splat with`, () => { - assertPageVisits([ - { - path: "/fs-api/with/with/", - status: 200, - }, - ]) - - cy.visit(`/fs-api/with/with/`) - .waitForRouteChange() - .assertRoute(`/fs-api/with/with/`) - }) - it(`fs-api client only splat without`, () => { - assertPageVisits([ - { - path: "/fs-api/without/without", - status: 200, - }, - ]) - - cy.visit(`/fs-api/without/without`) - .waitForRouteChange() - .assertRoute(`/fs-api/without/without`) - }) - it(`client-only with`, () => { - assertPageVisits([ - { - path: "/client-only/with/", - status: 200, - }, - ]) - - cy.visit(`/client-only/with/`) - .waitForRouteChange() - .assertRoute(`/client-only/with/`) - }) - it(`client-only without`, () => { - assertPageVisits([ - { - path: "/client-only/without", - status: 200, - }, - ]) - - cy.visit(`/client-only/without`) - .waitForRouteChange() - .assertRoute(`/client-only/without`) - }) - it(`client-only-splat with`, () => { - assertPageVisits([ - { - path: `/client-only-splat/with/with/`, - status: 200, - }, - ]) - - cy.visit(`/client-only-splat/with/with/`) - .waitForRouteChange() - .assertRoute(`/client-only-splat/with/with/`) - }) - it(`client-only-splat without`, () => { - assertPageVisits([ - { - path: "/client-only-splat/without/without", - status: 200, - }, - ]) - - cy.visit(`/client-only-splat/without/without`) - .waitForRouteChange() - .assertRoute(`/client-only-splat/without/without`) - }) - it(`query-param-hash with`, () => { - assertPageVisits([ - { - path: "/page-2/?query_param=hello#anchor", - status: 200, - }, - ]) - - cy.visit(`/page-2/?query_param=hello#anchor`) - .waitForRouteChange() - .assertRoute(`/page-2/?query_param=hello#anchor`) - }) - - it(`query-param-hash without`, () => { - assertPageVisits([ - { - path: "/page-2?query_param=hello#anchor", - status: IS_BUILD ? 301 : 200, - destinationPath: IS_BUILD ? `/page-2?query_param=hello#anchor` : false, - }, - ]) - - cy.visit(`/page-2?query_param=hello#anchor`) - .waitForRouteChange() - .assertRoute( - IS_BUILD - ? `/page-2/?query_param=hello#anchor` - : `/page-2?query_param=hello#anchor` - ) - }) -}) diff --git a/e2e-tests/trailing-slash/cypress/integration/never.js b/e2e-tests/trailing-slash/cypress/integration/never.js index 77caf305947cf..bb469580be857 100644 --- a/e2e-tests/trailing-slash/cypress/integration/never.js +++ b/e2e-tests/trailing-slash/cypress/integration/never.js @@ -1,5 +1,11 @@ import { assertPageVisits } from "../support/utils/trailing-slash" +Cypress.on('uncaught:exception', (err) => { + if (err.message.includes('Minified React error #418') || err.message.includes('Minified React error #423') || ('Minified React error #425')) { + return false + } +}) + describe(`never`, () => { beforeEach(() => { cy.visit(`/`).waitForRouteChange() diff --git a/e2e-tests/trailing-slash/gatsby-config.js b/e2e-tests/trailing-slash/gatsby-config.js index 01a4fb4540838..8a241c3bab122 100644 --- a/e2e-tests/trailing-slash/gatsby-config.js +++ b/e2e-tests/trailing-slash/gatsby-config.js @@ -1,4 +1,4 @@ -const trailingSlash = process.env.TRAILING_SLASH || `legacy` +const trailingSlash = process.env.TRAILING_SLASH || `always` console.info(`TrailingSlash: ${trailingSlash}`) module.exports = { diff --git a/e2e-tests/trailing-slash/package.json b/e2e-tests/trailing-slash/package.json index e9b21daeb3951..9bf9efef35270 100644 --- a/e2e-tests/trailing-slash/package.json +++ b/e2e-tests/trailing-slash/package.json @@ -6,8 +6,8 @@ "dependencies": { "cypress": "^9.7.0", "gatsby": "next", - "react": "^17.0.2", - "react-dom": "^17.0.2" + "react": "^18.2.0", + "react-dom": "^18.2.0" }, "license": "MIT", "scripts": { @@ -29,8 +29,7 @@ "test:always": "cross-env OPTION=always npm run build:option && cross-env OPTION=always npm run t:opt:build && npm run clean && cross-env OPTION=always npm run t:opt:develop", "test:never": "cross-env OPTION=never npm run build:option && cross-env OPTION=never npm run t:opt:build && npm run clean && cross-env OPTION=never npm run t:opt:develop", "test:ignore": "cross-env OPTION=ignore npm run build:option && cross-env OPTION=ignore npm run t:opt:build && npm run clean && cross-env OPTION=ignore npm run t:opt:develop", - "test:legacy": "cross-env OPTION=legacy npm run build:option && cross-env OPTION=legacy npm run t:opt:build && npm run clean && cross-env OPTION=legacy npm run t:opt:develop", - "test": "npm-run-all -c -s test:always test:never test:ignore test:legacy" + "test": "npm-run-all -c -s test:always test:never test:ignore" }, "devDependencies": { "cross-env": "^7.0.3", diff --git a/integration-tests/artifacts/__tests__/index.js b/integration-tests/artifacts/__tests__/index.js index 5f55451fe0914..1d2500aa1cd41 100644 --- a/integration-tests/artifacts/__tests__/index.js +++ b/integration-tests/artifacts/__tests__/index.js @@ -521,7 +521,7 @@ describe(`First run (baseline)`, () => { `stale-pages/stable`, `stale-pages/only-in-first`, `page-that-will-have-trailing-slash-removed`, - `/stale-pages/sometimes-i-have-trailing-slash-sometimes-i-dont`, + `/stale-pages/sometimes-i-have-trailing-slash-sometimes-i-dont/`, ] const unexpectedPages = [`stale-pages/only-not-in-first`] @@ -622,7 +622,7 @@ describe(`Second run (different pages created, data changed)`, () => { const runNumber = 2 const expectedPagesToBeGenerated = [ - `/stale-pages/only-not-in-first`, + `/stale-pages/only-not-in-first/`, `/page-query-changing-data-but-not-id/`, `/page-query-dynamic-2/`, `/static-query-result-tracking/should-invalidate/`, @@ -733,7 +733,7 @@ describe( `/gatsby-browser/`, // those change happen on every build `/page-query-dynamic-3/`, - `/stale-pages/sometimes-i-have-trailing-slash-sometimes-i-dont`, + `/stale-pages/sometimes-i-have-trailing-slash-sometimes-i-dont/`, `/changing-context/`, ] diff --git a/integration-tests/node-manifest/__tests__/create-node-manifest.test.js b/integration-tests/node-manifest/__tests__/create-node-manifest.test.js index 1095e2174f695..3c8cf2f176cb9 100644 --- a/integration-tests/node-manifest/__tests__/create-node-manifest.test.js +++ b/integration-tests/node-manifest/__tests__/create-node-manifest.test.js @@ -82,7 +82,7 @@ describe(`Node Manifest API in "gatsby ${gatsbyCommandName}"`, () => { const manifestFileContents = await getManifestContents(1) expect(manifestFileContents.node.id).toBe(`1`) - expect(manifestFileContents.page.path).toBe(`/one`) + expect(manifestFileContents.page.path).toBe(`/one/`) expect(manifestFileContents.foundPageBy).toBe(`ownerNodeId`) }) @@ -90,7 +90,7 @@ describe(`Node Manifest API in "gatsby ${gatsbyCommandName}"`, () => { const manifestFileContents = await getManifestContents(2) expect(manifestFileContents.node.id).toBe(`2`) - expect(manifestFileContents.page.path).toBe(`/two`) + expect(manifestFileContents.page.path).toBe(`/two/`) expect(manifestFileContents.foundPageBy).toBe(`context.id`) }) @@ -98,7 +98,7 @@ describe(`Node Manifest API in "gatsby ${gatsbyCommandName}"`, () => { const manifestFileContents = await getManifestContents(5) expect(manifestFileContents.node.id).toBe(`5`) - expect(manifestFileContents.page.path).toBe(`/slug-test-path`) + expect(manifestFileContents.page.path).toBe(`/slug-test-path/`) expect(manifestFileContents.foundPageBy).toBe(`context.slug`) }) @@ -110,7 +110,7 @@ describe(`Node Manifest API in "gatsby ${gatsbyCommandName}"`, () => { expect(manifestFileContents.node.id).toBe(`3`) expect( - [`/three`, `/three-alternative`].includes( + [`/three/`, `/three-alternative/`].includes( manifestFileContents.page.path ) ).toBe(true) diff --git a/packages/gatsby-link/src/__tests__/rewrite-link-path.js b/packages/gatsby-link/src/__tests__/rewrite-link-path.js index 64829a2a25a7a..416c9d89e74cc 100644 --- a/packages/gatsby-link/src/__tests__/rewrite-link-path.js +++ b/packages/gatsby-link/src/__tests__/rewrite-link-path.js @@ -12,21 +12,16 @@ const getRewriteLinkPath = (option = `legacy`, pathPrefix = undefined) => { } describe(`rewriteLinkPath`, () => { - it(`handles legacy option`, () => { - expect(getRewriteLinkPath()(`/path`, `/`)).toBe(`/path`) - expect(getRewriteLinkPath()(`/path/`, `/`)).toBe(`/path/`) - expect(getRewriteLinkPath()(`/path#hash`, `/`)).toBe(`/path#hash`) - expect(getRewriteLinkPath()(`/path?query_param=hello`, `/`)).toBe( - `/path?query_param=hello` - ) - expect(getRewriteLinkPath()(`/path?query_param=hello#anchor`, `/`)).toBe( - `/path?query_param=hello#anchor` - ) - expect(getRewriteLinkPath(`legacy`, `/prefix`)(`/`, `/`)).toBe(`/prefix/`) - }) it(`handles always option`, () => { expect(getRewriteLinkPath(`always`)(`/path`, `/`)).toBe(`/path/`) expect(getRewriteLinkPath(`always`)(`/path/`, `/`)).toBe(`/path/`) + expect(getRewriteLinkPath(`always`)(`#hash`, `/`)).toBe(`/#hash`) + expect(getRewriteLinkPath(`always`)(`?query_param=hello`, `/`)).toBe( + `/?query_param=hello` + ) + expect(getRewriteLinkPath(`always`)(`?query_param=hello#anchor`, `/`)).toBe( + `/?query_param=hello#anchor` + ) expect(getRewriteLinkPath(`always`)(`/path#hash`, `/`)).toBe(`/path/#hash`) expect(getRewriteLinkPath(`always`)(`/path?query_param=hello`, `/`)).toBe( `/path/?query_param=hello` @@ -39,6 +34,13 @@ describe(`rewriteLinkPath`, () => { it(`handles never option`, () => { expect(getRewriteLinkPath(`never`)(`/path`, `/`)).toBe(`/path`) expect(getRewriteLinkPath(`never`)(`/path/`, `/`)).toBe(`/path`) + expect(getRewriteLinkPath(`always`)(`#hash`, `/`)).toBe(`/#hash`) + expect(getRewriteLinkPath(`always`)(`?query_param=hello`, `/`)).toBe( + `/?query_param=hello` + ) + expect(getRewriteLinkPath(`always`)(`?query_param=hello#anchor`, `/`)).toBe( + `/?query_param=hello#anchor` + ) expect(getRewriteLinkPath(`never`)(`/path/#hash`, `/`)).toBe(`/path#hash`) expect(getRewriteLinkPath(`never`)(`/path/?query_param=hello`, `/`)).toBe( `/path?query_param=hello` @@ -51,6 +53,13 @@ describe(`rewriteLinkPath`, () => { it(`handles ignore option`, () => { expect(getRewriteLinkPath(`ignore`)(`/path`, `/`)).toBe(`/path`) expect(getRewriteLinkPath(`ignore`)(`/path/`, `/`)).toBe(`/path/`) + expect(getRewriteLinkPath(`always`)(`#hash`, `/`)).toBe(`/#hash`) + expect(getRewriteLinkPath(`always`)(`?query_param=hello`, `/`)).toBe( + `/?query_param=hello` + ) + expect(getRewriteLinkPath(`always`)(`?query_param=hello#anchor`, `/`)).toBe( + `/?query_param=hello#anchor` + ) expect(getRewriteLinkPath(`ignore`)(`/path#hash`, `/`)).toBe(`/path#hash`) expect(getRewriteLinkPath(`ignore`)(`/path?query_param=hello`, `/`)).toBe( `/path?query_param=hello` diff --git a/packages/gatsby-link/src/rewrite-link-path.js b/packages/gatsby-link/src/rewrite-link-path.js index 0722449326656..e1b2b496716c7 100644 --- a/packages/gatsby-link/src/rewrite-link-path.js +++ b/packages/gatsby-link/src/rewrite-link-path.js @@ -10,6 +10,13 @@ const isAbsolutePath = path => path?.startsWith(`/`) const getGlobalTrailingSlash = () => typeof __TRAILING_SLASH__ !== `undefined` ? __TRAILING_SLASH__ : undefined +function applyTrailingSlashOptionOnPathnameOnly(path, option) { + const { pathname, search, hash } = parsePath(path) + const output = applyTrailingSlashOption(pathname, option) + + return `${output}${search}${hash}` +} + function absolutify(path, current) { // If it's already absolute, return as-is if (isAbsolutePath(path)) { @@ -20,7 +27,7 @@ function absolutify(path, current) { const absolutePath = resolve(path, current) if (option === `always` || option === `never`) { - return applyTrailingSlashOption(absolutePath, option) + return applyTrailingSlashOptionOnPathnameOnly(absolutePath, option) } return absolutePath @@ -31,10 +38,7 @@ function applyPrefix(path) { const option = getGlobalTrailingSlash() if (option === `always` || option === `never`) { - const { pathname, search, hash } = parsePath(prefixed) - const output = applyTrailingSlashOption(pathname, option) - - return `${output}${search}${hash}` + return applyTrailingSlashOptionOnPathnameOnly(prefixed, option) } return prefixed diff --git a/packages/gatsby-page-utils/src/__tests__/apply-trailing-slash-option.ts b/packages/gatsby-page-utils/src/__tests__/apply-trailing-slash-option.ts index 1ab370dede016..92b8bd877c951 100644 --- a/packages/gatsby-page-utils/src/__tests__/apply-trailing-slash-option.ts +++ b/packages/gatsby-page-utils/src/__tests__/apply-trailing-slash-option.ts @@ -41,10 +41,4 @@ describe(`applyTrailingSlashOption`, () => { ) }) }) - describe(`legacy`, () => { - it(`should do nothing`, () => { - expect(applyTrailingSlashOption(withSlash)).toEqual(withSlash) - expect(applyTrailingSlashOption(withoutSlash)).toEqual(withoutSlash) - }) - }) }) diff --git a/packages/gatsby-page-utils/src/__tests__/create-path.ts b/packages/gatsby-page-utils/src/__tests__/create-path.ts index 82364759ffee8..f2905fbc7c22e 100644 --- a/packages/gatsby-page-utils/src/__tests__/create-path.ts +++ b/packages/gatsby-page-utils/src/__tests__/create-path.ts @@ -6,21 +6,23 @@ describe(`create-path`, () => { expect(paths.map(p => createPath(p))).toMatchInlineSnapshot(` Array [ - "/b/c/de/", - "/bee/", - "/b/d/c/", + "/b/c/de", + "/bee", + "/b/d/c", ] `) }) it(`should convert windows seperator to unix seperator`, () => { - expect(createPath(`lorem\\foo\\bar`)).toEqual(`/lorem/foo/bar/`) + expect(createPath(`lorem\\foo\\bar`)).toEqual(`/lorem/foo/bar`) }) it(`should parse index`, () => { - expect(createPath(`foo/bar/index`)).toEqual(`/foo/bar/`) + expect(createPath(`foo/bar/index`)).toEqual(`/foo/bar`) }) it(`should have working trailingSlash option`, () => { expect(createPath(`foo/bar/`, false)).toEqual(`/foo/bar`) expect(createPath(`foo/bar/index`, false)).toEqual(`/foo/bar`) + expect(createPath(`foo/bar/`, true)).toEqual(`/foo/bar/`) + expect(createPath(`foo/bar/index`, true)).toEqual(`/foo/bar/`) }) it(`should support client-only routes`, () => { expect(createPath(`foo/[name]`, false, true)).toEqual(`/foo/[name]`) diff --git a/packages/gatsby-page-utils/src/apply-trailing-slash-option.ts b/packages/gatsby-page-utils/src/apply-trailing-slash-option.ts index 43bbcafe7806d..4a7266a52d568 100644 --- a/packages/gatsby-page-utils/src/apply-trailing-slash-option.ts +++ b/packages/gatsby-page-utils/src/apply-trailing-slash-option.ts @@ -1,9 +1,8 @@ -// TODO(v5): Remove legacy setting and default to "always" -export type TrailingSlash = "always" | "never" | "ignore" | "legacy" +export type TrailingSlash = "always" | "never" | "ignore" export const applyTrailingSlashOption = ( input: string, - option: TrailingSlash = `legacy` + option: TrailingSlash = `always` ): string => { const hasHtmlSuffix = input.endsWith(`.html`) const hasXmlSuffix = input.endsWith(`.xml`) diff --git a/packages/gatsby-page-utils/src/create-path.ts b/packages/gatsby-page-utils/src/create-path.ts index 7572df7b0959f..d098f8dbbf4c8 100644 --- a/packages/gatsby-page-utils/src/create-path.ts +++ b/packages/gatsby-page-utils/src/create-path.ts @@ -3,8 +3,7 @@ import { slash } from "gatsby-core-utils/path" export function createPath( filePath: string, - // TODO(v5): Set this default to false - withTrailingSlash: boolean = true, + withTrailingSlash: boolean = false, usePathBase: boolean = false ): string { const { dir, name, base } = parse(filePath) diff --git a/packages/gatsby-plugin-page-creator/src/gatsby-node.ts b/packages/gatsby-plugin-page-creator/src/gatsby-node.ts index cd024ba8f22e0..38c5070a8982b 100644 --- a/packages/gatsby-plugin-page-creator/src/gatsby-node.ts +++ b/packages/gatsby-plugin-page-creator/src/gatsby-node.ts @@ -81,7 +81,7 @@ export async function createPagesStatefully( try { const { deletePage } = actions const { program, config } = store.getState() - const { trailingSlash = `legacy` } = config + const { trailingSlash = `always` } = config const exts = program.extensions.map(e => `${e.slice(1)}`).join(`,`) @@ -177,10 +177,9 @@ Please pick a path to an existing directory.`, reporter, slugifyOptions ) - // TODO(v5): Remove legacy handling - const isLegacy = trailingSlash === `legacy` + const hasTrailingSlash = derivedPath.endsWith(`/`) - const path = createPath(derivedPath, isLegacy || hasTrailingSlash, true) + const path = createPath(derivedPath, hasTrailingSlash, true) const modifiedPath = applyTrailingSlashOption(path, trailingSlash) return modifiedPath } @@ -199,10 +198,9 @@ Please pick a path to an existing directory.`, reporter, slugifyOptions ) - // TODO(v5): Remove legacy handling - const isLegacy = trailingSlash === `legacy` + const hasTrailingSlash = derivedPath.endsWith(`/`) - const path = createPath(derivedPath, isLegacy || hasTrailingSlash, true) + const path = createPath(derivedPath, hasTrailingSlash, true) // We've already created a page with this path if (this.knownPagePaths.has(path)) { return undefined @@ -364,7 +362,7 @@ export function setFieldsOnGraphQLNodeType( ): Record { try { const extensions = store.getState().program.extensions - const { trailingSlash = `legacy` } = store.getState().config + const { trailingSlash = `always` } = store.getState().config const collectionQuery = _.camelCase(`all ${type.name}`) if (knownCollections.has(collectionQuery)) { return { @@ -398,14 +396,9 @@ export function setFieldsOnGraphQLNodeType( reporter, slugifyOptions ) - // TODO(v5): Remove legacy handling - const isLegacy = trailingSlash === `legacy` + const hasTrailingSlash = derivedPath.endsWith(`/`) - const path = createPath( - derivedPath, - isLegacy || hasTrailingSlash, - true - ) + const path = createPath(derivedPath, hasTrailingSlash, true) const modifiedPath = applyTrailingSlashOption(path, trailingSlash) return modifiedPath diff --git a/packages/gatsby/index.d.ts b/packages/gatsby/index.d.ts index 4cdffd10f1534..48dc6ec78cf57 100644 --- a/packages/gatsby/index.d.ts +++ b/packages/gatsby/index.d.ts @@ -324,8 +324,8 @@ export interface GatsbyConfig { flags?: Record /** It’s common for sites to be hosted somewhere other than the root of their domain. Say we have a Gatsby site at `example.com/blog/`. In this case, we would need a prefix (`/blog`) added to all paths on the site. */ pathPrefix?: string - /** `never` removes all trailing slashes, `always` adds it, and `ignore` doesn't automatically change anything and it's in user hands to keep things consistent. By default `legacy` is used which is the behavior until v5. With Gatsby v5 "always" will be the default. */ - trailingSlash?: "always" | "never" | "ignore" | "legacy" + /** `never` removes all trailing slashes, `always` adds it, and `ignore` doesn't automatically change anything and it's in user hands to keep things consistent. By default `always` is used. */ + trailingSlash?: "always" | "never" | "ignore" /** In some circumstances you may want to deploy assets (non-HTML resources such as JavaScript, CSS, etc.) to a separate domain. `assetPrefix` allows you to use Gatsby with assets hosted from a separate domain */ assetPrefix?: string /** More easily incorporate content into your pages through automatic TypeScript type generation and better GraphQL IntelliSense. If set to true, the default GraphQLTypegenOptions are used. See https://www.gatsbyjs.com/docs/reference/config-files/gatsby-config/ for all options. */ diff --git a/packages/gatsby/src/bootstrap/__tests__/redirects-writer.ts b/packages/gatsby/src/bootstrap/__tests__/redirects-writer.ts index 63a3b5fc5b091..cc1be69b749c5 100644 --- a/packages/gatsby/src/bootstrap/__tests__/redirects-writer.ts +++ b/packages/gatsby/src/bootstrap/__tests__/redirects-writer.ts @@ -109,7 +109,7 @@ describe(`redirect-writer`, () => { const warningMessage = reporterWarnMock.mock.calls[0][0] expect(warningMessage).toMatchInlineSnapshot(` "There are routes that match both page and redirect. Pages take precedence over redirects so the redirect will not work: - - page: \\"/server-overlap\\" and redirect: \\"/server-overlap/\\" -> \\"/server-overlap/redirect/\\" + - page: \\"/server-overlap/\\" and redirect: \\"/server-overlap/\\" -> \\"/server-overlap/redirect/\\" - page: \\"/client-overlap/\\" and redirect: \\"/client-overlap\\" -> \\"/client-overlap/redirect/\\"" `) }) diff --git a/packages/gatsby/src/joi-schemas/__tests__/joi.ts b/packages/gatsby/src/joi-schemas/__tests__/joi.ts index 4ba077388a4ed..f0b089cbfdbf0 100644 --- a/packages/gatsby/src/joi-schemas/__tests__/joi.ts +++ b/packages/gatsby/src/joi-schemas/__tests__/joi.ts @@ -108,13 +108,13 @@ describe(`gatsby config`, () => { ) }) - it(`returns "legacy" for trailingSlash when not set`, () => { + it(`returns "always" for trailingSlash when not set`, () => { const config = {} const result = gatsbyConfigSchema.validate(config) expect(result.value).toEqual( expect.objectContaining({ - trailingSlash: `legacy`, + trailingSlash: `always`, }) ) }) @@ -126,7 +126,7 @@ describe(`gatsby config`, () => { const result = gatsbyConfigSchema.validate(config) expect(result.error).toMatchInlineSnapshot( - `[ValidationError: "trailingSlash" must be one of [always, never, ignore, legacy]]` + `[ValidationError: "trailingSlash" must be one of [always, never, ignore]]` ) }) @@ -137,7 +137,7 @@ describe(`gatsby config`, () => { const result = gatsbyConfigSchema.validate(config) expect(result.error).toMatchInlineSnapshot( - `[ValidationError: "trailingSlash" must be one of [always, never, ignore, legacy]]` + `[ValidationError: "trailingSlash" must be one of [always, never, ignore]]` ) }) diff --git a/packages/gatsby/src/joi-schemas/joi.ts b/packages/gatsby/src/joi-schemas/joi.ts index a41dc284396e9..2e708c58f1e10 100644 --- a/packages/gatsby/src/joi-schemas/joi.ts +++ b/packages/gatsby/src/joi-schemas/joi.ts @@ -54,8 +54,8 @@ export const gatsbyConfigSchema: Joi.ObjectSchema = Joi.object() jsxRuntime: Joi.string().valid(`automatic`, `classic`).default(`classic`), jsxImportSource: Joi.string(), trailingSlash: Joi.string() - .valid(`always`, `never`, `ignore`, `legacy`) // TODO(v5): Remove legacy - .default(`legacy`), + .valid(`always`, `never`, `ignore`) + .default(`always`), graphqlTypegen: Joi.alternatives( Joi.boolean(), Joi.object() diff --git a/packages/gatsby/src/query/__tests__/data-tracking.js b/packages/gatsby/src/query/__tests__/data-tracking.js index 27e3b676f35ac..0454b46aeab2a 100644 --- a/packages/gatsby/src/query/__tests__/data-tracking.js +++ b/packages/gatsby/src/query/__tests__/data-tracking.js @@ -281,6 +281,8 @@ const setup = async ({ restart = isFirstRun, clearCache = false } = {}) => { } } +const indexBarFooArray = [`/`, `/bar/`, `/foo/`] + describe(`query caching between builds`, () => { beforeAll(() => { setAPIhooks({ @@ -376,10 +378,10 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`, `/foo`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual(indexBarFooArray) expect(staticQueriesThatRan).toEqual([`static-query-1`]) }, 99999) @@ -388,7 +390,7 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // no changes should mean no queries to run expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -402,7 +404,7 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // no changes should mean no queries to run expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -417,10 +419,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`, `/foo`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual(indexBarFooArray) expect(staticQueriesThatRan).toEqual([`static-query-1`]) }, 99999) }) @@ -466,10 +468,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`, `/foo`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual(indexBarFooArray) expect(staticQueriesThatRan).toEqual([`static-query-1`]) }, 99999) @@ -478,7 +480,7 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // it should not rerun any page query expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -492,7 +494,7 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // it should not rerun any page query expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -541,10 +543,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`, `/foo`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual(indexBarFooArray) expect(staticQueriesThatRan).toEqual([`static-query-1`]) }, 99999) @@ -553,10 +555,10 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) - // we changed one node, so connection (`/`) and one detail page (`/bar`) should run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`]) + // we changed one node, so connection (`/`) and one detail page (`/bar/`) should run + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar/`]) expect(staticQueriesThatRan).toEqual([]) }, 999999) @@ -567,10 +569,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) - // we changed one node, so connection (`/`) and one detail page (`/bar`) should run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`]) + // we changed one node, so connection (`/`) and one detail page (`/bar/`) should run + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar/`]) expect(staticQueriesThatRan).toEqual([]) }, 999999) }) @@ -619,10 +621,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/`, `/bar`, `/foo`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual(indexBarFooArray) expect(staticQueriesThatRan).toEqual([`static-query-1`]) }, 99999) @@ -631,7 +633,7 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // no queries should run expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -645,7 +647,7 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/`, `/bar`, `/foo`]) + expect(pages).toEqual(indexBarFooArray) // no queries should run expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -1257,10 +1259,10 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/no-dep-page`]) + expect(pages).toEqual([`/no-dep-page/`]) // on initial we want all queries to run - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/no-dep-page`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/no-dep-page/`]) expect(staticQueriesThatRan).toEqual([]) }, 99999) @@ -1269,7 +1271,7 @@ describe(`query caching between builds`, () => { await setup() // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/no-dep-page`]) + expect(pages).toEqual([`/no-dep-page/`]) // no queries should run expect(pathsOfPagesWithQueriesThatRan).toEqual([]) @@ -1283,7 +1285,7 @@ describe(`query caching between builds`, () => { }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/no-dep-page`]) + expect(pages).toEqual([`/no-dep-page/`]) // no queries should run // Currently it actually re-run query for `/no-dep-page` @@ -1447,14 +1449,14 @@ describe(`query caching between builds`, () => { clearCache: true, }) // sanity check, to make sure test setup is correct - expect(pages).toEqual([`/bar`, `/bar-list`, `/foo`, `/foo-list`]) + expect(pages).toEqual([`/bar-list/`, `/bar/`, `/foo-list/`, `/foo/`]) // on initial we want query to run expect(pathsOfPagesWithQueriesThatRan).toEqual([ - `/bar`, - `/bar-list`, - `/foo`, - `/foo-list`, + `/bar-list/`, + `/bar/`, + `/foo-list/`, + `/foo/`, ]) }, 999999) @@ -1466,7 +1468,7 @@ describe(`query caching between builds`, () => { // Since there was no single "foo" node before - we will also re-run the "/foo" // (as maybe the new node matches this query now) - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/foo`, `/foo-list`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/foo-list/`, `/foo/`]) }, 999999) it(`re-runs queries when a foo node is deleted`, async () => { @@ -1475,7 +1477,7 @@ describe(`query caching between builds`, () => { restart: withRestarts, }) - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/foo`, `/foo-list`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/foo-list/`, `/foo/`]) }, 999999) it(`re-runs queries when a bar-2 node is added`, async () => { @@ -1489,7 +1491,7 @@ describe(`query caching between builds`, () => { // FIXME: this can be actually wrong behavior when filtering by any field other than "id" // we should rework this to use connection + filter for tracking. // In this case this test will also re-run "/bar" query (also affects other bar-related tests) - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar-list`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar-list/`]) }, 999999) it(`re-runs queries when a bar-2 node is deleted`, async () => { @@ -1498,7 +1500,7 @@ describe(`query caching between builds`, () => { restart: withRestarts, }) - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar-list`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar-list/`]) }, 999999) it(`re-runs queries when a bar-1 node is deleted`, async () => { @@ -1507,7 +1509,7 @@ describe(`query caching between builds`, () => { restart: withRestarts, }) - expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar`, `/bar-list`]) + expect(pathsOfPagesWithQueriesThatRan).toEqual([`/bar-list/`, `/bar/`]) }, 999999) } diff --git a/packages/gatsby/src/redux/actions/__tests__/internal.ts b/packages/gatsby/src/redux/actions/__tests__/internal.ts index 899e058a6544c..ea01df2b77a0f 100644 --- a/packages/gatsby/src/redux/actions/__tests__/internal.ts +++ b/packages/gatsby/src/redux/actions/__tests__/internal.ts @@ -28,7 +28,7 @@ describe(`setSiteConfig`, () => { "siteMetadata": Object { "hi": true, }, - "trailingSlash": "legacy", + "trailingSlash": "always", }, "type": "SET_SITE_CONFIG", } @@ -44,7 +44,7 @@ describe(`setSiteConfig`, () => { "jsxRuntime": "classic", "pathPrefix": "", "polyfill": true, - "trailingSlash": "legacy", + "trailingSlash": "always", }, "type": "SET_SITE_CONFIG", } diff --git a/packages/gatsby/src/services/__tests__/create-pages.ts b/packages/gatsby/src/services/__tests__/create-pages.ts index 4ec43929473d0..e3c6a90f68a5b 100644 --- a/packages/gatsby/src/services/__tests__/create-pages.ts +++ b/packages/gatsby/src/services/__tests__/create-pages.ts @@ -142,15 +142,15 @@ describe(`createPages service cleans up not recreated pages`, () => { expect(store.getState().pages.size).toEqual(2) expect(Array.from(store.getState().pages.keys())).toEqual([ - `/stateless/junk`, - `/stateful/junk`, + `/stateless/junk/`, + `/stateful/junk/`, ]) expect( - store.getState().pages.get(`/stateless/junk`) + store.getState().pages.get(`/stateless/junk/`) .isCreatedByStatefulCreatePages ).toEqual(false) expect( - store.getState().pages.get(`/stateful/junk`) + store.getState().pages.get(`/stateful/junk/`) .isCreatedByStatefulCreatePages ).toEqual(true) } else { @@ -167,10 +167,10 @@ describe(`createPages service cleans up not recreated pages`, () => { expect(store.getState().pages.size).toEqual(4) expect(Array.from(store.getState().pages.keys())).toEqual( expect.arrayContaining([ - `/stateless/stable`, - `/stateless/dynamic/1`, - `/stateful/stable`, - `/stateful/dynamic/1`, + `/stateless/stable/`, + `/stateless/dynamic/1/`, + `/stateful/stable/`, + `/stateful/dynamic/1/`, ]) ) @@ -181,7 +181,7 @@ describe(`createPages service cleans up not recreated pages`, () => { expect.objectContaining({ type: `DELETE_PAGE`, payload: expect.objectContaining({ - path: `/stateless/junk`, + path: `/stateless/junk/`, }), }), ]) @@ -191,7 +191,7 @@ describe(`createPages service cleans up not recreated pages`, () => { expect.objectContaining({ type: `DELETE_PAGE`, payload: expect.objectContaining({ - path: `/stateful/junk`, + path: `/stateful/junk/`, }), }), ]) @@ -207,10 +207,10 @@ describe(`createPages service cleans up not recreated pages`, () => { expect(Array.from(store.getState().pages.keys())).toEqual( expect.arrayContaining([ - `/stateless/stable`, - `/stateless/dynamic/2`, - `/stateful/stable`, - `/stateful/dynamic/1`, + `/stateless/stable/`, + `/stateless/dynamic/2/`, + `/stateful/stable/`, + `/stateful/dynamic/1/`, ]) ) @@ -220,7 +220,7 @@ describe(`createPages service cleans up not recreated pages`, () => { expect.objectContaining({ type: `DELETE_PAGE`, payload: expect.objectContaining({ - path: `/stateless/dynamic/1`, + path: `/stateless/dynamic/1/`, }), }), ]) diff --git a/packages/gatsby/src/utils/__tests__/gatsby-cloud-config.ts b/packages/gatsby/src/utils/__tests__/gatsby-cloud-config.ts index 53621e57fa691..3c05c07c3251c 100644 --- a/packages/gatsby/src/utils/__tests__/gatsby-cloud-config.ts +++ b/packages/gatsby/src/utils/__tests__/gatsby-cloud-config.ts @@ -3,7 +3,7 @@ import { constructConfigObject } from "../gatsby-cloud-config" describe(`constructConfigObject`, () => { it(`should return defaults with empty config`, () => { expect(constructConfigObject({})).toEqual({ - trailingSlash: `legacy`, + trailingSlash: `always`, pathPrefix: ``, }) }) diff --git a/packages/gatsby/src/utils/gatsby-cloud-config.ts b/packages/gatsby/src/utils/gatsby-cloud-config.ts index 5219c1d84b1a1..d8fd44b82d863 100644 --- a/packages/gatsby/src/utils/gatsby-cloud-config.ts +++ b/packages/gatsby/src/utils/gatsby-cloud-config.ts @@ -9,7 +9,7 @@ export function constructConfigObject( gatsbyConfig: IGatsbyConfig ): ConstructConfigObjectResponse { return { - trailingSlash: gatsbyConfig.trailingSlash ?? `legacy`, + trailingSlash: gatsbyConfig.trailingSlash ?? `always`, pathPrefix: gatsbyConfig.pathPrefix ?? ``, ...(gatsbyConfig.assetPrefix ? { assetPrefix: gatsbyConfig.assetPrefix }