Skip to content

Commit

Permalink
fix(gatsby): Fix client side routing match paths regression (#15010)
Browse files Browse the repository at this point in the history
* export loader.findMatchPath

* reorganize production-app explicit navigate if statement

* strip BASE_PATH prefix before finding matchPath

* Add end to end tests

* Remove screenshots

* Update tests

* Add .gitignore

* Move tests to development runtime

* Hard code path in tests

* Remove serve e2e tests

* Add tests in production runtime
  • Loading branch information
Moocar authored and GatsbyJS Bot committed Jun 24, 2019
1 parent 56b4516 commit f52bbac
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
describe(`develop`, () => {
it(`works with the client side path`, () => {
cy.visit(`/paths/app`).waitForRouteChange()
cy.get(`h1`).contains(`App Page`)
})

it(`works with a child of the client side path`, () => {
cy.visit(`/paths/app/12345`).waitForRouteChange()
cy.get(`h1`).contains(`Not Found in App`)
})
})
10 changes: 10 additions & 0 deletions e2e-tests/development-runtime/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,13 @@ exports.createPages = async function createPages({
})
})
}

exports.onCreatePage = async ({ page, actions }) => {
const { createPage } = actions

if (page.path.match(/^\/paths/)) {
page.matchPath = `/paths/*`
}

createPage(page)
}
15 changes: 15 additions & 0 deletions e2e-tests/development-runtime/src/pages/paths.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Router } from "@reach/router"
import React from "react"

const App = () => <h1>App Page</h1>

const NotFound = () => <h1>Not Found in App</h1>

const AppPage = () => (
<Router>
<App path="/paths/app" />
<NotFound default />
</Router>
)

export default AppPage
11 changes: 11 additions & 0 deletions e2e-tests/production-runtime/cypress/integration/paths.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
describe(`serve`, () => {
it(`works with the client side path`, () => {
cy.visit(`/paths/app`).waitForRouteChange()
cy.get(`h1`).contains(`App Page`)
})

it(`works with a child of the client side path`, () => {
cy.visit(`/paths/app/12345`).waitForRouteChange()
cy.get(`h1`).contains(`Not Found in App`)
})
})
4 changes: 4 additions & 0 deletions e2e-tests/production-runtime/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ exports.onCreatePage = ({ page, actions }) => {
})
break
}
if (page.path.match(/^\/paths/)) {
page.matchPath = `/paths/*`
actions.createPage(page)
}
}
15 changes: 15 additions & 0 deletions e2e-tests/production-runtime/src/pages/paths.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Router } from '@reach/router'
import React from 'react'

const App = () => <h1>App Page</h1>

const NotFound = () => <h1>Not Found in App</h1>

const AppPage = () => (
<Router>
<App path="/paths/app" />
<NotFound default />
</Router>
)

export default AppPage
6 changes: 4 additions & 2 deletions packages/gatsby/cache-dir/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ if (process.env.NODE_ENV !== `production`) {
// Cache for `cleanAndFindPath()`. In case `match-paths.json` is large
const cleanAndFindPathCache = {}

const findMatchPath = (matchPaths, trimmedPathname) => {
const findMatchPath = trimmedPathname => {
for (const { matchPath, path } of matchPaths) {
if (match(matchPath, trimmedPathname)) {
return path
Expand Down Expand Up @@ -66,7 +66,7 @@ const cleanAndFindPath = rawPathname => {
return cleanAndFindPathCache[trimmedPathname]
}

let foundPath = findMatchPath(matchPaths, trimmedPathname)
let foundPath = findMatchPath(trimmedPathname)
if (!foundPath) {
if (trimmedPathname === `/index.html`) {
foundPath = `/`
Expand Down Expand Up @@ -371,6 +371,8 @@ const queue = {

doesPageHtmlExistSync: rawPath =>
pageHtmlExistsResults[cleanAndFindPath(rawPath)],

findMatchPath,
}

export const postInitialRenderWork = () => {
Expand Down
23 changes: 15 additions & 8 deletions packages/gatsby/cache-dir/production-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import asyncRequires from "./async-requires"
import matchPaths from "./match-paths.json"
import loader, { setApiRunnerForLoader } from "./loader"
import EnsureResources from "./ensure-resources"
import stripPrefix from "./strip-prefix"

window.asyncRequires = asyncRequires
window.___emitter = emitter
Expand Down Expand Up @@ -62,17 +63,23 @@ apiRunnerAsync(`onClientEntry`).then(() => {
}

const { pagePath, location: browserLoc } = window

// Explicitly call navigate if the canonical path (window.pagePath)
// is different to the browser path (window.location.pathname). But
// only if NONE of the following conditions hold:
//
// - The url matches a client side route (page.matchPath)
// - it's a 404 page
// - it's the offline plugin shell (/offline-plugin-app-shell-fallback/)
if (
// Make sure the window.page object is defined
pagePath &&
// The canonical path doesn't match the actual path (i.e. the address bar)
__BASE_PATH__ + pagePath !== browserLoc.pathname &&
// Ignore 404 pages, since we want to keep the same URL
pagePath !== `/404.html` &&
!pagePath.match(/^\/404\/?$/) &&
// Also ignore the offline shell (since when using the offline plugin, all
// pages have this canonical path)
!pagePath.match(/^\/offline-plugin-app-shell-fallback\/?$/)
!(
loader.findMatchPath(stripPrefix(browserLoc.pathname, __BASE_PATH__)) ||
pagePath === `/404.html` ||
pagePath.match(/^\/404\/?$/) ||
pagePath.match(/^\/offline-plugin-app-shell-fallback\/?$/)
)
) {
navigate(__BASE_PATH__ + pagePath + browserLoc.search + browserLoc.hash, {
replace: true,
Expand Down

0 comments on commit f52bbac

Please sign in to comment.