Skip to content

Commit

Permalink
perf(gatsby-adapter-netlify): improve adapt() performance (#38988)
Browse files Browse the repository at this point in the history
* test: add ssg specific headers

* fix: headers rule matching with path prefix

* test: add NODE_NO_WARNING, axios seems to use fetch now under the hood causing fetch warning on node 18 impacting some logs assertions

* perf: don't recreate headers from headerRoutes for each static route in route manifest

* test: apply NODE_NO_WARNINGS env in correct place
  • Loading branch information
pieh authored May 29, 2024
1 parent 9584173 commit ec77bed
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 34 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ executors:
environment:
GATSBY_CPU_COUNT: 2
COMPILER_OPTIONS: GATSBY_MAJOR=<< parameters.gatsby_major >>
NODE_NO_WARNINGS: 1

aliases:
e2e-executor-env: &e2e-executor-env
Expand Down
20 changes: 20 additions & 0 deletions e2e-tests/adapters/cypress/e2e/headers.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ describe("Headers", () => {

beforeEach(() => {
cy.intercept(PATH_PREFIX + "/", WorkaroundCachedResponse).as("index")
cy.intercept(
PATH_PREFIX + "/routes/ssg/static",
WorkaroundCachedResponse
).as("ssg")
cy.intercept(
PATH_PREFIX + "/routes/ssr/static",
WorkaroundCachedResponse
Expand Down Expand Up @@ -128,6 +132,22 @@ describe("Headers", () => {
checkHeaders("@js")
})

it("should contain correct headers for ssg page", () => {
cy.visit("routes/ssg/static").waitForRouteChange()

checkHeaders("@ssg", {
...defaultHeaders,
"x-custom-header": "my custom header value",
"x-ssg-header": "my custom header value",
"cache-control": "public,max-age=0,must-revalidate",
})

checkHeaders("@app-data")
checkHeaders("@page-data")
checkHeaders("@slice-data")
checkHeaders("@static-query-result")
})

it("should contain correct headers for ssr page", () => {
cy.visit("routes/ssr/static").waitForRouteChange()

Expand Down
4 changes: 2 additions & 2 deletions e2e-tests/adapters/cypress/e2e/remote-file.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ const PATH_PREFIX = Cypress.env(`PATH_PREFIX`) || ``
const configs = [
{
title: `remote-file (SSG, Page Query)`,
pagePath: `/routes/remote-file/`,
pagePath: `/routes/ssg/remote-file/`,
placeholders: true,
},
{
title: `remote-file (SSG, Page Context)`,
pagePath: `/routes/remote-file-data-from-context/`,
pagePath: `/routes/ssg/remote-file-data-from-context/`,
placeholders: true,
},
{
Expand Down
46 changes: 35 additions & 11 deletions e2e-tests/adapters/cypress/e2e/trailing-slash.cy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { assertPageVisits } from "../utils/assert-page-visits"
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 @@ -12,38 +12,62 @@ const TRAILING_SLASH = Cypress.env(`TRAILING_SLASH`) || `never`
describe("trailingSlash", () => {
describe(TRAILING_SLASH, () => {
it("should work when using Gatsby Link (without slash)", () => {
cy.visit('/').waitForRouteChange()
cy.visit("/").waitForRouteChange()

cy.get(`[data-testid="static-without-slash"]`).click().waitForRouteChange().assertRoute(applyTrailingSlashOption(`/routes/static`, TRAILING_SLASH))
cy.get(`[data-testid="static-without-slash"]`)
.click()
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/ssg/static`, TRAILING_SLASH)
)
})
it("should work when using Gatsby Link (with slash)", () => {
cy.visit('/').waitForRouteChange()
cy.visit("/").waitForRouteChange()

cy.get(`[data-testid="static-with-slash"]`).click().waitForRouteChange().assertRoute(applyTrailingSlashOption(`/routes/static`, TRAILING_SLASH))
cy.get(`[data-testid="static-with-slash"]`)
.click()
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/ssg/static`, TRAILING_SLASH)
)
})
it("should work on direct visit (with other setting)", () => {
const destination = applyTrailingSlashOption("/routes/static", TRAILING_SLASH)
const inverse = TRAILING_SLASH === `always` ? "/routes/static" : "/routes/static/"
const destination = applyTrailingSlashOption(
"/routes/ssg/static",
TRAILING_SLASH
)
const inverse =
TRAILING_SLASH === `always`
? "/routes/ssg/static"
: "/routes/ssg/static/"

assertPageVisits([
{
path: destination,
status: 200,
},
{ path: inverse, status: 301, destinationPath: destination }
{ path: inverse, status: 301, destinationPath: destination },
])

cy.visit(inverse).waitForRouteChange().assertRoute(applyTrailingSlashOption(`/routes/static`, TRAILING_SLASH))
cy.visit(inverse)
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/ssg/static`, TRAILING_SLASH)
)
})
it("should work on direct visit (with current setting)", () => {
assertPageVisits([
{
path: applyTrailingSlashOption("/routes/static", TRAILING_SLASH),
path: applyTrailingSlashOption("/routes/ssg/static", TRAILING_SLASH),
status: 200,
},
])

cy.visit(applyTrailingSlashOption("/routes/static", TRAILING_SLASH)).waitForRouteChange().assertRoute(applyTrailingSlashOption(`/routes/static`, TRAILING_SLASH))
cy.visit(applyTrailingSlashOption("/routes/ssg/static", TRAILING_SLASH))
.waitForRouteChange()
.assertRoute(
applyTrailingSlashOption(`/routes/ssg/static`, TRAILING_SLASH)
)
})
})
})
9 changes: 9 additions & 0 deletions e2e-tests/adapters/gatsby-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ const config: GatsbyConfig = {
},
],
},
{
source: `routes/ssg/*`,
headers: [
{
key: "x-ssg-header",
value: "my custom header value",
},
],
},
],
...configOverrides,
}
Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/adapters/gatsby-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const createPages: GatsbyNode["createPages"] = async ({

createPage({
path: applyTrailingSlashOption(
`/routes/remote-file-data-from-context/`,
`/routes/ssg/remote-file-data-from-context/`,
TRAILING_SLASH
),
component: path.resolve(`./src/templates/remote-file-from-context.jsx`),
Expand Down
8 changes: 4 additions & 4 deletions e2e-tests/adapters/src/pages/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import "./index.css"
const routes = [
{
text: "Static",
url: "/routes/static",
url: "/routes/ssg/static",
id: "static-without-slash",
},
{
text: "Static (With Slash)",
url: "/routes/static/",
url: "/routes/ssg/static/",
id: "static-with-slash",
},
{
Expand Down Expand Up @@ -41,11 +41,11 @@ const routes = [
},
{
text: "RemoteFile (ImageCDN and FileCDN) (SSG, Page Query)",
url: "/routes/remote-file",
url: "/routes/ssg/remote-file",
},
{
text: "RemoteFile (ImageCDN and FileCDN) (SSG, Page Context)",
url: "/routes/remote-file-data-from-context",
url: "/routes/ssg/remote-file-data-from-context",
},
{
text: "RemoteFile (ImageCDN and FileCDN) (SSR, Page Query)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { graphql } from "gatsby"
import React from "react"

import { GatsbyImage } from "gatsby-plugin-image"
import Layout from "../../components/layout"
import Layout from "../../../components/layout"

const RemoteFile = ({ data }) => {
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react"
import Layout from "../../components/layout"
import Layout from "../../../components/layout"

const StaticPage = () => {
return (
Expand All @@ -11,4 +11,4 @@ const StaticPage = () => {

export default StaticPage

export const Head = () => <title>Static</title>
export const Head = () => <title>Static</title>
12 changes: 6 additions & 6 deletions packages/gatsby-adapter-netlify/src/route-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,13 @@ export function processRoutesManifest(
_headers += buildHeaderString(route.path, route.headers)
}
}
}

if (headerRoutes) {
_headers = headerRoutes.reduce((acc, curr) => {
acc += buildHeaderString(curr.path, curr.headers)
return acc
}, ``)
}
if (headerRoutes) {
_headers = headerRoutes.reduce((acc, curr) => {
acc += buildHeaderString(curr.path, curr.headers)
return acc
}, ``)
}

return {
Expand Down
29 changes: 24 additions & 5 deletions packages/gatsby/src/utils/adapter/create-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,17 @@ const normalizePath = (input: string): string =>
input.endsWith(`/`) ? input : `${input}/`

export const createHeadersMatcher = (
headers: Array<IHeader> | undefined
headers: Array<IHeader> | undefined,
pathPrefix: string
): ((path: string, defaultHeaders: Headers) => Headers) => {
function stripPathPrefix(path: string): string {
if (pathPrefix && path.startsWith(pathPrefix)) {
path = path.slice(pathPrefix.length)
}

return path
}

// Split the incoming user headers into two buckets:
// - dynamicHeaders: Headers with dynamic paths (e.g. /* or /:tests)
// - staticHeaders: Headers with fully static paths (e.g. /static/)
Expand All @@ -27,13 +36,21 @@ export const createHeadersMatcher = (
}

for (const header of headers) {
if (header.source.includes(`:`) || header.source.includes(`*`)) {
const source = stripPathPrefix(header.source)
if (source.includes(`:`) || source.includes(`*`)) {
// rankRoute is the internal function that also "match" uses
const score = rankRoute(header.source)
const score = rankRoute(source)

dynamicHeaders.push({ ...header, score })
dynamicHeaders.push({
...header,
score,
source,
})
} else {
staticHeaders.set(normalizePath(header.source), header)
staticHeaders.set(normalizePath(source), {
...header,
source,
})
}
}

Expand All @@ -48,6 +65,8 @@ export const createHeadersMatcher = (
})

return (path: string, defaultHeaders: Headers): Headers => {
path = stripPathPrefix(path)

// Create a map of headers for the given path
// The key will be the header key. Since a key may only appear once in a map, the last header with the same key will win
const uniqueHeaders: Map<string, string> = new Map()
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/src/utils/adapter/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,10 @@ function getRoutesManifest(): {
} {
const routes: Array<RouteWithScore> = []
const state = store.getState()
const createHeaders = createHeadersMatcher(state.config.headers)
const pathPrefix = state.program.prefixPaths
? state.config.pathPrefix ?? ``
: ``
const createHeaders = createHeadersMatcher(state.config.headers, pathPrefix)

const headerRoutes: HeaderRoutes = [...getDefaultHeaderRoutes(pathPrefix)]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export async function createPageSSRBundle({
)
: ``
),
PATH_PREFIX: JSON.stringify(pathPrefix),
// eslint-disable-next-line @typescript-eslint/naming-convention
"process.env.GATSBY_LOGGER": JSON.stringify(`yurnalist`),
// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/src/utils/page-ssr-module/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ declare global {
const WEBPACK_COMPILATION_HASH: string
const GATSBY_SLICES_SCRIPT: string
const GATSBY_PAGES: Array<[string, EnginePage]>
const PATH_PREFIX: string
}

const tracerReadyPromise = initTracer(
Expand All @@ -85,7 +86,7 @@ type MaybePhantomActivity =
| ReturnType<typeof reporter.phantomActivity>
| undefined

const createHeaders = createHeadersMatcher(INLINED_HEADERS_CONFIG)
const createHeaders = createHeadersMatcher(INLINED_HEADERS_CONFIG, PATH_PREFIX)

interface IGetDataBaseArgs {
pathName: string
Expand Down

0 comments on commit ec77bed

Please sign in to comment.