Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gatsby): trailingSlash config option #34268

Merged
merged 76 commits into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
49f1672
config option
LekoArts Dec 14, 2021
1e7646c
Merge branch 'master' into trailing-slash
LekoArts Dec 14, 2021
db532e4
ssr part
LekoArts Dec 15, 2021
0f8f353
docs, types
LekoArts Dec 15, 2021
55d47aa
move applyTrailingSlash to gatsby-page-utils
LekoArts Dec 15, 2021
248877f
dev-404-page handling
LekoArts Dec 15, 2021
396f986
snapshot update
LekoArts Dec 15, 2021
40aea6f
wip
LekoArts Dec 15, 2021
d590d4f
docs
LekoArts Dec 15, 2021
d4a149c
add todo
LekoArts Dec 15, 2021
dae6ddb
gatsby-link :yey:
LekoArts Dec 16, 2021
5e02152
move func into option if case
LekoArts Dec 16, 2021
8949a3e
put rewriteLinkPath to own file + add unit tests
LekoArts Dec 16, 2021
6648b77
fix order of args
LekoArts Dec 16, 2021
992c6d6
fix number behavior
LekoArts Dec 17, 2021
9b2acc5
e2e test addition
LekoArts Dec 17, 2021
b6fdb4d
circle ci addition
LekoArts Dec 17, 2021
429c75e
silence warning
LekoArts Dec 17, 2021
ba4dbd9
revert create-path
LekoArts Dec 17, 2021
07f9343
use createPath + applyTrailingSlashOption
LekoArts Dec 17, 2021
3909933
begin derive-path work
LekoArts Dec 17, 2021
aeec8fd
Merge branch 'master' into trailing-slash
LekoArts Dec 19, 2021
9a89baa
Merge branch 'master' into trailing-slash
LekoArts Jan 4, 2022
abd7c02
change derivePath function
LekoArts Jan 4, 2022
c539129
add client-only and index page
LekoArts Jan 4, 2022
7213133
remove unused import
LekoArts Jan 4, 2022
be16962
handle route params + make ignore work?
LekoArts Jan 4, 2022
6c8e3bc
splat routes + gatsbyPath
LekoArts Jan 5, 2022
0623374
fix ts error
LekoArts Jan 5, 2022
1074661
use npm-run-all
LekoArts Jan 5, 2022
531d42a
prep tests
LekoArts Jan 6, 2022
b062129
always
LekoArts Jan 6, 2022
3a5c0a4
fix gatsbyPath
LekoArts Jan 6, 2022
8147f56
update tests
LekoArts Jan 6, 2022
97e6a3c
update trailingSlash ts type
LekoArts Jan 6, 2022
5a81041
Merge branch 'master' into trailing-slash
LekoArts Jan 6, 2022
0708e46
Update types.ts
LekoArts Jan 6, 2022
a5c77da
add cypress dashboard entries
LekoArts Jan 6, 2022
d7963af
update cypress script to record
LekoArts Jan 6, 2022
10996bd
Merge branch 'master' into trailing-slash
LekoArts Jan 7, 2022
5cab2e4
update package.json from merge
LekoArts Jan 7, 2022
f8b9d2e
tmp
LekoArts Jan 7, 2022
a370934
Merge branch 'master' into trailing-slash
LekoArts Jan 10, 2022
0a522d4
use regex for local-link and add test
LekoArts Jan 10, 2022
a6244b4
clean between test runs
LekoArts Jan 10, 2022
d2477bb
feat(gatsby): Send config keys over IPC (#34411)
LekoArts Jan 11, 2022
917d2be
Merge branch 'master' into trailing-slash
LekoArts Jan 11, 2022
1b3b47f
get tests to pass?
LekoArts Jan 11, 2022
1887b51
Merge branch 'master' into trailing-slash
LekoArts Jan 12, 2022
ecdfa26
update script
LekoArts Jan 12, 2022
1bd7cca
add legacy tests
LekoArts Jan 13, 2022
94b1e78
wip dev
LekoArts Jan 13, 2022
267f6cd
add express middleware
LekoArts Jan 13, 2022
a583544
tests
LekoArts Jan 13, 2022
84fb2cf
Update legacy.js
LekoArts Jan 13, 2022
3cb65ed
Update legacy.js
LekoArts Jan 13, 2022
bb9b1d8
Update serve.ts
LekoArts Jan 14, 2022
0970fbf
Merge branch 'master' into trailing-slash
LekoArts Jan 14, 2022
43537d8
split up tests into more fine-grained
LekoArts Jan 16, 2022
3cf0dc2
format
LekoArts Jan 16, 2022
0a8dfc0
add clean in between build & develop + darkmode
LekoArts Jan 16, 2022
257febb
fix two smaller todos
LekoArts Jan 17, 2022
8b448ce
fix(gatsby-plugin-gatsby-cloud): Revert removal of _gatsby-config.jso…
marvinjude Jan 19, 2022
60ff0ab
Update packages/gatsby-plugin-gatsby-cloud/src/create-site-config.js
LekoArts Jan 19, 2022
b3f4783
feat(gatsby): Add telemetry tracking for trailing slash option (#34529)
marvinjude Jan 19, 2022
a501683
change local-link
LekoArts Jan 20, 2022
4ee7ff6
Merge branch 'master' into trailing-slash
LekoArts Jan 20, 2022
159969b
feat(gatsby): clear cache when trailing slash option changes (#34547)
marvinjude Jan 24, 2022
dd8cc06
test(gatsby): Trailing slash client only splat (#34538)
tyhopp Jan 24, 2022
9bffe80
Merge branch 'master' into trailing-slash
LekoArts Jan 24, 2022
820bba8
Update package.json
LekoArts Jan 24, 2022
8f034c6
test(gatsby): Assert 301 redirects in E2E Tests (#34554)
marvinjude Jan 26, 2022
8235639
Apply suggestions from code review
LekoArts Jan 27, 2022
50f1ae5
fix lint
LekoArts Jan 27, 2022
ad04771
code review
LekoArts Jan 28, 2022
caafd5c
Merge remote-tracking branch 'upstream/master' into trailing-slash
Jan 28, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports = {
__ASSET_PREFIX__: true,
_CFLAGS_: true,
__GATSBY: true,
__TRAILING_SLASH__: true,
},
rules: {
"@babel/no-unused-expressions": [
Expand Down
33 changes: 23 additions & 10 deletions docs/docs/reference/config-files/gatsby-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,16 @@ module.exports = {
Options available to set within `gatsby-config.js` include:

1. [siteMetadata](#sitemetadata) (object)
2. [plugins](#plugins) (array)
3. [flags](#flags) (object)
4. [pathPrefix](#pathprefix) (string)
5. [polyfill](#polyfill) (boolean)
6. [mapping](#mapping-node-types) (object)
7. [proxy](#proxy) (object)
8. [developMiddleware](#advanced-proxying-with-developmiddleware) (function)
1. [plugins](#plugins) (array)
1. [flags](#flags) (object)
1. [pathPrefix](#pathprefix) (string)
1. [trailingSlash](#trailingslash) (string)
1. [polyfill](#polyfill) (boolean)
1. [mapping](#mapping-node-types) (object)
1. [proxy](#proxy) (object)
1. [developMiddleware](#advanced-proxying-with-developmiddleware) (function)
1. [jsxRuntime](#jsxruntime) (string)
1. [jsxImportSource](#jsximportsource) (string)

## siteMetadata

Expand All @@ -68,7 +71,7 @@ This way you can store it in one place, and pull it whenever you need it. If you

See a full description and sample usage in [Gatsby.js Tutorial Part Four](/docs/tutorial/part-4/#data-in-gatsby).

## Plugins
## plugins

Plugins are Node.js packages that implement Gatsby APIs. The config file accepts an array of plugins. Some plugins may need only to be listed by name, while others may take options (see the docs for individual plugins).

Expand Down Expand Up @@ -135,7 +138,7 @@ module.exports = {

See more about [Plugins](/docs/plugins/) for more on utilizing plugins, and to see available official and community plugins.

## Flags
## flags

Flags let sites enable experimental or upcoming changes that are still in testing or waiting for the next major release.

Expand All @@ -161,7 +164,17 @@ module.exports = {

See more about [Adding a Path Prefix](/docs/how-to/previews-deploys-hosting/path-prefix/).

## Polyfill
## trailingSlash

Configures the creation of URLs and whether to remove, append, or ignore trailing slashes.
LekoArts marked this conversation as resolved.
Show resolved Hide resolved

- `always`: Always add trailing slashes to each URL, e.g. `/x` to `/x/`.
- `never`: Remove all trailing slashes on each URL, e.g. `/x/` to `/x`.
- `ignore`: Don't automatically modify the URL

Until Gatsby v4 it'll be set to `legacy` by default, in Gatsby v5 the default mode will be `always`.
LekoArts marked this conversation as resolved.
Show resolved Hide resolved

## polyfill

Gatsby uses the ES6 Promise API. Because some browsers don't support this, Gatsby includes a Promise polyfill by default.

Expand Down
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ module.exports = {
"^weak-lru-cache$": `<rootDir>/node_modules/weak-lru-cache/dist/index.cjs`,
"^ordered-binary$": `<rootDir>/node_modules/ordered-binary/dist/index.cjs`,
"^msgpackr$": `<rootDir>/node_modules/msgpackr/dist/node.cjs`,
"^gatsby-page-utils/(.*)$": `gatsby-page-utils/dist/$1`, // Workaround for https://github.com/facebook/jest/issues/9771
},
snapshotSerializers: [`jest-serializer-path`],
collectCoverageFrom: coverageDirs,
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby-link/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"dependencies": {
"@babel/runtime": "^7.15.4",
"@types/reach__router": "^1.3.9",
"gatsby-page-utils": "^2.5.0-next.0",
"prop-types": "^15.7.2"
},
"devDependencies": {
Expand Down
57 changes: 57 additions & 0 deletions packages/gatsby-link/src/__tests__/rewrite-link-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { rewriteLinkPath } from "../rewrite-link-path"

beforeEach(() => {
global.__TRAILING_SLASH__ = ``
})

const getRewriteLinkPath = (option = `legacy`) => {
global.__TRAILING_SLASH__ = option
return rewriteLinkPath
}

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`
)
})
it(`handles always option`, () => {
expect(getRewriteLinkPath(`always`)(`/path`, `/`)).toBe(`/path/`)
expect(getRewriteLinkPath(`always`)(`/path/`, `/`)).toBe(`/path/`)
expect(getRewriteLinkPath(`always`)(`/path#hash`, `/`)).toBe(`/path/#hash`)
expect(getRewriteLinkPath(`always`)(`/path?query_param=hello`, `/`)).toBe(
`/path/?query_param=hello`
)
expect(
getRewriteLinkPath(`always`)(`/path?query_param=hello#anchor`, `/`)
).toBe(`/path/?query_param=hello#anchor`)
})
it(`handles never option`, () => {
expect(getRewriteLinkPath(`never`)(`/path`, `/`)).toBe(`/path`)
expect(getRewriteLinkPath(`never`)(`/path/`, `/`)).toBe(`/path`)
expect(getRewriteLinkPath(`never`)(`/path/#hash`, `/`)).toBe(`/path#hash`)
expect(getRewriteLinkPath(`never`)(`/path/?query_param=hello`, `/`)).toBe(
`/path?query_param=hello`
)
expect(
getRewriteLinkPath(`never`)(`/path/?query_param=hello#anchor`, `/`)
).toBe(`/path?query_param=hello#anchor`)
})
it(`handles ignore option`, () => {
expect(getRewriteLinkPath(`ignore`)(`/path`, `/`)).toBe(`/path`)
expect(getRewriteLinkPath(`ignore`)(`/path/`, `/`)).toBe(`/path/`)
expect(getRewriteLinkPath(`ignore`)(`/path#hash`, `/`)).toBe(`/path#hash`)
expect(getRewriteLinkPath(`ignore`)(`/path?query_param=hello`, `/`)).toBe(
`/path?query_param=hello`
)
expect(
getRewriteLinkPath(`ignore`)(`/path?query_param=hello#anchor`, `/`)
).toBe(`/path?query_param=hello#anchor`)
})
})
30 changes: 2 additions & 28 deletions packages/gatsby-link/src/index.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import PropTypes from "prop-types"
import React from "react"
import { Link, Location } from "@gatsbyjs/reach-router"
import { resolve } from "@gatsbyjs/reach-router/lib/utils"

import { parsePath } from "./parse-path"
import { isLocalLink } from "./is-local-link"
import { rewriteLinkPath } from "./rewrite-link-path"

export { parsePath }

const isAbsolutePath = path => path?.startsWith(`/`)

export function withPrefix(path, prefix = getGlobalBasePrefix()) {
if (!isLocalLink(path)) {
return path
Expand Down Expand Up @@ -39,34 +37,10 @@ const getGlobalBasePrefix = () =>
: undefined
: __BASE_PATH__

const isLocalLink = path =>
path &&
!path.startsWith(`http://`) &&
!path.startsWith(`https://`) &&
!path.startsWith(`//`)

export function withAssetPrefix(path) {
return withPrefix(path, getGlobalPathPrefix())
}

function absolutify(path, current) {
// If it's already absolute, return as-is
if (isAbsolutePath(path)) {
return path
}
return resolve(path, current)
}

const rewriteLinkPath = (path, relativeTo) => {
if (typeof path === `number`) {
return path
}
if (!isLocalLink(path)) {
return path
}
return isAbsolutePath(path) ? withPrefix(path) : absolutify(path, relativeTo)
}

const NavLinkPropTypes = {
activeClassName: PropTypes.string,
activeStyle: PropTypes.object,
Expand Down
5 changes: 5 additions & 0 deletions packages/gatsby-link/src/is-local-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const isLocalLink = path =>
LekoArts marked this conversation as resolved.
Show resolved Hide resolved
path &&
!path.startsWith(`http://`) &&
!path.startsWith(`https://`) &&
!path.startsWith(`//`)
45 changes: 45 additions & 0 deletions packages/gatsby-link/src/rewrite-link-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { resolve } from "@gatsbyjs/reach-router/lib/utils"
// Specific import to treeshake Node.js stuff
import { applyTrailingSlashOption } from "gatsby-page-utils/apply-trailing-slash-option"
import { parsePath } from "./parse-path"
import { isLocalLink } from "./is-local-link"
import { withPrefix } from "."

const isAbsolutePath = path => path?.startsWith(`/`)

const getGlobalTrailingSlash = () =>
process.env.NODE_ENV !== `production`
? typeof __TRAILING_SLASH__ !== `undefined`
? __TRAILING_SLASH__
: undefined
: __TRAILING_SLASH__

function absolutify(path, current) {
// If it's already absolute, return as-is
if (isAbsolutePath(path)) {
return path
}
return resolve(path, current)
}

export const rewriteLinkPath = (path, relativeTo) => {
const { pathname, search, hash } = parsePath(path)
const option = getGlobalTrailingSlash()
let adjustedPath = path

if (typeof path === `number`) {
return path
}
if (!isLocalLink(path)) {
return path
}

if (option === `always` || option === `never`) {
const output = applyTrailingSlashOption(pathname, option)
adjustedPath = `${output}${search}${hash}`
}

return isAbsolutePath(adjustedPath)
? withPrefix(adjustedPath)
: absolutify(adjustedPath, relativeTo)
}
4 changes: 4 additions & 0 deletions packages/gatsby-page-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
"description": "Gatsby library that helps creating pages",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"exports": {
".": "./dist/index.js",
"./*": "./dist/*.js"
},
"scripts": {
"build": "babel src --out-dir dist/ --ignore \"**/__tests__\" --extensions \".ts\"",
"typegen": "rimraf \"dist/**/*.d.ts\" && tsc --emitDeclarationOnly --declaration --declarationDir dist/",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { applyTrailingSlashOption } from "../apply-trailing-slash-option"

describe(`applyTrailingSlashOption`, () => {
const indexPage = `/`
const withoutSlash = `/nested/path`
const withSlash = `/nested/path/`

it(`returns / for root index page`, () => {
expect(applyTrailingSlashOption(indexPage)).toEqual(indexPage)
})
describe(`always`, () => {
it(`should add trailing slash`, () => {
expect(applyTrailingSlashOption(withoutSlash, `always`)).toEqual(
withSlash
)
})
it(`should leave existing slash`, () => {
expect(applyTrailingSlashOption(withSlash, `always`)).toEqual(withSlash)
})
})
describe(`never`, () => {
it(`should leave root index`, () => {
expect(applyTrailingSlashOption(indexPage, `never`)).toEqual(indexPage)
})
it(`should remove trailing slashes`, () => {
expect(applyTrailingSlashOption(withSlash, `never`)).toEqual(withoutSlash)
})
it(`should leave non-trailing paths`, () => {
expect(applyTrailingSlashOption(withoutSlash, `never`)).toEqual(
withoutSlash
)
})
})
describe(`ignore`, () => {
it(`should return input (trailing)`, () => {
expect(applyTrailingSlashOption(withSlash, `ignore`)).toEqual(withSlash)
})
it(`should return input (non-trailing)`, () => {
expect(applyTrailingSlashOption(withoutSlash, `ignore`)).toEqual(
withoutSlash
)
})
})
describe(`legacy`, () => {
it(`should do nothing`, () => {
expect(applyTrailingSlashOption(withSlash)).toEqual(withSlash)
expect(applyTrailingSlashOption(withoutSlash)).toEqual(withoutSlash)
})
})
})
30 changes: 30 additions & 0 deletions packages/gatsby-page-utils/src/__tests__/create-path.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,39 @@
import { createPath } from "../create-path"

const withoutSlash = `/nested/path`
const withSlash = `/nested/path/`

describe(`create-path`, () => {
it(`should create unix paths`, () => {
const paths = [`b/c/de`, `bee`, `b/d/c/`]

expect(paths.map(p => createPath(p))).toMatchSnapshot()
})
it(`should handle index page`, () => {
expect(createPath(`/index/`)).toEqual(`/`)
expect(createPath(`/index`)).toEqual(`/`)
})
it(`should output with trailing slash in legacy mode`, () => {
expect(createPath(withoutSlash)).toEqual(withSlash)
expect(createPath(`/index/`)).toEqual(`/`)
expect(createPath(`/index`)).toEqual(`/`)
})
it(`should add trailing slash with always mode`, () => {
expect(createPath(withoutSlash, `always`)).toEqual(withSlash)
expect(createPath(withSlash, `always`)).toEqual(withSlash)
expect(createPath(`/index/`, `always`)).toEqual(`/`)
expect(createPath(`/index`, `always`)).toEqual(`/`)
})
it(`should not add trailing slash with never mode`, () => {
expect(createPath(withSlash, `never`)).toEqual(withoutSlash)
expect(createPath(withoutSlash, `never`)).toEqual(withoutSlash)
expect(createPath(`/index/`, `never`)).toEqual(`/`)
expect(createPath(`/index`, `never`)).toEqual(`/`)
})
it(`should not change anything in ignore mode`, () => {
expect(createPath(withSlash, `ignore`)).toEqual(withSlash)
expect(createPath(withoutSlash, `ignore`)).toEqual(withoutSlash)
expect(createPath(`/index/`, `ignore`)).toEqual(`/`)
expect(createPath(`/index`, `ignore`)).toEqual(`/`)
})
})
21 changes: 21 additions & 0 deletions packages/gatsby-page-utils/src/apply-trailing-slash-option.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// TODO(v5): Remove legacy setting and default to "always"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a task/issue for all of these v5 TODOs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll eventually look at those once v5 approaches

export type TrailingSlash = "always" | "never" | "ignore" | "legacy"

export const applyTrailingSlashOption = (
input: string,
option: TrailingSlash = `legacy`
): string => {
const hasHtmlSuffix = input.endsWith(`.html`)
if (input === `/`) return input
if (hasHtmlSuffix) {
option = `never`
}
if (option === `always`) {
return input.endsWith(`/`) ? input : `${input}/`
}
if (option === `never`) {
return input.endsWith(`/`) ? input.slice(0, -1) : input
}

return input
}
22 changes: 20 additions & 2 deletions packages/gatsby-page-utils/src/create-path.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { parse, posix } from "path"
import {
applyTrailingSlashOption,
TrailingSlash,
} from "./apply-trailing-slash-option"

export function createPath(filePath: string): string {
export function createPath(
filePath: string,
option: TrailingSlash = `legacy`
): string {
const { dir, name } = parse(filePath)
const parsedName = name === `index` ? `` : name
const hasHtmlSuffix = parsedName.endsWith(`.html`)

return posix.join(`/`, dir, parsedName, `/`)
if (hasHtmlSuffix) {
option = `never`
}

const hasTrailing = filePath.endsWith(`/`)
const parsedNameWithSlash = `${parsedName}${
option === `legacy` || hasTrailing ? `/` : ``
}`
const postfix = applyTrailingSlashOption(parsedNameWithSlash, option)

return posix.join(`/`, dir, postfix)
}
Loading