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

Migrate Deno Bundle to ESbuild #71

Merged
merged 13 commits into from
Oct 26, 2023
4 changes: 2 additions & 2 deletions deno.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]/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/[email protected]/cli.ts"
},
"lock": false
}
71 changes: 45 additions & 26 deletions src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -37,6 +39,7 @@ export const validateAndCreateFunctions = async (
fnDef.source_file,
);
await createFunctionFile(
workingDirectory,
outputDirectory,
fnId,
fnFilePath,
Expand All @@ -45,7 +48,27 @@ export const validateAndCreateFunctions = async (
}
};

async function resolveDenoConfigPath(
directory: string = Deno.cwd(),
): Promise<string> {
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,
Expand All @@ -54,36 +77,32 @@ 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;
}

protocol.warn(
WilliamBergamin marked this conversation as resolved.
Show resolved Hide resolved
`Failed bundling function file "${fnId}" with Deno falling back to esbuild`,
);

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);
filmaj marked this conversation as resolved.
Show resolved Hide resolved
} catch (esbuildError) {
protocol.error(`Error bundling function file "${fnId}" with esbuild`);
throw esbuildError;
}
} catch (e) {
protocol.error(`Error bundling function file: ${fnId}`);
throw e;
}
};

Expand Down
29 changes: 29 additions & 0 deletions src/bundler/deno_bundler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { BundleError } from "../errors.ts";

export type DenoBundleOptions = {
WilliamBergamin marked this conversation as resolved.
Show resolved Hide resolved
/** The path to the file being bundled */
entrypoint: string;
/** The path to the deno.json / deno.jsonc config file. */
outFile: string;
};

export const DenoBundler = {
bundle: async (options: DenoBundleOptions): Promise<void> => {
// 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),
});
}
},
};
71 changes: 71 additions & 0 deletions src/bundler/deno_bundler_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { assertRejects, assertSpyCalls, 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) => {
await tt.step(
"should invoke 'deno bundle' successfully",
Copy link

Choose a reason for hiding this comment

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

Technically we are not testing whether bundle is invoked here, we are testing whether Deno.Command was invoked. I think we may want to assert on the arguments passed into the Command stub to make sure it is bundle - assuming that's the right hypothesis we are testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, Ive added argument validation 💯

async () => {
const commandResp = {
output: () => Promise.resolve({ code: 0 }),
} as Deno.Command;

// Stub out call to `Deno.Command` and fake return a success
Copy link

Choose a reason for hiding this comment

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

Nice!

const commandStub = stub(
Deno,
"Command",
() => commandResp,
);

try {
await DenoBundler.bundle(
{ entrypoint: "./function.ts", outFile: "./dist/bundle.ts" },
);
assertSpyCalls(commandStub, 1);
} 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'",
Copy link

Choose a reason for hiding this comment

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

Love it

),
}),
} 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: "./function.ts",
outFile: "./dist/bundle.ts",
},
),
BundleError,
"Error bundling function file",
);
assertSpyCalls(commandStub, 1);
} finally {
commandStub.restore();
}
},
);
});
});
35 changes: 35 additions & 0 deletions src/bundler/esbuild_bundler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { denoPlugins, esbuild } from "../deps.ts";

export type EsbuildBundleOptions = {
WilliamBergamin marked this conversation as resolved.
Show resolved Hide resolved
/** 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<Uint8Array> => {
try {
Deno.version.deno;
// 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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Esbuild requires a deno version for the target value, currently the user is not able to specify the version their app should run on. Once they are able to, I believe the version they specify should be used here

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();
}
},
};
22 changes: 22 additions & 0 deletions src/bundler/esbuild_bundler_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { assertEquals } from "https://deno.land/[email protected]/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);
},
);
});
});
4 changes: 4 additions & 0 deletions src/bundler/mods.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export { EsbuildBundler } from "./esbuild_bundler.ts";
export { DenoBundler } from "./deno_bundler.ts";
export type { DenoBundleOptions } from "./deno_bundler.ts";
export type { EsbuildBundleOptions } from "./esbuild_bundler.ts";
2 changes: 2 additions & 0 deletions src/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ export type { JSONValue } from "https://deno.land/[email protected]/encoding/jsonc.ts"
export { deepMerge } from "https://deno.land/[email protected]/collections/deep_merge.ts";
export { getProtocolInterface } from "https://deno.land/x/[email protected]/mod.ts";
export type { Protocol } from "https://deno.land/x/[email protected]/types.ts";
export * as esbuild from "https://deno.land/x/[email protected]/mod.js";
export { denoPlugins } from "https://deno.land/x/[email protected]/mod.ts";
5 changes: 5 additions & 0 deletions src/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class BundleError extends Error {
constructor(options?: ErrorOptions) {
super("Error bundling function file", options);
}
}
Loading
Loading