fix(packages): declare runtime imports as production dependencies
@ekklesia/module-sdk imports drizzle-orm at run time but declared it only as a peer and a development dependency. A workspace package is linked into its consumers as a directory, so its imports resolve from its own node_modules, where a consumer never satisfies a peer; a production install omits development dependencies. In the production image this made every module with a schema fail: the API exited at startup and Doctor reported NOT READY. - Move drizzle-orm to module-sdk's dependencies, keeping the peer. - Move react to @ekklesia/ui's dependencies, keeping the peer. The new guard flags it the same way; it is bundled at build time today, so it was not a live failure. Both lockfile entries keep the identical resolution, so nothing new is installed and the shell bundles still contain one React. - Add the boundary rule runtime-imports-are-production-dependencies: code a package ships (src/, bin/, excluding tests and .d.ts) must declare every runtime import in dependencies or optionalDependencies. Type-only imports are exempt; .tsx/.jsx implies react (automatic JSX runtime). Fixtures cover peer-only, dev-only, JSX, undeclared, and the accepted cases. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
d5d4928b9f
commit
fc422b6d8d
@@ -1,4 +1,5 @@
|
||||
import { readdir, readFile, stat } from "node:fs/promises";
|
||||
import { builtinModules } from "node:module";
|
||||
import path from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { parse } from "yaml";
|
||||
@@ -506,6 +507,82 @@ function collectImportSpecifiers(sourceText: string): string[] {
|
||||
return [...specifiers];
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a file is code a workspace package ships and executes: anything under its `src/` or
|
||||
* `bin/`, except tests and declaration files. Build scripts, configuration files, and tests run with
|
||||
* development dependencies installed, so they are outside this rule.
|
||||
*/
|
||||
function isRuntimeSourceFile(packageRelativePath: string): boolean {
|
||||
const normalized = normalizePath(packageRelativePath);
|
||||
if (!normalized.startsWith("src/") && !normalized.startsWith("bin/")) {
|
||||
return false;
|
||||
}
|
||||
const segments = normalized.split("/");
|
||||
if (segments.some((segment) => ["__tests__", "test", "tests"].includes(segment))) {
|
||||
return false;
|
||||
}
|
||||
const fileName = segments.at(-1) ?? "";
|
||||
return !/\.(?:test|spec)\.[cm]?[jt]sx?$/u.test(fileName) && !/\.d\.[cm]?ts$/u.test(fileName);
|
||||
}
|
||||
|
||||
/** `import { type A, type B } from "x"` has no runtime binding: without `verbatimModuleSyntax`, the
|
||||
* compiler erases it exactly like `import type`. A default or namespace binding, or any specifier
|
||||
* without `type`, is a runtime import. */
|
||||
function importClauseIsTypeOnly(clause: string): boolean {
|
||||
const trimmed = clause.trim();
|
||||
if (!trimmed.startsWith("{") || !trimmed.endsWith("}")) {
|
||||
return false;
|
||||
}
|
||||
const specifiers = trimmed
|
||||
.slice(1, -1)
|
||||
.split(",")
|
||||
.map((specifier) => specifier.trim())
|
||||
.filter((specifier) => specifier.length > 0);
|
||||
return specifiers.length > 0 && specifiers.every((specifier) => /^type\s/u.test(specifier));
|
||||
}
|
||||
|
||||
/**
|
||||
* Bare specifiers a source file needs at run time: every static, side-effect, and dynamic import
|
||||
* except type-only ones. A `.tsx`/`.jsx` file also needs `react`, because the platform compiles JSX
|
||||
* with the automatic runtime (`"jsx": "react-jsx"`), which imports `react/jsx-runtime` without that
|
||||
* import appearing in the source.
|
||||
*/
|
||||
function collectRuntimeImportSpecifiers(sourceText: string, extension: string): string[] {
|
||||
const specifiers = new Set<string>();
|
||||
const staticImport = /\b(?:import|export)\s+(type\s+)?([^;]*?)\bfrom\s*["']([^"'\n]+)["']/gu;
|
||||
for (const match of sourceText.matchAll(staticImport)) {
|
||||
const [, typeKeyword, clause = "", specifier] = match;
|
||||
if (specifier && !typeKeyword && !importClauseIsTypeOnly(clause)) {
|
||||
specifiers.add(specifier);
|
||||
}
|
||||
}
|
||||
const sideEffectImport = /\bimport\s*["']([^"'\n]+)["']/gu;
|
||||
const dynamicImport = /\b(?:import|require)\s*\(\s*["']([^"'\n]+)["']\s*\)/gu;
|
||||
for (const pattern of [sideEffectImport, dynamicImport]) {
|
||||
for (const match of sourceText.matchAll(pattern)) {
|
||||
if (match[1]) {
|
||||
specifiers.add(match[1]);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (extension === ".tsx" || extension === ".jsx") {
|
||||
specifiers.add("react/jsx-runtime");
|
||||
}
|
||||
return [...specifiers];
|
||||
}
|
||||
|
||||
const NODE_BUILTIN_MODULES = new Set(builtinModules);
|
||||
|
||||
function isBareSpecifier(specifier: string): boolean {
|
||||
return !(
|
||||
specifier.startsWith(".") ||
|
||||
specifier.startsWith("/") ||
|
||||
specifier.startsWith("#") ||
|
||||
specifier.startsWith("node:") ||
|
||||
/^[a-z][a-z0-9+.-]*:/u.test(specifier)
|
||||
);
|
||||
}
|
||||
|
||||
async function resolveSourceImport(
|
||||
fromFile: string,
|
||||
specifier: string,
|
||||
@@ -906,6 +983,49 @@ export async function checkBoundaries(rootDirectory: string): Promise<BoundaryRe
|
||||
sourceGraph.set(relativeFile, new Set());
|
||||
|
||||
const sourceText = await readFile(codeFile, "utf8");
|
||||
|
||||
// A workspace package is linked into its consumers as a directory, so Node resolves the
|
||||
// package's own imports from the package's own `node_modules`. A peer dependency is never
|
||||
// satisfied there by the consumer, and a production install (`pnpm install --prod`, as the
|
||||
// production image runs) omits development dependencies. Code a package ships must therefore
|
||||
// declare every runtime import as a production dependency. A peer declaration may stay beside it.
|
||||
const runtimePackage = findContainingPackage(codeFile, packagesByDepth);
|
||||
if (runtimePackage && isRuntimeSourceFile(path.relative(runtimePackage.directory, codeFile))) {
|
||||
const reported = new Set<string>();
|
||||
for (const specifier of collectRuntimeImportSpecifiers(
|
||||
sourceText,
|
||||
path.extname(codeFile).toLowerCase(),
|
||||
)) {
|
||||
if (!isBareSpecifier(specifier)) {
|
||||
continue;
|
||||
}
|
||||
const dependencyName = packageNameFromSpecifier(specifier);
|
||||
const manifest = runtimePackage.manifest;
|
||||
if (
|
||||
reported.has(dependencyName) ||
|
||||
dependencyName === runtimePackage.name ||
|
||||
NODE_BUILTIN_MODULES.has(dependencyName) ||
|
||||
manifest.dependencies?.[dependencyName] !== undefined ||
|
||||
manifest.optionalDependencies?.[dependencyName] !== undefined
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
reported.add(dependencyName);
|
||||
const declaredAs =
|
||||
manifest.peerDependencies?.[dependencyName] !== undefined
|
||||
? "only as a peer dependency"
|
||||
: manifest.devDependencies?.[dependencyName] !== undefined
|
||||
? "only as a development dependency"
|
||||
: "not at all";
|
||||
violations.push({
|
||||
from: relativeFile,
|
||||
message: `${runtimePackage.name} imports ${dependencyName} at run time but declares it ${declaredAs}; a runtime import must be in dependencies.`,
|
||||
rule: "runtime-imports-are-production-dependencies",
|
||||
to: dependencyName,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const specifier of collectImportSpecifiers(sourceText)) {
|
||||
const dependencyName = packageNameFromSpecifier(specifier);
|
||||
if (from.layer === "module" && isModuleUiPackage(dependencyName)) {
|
||||
|
||||
Reference in New Issue
Block a user