From a7c7cd6ab1ca3090562120312a754e5c75773290 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 26 Oct 2023 11:55:34 -0400 Subject: [PATCH] Migrate Deno Bundle to ESbuild (#71) * Improve bundle to fallback to es build --- deno.jsonc | 4 +- src/build.ts | 69 +++++--- src/bundler/deno_bundler.ts | 29 ++++ src/bundler/deno_bundler_test.ts | 98 +++++++++++ src/bundler/esbuild_bundler.ts | 34 ++++ src/bundler/esbuild_bundler_test.ts | 22 +++ src/bundler/mods.ts | 2 + src/deps.ts | 2 + src/errors.ts | 5 + src/tests/build_test.ts | 252 ++++++++++++++++++++-------- 10 files changed, 415 insertions(+), 102 deletions(-) create mode 100644 src/bundler/deno_bundler.ts create mode 100644 src/bundler/deno_bundler_test.ts create mode 100644 src/bundler/esbuild_bundler.ts create mode 100644 src/bundler/esbuild_bundler_test.ts create mode 100644 src/bundler/mods.ts create mode 100644 src/errors.ts diff --git a/deno.jsonc b/deno.jsonc index 3fd2323..d20a800 100644 --- a/deno.jsonc +++ b/deno.jsonc @@ -31,8 +31,8 @@ } }, "tasks": { - "test": "deno fmt --check && deno lint && deno test --allow-read --allow-net --allow-write --allow-run src", - "coverage": "rm -rf .coverage && deno test --reporter=dot --allow-read --allow-net --allow-write --allow-run --coverage=.coverage src && deno coverage --exclude=fixtures --exclude=test --lcov --output=lcov.info .coverage && deno run --allow-read https://deno.land/x/code_coverage@0.2.0/cli.ts" + "test": "deno fmt --check && deno lint && deno test --allow-read --allow-net --allow-write --allow-run --allow-env src", + "coverage": "rm -rf .coverage && deno test --reporter=dot --allow-read --allow-net --allow-write --allow-run --allow-env --coverage=.coverage src && deno coverage --exclude=fixtures --exclude=test --lcov --output=lcov.info .coverage && deno run --allow-read https://deno.land/x/code_coverage@0.2.0/cli.ts" }, "lock": false } diff --git a/src/build.ts b/src/build.ts index f5527c9..cb7dcd1 100644 --- a/src/build.ts +++ b/src/build.ts @@ -7,6 +7,8 @@ import { import type { Protocol } from "./deps.ts"; import { cleanManifest, getManifest } from "./get_manifest.ts"; import { validateManifestFunctions } from "./utilities.ts"; +import { DenoBundler, EsbuildBundler } from "./bundler/mods.ts"; +import { BundleError } from "./errors.ts"; export const validateAndCreateFunctions = async ( workingDirectory: string, @@ -37,6 +39,7 @@ export const validateAndCreateFunctions = async ( fnDef.source_file, ); await createFunctionFile( + workingDirectory, outputDirectory, fnId, fnFilePath, @@ -45,7 +48,27 @@ export const validateAndCreateFunctions = async ( } }; +async function resolveDenoConfigPath( + directory: string = Deno.cwd(), +): Promise { + for (const name of ["deno.json", "deno.jsonc"]) { + const denoConfigPath = path.join(directory, name); + try { + await Deno.stat(denoConfigPath); + return denoConfigPath; + } catch (error) { + if (!(error instanceof Deno.errors.NotFound)) { + throw error; + } + } + } + throw new Error( + `Could not find a deno.json or deno.jsonc file in the current directory.`, + ); +} + const createFunctionFile = async ( + workingDirectory: string, outputDirectory: string, fnId: string, fnFilePath: string, @@ -54,36 +77,30 @@ const createFunctionFile = async ( const fnFileRelative = path.join("functions", `${fnId}.js`); const fnBundledPath = path.join(outputDirectory, fnFileRelative); - // We'll default to just using whatever Deno executable is on the path - // Ideally we should be able to rely on Deno.execPath() so we make sure to bundle with the same version of Deno - // that called this script. This is perhaps a bit overly cautious, so we can look to remove the defaulting here in the future. - let denoExecutablePath = "deno"; - try { - denoExecutablePath = Deno.execPath(); - } catch (e) { - protocol.error("Error calling Deno.execPath()", e); - } - try { - // call out to deno to handle bundling - const p = Deno.run({ - cmd: [ - denoExecutablePath, - "bundle", - "--quiet", - fnFilePath, - fnBundledPath, - ], + await DenoBundler.bundle({ + entrypoint: fnFilePath, + outFile: fnBundledPath, }); + } catch (denoBundleErr) { + if (!(denoBundleErr instanceof BundleError)) { + protocol.error(`Error bundling function file "${fnId}" with Deno`); + throw denoBundleErr; + } + + // TODO: once Protocol can handle debug add a debug statement here - const status = await p.status(); - p.close(); - if (status.code !== 0 || !status.success) { - throw new Error(`Error bundling function file: ${fnId}`); + try { + const bundle = await EsbuildBundler.bundle({ + entrypoint: fnFilePath, + absWorkingDir: workingDirectory, + configPath: await resolveDenoConfigPath(workingDirectory), + }); + await Deno.writeFile(fnBundledPath, bundle); + } catch (esbuildError) { + protocol.error(`Error bundling function file "${fnId}" with esbuild`); + throw esbuildError; } - } catch (e) { - protocol.error(`Error bundling function file: ${fnId}`); - throw e; } }; diff --git a/src/bundler/deno_bundler.ts b/src/bundler/deno_bundler.ts new file mode 100644 index 0000000..4882db9 --- /dev/null +++ b/src/bundler/deno_bundler.ts @@ -0,0 +1,29 @@ +import { BundleError } from "../errors.ts"; + +type DenoBundleOptions = { + /** The path to the file being bundled */ + entrypoint: string; + /** The path where the bundled file should be written. */ + outFile: string; +}; + +export const DenoBundler = { + bundle: async (options: DenoBundleOptions): Promise => { + // call out to deno to handle bundling + const command = new Deno.Command(Deno.execPath(), { + args: [ + "bundle", + "--quiet", + options.entrypoint, + options.outFile, + ], + }); + + const { code, stderr } = await command.output(); + if (code !== 0) { + throw new BundleError({ + cause: new TextDecoder().decode(stderr), + }); + } + }, +}; diff --git a/src/bundler/deno_bundler_test.ts b/src/bundler/deno_bundler_test.ts new file mode 100644 index 0000000..e7a4fa6 --- /dev/null +++ b/src/bundler/deno_bundler_test.ts @@ -0,0 +1,98 @@ +import { assertRejects, assertSpyCall, stub } from "../dev_deps.ts"; +import { BundleError } from "../errors.ts"; +import { DenoBundler } from "./deno_bundler.ts"; + +Deno.test("Deno Bundler tests", async (t) => { + await t.step(DenoBundler.bundle.name, async (tt) => { + const expectedEntrypoint = "./function.ts"; + const expectedOutFile = "./dist/bundle.ts"; + + await tt.step( + "should invoke 'deno bundle' successfully", + async () => { + const commandResp = { + output: () => Promise.resolve({ code: 0 }), + } as Deno.Command; + + // Stub out call to `Deno.Command` and fake return a success + const commandStub = stub( + Deno, + "Command", + () => commandResp, + ); + + try { + await DenoBundler.bundle( + { entrypoint: expectedEntrypoint, outFile: expectedOutFile }, + ); + assertSpyCall(commandStub, 0, { + args: [ + Deno.execPath(), + { + args: [ + "bundle", + "--quiet", + expectedEntrypoint, + expectedOutFile, + ], + }, + ], + }); + } finally { + commandStub.restore(); + } + }, + ); + + await tt.step( + "should throw an exception if the 'deno bundle' command fails", + async () => { + const commandResp = { + output: () => + Promise.resolve({ + code: 1, + stderr: new TextEncoder().encode( + "error: unrecognized subcommand 'bundle'", + ), + }), + } as Deno.Command; + + // Stub out call to `Deno.Command` and fake return a success + const commandStub = stub( + Deno, + "Command", + () => commandResp, + ); + + try { + await assertRejects( + () => + DenoBundler.bundle( + { + entrypoint: expectedEntrypoint, + outFile: expectedOutFile, + }, + ), + BundleError, + "Error bundling function file", + ); + assertSpyCall(commandStub, 0, { + args: [ + Deno.execPath(), + { + args: [ + "bundle", + "--quiet", + expectedEntrypoint, + expectedOutFile, + ], + }, + ], + }); + } finally { + commandStub.restore(); + } + }, + ); + }); +}); diff --git a/src/bundler/esbuild_bundler.ts b/src/bundler/esbuild_bundler.ts new file mode 100644 index 0000000..33d9f18 --- /dev/null +++ b/src/bundler/esbuild_bundler.ts @@ -0,0 +1,34 @@ +import { denoPlugins, esbuild } from "../deps.ts"; + +type EsbuildBundleOptions = { + /** The path to the file being bundled */ + entrypoint: string; + /** The path to the deno.json / deno.jsonc config file. */ + configPath: string; + /** specify the working directory to use for the build */ + absWorkingDir: string; +}; + +export const EsbuildBundler = { + bundle: async (options: EsbuildBundleOptions): Promise => { + try { + // esbuild configuration options https://esbuild.github.io/api/#overview + const result = await esbuild.build({ + entryPoints: [options.entrypoint], + platform: "neutral", + target: "deno1", // TODO: the versions should come from the user defined input + format: "esm", // esm format stands for "ECMAScript module" + bundle: true, // inline any imported dependencies into the file itself + absWorkingDir: options.absWorkingDir, + write: false, // Favor returning the contents + outdir: "out", // Nothing is being written to file here + plugins: [ + ...denoPlugins({ configPath: options.configPath }), + ], + }); + return result.outputFiles[0].contents; + } finally { + esbuild.stop(); + } + }, +}; diff --git a/src/bundler/esbuild_bundler_test.ts b/src/bundler/esbuild_bundler_test.ts new file mode 100644 index 0000000..0d9893d --- /dev/null +++ b/src/bundler/esbuild_bundler_test.ts @@ -0,0 +1,22 @@ +import { assertEquals } from "https://deno.land/std@0.138.0/testing/asserts.ts"; +import { assertExists } from "../dev_deps.ts"; +import { EsbuildBundler } from "./esbuild_bundler.ts"; + +Deno.test("Esbuild Bundler tests", async (t) => { + await t.step(EsbuildBundler.bundle.name, async (tt) => { + await tt.step( + "should invoke 'esbuild.build' successfully", + async () => { + const bundle = await EsbuildBundler.bundle( + { + entrypoint: "src/tests/fixtures/functions/test_function_file.ts", + configPath: `${Deno.cwd()}/deno.jsonc`, + absWorkingDir: Deno.cwd(), + }, + ); + assertExists(bundle); + assertEquals(bundle.length, 195); + }, + ); + }); +}); diff --git a/src/bundler/mods.ts b/src/bundler/mods.ts new file mode 100644 index 0000000..e1191dd --- /dev/null +++ b/src/bundler/mods.ts @@ -0,0 +1,2 @@ +export { EsbuildBundler } from "./esbuild_bundler.ts"; +export { DenoBundler } from "./deno_bundler.ts"; diff --git a/src/deps.ts b/src/deps.ts index e3e52a3..36b79b1 100644 --- a/src/deps.ts +++ b/src/deps.ts @@ -6,3 +6,5 @@ export type { JSONValue } from "https://deno.land/std@0.149.0/encoding/jsonc.ts" export { deepMerge } from "https://deno.land/std@0.134.0/collections/deep_merge.ts"; export { getProtocolInterface } from "https://deno.land/x/deno_slack_protocols@0.0.2/mod.ts"; export type { Protocol } from "https://deno.land/x/deno_slack_protocols@0.0.2/types.ts"; +export * as esbuild from "https://deno.land/x/esbuild@v0.19.4/mod.js"; +export { denoPlugins } from "https://deno.land/x/esbuild_deno_loader@0.8.2/mod.ts"; diff --git a/src/errors.ts b/src/errors.ts new file mode 100644 index 0000000..1f05803 --- /dev/null +++ b/src/errors.ts @@ -0,0 +1,5 @@ +export class BundleError extends Error { + constructor(options?: ErrorOptions) { + super("Error bundling function file", options); + } +} diff --git a/src/tests/build_test.ts b/src/tests/build_test.ts index 87d76ee..a1ce3e6 100644 --- a/src/tests/build_test.ts +++ b/src/tests/build_test.ts @@ -1,12 +1,14 @@ import { validateAndCreateFunctions } from "../build.ts"; +import { DenoBundler, EsbuildBundler } from "../bundler/mods.ts"; import { assertExists, assertRejects, assertSpyCalls, - returnsNext, + spy, stub, } from "../dev_deps.ts"; import { MockProtocol } from "../dev_deps.ts"; +import { BundleError } from "../errors.ts"; Deno.test("build hook tests", async (t) => { await t.step("validateAndCreateFunctions", async (tt) => { @@ -14,76 +16,167 @@ Deno.test("build hook tests", async (t) => { assertExists(validateAndCreateFunctions); }); + const validManifest = { + "functions": { + "test_function_one": { + "title": "Test function 1", + "description": "this is a test", + "source_file": "src/tests/fixtures/functions/test_function_file.ts", + "input_parameters": { + "required": [], + "properties": {}, + }, + "output_parameters": { + "required": [], + "properties": {}, + }, + }, + "test_function_two": { + "title": "Test function 2", + "description": "this is a test", + "source_file": "src/tests/fixtures/functions/test_function_file.ts", + "input_parameters": { + "required": [], + "properties": {}, + }, + "output_parameters": { + "required": [], + "properties": {}, + }, + }, + "api_function_that_should_not_be_built": { + "type": "API", + "title": "API function", + "description": "should most definitely not be bundled", + "source_file": "src/tests/fixtures/functions/this_shouldnt_matter.ts", + "input_parameters": { + "required": [], + "properties": {}, + }, + "output_parameters": { + "required": [], + "properties": {}, + }, + }, + }, + }; + await tt.step( "should invoke `deno bundle` once per non-API function", async () => { const protocol = MockProtocol(); - const manifest = { - "functions": { - "test_function_one": { - "title": "Test function 1", - "description": "this is a test", - "source_file": - "src/tests/fixtures/functions/test_function_file.ts", - "input_parameters": { - "required": [], - "properties": {}, - }, - "output_parameters": { - "required": [], - "properties": {}, - }, - }, - "test_function_two": { - "title": "Test function 2", - "description": "this is a test", - "source_file": - "src/tests/fixtures/functions/test_function_file.ts", - "input_parameters": { - "required": [], - "properties": {}, - }, - "output_parameters": { - "required": [], - "properties": {}, - }, - }, - "api_function_that_should_not_be_built": { - "type": "API", - "title": "API function", - "description": "should most definitely not be bundled", - "source_file": - "src/tests/fixtures/functions/this_shouldnt_matter.ts", - "input_parameters": { - "required": [], - "properties": {}, - }, - "output_parameters": { - "required": [], - "properties": {}, - }, - }, - }, - }; const outputDir = await Deno.makeTempDir(); - // Stub out call to `Deno.run` and fake return a success - const runResponse = { - close: () => {}, - status: () => Promise.resolve({ code: 0, success: true }), - } as unknown as Deno.Process; - const runStub = stub( + + const commandResp = { + output: () => Promise.resolve({ code: 0 }), + } as Deno.Command; + + // Stub out call to `Deno.Command` and fake return a success + const commandStub = stub( Deno, - "run", - returnsNext([runResponse, runResponse]), + "Command", + () => commandResp, ); - await validateAndCreateFunctions( - Deno.cwd(), - outputDir, - manifest, - protocol, + + const esbuildBundlerSpy = spy( + EsbuildBundler, + "bundle", + ); + + try { + await validateAndCreateFunctions( + Deno.cwd(), + outputDir, + validManifest, + protocol, + ); + assertSpyCalls(commandStub, 2); + assertSpyCalls(esbuildBundlerSpy, 0); + } finally { + commandStub.restore(); + esbuildBundlerSpy.restore(); + } + }, + ); + + await tt.step( + "should invoke `esbuild` once per non-API function if bundle fails", + async () => { + const protocol = MockProtocol(); + + const outputDir = await Deno.makeTempDir(); + + // Stub out call to `Deno.Command` and fake throw error + const commandStub = stub( + DenoBundler, + "bundle", + () => { + throw new BundleError(); + }, + ); + + // Stub out call to `Deno.writeFile` and fake response + const writeFileStub = stub( + Deno, + "writeFile", + async () => {}, ); - assertSpyCalls(runStub, 2); - runStub.restore(); + + try { + await validateAndCreateFunctions( + Deno.cwd(), + outputDir, + validManifest, + protocol, + ); + assertSpyCalls(commandStub, 2); + assertSpyCalls(writeFileStub, 2); + } finally { + commandStub.restore(); + writeFileStub.restore(); + } + }, + ); + + await tt.step( + "should throw an exception if `DenoBundler.bundle` fails unexpectedly", + async () => { + const protocol = MockProtocol(); + const outputDir = await Deno.makeTempDir(); + + // Stub out call to `Deno.Command` and fake throw error + const commandStub = stub( + DenoBundler, + "bundle", + () => { + throw new Error("something was unexpected"); + }, + ); + + // Spy on `Deno.writeFile` + const writeFileStub = spy( + Deno, + "writeFile", + ); + + try { + await assertRejects( + () => + validateAndCreateFunctions( + Deno.cwd(), + outputDir, + validManifest, + protocol, + ), + Error, + "something was unexpected", + ); + assertSpyCalls(commandStub, 1); + assertSpyCalls(writeFileStub, 0); + } finally { + commandStub.restore(); + writeFileStub.restore(); + } }, ); @@ -261,19 +354,30 @@ Deno.test("build hook tests", async (t) => { manifest, protocol, ); - // Stub out call to `Deno.run` and fake return a success - const runResponse = { - close: () => {}, - status: () => Promise.resolve({ code: 0, success: true }), - } as unknown as Deno.Process; - const runStub = stub( + + // Spy on `Deno.Command` and `writeFile` + const commandStub = spy( + Deno, + "Command", + ); + const writeFileStub = spy( Deno, - "run", - returnsNext([runResponse, runResponse]), + "writeFile", ); - // Make sure we didn't shell out to Deno.run - assertSpyCalls(runStub, 0); - runStub.restore(); + + try { + await validateAndCreateFunctions( + Deno.cwd(), + outputDir, + manifest, + protocol, + ); + assertSpyCalls(commandStub, 0); + assertSpyCalls(writeFileStub, 0); + } finally { + commandStub.restore(); + writeFileStub.restore(); + } }); }); });