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

fix(gatsby): don't show error for proper redirects #19789

Merged
merged 12 commits into from
Apr 28, 2020
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
let spy
Cypress.on(`window:before:load`, win => {
spy = cy.spy(win.console, `error`).as(`spyWinConsoleError`)
})

const runTests = () => {
it(`should redirect page to index page when there is no such page`, () => {
cy.visit(`/redirect-without-page`).waitForRouteChange()

cy.location(`pathname`).should(`equal`, `/`)
cy.then(() => {
const calls = spy.getCalls()

const callsAboutRedirectMatchingPage = calls.filter(call => {
return call.args[0].includes(
"matches both a page and a redirect; this is probably not intentional."
)
})

expect(callsAboutRedirectMatchingPage.length).to.equal(0)
})
})

it(`should redirect page to index page even there is a such page`, () => {
cy.visit(`/redirect`).waitForRouteChange()

cy.location(`pathname`).should(`equal`, `/`)
cy.then(() => {
const calls = spy.getCalls()

const callsAboutRedirectMatchingPage = calls.filter(call => {
return call.args[0].includes(
"matches both a page and a redirect; this is probably not intentional."
)
})

expect(callsAboutRedirectMatchingPage.length).not.to.equal(0)
expect(spy).to.be.calledWith(
`The route "/redirect" matches both a page and a redirect; this is probably not intentional.`
)
})
})
}

describe(`redirect`, () => {
describe("404 is present", () => {
before(() => {
cy.task(`restoreAllBlockedResources`)
})

// this is sanity check for this group
it(`make sure 404 is present`, () => {
cy.visit(`/______not_existing_page`).waitForRouteChange()
cy.queryByText("Preview custom 404 page").click()
cy.queryByText("A custom 404 page wasn't detected", {
exact: false,
}).should(`not.exist`)
cy.queryByText(
"You just hit a route that does not exist... the sadness."
).should(`exist`)
})

runTests()
})

describe("no 404", () => {
before(() => {
cy.task(`restoreAllBlockedResources`)

cy.task(`blockAssetsForPage`, {
pagePath: `/404`,
filter: `page-data`,
})
cy.task(`blockAssetsForPage`, {
pagePath: `/404.html`,
filter: `page-data`,
})
})

after(() => {
cy.task(`restoreAllBlockedResources`)
})

it(`make sure 404 is NOT present`, () => {
cy.visit(`/______not_existing_page`).waitForRouteChange()
cy.queryByText("Preview custom 404 page").click()
cy.queryByText("A custom 404 page wasn't detected", {
exact: false,
}).should(`exist`)
cy.queryByText(
"You just hit a route that does not exist... the sadness.",
{ exact: false }
).should(`not.exist`)
})

runTests()
})
})
66 changes: 66 additions & 0 deletions e2e-tests/development-runtime/cypress/plugins/block-resources.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// this is partial copy of e2e-tests/production-runtime/cypress/plugins/block-resources.js
// partial because there is no asset blocking possibly in develop (we have single, virtual development bundle)

const fs = require(`fs-extra`)
const path = require(`path`)
const glob = require(`glob`)

const publicDir = path.join(__dirname, `..`, `..`, `public`)

const moveAsset = (from, to) => {
const fromExists = fs.existsSync(from)
const toExists = fs.existsSync(to)

if (fromExists && !toExists) {
fs.moveSync(from, to, {
overwrite: true,
})
}
}

const restorePageData = hiddenPath => {
if (path.basename(hiddenPath).charAt(0) !== `_`) {
throw new Error(`hiddenPath should have _ prefix`)
}
const restoredPath = path.join(
path.dirname(hiddenPath),
path.basename(hiddenPath).slice(1)
)
moveAsset(hiddenPath, restoredPath)
}

const getPageDataPath = pagePath => {
const fixedPagePath = pagePath === `/` ? `index` : pagePath
return path.join(publicDir, `page-data`, fixedPagePath, `page-data.json`)
}

const getHiddenPageDataPath = pagePath => {
const fixedPagePath = pagePath === `/` ? `index` : pagePath
return path.join(publicDir, `page-data`, fixedPagePath, `_page-data.json`)
}

const blockPageData = pagePath =>
moveAsset(getPageDataPath(pagePath), getHiddenPageDataPath(pagePath))

const blockAssetsForPage = ({ pagePath, filter }) => {
if (filter === `all` || filter === `page-data`) {
blockPageData(pagePath)
}

console.log(`Blocked assets for path "${pagePath}" [${filter}]`)
return null
}

const restore = () => {
const globPattern = path.join(publicDir, `/page-data/**`, `_page-data.json`)
const hiddenPageDatas = glob.sync(globPattern)
hiddenPageDatas.forEach(restorePageData)

console.log(`Restored resources`)
return null
}

module.exports = {
restoreAllBlockedResources: restore,
blockAssetsForPage,
}
2 changes: 2 additions & 0 deletions e2e-tests/development-runtime/cypress/plugins/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@

// This function is called when a project is opened or re-opened (e.g. due to
// the project's config changing)
const blockResources = require(`./block-resources`)

module.exports = (on, config) => {
// `on` is used to hook into various events Cypress emits
// `config` is the resolved Cypress config
on(`task`, Object.assign({}, blockResources))
}
16 changes: 15 additions & 1 deletion e2e-tests/development-runtime/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ exports.onCreateNode = function onCreateNode({
}

exports.createPages = async function createPages({
actions: { createPage },
actions: { createPage, createRedirect },
graphql,
}) {
const { data } = await graphql(`
Expand Down Expand Up @@ -87,6 +87,20 @@ exports.createPages = async function createPages({
path: `/client-only-paths/static`,
component: path.resolve(`src/templates/static-page.js`),
})

createRedirect({
fromPath: `/redirect-without-page`,
toPath: `/`,
isPermanent: true,
redirectInBrowser: true,
})

createRedirect({
fromPath: `/redirect`,
toPath: `/`,
isPermanent: true,
redirectInBrowser: true,
})
}

exports.onCreatePage = async ({ page, actions }) => {
Expand Down
13 changes: 13 additions & 0 deletions e2e-tests/development-runtime/src/pages/redirect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from "react"

import Layout from "../components/layout"
import SEO from "../components/seo"

const Redirect = () => (
<Layout>
<SEO title="Redirect" />
<p>Redirecting...</p>
</Layout>
)

export default Redirect
4 changes: 1 addition & 3 deletions packages/gatsby/cache-dir/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ function maybeRedirect(pathname) {

if (redirect != null) {
if (process.env.NODE_ENV !== `production`) {
const pageResources = loader.loadPageSync(pathname)

if (pageResources != null) {
pvdz marked this conversation as resolved.
Show resolved Hide resolved
if (!loader.isPageNotFound(pathname)) {
pieh marked this conversation as resolved.
Show resolved Hide resolved
console.error(
`The route "${pathname}" matches both a page and a redirect; this is probably not intentional.`
)
Expand Down