From 36ca54301151404e2a3e3a44add93e2b475aba90 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 5 Oct 2026 13:13:39 +0200 Subject: [PATCH] chore(appkit): fail packaging when the CLI imports a module outside dist/cli The published CLI imports a few leaf modules that live outside dist/cli (for example naming.js), which dist-appkit.ts copies into the tarball by hand. If a new import is added without a matching copy, the CLI breaks only after publish. Add a pack-time guard: after assembling tmp/, dist-appkit.ts walks tmp/dist/cli and fails if any relative import does not resolve inside the packed tree. Includes a unit test for the guard, the knip entry for the script, and the root @ast-grep/napi devDependency it uses. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- knip.json | 3 + package.json | 1 + .../src/tsdown/tests/package-imports.test.ts | 47 +++++++++++++++ pnpm-lock.yaml | 3 + tools/dist-appkit.ts | 4 ++ tools/validate-package-imports.ts | 58 +++++++++++++++++++ 6 files changed, 116 insertions(+) create mode 100644 packages/appkit/src/tsdown/tests/package-imports.test.ts create mode 100644 tools/validate-package-imports.ts diff --git a/knip.json b/knip.json index 02320c52b..e3d41b06a 100644 --- a/knip.json +++ b/knip.json @@ -7,6 +7,9 @@ "docs" ], "workspaces": { + ".": { + "entry": ["tools/validate-package-imports.ts"] + }, "packages/appkit": { "ignoreDependencies": [ "vitest", diff --git a/package.json b/package.json index 87de02a75..b47a77c8a 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ }, "devDependencies": { "@arethetypeswrong/core": "0.18.4", + "@ast-grep/napi": "0.37.0", "@commitlint/cli": "19.8.1", "@commitlint/config-conventional": "19.8.1", "@cyclonedx/cdxgen": "12.1.2", diff --git a/packages/appkit/src/tsdown/tests/package-imports.test.ts b/packages/appkit/src/tsdown/tests/package-imports.test.ts new file mode 100644 index 000000000..5c02337b3 --- /dev/null +++ b/packages/appkit/src/tsdown/tests/package-imports.test.ts @@ -0,0 +1,47 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { assertPackageImportsResolve } from "@tools/validate-package-imports"; +import { afterEach, describe, expect, test } from "vitest"; + +const dirs: string[] = []; + +afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true }); +}); + +function fixture(files: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-package-imports-")); + dirs.push(dir); + for (const [name, contents] of Object.entries(files)) { + const file = path.join(dir, name); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, contents); + } + return dir; +} + +describe("published package imports", () => { + test("accepts relative imports included in the package", () => { + const dir = fixture({ + "cli/command.js": [ + 'import { helper } from "../helper.js";', + 'const example = `import missing from "../not-an-import.js"`;', + ].join("\n"), + "helper.js": "export const helper = true;", + }); + + expect(() => assertPackageImportsResolve(dir)).not.toThrow(); + }); + + test("rejects a CLI command whose relative helper was not packaged", () => { + const dir = fixture({ + "cli/commands/example.js": 'import { helper } from "../../helper.js";', + }); + + expect(() => assertPackageImportsResolve(dir)).toThrow( + "cli/commands/example.js -> ../../helper.js", + ); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4ced623e4..bb8ac1c13 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -18,6 +18,9 @@ importers: '@arethetypeswrong/core': specifier: 0.18.4 version: 0.18.4 + '@ast-grep/napi': + specifier: 0.37.0 + version: 0.37.0 '@commitlint/cli': specifier: 19.8.1 version: 19.8.1(@types/node@24.7.2)(typescript@5.9.3) diff --git a/tools/dist-appkit.ts b/tools/dist-appkit.ts index fee75f96a..1f35234be 100644 --- a/tools/dist-appkit.ts +++ b/tools/dist-appkit.ts @@ -2,6 +2,8 @@ import fs from "node:fs"; import path from "node:path"; import { parseArgs } from "node:util"; +import { assertPackageImportsResolve } from "./validate-package-imports"; + const __dirname = path.dirname(new URL(import.meta.url).pathname); const { values } = parseArgs({ options: { @@ -140,6 +142,8 @@ if (fs.existsSync(sharedPostinstall)) { fs.copyFileSync(sharedPostinstall, "tmp/scripts/postinstall.js"); } +assertPackageImportsResolve("tmp/dist/cli"); + // Copy documentation from docs/build into tmp/docs/ const docsBuildPath = path.join(__dirname, "../docs/build"); diff --git a/tools/validate-package-imports.ts b/tools/validate-package-imports.ts new file mode 100644 index 000000000..7b078e732 --- /dev/null +++ b/tools/validate-package-imports.ts @@ -0,0 +1,58 @@ +import fs from "node:fs"; +import path from "node:path"; + +import { Lang, parse } from "@ast-grep/napi"; + +function findJavaScriptFiles(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const file = path.join(dir, entry.name); + if (entry.isDirectory()) return findJavaScriptFiles(file); + return /\.[cm]?js$/.test(entry.name) ? [file] : []; + }); +} + +function relativeImports(source: string): string[] { + const root = parse(Lang.JavaScript, source).root(); + const imports = root + .findAll({ + rule: { + any: [{ kind: "import_statement" }, { kind: "export_statement" }], + }, + }) + .map((node) => node.field("source")?.text()) + .filter((value): value is string => Boolean(value)); + + for (const call of root.findAll({ rule: { kind: "call_expression" } })) { + if (call.field("function")?.text() !== "import") continue; + const argument = call + .field("arguments") + ?.children() + .find((node) => node.kind() === "string"); + if (argument) imports.push(argument.text()); + } + + return imports + .map((value) => value.slice(1, -1)) + .filter((value) => value.startsWith(".")); +} + +/** Fail packaging when a built JavaScript module has a missing relative import. */ +export function assertPackageImportsResolve(dir: string): void { + const missing = findJavaScriptFiles(dir).flatMap((file) => + relativeImports(fs.readFileSync(file, "utf8")) + .map((specifier) => ({ + file, + specifier, + target: path.resolve(path.dirname(file), specifier), + })) + .filter(({ target }) => !fs.existsSync(target)), + ); + + if (missing.length) { + throw new Error( + `Package contains missing relative imports:\n${missing + .map(({ file, specifier }) => `- ${file} -> ${specifier}`) + .join("\n")}`, + ); + } +}