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): use pathPrefix for SSR/DSG page-data requests with gatsby serve #36231

Merged
merged 3 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions e2e-tests/path-prefix/cypress/integration/navigate.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ describe(`navigate`, () => {
.location(`pathname`)
.should(`eq`, `${pathPrefix}/subdirectory/page-2`)
})

it(`can navigate to SSR page`, () => {
cy.getTestElement(`page-ssr-button-link`)
.click()
.waitForRouteChange()
.location(`pathname`)
.should(`eq`, withTrailingSlash(`${pathPrefix}/ssr`))

cy.getTestElement(`server-data`).contains(`foo`)
})
})

it(`can navigate to 404`, () => {
Expand Down
69 changes: 46 additions & 23 deletions e2e-tests/path-prefix/cypress/integration/path-prefix.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,43 @@ const { pathPrefix } = require(`../../gatsby-config`)
const withTrailingSlash = url => `${url}/`

describe(`Production pathPrefix`, () => {
beforeEach(() => {
cy.visit(`/`).waitForRouteChange()
})
describe(`navigation`, () => {
beforeEach(() => {
cy.visit(`/`).waitForRouteChange()
})

it(`returns 200 on base route`, () => {
cy.location(`pathname`).should(`eq`, withTrailingSlash(pathPrefix))
})
it(`returns 200 on base route`, () => {
cy.location(`pathname`).should(`eq`, withTrailingSlash(pathPrefix))
})

it(`renders static image`, () => {
cy.getTestElement(`static-image`)
.should(`have.attr`, `srcset`)
.and(srcset => {
srcset.split(/\s*,\s*/).forEach(part => {
expect(part).to.contain(`/blog`)
it(`renders static image`, () => {
cy.getTestElement(`static-image`)
.should(`have.attr`, `srcset`)
.and(srcset => {
srcset.split(/\s*,\s*/).forEach(part => {
expect(part).to.contain(`/blog`)
})
})
})
})
})

it(`renders dynamic image`, () => {
cy.getTestElement(`gatsby-image`)
.should(`have.attr`, `srcset`)
.and(srcset => {
srcset.split(/\s*,\s*/).forEach(part => {
expect(part).to.contain(`/blog`)
it(`renders dynamic image`, () => {
cy.getTestElement(`gatsby-image`)
.should(`have.attr`, `srcset`)
.and(srcset => {
srcset.split(/\s*,\s*/).forEach(part => {
expect(part).to.contain(`/blog`)
})
})
})
})
})

describe(`navigation`, () => {
it(`prefixes link with /blog`, () => {
cy.getTestElement(`page-2-link`)
.should(`have.attr`, `href`)
.and(`include`, `/blog`)

cy.getTestElement(`page-ssr-link`)
.should(`have.attr`, `href`)
.and(`include`, `/blog`)
})

it(`can navigate to secondary page`, () => {
Expand Down Expand Up @@ -73,5 +77,24 @@ describe(`Production pathPrefix`, () => {
withTrailingSlash(`${pathPrefix}/blogtest`)
)
})

it(`can navigate to ssr page`, () => {
cy.getTestElement(`page-ssr-link`).click()

cy.location(`pathname`).should(
`eq`,
withTrailingSlash(`${pathPrefix}/ssr`)
)

cy.getTestElement(`server-data`).contains(`foo`)
})
})

it(`can visit ssr page`, () => {
cy.visit(`/ssr/`).waitForRouteChange()

cy.location(`pathname`).should(`eq`, withTrailingSlash(`${pathPrefix}/ssr`))

cy.getTestElement(`server-data`).contains(`foo`)
})
})
11 changes: 6 additions & 5 deletions e2e-tests/path-prefix/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"gatsby-plugin-sitemap": "next",
"gatsby-source-filesystem": "next",
"gatsby-transformer-sharp": "next",
"http-proxy": "^1.18.1",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-helmet": "^5.2.0"
Expand All @@ -25,11 +26,11 @@
"license": "MIT",
"scripts": {
"prebuild": "del-cli -f assets && make-dir assets/blog",
"build": "gatsby build --prefix-paths",
"build": "cross-env CYPRESS_SUPPORT=y gatsby build --prefix-paths",
"postbuild": "cpy --cwd=./public --parents '**/*' '../assets/blog'",
"develop": "gatsby develop",
"format": "prettier --write '**/*.js'",
"test": "cross-env CYPRESS_SUPPORT=y npm run build && npm run start-server-and-test",
"test": "npm run build && npm run start-server-and-test",
"start-server-and-test": "start-server-and-test serve \"http://localhost:9000/blog/|http://localhost:9001/blog/\" cy:run",
"serve": "npm-run-all --parallel serve:*",
"serve:site": "gatsby serve --prefix-paths",
Expand All @@ -46,11 +47,11 @@
"make-dir-cli": "^2.0.0",
"npm-run-all": "^4.1.5",
"prettier": "2.0.4",
"serve-handler": "^6.0.0",
"start-server-and-test": "^1.7.1"
"start-server-and-test": "^1.7.1",
"wait-on": "^6.0.0"
},
"repository": {
"type": "git",
"url": "https://github.com/gatsbyjs/gatsby-starter-default"
}
}
}
42 changes: 21 additions & 21 deletions e2e-tests/path-prefix/scripts/serve.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
const handler = require(`serve-handler`)
const http = require(`http`)
const path = require(`path`)
const httpProxy = require("http-proxy")
const waitOn = require("wait-on")

const server = http.createServer((request, response) =>
handler(request, response, {
public: path.resolve(`assets`),
headers: [
{
source: `**/*`,
headers: [
{
key: `Access-Control-Allow-Origin`,
value: `*`,
},
],
},
],
})
)
waitOn(
{
resources: [`http://localhost:9000/blog/`],
},
function (err) {
if (err) {
console.error(err)
process.exit(1)
}

const proxy = httpProxy.createProxyServer()
const server = http.createServer((request, response) =>
proxy.web(request, response, { target: "http://localhost:9000" })
)

server.listen(9001, () => {
console.log(`Running at http://localhost:9001`)
})
server.listen(9001, () => {
console.log(`Assets server running at http://localhost:9001`)
})
}
)
9 changes: 9 additions & 0 deletions e2e-tests/path-prefix/src/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ const IndexPage = ({ data }) => {
<Link data-testid="subdir-link" to="subdirectory/page-1">
Go to subdirectory
</Link>
<Link data-testid="page-ssr-link" to="/ssr/">
Go to SSR page
</Link>
<button
data-testid="page-ssr-button-link"
onClick={() => navigate(`/ssr/`)}
>
Go to SSR page with navigate()
</button>
</Layout>
)
}
Expand Down
25 changes: 25 additions & 0 deletions e2e-tests/path-prefix/src/pages/ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as React from "react"
import { Link } from "gatsby"

import Layout from "../components/layout"

const SSRPage = ({ serverData }) => (
<Layout>
<h1>Hi from the SSR page</h1>
<p>Welcome to SSR page</p>
<Link data-testid="index-link" to="/">
Go back to the homepage
</Link>
<code data-testid="server-data">{serverData?.dataFromServer}</code>
</Layout>
)

export default SSRPage

export function getServerData() {
return {
props: {
dataFromServer: `foo`,
},
}
}
2 changes: 1 addition & 1 deletion packages/gatsby/src/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ module.exports = async (program: IServeProgram): Promise<void> => {
dbPath: path.join(program.directory, `.cache`, `data`, `datastore`),
})

app.get(
router.get(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actual fix - router strips path prefix and enables engines to find proper pages

`/page-data/:pagePath(*)/page-data.json`,
async (req, res, next) => {
const requestedPagePath = req.params.pagePath
Expand Down