Skip to content

Commit

Permalink
fix(gatsby-adapter-netlify): handler generation on windows (#38900)
Browse files Browse the repository at this point in the history
* test: add unit test for produced handler

* actually failing test in windows

* fix(gatsby-adapter-netlify): produce working function handlers on windows

* fix(gatsby): functions compilation on windows

* tmp: prepare cross-platform binaries for SSR/DSG

* fix: lint

* feat: add a way to configure functions executing platform/arch and add early check in DSG/SSR

* refactor: move some utility functions around, cleanup standalone-regenrate, restore engine validation, add structured error and better error messages

* chore: add jsdocs description for functionsPlatform and functionsArch optional config values passed by adapter

* chore: make sure fs wrapper is first

* fix: actually use values reported by adapter

* test: try to setup windows adapters smoke test

* test: typo

* test: maybe cd into dirs?

* test: no powershell fro smoke test

* chore: single quote to double

* chore: install node-gyp requirements

* chore: install deps in win smoke

* ?

* newer node needed for ntl-cli

* run ntl through yarn

* Revert "run ntl through yarn"

This reverts commit 8c55e40.

* install ntl-cli in circleci pipeline

* test: adjust lmdb regeneration test to changed internal-packages location

* test: run windows deploy/smoke test after unit tests passed

* chore: use path.posix to load engines in serve command

* chore: use default value when destructuring instead of nullish coalescing later
  • Loading branch information
pieh authored Apr 10, 2024
1 parent 5723972 commit c91ed28
Show file tree
Hide file tree
Showing 28 changed files with 716 additions and 204 deletions.
45 changes: 45 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,43 @@ jobs:
- store_test_results:
path: ./test-results/jest-node/

windows_adapters_smoke:
executor:
name: win/default
shell: bash.exe
steps:
- checkout
- run:
command: ./scripts/assert-changed-files.sh "packages/*|(e2e|integration)-tests/*|.circleci/*|scripts/e2e-test.sh|yarn.lock"
- <<: *attach_to_bootstrap
- run:
name: Install node 18.19.0, yarn and netlify-cli
command: |
nvm install 18.19.0
nvm alias default 18.19.0
nvm use 18.19.0
npm install -g yarn netlify-cli
- run:
name: Clear out sharp
command: |
Remove-Item -Recurse -Force -Path "node_modules/sharp/"
shell: powershell.exe
- run:
command: yarn
- run:
command: mkdir -p /tmp/e2e-tests/
- run:
command: cp -r ./e2e-tests/adapters /tmp/e2e-tests/adapters
- run:
command: pwd && ls
working_directory: /tmp/e2e-tests/adapters
- run: # Set project dir
command: node ./packages/gatsby-dev-cli/dist/index.js --set-path-to-repo .
- run: # Copy over packages
command: cd /tmp/e2e-tests/adapters && node ~/project/packages/gatsby-dev-cli/dist/index.js --force-install --scan-once
- run: # run smoke test
command: cd /tmp/e2e-tests/adapters && node scripts/deploy-and-run/netlify.mjs test:smoke

workflows:
version: 2

Expand Down Expand Up @@ -611,6 +648,14 @@ workflows:
requires:
- lint
- bootstrap
- windows_adapters_smoke:
requires:
# ideally we wait for windows unit tests here, but because those are flaky
# feedback loop would be not practical, so at least wait for linux unit tests
# to resemble setup for more robust E2E tests
- lint
- bootstrap
- unit_tests_node18
- unit_tests_node18:
<<: *ignore_docs
requires:
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/adapters/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"test:template:debug": "cross-env-shell CYPRESS_GROUP_NAME=\"adapter:$ADAPTER / trailingSlash:${TRAILING_SLASH:-always} / pathPrefix:${PATH_PREFIX:--}\" TRAILING_SLASH=$TRAILING_SLASH PATH_PREFIX=$PATH_PREFIX npm run cy:open -- --config-file \"cypress/configs/$ADAPTER.ts\" --env TRAILING_SLASH=$TRAILING_SLASH,PATH_PREFIX=$PATH_PREFIX",
"test:debug": "npm-run-all -s build:debug ssat:debug",
"test:netlify": "cross-env TRAILING_SLASH=always node scripts/deploy-and-run/netlify.mjs test:template",
"test:smoke": "node smoke-test.mjs",
"test:netlify:debug": "cross-env TRAILING_SLASH=always node scripts/deploy-and-run/netlify.mjs test:template:debug",
"test:netlify:prefix-never": "cross-env TRAILING_SLASH=never PATH_PREFIX=/prefix node scripts/deploy-and-run/netlify.mjs test:template",
"test:netlify:prefix-never:debug": "cross-env TRAILING_SLASH=never PATH_PREFIX=/prefix node scripts/deploy-and-run/netlify.mjs test:template:debug",
Expand Down
24 changes: 24 additions & 0 deletions e2e-tests/adapters/smoke-test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import assert from "node:assert"

{
// check index page (SSG)
const response = await fetch(process.env.DEPLOY_URL)
assert.equal(response.status, 200)

const body = await response.text()
assert.match(body, /<h1>Adapters<\/h1>/)
assert.match(body, /<title[^>]*>Adapters E2E<\/title>/)
}

{
// check SSR page
const response = await fetch(
process.env.DEPLOY_URL + `/routes/ssr/remote-file/`
)
assert.equal(response.status, 200)

const body = await response.text()
// inline css for placeholder - this tests both LMDB and SHARP
// (LMDB because of page query and sharp because page query will use sharp to generate placeholder values)
assert.match(body, /background-color:rgb\(232,184,8\)/)
}
8 changes: 7 additions & 1 deletion integration-tests/lmdb-regeneration/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ describe(`Lmdb regeneration`, () => {

// If the fix worked correctly we should have installed the prebuilt binary for our platform under our `.cache` directory
const lmdbRequire = mod.createRequire(
path.resolve(rootPath, ".cache", "internal-packages", "package.json")
path.resolve(
rootPath,
".cache",
"internal-packages",
`${process.platform}-${process.arch}`,
"package.json"
)
)
expect(() => {
lmdbRequire.resolve(lmdbPackage)
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby-adapter-netlify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
"@netlify/functions": "^1.6.0",
"cookie": "^0.6.0",
"fastq": "^1.15.0",
"fs-extra": "^11.2.0"
"fs-extra": "^11.2.0",
"gatsby-core-utils": "^4.14.0-next.2"
},
"devDependencies": {
"@babel/cli": "^7.20.7",
Expand Down
Empty file.
Empty file.
46 changes: 46 additions & 0 deletions packages/gatsby-adapter-netlify/src/__tests__/lambda-handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import fs from "fs-extra"
import { prepareFunction } from "../lambda-handler"
import { join, relative } from "path"
import { slash } from "gatsby-core-utils/path"

const writeFileSpy = jest
.spyOn(fs, `writeFile`)
.mockImplementation(async () => {})
const writeJsonSpy = jest
.spyOn(fs, `writeJSON`)
.mockImplementation(async () => {})

const fixturePath = join(
relative(process.cwd(), __dirname),
`fixtures`,
`lambda-handler`
)
const pathToEntryPoint = join(fixturePath, `entry.js`)
const requiredFile = join(fixturePath, `included.js`)

test(`produced handler is correct`, async () => {
await prepareFunction({
functionId: `test`,
name: `test`,
pathToEntryPoint,
requiredFiles: [requiredFile],
})
const handlerCode = writeFileSpy.mock.calls[0][1]
// expect require in produced code (this is to mostly to make sure handlerCode is actual handler code)
expect(handlerCode).toMatch(/require\(["'][^"']*["']\)/)
// require paths should not have backward slashes (win paths)
expect(handlerCode).not.toMatch(/require\(["'][^"']*\\[^"']*["']\)/)

expect(writeJsonSpy).toBeCalledWith(
expect.any(String),
expect.objectContaining({
config: expect.objectContaining({
name: `test`,
generator: expect.stringContaining(`gatsby-adapter-netlify`),
includedFiles: [slash(requiredFile)],
externalNodeModules: [`msgpackr-extract`],
}),
version: 1,
})
)
})
2 changes: 2 additions & 0 deletions packages/gatsby-adapter-netlify/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ const createNetlifyAdapter: AdapterInit<INetlifyAdapterOptions> = options => {
fileCDNUrlGeneratorModulePath: useNetlifyImageCDN
? require.resolve(`./file-cdn-url-generator`)
: undefined,
functionsPlatform: `linux`,
functionsArch: `x64`,
}
},
}
Expand Down
10 changes: 7 additions & 3 deletions packages/gatsby-adapter-netlify/src/lambda-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { IFunctionDefinition } from "gatsby"
import packageJson from "gatsby-adapter-netlify/package.json"
import fs from "fs-extra"
import * as path from "path"
import { slash } from "gatsby-core-utils/path"

interface INetlifyFunctionConfig {
externalNodeModules?: Array<string>
Expand All @@ -25,7 +26,7 @@ interface INetlifyFunctionManifest {
version: number
}

async function prepareFunction(
export async function prepareFunction(
fun: IFunctionDefinition,
odbfunctionName?: string
): Promise<void> {
Expand Down Expand Up @@ -58,7 +59,7 @@ async function prepareFunction(
name: displayName,
generator: `gatsby-adapter-netlify@${packageJson?.version ?? `unknown`}`,
includedFiles: fun.requiredFiles.map(file =>
file.replace(/\[/g, `*`).replace(/]/g, `*`)
slash(file).replace(/\[/g, `*`).replace(/]/g, `*`)
),
externalNodeModules: [`msgpackr-extract`],
},
Expand All @@ -73,7 +74,10 @@ async function prepareFunction(
function getRelativePathToModule(modulePath: string): string {
const absolutePath = require.resolve(modulePath)

return `./` + path.relative(internalFunctionsDir, absolutePath)
return (
`./` +
path.posix.relative(slash(internalFunctionsDir), slash(absolutePath))
)
}

const handlerSource = /* javascript */ `
Expand Down
8 changes: 8 additions & 0 deletions packages/gatsby-cli/src/create-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,14 @@ function buildLocalCommands(cli: yargs.Argv, isLocalSite: boolean): void {
default: false,
describe: `Save the log of changed pages for future comparison.`,
hidden: true,
})
.option(`functions-platform`, {
type: `string`,
describe: `The platform bundled functions will execute on. Defaults to current platform or settings provided by used adapter.`,
})
.option(`functions-arch`, {
type: `string`,
describe: `The architecture bundled functions will execute on. Defaults to current architecture or settings provided by used adapter.`,
}),
handler: handlerP(
getCommandHandler(
Expand Down
6 changes: 6 additions & 0 deletions packages/gatsby-cli/src/structured-errors/error-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ const errors: Record<string, IErrorMapEntry> = {
level: Level.ERROR,
category: ErrorCategory.USER,
},
"98051": {
text: (): string => `Built Rendering Engines failed to load.`,
type: Type.ENGINE_EXECUTION,
level: Level.ERROR,
category: ErrorCategory.UNKNOWN,
},
"98123": {
text: (context): string =>
`${context.stageLabel} failed\n\n${
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-legacy-polyfills/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"license": "MIT",
"scripts": {
"build": "npm-run-all --npm-path npm -p build:*",
"build:exclude": "cpy 'exclude.js' '../dist' --cwd=./src",
"build:exclude": "cpy \"exclude.js\" \"../dist\" --cwd=./src",
"build:polyfills": "microbundle -f iife -i src/polyfills.js --no-sourcemap --external=none",
"prepare": "cross-env NODE_ENV=production npm run build",
"watch": "npm-run-all --npm-path npm -p watch:*",
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-plugin-offline/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"scripts": {
"build": "npm run build:src && npm run build:sw-append",
"build:src": "babel src --out-dir . --ignore \"**/__tests__,src/sw-append.js\"",
"build:sw-append": "cpy 'sw-append.js' '../' --cwd=./src",
"build:sw-append": "cpy \"sw-append.js\" \"../\" --cwd=./src",
"prepare": "cross-env NODE_ENV=production npm run build",
"watch": "npm run build:sw-append -- --watch & npm run build:src -- --watch"
},
Expand Down
2 changes: 2 additions & 0 deletions packages/gatsby/src/commands/build-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export interface IBuildArgs extends IProgram {
profile: boolean
graphqlTracing: boolean
openTracingConfigFile: string
functionsPlatform?: string
functionsArch?: string
// TODO remove in v4
keepPageRenderer: boolean
}
Expand Down
17 changes: 2 additions & 15 deletions packages/gatsby/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import {
getPageMode,
preparePageTemplateConfigs,
} from "../utils/page-mode"
import { validateEngines } from "../utils/validate-engines"
import { validateEnginesWithActivity } from "../utils/validate-engines"
import { constructConfigObject } from "../utils/gatsby-cloud-config"
import { waitUntilWorkerJobsAreComplete } from "../utils/jobs/worker-messaging"
import { getSSRChunkHashes } from "../utils/webpack/get-ssr-chunk-hashes"
Expand Down Expand Up @@ -295,20 +295,7 @@ module.exports = async function build(
}

if (shouldGenerateEngines()) {
const validateEnginesActivity = report.activityTimer(
`Validating Rendering Engines`,
{
parentSpan: buildSpan,
}
)
validateEnginesActivity.start()
try {
await validateEngines(store.getState().program.directory)
} catch (error) {
validateEnginesActivity.panic({ id: `98001`, context: {}, error })
} finally {
validateEnginesActivity.end()
}
await validateEnginesWithActivity(program.directory, buildSpan)
}

const cacheActivity = report.activityTimer(`Caching Webpack compilations`, {
Expand Down
Loading

0 comments on commit c91ed28

Please sign in to comment.