Skip to content

Commit

Permalink
fix: respect 'force' and 'conditions' properties on redirects (#38657)
Browse files Browse the repository at this point in the history
* move force redirects changes to main repo

* empty commit

* fix: conditions and add e2e test case

* test: fix country condition assertion

---------

Co-authored-by: Michal Piechowiak <[email protected]>
  • Loading branch information
jenae-janzen and pieh authored Oct 24, 2023
1 parent f642940 commit 48d311e
Show file tree
Hide file tree
Showing 13 changed files with 373 additions and 38 deletions.
145 changes: 125 additions & 20 deletions e2e-tests/adapters/cypress/e2e/redirects.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { applyTrailingSlashOption } from "../../utils"

Cypress.on("uncaught:exception", (err) => {
Cypress.on("uncaught:exception", err => {
if (err.message.includes("Minified React error")) {
return false
}
Expand All @@ -14,44 +14,149 @@ describe("Redirects", () => {
it("should redirect from non-existing page to existing", () => {
cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH), {
failOnStatusCode: false,
}).waitForRouteChange()
.assertRoute(applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
})
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Hit`)
})
it("should respect that pages take precedence over redirects", () => {
cy.visit(applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH), {
failOnStatusCode: false,
}).waitForRouteChange()
.assertRoute(applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH))
cy.visit(
applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH),
{
failOnStatusCode: false,
}
)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/existing`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Existing`)
})
it("should respect force redirect", () => {
cy.visit(
applyTrailingSlashOption(
`/routes/redirect/existing-force`,
TRAILING_SLASH
),
{
failOnStatusCode: false,
}
)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Hit`)
})
it("should respect country condition on redirect", () => {
cy.visit(
applyTrailingSlashOption(
`/routes/redirect/country-condition`,
TRAILING_SLASH
),
{
failOnStatusCode: false,
headers: {
"x-nf-country": "us",
},
}
)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/hit-us`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Hit US`)

cy.visit(
applyTrailingSlashOption(
`/routes/redirect/country-condition`,
TRAILING_SLASH
),
{
failOnStatusCode: false,
headers: {
"x-nf-country": "de",
},
}
)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/hit-de`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Hit DE`)

// testing fallback
cy.visit(
applyTrailingSlashOption(
`/routes/redirect/country-condition`,
TRAILING_SLASH
),
{
failOnStatusCode: false,
headers: {
"x-nf-country": "fr",
},
}
)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)

cy.get(`h1`).should(`have.text`, `Hit`)
})
it("should support hash parameter on direct visit", () => {
cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `#anchor`, {
failOnStatusCode: false,
}).waitForRouteChange()
cy.visit(
applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `#anchor`,
{
failOnStatusCode: false,
}
).waitForRouteChange()

cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
cy.location(`pathname`).should(
`equal`,
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)
cy.location(`hash`).should(`equal`, `#anchor`)
cy.location(`search`).should(`equal`, ``)
})
it("should support search parameter on direct visit", () => {
cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `?query_param=hello`, {
failOnStatusCode: false,
}).waitForRouteChange()
cy.visit(
applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) +
`?query_param=hello`,
{
failOnStatusCode: false,
}
).waitForRouteChange()

cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
cy.location(`pathname`).should(
`equal`,
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)
cy.location(`hash`).should(`equal`, ``)
cy.location(`search`).should(`equal`, `?query_param=hello`)
})
it("should support search & hash parameter on direct visit", () => {
cy.visit(applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) + `?query_param=hello#anchor`, {
failOnStatusCode: false,
}).waitForRouteChange()
cy.visit(
applyTrailingSlashOption(`/redirect`, TRAILING_SLASH) +
`?query_param=hello#anchor`,
{
failOnStatusCode: false,
}
).waitForRouteChange()

cy.location(`pathname`).should(`equal`, applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH))
cy.location(`pathname`).should(
`equal`,
applyTrailingSlashOption(`/routes/redirect/hit`, TRAILING_SLASH)
)
cy.location(`hash`).should(`equal`, `#anchor`)
cy.location(`search`).should(`equal`, `?query_param=hello`)
})
})
})
50 changes: 46 additions & 4 deletions e2e-tests/adapters/gatsby-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,57 @@ import * as path from "path"
import type { GatsbyNode, GatsbyConfig } from "gatsby"
import { applyTrailingSlashOption } from "./utils"

const TRAILING_SLASH = (process.env.TRAILING_SLASH || `never`) as GatsbyConfig["trailingSlash"]
const TRAILING_SLASH = (process.env.TRAILING_SLASH ||
`never`) as GatsbyConfig["trailingSlash"]

