Skip to content

Commit

Permalink
Refactor build system for better CJS + ESM support, switch to esbuild…
Browse files Browse the repository at this point in the history
… vs rollupjs. (#102)

This PR migrates Pepr's build and bundling systems from RollupJS ->
esbuild + tsc. It also unifies the `pepr dev` and `pepr build` code in
addition to addressing issues with ESM imports. See the ADR for further
details.
  • Loading branch information
jeff-mccoy authored May 22, 2023
1 parent 2c10e7c commit 87de7a8
Show file tree
Hide file tree
Showing 45 changed files with 843 additions and 701 deletions.
28 changes: 16 additions & 12 deletions .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,23 @@
.prettierrc
.vscode
adr
CODEOWNERS
Dockerfile
insecure*
rollup.config.mjs
tsconfig.json
tsconfig.build.json
/src/cli
/dist/src/cli/**/*.d.*
/fixtures
/docs/
/hack
pepr-test-module
build.mjs

/dist/cli/**/*.d.*
/dist/fixtures
/dist/test
/pepr-test-module

/src/cli
/src/fixtures

/CODEOWNERS
/Dockerfile
/insecure*
/tsconfig.*

*.toml
docs/
dist/fixtures
*.test.*
*.tgz
27 changes: 27 additions & 0 deletions adr/0003-use-commonjs-and-esbuild.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# 3. Use CommonJS and ESBuild

Date: 2023-05-22

## Status

Accepted

## Context

As a Typescript-based project that is trying to create a Node.js Command Line Interface (CLI), a Node.js runtime component for Kubernetes (K8s), and a TypeScript (TS) Software Development Kit (SDK), there are some edge cases that we have encountered. The bottom line is CommonJS (CJS) and ECMA Script Modules (ESM) don't work well together in all cases. TS further complicates this by it's [odd stance](https://github.com/microsoft/TypeScript/issues/49083#issuecomment-1125503248) on making a pure import system.

Producing a clean JS SDK and JS CLI for NodeJS out of TS project is well-documented and suited the initial Pepr needs well. However, there were limitations to this model. Specifically a key value of Pepr is the `pepr dev` experience that allows zero-config breakpoint inspection in VSCode. Without leaning on `ts-node`, we [ran into issues](https://github.com/defenseunicorns/pepr/pull/94) preserving this experience or not creating very odd offset behaviors between the TS source and the transpiled code in the debugger. This same code must also produce the smallest possible output for the runtime controller in a single file, the presented the second issue where `pepr dev` would work due to `ts-node` magic, but the bundled output would break on ESM imports.

During the evaluation of alternative solutions, the codebase was [rewritten](https://github.com/defenseunicorns/pepr/pull/102/commits/80ed6c88d1b7789c318e04ca3adb98d40f1b46c5) to fully leverage ESM instead of CJS. Though this did solve some of the bundling issues, the DevX was very confusing due to the forced use of phantom `.js` files for imports in addition to odd behaviors with default imports. This made both developing and testing Pepr Modules confusing and frustrating.

Other build systems were explored, including [esbuild](https://esbuild.github.io/) to replace RollupJS.

## Decision

The decision has been made to implement CommonJS (CJS) in combination with ESBuild and TypeScript (tsc) in the Pepr project. This decision has been made with the objective of sustaining efficient debugging and breakpoints, operational CLI, SDK publishing, and successful bundling. esbuild is also what [Typescript themselves](https://github.com/microsoft/TypeScript/pull/51387) chose to use in their module system modernization efforts. This change also allowed the unification of the `pepr dev` and `pepr build` process to use the same code and configurations reducing risk of bugs in prod not being caught in dev.

## Consequences

The adoption of CJS with ESBuild and tsc is expected to resolve the disruptions caused by the integration of ESM dynamic imports and bundled output. The integration of ESBuild's superior bundling capabilities with tsc for type-checking and typedef generation is expected to enhance the build process and the overall user experience.

This decision does not introduce breaking changes in the existing system, highlighting its backward compatibility.
54 changes: 54 additions & 0 deletions build.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* eslint-disable no-undef */

import { analyzeMetafile, build } from "esbuild";

import packageJSON from "./package.json" assert { type: "json" };

const { dependencies, peerDependencies } = packageJSON;
const external = Object.keys(dependencies).concat(Object.keys(peerDependencies));

const buildOpts = {
bundle: true,
external,
format: "cjs",
legalComments: "eof",
metafile: true,
platform: "node",
};

async function builder() {
try {
// Build the CLI
const cli = await build({
...buildOpts,
entryPoints: ["src/cli.ts"],
outfile: "dist/cli.js",
});

console.log(await analyzeMetafile(cli.metafile));

// Build the controller runtime
const controller = await build({
...buildOpts,
entryPoints: ["src/cli/run.ts"],
outfile: "dist/controller.js",
});

console.log(await analyzeMetafile(controller.metafile));

// Build the library
const lib = await build({
...buildOpts,
entryPoints: ["src/lib.ts"],
outfile: "dist/lib.js",
sourcemap: true,
});

console.log(await analyzeMetafile(lib.metafile));
} catch (e) {
console.error(e);
process.exit(1);
}
}

builder();
10 changes: 6 additions & 4 deletions hack/build-template-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@ const path = require("path");
const baseDir = path.join(__dirname, "..", "src", "cli", "init", "templates");

// Read the text file
const gitignore = fs.readFileSync(path.join(baseDir, "gitignore"), "utf8");
const readme = fs.readFileSync(path.join(baseDir, "README.md"), "utf8");
const gitIgnore = fs.readFileSync(path.join(baseDir, "gitignore"), "utf8");
const readmeMd = fs.readFileSync(path.join(baseDir, "README.md"), "utf8");
const peprTS = fs.readFileSync(path.join(baseDir, "pepr.ts"), "utf8");
const helloPeprTS = fs.readFileSync(path.join(baseDir, "capabilities", "hello-pepr.ts"), "utf8");
const packageJSON = JSON.parse(fs.readFileSync(path.join(__dirname, "..", "package.json"), "utf8"));

fs.writeFileSync(
path.join(baseDir, "data.json"),
JSON.stringify({
gitignore,
readme,
gitIgnore,
readmeMd,
peprTS,
helloPeprTS,
packageJSON,
})
);
38 changes: 20 additions & 18 deletions hack/e2e.test.js → hack/e2e.test.mjs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
/* eslint-disable */
const test = require("ava");
const fs = require("fs").promises;
const path = require("path");
const { spawn, execSync } = require("child_process");
const k8s = require("@kubernetes/client-node");

const kc = new k8s.KubeConfig();
import { AppsV1Api, CoreV1Api, KubeConfig } from "@kubernetes/client-node";
import test from "ava";
import { execSync, spawn } from "child_process";
import { promises as fs } from "fs";
import { resolve } from "path";

const kc = new KubeConfig();
kc.loadFromDefault();

const k8sApi = kc.makeApiClient(k8s.AppsV1Api);
const k8sCoreApi = kc.makeApiClient(k8s.CoreV1Api);
const k8sApi = kc.makeApiClient(AppsV1Api);
const k8sCoreApi = kc.makeApiClient(CoreV1Api);

// Timeout in milliseconds
const TIMEOUT = 120 * 1000;
Expand All @@ -21,16 +20,16 @@ let expectedLines = [
"hello-pepr: V1ConfigMap Binding created",
"hello-pepr: V1ConfigMap Binding action created",
"Server listening on port 3000",
"Using beforeHook: req => pepr_1.Log.debug(`beforeHook: ${req.uid}`)",
"Using afterHook: res => pepr_1.Log.debug(`afterHook: ${res.uid}`)",
"Using beforeHook: (req) => import_pepr2.Log.debug(`beforeHook: ${req.uid}`)",
"Using afterHook: (res) => import_pepr2.Log.debug(`afterHook: ${res.uid}`)",
];

function stripAnsiCodes(input) {
return input.replace(/\u001B\[[0-9;]*[a-zA-Z]/g, "");
}

test.before(async t => {
const dir = path.resolve(testDir);
const dir = resolve(testDir);

try {
await fs.access(dir);
Expand All @@ -56,8 +55,8 @@ test.serial("E2E: Pepr Build", async t => {
try {
execSync("pepr build", { cwd: testDir, stdio: "inherit" });
// check if the file exists
await fs.access(path.resolve(testDir, "dist", "zarf.yaml"));
await fs.access(path.resolve(testDir, "dist", "pepr-module-static-test.yaml"));
await fs.access(resolve(testDir, "dist", "zarf.yaml"));
await fs.access(resolve(testDir, "dist", "pepr-module-static-test.yaml"));

t.pass();
} catch (e) {
Expand All @@ -77,7 +76,7 @@ test.serial("E2E: Pepr Deploy", async t => {

// Apply the sample yaml for the HelloPepr capability
execSync("kubectl apply -f hello-pepr.samples.yaml", {
cwd: path.resolve(testDir, "capabilities"),
cwd: resolve(testDir, "capabilities"),
stdio: "inherit",
});

Expand Down Expand Up @@ -137,7 +136,7 @@ test.serial("E2E: Pepr Deploy", async t => {

// Remove the sample yaml for the HelloPepr capability
execSync("kubectl delete -f hello-pepr.samples.yaml", {
cwd: path.resolve(testDir, "capabilities"),
cwd: resolve(testDir, "capabilities"),
stdio: "inherit",
});

Expand Down Expand Up @@ -165,7 +164,10 @@ function peprDev(resolve, reject) {
return !data.replace(/\s+/g, " ").includes(expectedLine);
});

console.log("\x1b[36m%s\x1b[0m'", "Remaining expected lines:" + JSON.stringify(expectedLines, null, 2));
console.log(
"\x1b[36m%s\x1b[0m'",
"Remaining expected lines:" + JSON.stringify(expectedLines, null, 2)
);

// If all expected lines are found, resolve the promise
if (expectedLines.length < 1) {
Expand Down
Loading

0 comments on commit 87de7a8

Please sign in to comment.