mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 21:59:38 +01:00
refactor: remove code comments added by recent PRs (#16681)
CONTRIBUTING.md says PRs add no code comments, but #16485, #16586, #16621 and #16672 added some. This removes the 215 comment-only lines git blame on master attributes to those merge commits, across 12 code files, plus one blank line the removal left doubled. Rewritten (not added) comments in scripts/assert-cloud-image-sentry.mjs and comments in .github workflows are left alone. No code changes. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
bebeb389f1
commit
e30550ef39
12 files changed
-216
No files matched your search
@@ -1,36 +1,3 @@
|
||||
// Bundle the built server into `dist-runtime/` for production startup.
|
||||
//
|
||||
// The server's tsc output imports the workspace packages as TypeScript source
|
||||
// (their package exports point at `src/*.ts`), so production used to start
|
||||
// Node with the tsx loader. Every boot then resolved and transpiled thousands
|
||||
// of modules through tsx's hooks: about 8-10 s warm and over 20 s on a cold
|
||||
// container, all before the server could listen. This script runs after the
|
||||
// server `build` and bundles `dist/index.js`, with the workspace packages
|
||||
// compiled in, so production starts with plain `node` in a few seconds.
|
||||
//
|
||||
// The bundle keeps runtime behavior the same as running `dist/index.js`:
|
||||
//
|
||||
// - Third-party packages stay external. A package the server package itself
|
||||
// resolves to the same file stays a bare specifier, so Node resolves it at
|
||||
// runtime from server/node_modules exactly as before (the cloud image adds
|
||||
// optional peers such as @sentry/node there after the build). Only a
|
||||
// dependency that pnpm's strict layout hides from the server package, one
|
||||
// that only a workspace package can see, is emitted as an absolute path.
|
||||
// Resolution follows Node's rules (the `main` field and `node` condition),
|
||||
// not bundler-only `module` builds.
|
||||
// - Every bundled module keeps its own location: `import.meta.url`,
|
||||
// `import.meta.dirname` and `import.meta.filename` are replaced with the
|
||||
// module's original path, so code that finds files next to its source
|
||||
// (scripts, assets, worker entry files) keeps finding them.
|
||||
// - Code splitting keeps dynamic imports as separate chunks. Bundled into a
|
||||
// single file, the server's top-level await never settled (observed on the
|
||||
// production build); split chunks keep the module evaluation order Node
|
||||
// uses for the unbundled output.
|
||||
// - A `require` built from the chunk's location is provided for the few
|
||||
// modules that call `require` from ESM.
|
||||
// - The bundle's entry sets `process.argv[1]` to the original entry file
|
||||
// before loading it, so the server's main-module check (and anything else
|
||||
// that reads argv[1]) sees exactly what it saw when started unbundled.
|
||||
|
||||
import { readFile, rm } from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
@@ -42,14 +9,6 @@ const serverDir = join(scriptDir, "..");
|
||||
|
||||
const REQUIRE_SHIM = "import { createRequire as __paperclipCreateRequire } from 'node:module'; const require = __paperclipCreateRequire(import.meta.url);";
|
||||
|
||||
/**
|
||||
* Replace a module's `import.meta` location properties with its original
|
||||
* path, so bundling does not move where the module looks for nearby files.
|
||||
*
|
||||
* @param {string} source Module source.
|
||||
* @param {string} file Absolute path of the module's original file.
|
||||
* @returns {string}
|
||||
*/
|
||||
export function preserveModuleLocation(source, file) {
|
||||
return source
|
||||
.replace(/\bimport\.meta\.url\b/g, JSON.stringify(pathToFileURL(file).href))
|
||||
@@ -57,13 +16,6 @@ export function preserveModuleLocation(source, file) {
|
||||
.replace(/\bimport\.meta\.filename\b/g, JSON.stringify(file));
|
||||
}
|
||||
|
||||
/**
|
||||
* The bundle's entry: present the original entry file as the main module,
|
||||
* then load it.
|
||||
*
|
||||
* @param {string} entry Absolute path of the original entry file.
|
||||
* @returns {import("esbuild").Plugin}
|
||||
*/
|
||||
function runtimeEntry(entry) {
|
||||
return {
|
||||
name: "runtime-entry",
|
||||
@@ -78,7 +30,6 @@ function runtimeEntry(entry) {
|
||||
};
|
||||
}
|
||||
|
||||
/** @type {import("esbuild").Plugin} */
|
||||
const keepModuleLocations = {
|
||||
name: "keep-module-locations",
|
||||
setup(builder) {
|
||||
@@ -95,13 +46,6 @@ const keepModuleLocations = {
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* @param {(specifier: string) => boolean} isBundled Whether a bare specifier is
|
||||
* bundled (workspace packages) rather than kept external.
|
||||
* @param {string} runtimeDir A directory inside the server package, from which
|
||||
* bare specifiers resolve at runtime.
|
||||
* @returns {import("esbuild").Plugin}
|
||||
*/
|
||||
function externalizeThirdParty(isBundled, runtimeDir) {
|
||||
return {
|
||||
name: "externalize-third-party",
|
||||
@@ -114,8 +58,6 @@ function externalizeThirdParty(isBundled, runtimeDir) {
|
||||
importer: args.importer,
|
||||
pluginData: { externalizeThirdParty: true },
|
||||
});
|
||||
// Builtins and anything esbuild cannot resolve stay as written;
|
||||
// Node resolves them at runtime exactly as before.
|
||||
if (resolved.errors.length > 0 || resolved.external) return { path: args.path, external: true };
|
||||
const fromServer = await builder.resolve(args.path, {
|
||||
kind: args.kind,
|
||||
@@ -129,16 +71,6 @@ function externalizeThirdParty(isBundled, runtimeDir) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Bundle a built entry point and everything it imports from the workspace.
|
||||
*
|
||||
* @param {{
|
||||
* entry: string,
|
||||
* outdir: string,
|
||||
* isBundled?: (specifier: string) => boolean,
|
||||
* }} options
|
||||
* @returns {Promise<{ outfile: string, inputs: number, chunks: number }>}
|
||||
*/
|
||||
export async function bundleRuntime({ entry, outdir, isBundled = (specifier) => specifier.startsWith("@paperclipai/") }) {
|
||||
await rm(outdir, { recursive: true, force: true });
|
||||
const result = await build({
|
||||
|
||||
@@ -20,7 +20,6 @@ describe("bundleRuntime", () => {
|
||||
|
||||
beforeEach(() => {
|
||||
root = realpathSync(mkdtempSync(path.join(tmpdir(), "paperclip-bundle-runtime-")));
|
||||
// A workspace package that exports TypeScript source, like packages/*.
|
||||
const pkg = path.join(root, "packages", "fixture");
|
||||
write(path.join(pkg, "package.json"), JSON.stringify({ name: "@paperclipai/fixture", type: "module", exports: { ".": "./src/index.ts" } }));
|
||||
write(path.join(pkg, "src", "asset.txt"), "asset beside its source\n");
|
||||
@@ -35,31 +34,24 @@ describe("bundleRuntime", () => {
|
||||
"export const values = { doubled: double(21), onlyForPackage, nodeBuild };",
|
||||
"",
|
||||
].join("\n"));
|
||||
// A dependency only the workspace package can see (pnpm strict layout).
|
||||
write(path.join(pkg, "node_modules", "package-private-dep", "package.json"), JSON.stringify({ name: "package-private-dep", type: "module", main: "index.js" }));
|
||||
write(path.join(pkg, "node_modules", "package-private-dep", "index.js"), "export const onlyForPackage = \"resolved from the package\";\n");
|
||||
// A dependency whose bundler-only "module" build is not valid Node ESM;
|
||||
// Node ignores the "module" condition, bundlers prefer it.
|
||||
write(path.join(pkg, "node_modules", "dual-build-dep", "package.json"), JSON.stringify({
|
||||
name: "dual-build-dep", exports: { ".": { module: "./bundler/index.js", default: "./node.cjs" } },
|
||||
}));
|
||||
write(path.join(pkg, "node_modules", "dual-build-dep", "node.cjs"), "exports.nodeBuild = \"node build\";\n");
|
||||
write(path.join(pkg, "node_modules", "dual-build-dep", "bundler", "index.js"), "export * from \"./extensionless\";\n");
|
||||
|
||||
// The server package sees the workspace package through its node_modules.
|
||||
const server = path.join(root, "server");
|
||||
mkdirSync(path.join(server, "node_modules", "@paperclipai"), { recursive: true });
|
||||
symlinkSync(pkg, path.join(server, "node_modules", "@paperclipai", "fixture"), "dir");
|
||||
write(path.join(server, "package.json"), JSON.stringify({ name: "server", type: "module" }));
|
||||
// A dependency the server package resolves itself, linked from a store
|
||||
// directory the way pnpm installs packages.
|
||||
for (const [version, flavor] of [["1", "installed at build"], ["2", "replaced after build"]]) {
|
||||
const store = path.join(root, "node_modules", ".pnpm", `server-dep@${version}`);
|
||||
write(path.join(store, "package.json"), JSON.stringify({ name: "server-dep", type: "module", main: "index.js" }));
|
||||
write(path.join(store, "index.js"), `export const flavor = ${JSON.stringify(flavor)};\n`);
|
||||
}
|
||||
symlinkSync(path.join(root, "node_modules", ".pnpm", "server-dep@1"), path.join(server, "node_modules", "server-dep"), "dir");
|
||||
// The entry awaits a dynamic import of another built module.
|
||||
write(path.join(server, "dist", "late.js"), "import { values } from \"@paperclipai/fixture\";\nexport const late = () => values.doubled + 1;\n");
|
||||
write(path.join(server, "dist", "index.js"), [
|
||||
"import { asset, location, values } from \"@paperclipai/fixture\";",
|
||||
@@ -87,7 +79,6 @@ describe("bundleRuntime", () => {
|
||||
const output = JSON.parse(stdout);
|
||||
expect(output.values).toEqual({ doubled: 42, onlyForPackage: "resolved from the package", nodeBuild: "node build" });
|
||||
expect(output.late).toBe(43);
|
||||
// Modules keep their original locations, so nearby files still resolve.
|
||||
expect(output.asset).toBe("asset beside its source");
|
||||
expect(output.location).toBe(path.join(root, "packages", "fixture", "src"));
|
||||
expect(output.entryDir).toBe(path.join(root, "server", "dist"));
|
||||
@@ -96,9 +87,6 @@ describe("bundleRuntime", () => {
|
||||
it("leaves the server's own dependencies to runtime resolution", async () => {
|
||||
const outdir = path.join(root, "server", "dist-runtime");
|
||||
const summary = await bundleRuntime({ entry: path.join(root, "server", "dist", "index.js"), outdir });
|
||||
// Re-point the package after the build, as the cloud image does when it
|
||||
// installs optional peers into server/node_modules; the bundle must pick
|
||||
// up the replacement instead of the store path seen at build time.
|
||||
rmSync(path.join(root, "server", "node_modules", "server-dep"));
|
||||
symlinkSync(path.join(root, "node_modules", ".pnpm", "server-dep@2"), path.join(root, "server", "node_modules", "server-dep"), "dir");
|
||||
const { stdout } = await execFileAsync(process.execPath, [summary.outfile], { cwd: path.join(root, "server"), timeout: 30_000 });
|
||||
|
||||
Reference in new issue
Block a user