export const createPages: GatsbyNode["createPages"] = ({ actions: { createRedirect, createSlice } }) => {
export const createPages: GatsbyNode["createPages"] = ({
actions: { createRedirect, createSlice },
}) => {
createRedirect({
fromPath: applyTrailingSlashOption("/redirect", TRAILING_SLASH),
toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
})
createRedirect({
fromPath: applyTrailingSlashOption("/routes/redirect/existing", TRAILING_SLASH),
fromPath: applyTrailingSlashOption(
"/routes/redirect/existing",
TRAILING_SLASH
),
toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
})
createRedirect({
fromPath: applyTrailingSlashOption(
"/routes/redirect/existing-force",
TRAILING_SLASH
),
toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
force: true,
})
createRedirect({
fromPath: applyTrailingSlashOption(
"/routes/redirect/country-condition",
TRAILING_SLASH
),
toPath: applyTrailingSlashOption("/routes/redirect/hit-us", TRAILING_SLASH),
conditions: {
country: ["us"],
},
})
createRedirect({
fromPath: applyTrailingSlashOption(
"/routes/redirect/country-condition",
TRAILING_SLASH
),
toPath: applyTrailingSlashOption("/routes/redirect/hit-de", TRAILING_SLASH),
conditions: {
country: ["de"],
},
})
// fallback if not matching a country condition
createRedirect({
fromPath: applyTrailingSlashOption(
"/routes/redirect/country-condition",
TRAILING_SLASH
),
toPath: applyTrailingSlashOption("/routes/redirect/hit", TRAILING_SLASH),
})

Expand All @@ -19,4 +61,4 @@ export const createPages: GatsbyNode["createPages"] = ({ actions: { createRedire
component: path.resolve(`./src/components/footer.jsx`),
context: {},
})
}
}
2 changes: 2 additions & 0 deletions e2e-tests/adapters/scripts/deploy-and-run/netlify.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const deployInfo = JSON.parse(deployResults.stdout)

process.env.DEPLOY_URL = deployInfo.deploy_url

console.log(`Deployed to ${deployInfo.deploy_url}`)

try {
await execa(`npm`, [`run`, npmScriptToRun], { stdio: `inherit` })
} finally {
Expand Down
14 changes: 14 additions & 0 deletions e2e-tests/adapters/src/pages/routes/redirect/existing-force.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as React from "react"
import Layout from "../../../components/layout"

const ExistingForcePage = () => {
return (
<Layout>
<h1>Existing Force</h1>
</Layout>
)
}

export default ExistingForcePage

export const Head = () => <title>Existing Force</title>
14 changes: 14 additions & 0 deletions e2e-tests/adapters/src/pages/routes/redirect/hit-de.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as React from "react"
import Layout from "../../../components/layout"

const HitDEPage = () => {
return (
<Layout>
<h1>Hit DE</h1>
</Layout>
)
}

export default HitDEPage

export const Head = () => <title>Hit DE</title>
14 changes: 14 additions & 0 deletions e2e-tests/adapters/src/pages/routes/redirect/hit-us.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as React from "react"
import Layout from "../../../components/layout"

const HitUSPage = () => {
return (
<Layout>
<h1>Hit US</h1>
</Layout>
)
}

export default HitUSPage

export const Head = () => <title>Hit US</title>
48 changes: 48 additions & 0 deletions packages/gatsby-adapter-netlify/src/__tests__/route-handler.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import fs from "fs-extra"
import { tmpdir } from "os"
import { join } from "path"
import type { IRedirectRoute, RoutesManifest } from "gatsby"
import {
injectEntries,
ADAPTER_MARKER_START,
ADAPTER_MARKER_END,
NETLIFY_PLUGIN_MARKER_START,
NETLIFY_PLUGIN_MARKER_END,
GATSBY_PLUGIN_MARKER_START,
processRoutesManifest,
} from "../route-handler"

function generateLotOfContent(placeholderCharacter: string): string {
Expand Down Expand Up @@ -142,4 +144,50 @@ describe(`route-handler`, () => {
})
})
})

describe(`createRedirects`, () => {
it(`honors the force parameter`, async () => {
const manifest: RoutesManifest = [
{
path: `/old-url`,
type: `redirect`,
toPath: `/new-url`,
status: 301,
headers: [{ key: `string`, value: `string` }],
force: true,
},
{
path: `/old-url2`,
type: `redirect`,
toPath: `/new-url2`,
status: 308,
headers: [{ key: `string`, value: `string` }],
force: false,
},
]

const { redirects } = processRoutesManifest(manifest)

// `!` is appended to status to mark forced redirect
expect(redirects).toMatch(/^\/old-url\s+\/new-url\s+301!$/m)
// `!` is not appended to status to mark not forced redirect
expect(redirects).toMatch(/^\/old-url2\s+\/new-url2\s+308$/m)
})

it(`honors the conditions parameter`, async () => {
const redirect: IRedirectRoute = {
path: `/old-url`,
type: `redirect`,
toPath: `/new-url`,
status: 200,
headers: [{ key: `string`, value: `string` }],
conditions: { language: [`ca`, `us`] },
}

const { redirects } = processRoutesManifest([redirect])
expect(redirects).toMatch(
/^\/old-url\s+\/new-url\s+200\s+Language=ca,us$/m
)
})
})
})
Loading

0 comments on commit 48d311e

Please sign in to comment.