mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
test(runtime): gate published declarations on consumer-resolvable imports
OSS-899 shipped 81 strict-mode errors to consumers because nothing checked what the published .d.cts files reach for. validate-dts-ambient.ts checks their shape; this checks their imports against the one thing that matters -- whether someone who installed this package and nothing else can resolve them. Flags devDependencies, optional peers, dependencies whose types live in a devDependency @types package, relative imports of JS-only bundler chunks, and an explicit ban on graphql-yoga, whose types drag lru-cache@10 into every consumer program. Currently red on 18 real violations; the fixes follow.
This commit is contained in:
Benjamin Taylor
@@ -73,7 +73,7 @@
|
||||
"generate-graphql-schema": "node -e \"const fs=require('fs');fs.rmSync('__snapshots__',{recursive:true,force:true})\" && ts-node --transpile-only ./scripts/generate-gql-schema.ts",
|
||||
"link:global": "pnpm link --global",
|
||||
"unlink:global": "pnpm unlink --global",
|
||||
"check-dts": "tsx ../../scripts/validate-dts-ambient.ts dist",
|
||||
"check-dts": "tsx ../../scripts/validate-dts-ambient.ts dist && tsx ../../scripts/validate-dts-imports.ts dist",
|
||||
"publint": "publint .",
|
||||
"attw": "attw --pack . --profile node16"
|
||||
},
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import {
|
||||
findImportViolations,
|
||||
formatViolations,
|
||||
packageNameOf,
|
||||
} from "../validate-dts-imports.js";
|
||||
|
||||
interface Fixture {
|
||||
/** Files to write under `dist/`, keyed by path relative to it. */
|
||||
dist: Record<string, string>;
|
||||
/** The package's own manifest. */
|
||||
manifest: Record<string, unknown>;
|
||||
/** Packages to fake under `node_modules/`, keyed by name. */
|
||||
installed?: Record<string, Record<string, unknown>>;
|
||||
}
|
||||
|
||||
function setup({ dist, manifest, installed = {} }: Fixture): {
|
||||
distDir: string;
|
||||
root: string;
|
||||
} {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "validate-dts-imports-"));
|
||||
const distDir = path.join(root, "dist");
|
||||
|
||||
for (const [relative, contents] of Object.entries(dist)) {
|
||||
const full = path.join(distDir, relative);
|
||||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||||
fs.writeFileSync(full, contents);
|
||||
}
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(root, "package.json"),
|
||||
JSON.stringify({ name: "fixture", ...manifest }),
|
||||
);
|
||||
|
||||
for (const [name, packageJson] of Object.entries(installed)) {
|
||||
const dir = path.join(root, "node_modules", name);
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(dir, "package.json"),
|
||||
JSON.stringify({ name, ...packageJson }),
|
||||
);
|
||||
}
|
||||
|
||||
return { distDir, root };
|
||||
}
|
||||
|
||||
describe("findImportViolations", () => {
|
||||
let root: string;
|
||||
|
||||
afterEach(() => {
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("flags the graphql-yoga type that shipped in OSS-899", () => {
|
||||
const fixture = setup({
|
||||
dist: {
|
||||
"index.d.cts": 'import { YogaInitialContext } from "graphql-yoga";',
|
||||
},
|
||||
manifest: { dependencies: { "graphql-yoga": "^5.3.1" } },
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
const violations = findImportViolations(fixture.distDir, fixture.root);
|
||||
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({
|
||||
file: "index.d.cts",
|
||||
line: 1,
|
||||
specifier: "graphql-yoga",
|
||||
});
|
||||
// Declared as a real dependency, so only the explicit ban catches it.
|
||||
expect(violations[0].reason).toContain("retired");
|
||||
});
|
||||
|
||||
it("flags a relative import of a bundler chunk emitted as JavaScript only", () => {
|
||||
const fixture = setup({
|
||||
dist: {
|
||||
"channels/index.d.cts":
|
||||
'import { __exportAll } from "../_virtual/_rolldown/runtime.cjs";',
|
||||
"_virtual/_rolldown/runtime.cjs": "module.exports = {};",
|
||||
},
|
||||
manifest: {},
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
expect(findImportViolations(fixture.distDir, fixture.root)).toEqual([
|
||||
{
|
||||
file: path.join("channels", "index.d.cts"),
|
||||
line: 1,
|
||||
specifier: "../_virtual/_rolldown/runtime.cjs",
|
||||
reason: "relative import has no declaration file next to it",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("flags an optional peer dependency a consumer may not have installed", () => {
|
||||
const fixture = setup({
|
||||
dist: { "adapter.d.cts": 'import Anthropic from "@anthropic-ai/sdk";' },
|
||||
manifest: {
|
||||
peerDependencies: { "@anthropic-ai/sdk": ">=0.57.0" },
|
||||
peerDependenciesMeta: { "@anthropic-ai/sdk": { optional: true } },
|
||||
},
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
const violations = findImportViolations(fixture.distDir, fixture.root);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0].reason).toContain("optional peer");
|
||||
});
|
||||
|
||||
it("flags a devDependency, which consumers never install", () => {
|
||||
const fixture = setup({
|
||||
dist: { "index.d.cts": 'import type { X } from "@copilotkit/channels";' },
|
||||
manifest: { devDependencies: { "@copilotkit/channels": "workspace:*" } },
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
const violations = findImportViolations(fixture.distDir, fixture.root);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0].reason).toContain("devDependency");
|
||||
});
|
||||
|
||||
it("flags a dependency whose types live in a devDependency @types package", () => {
|
||||
const fixture = setup({
|
||||
dist: { "express.d.cts": 'import type { CorsOptions } from "cors";' },
|
||||
manifest: {
|
||||
dependencies: { cors: "^2.8.5" },
|
||||
devDependencies: { "@types/cors": "^2.8.17" },
|
||||
},
|
||||
installed: { cors: { main: "lib/index.js" } },
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
const violations = findImportViolations(fixture.distDir, fixture.root);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0].reason).toContain("@types/cors must be a dependency");
|
||||
});
|
||||
|
||||
it("accepts a dependency once its @types package is promoted to a dependency", () => {
|
||||
const fixture = setup({
|
||||
dist: { "express.d.cts": 'import type { CorsOptions } from "cors";' },
|
||||
manifest: {
|
||||
dependencies: { cors: "^2.8.5", "@types/cors": "^2.8.17" },
|
||||
},
|
||||
installed: { cors: { main: "lib/index.js" } },
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
expect(findImportViolations(fixture.distDir, fixture.root)).toEqual([]);
|
||||
});
|
||||
|
||||
it("accepts builtins, resolvable relatives, real deps, and required peers", () => {
|
||||
const fixture = setup({
|
||||
dist: {
|
||||
"index.d.cts": [
|
||||
'import { Readable } from "node:stream";',
|
||||
'import { EventEmitter } from "events";',
|
||||
'import type { Foo } from "./foo.cjs";',
|
||||
'import type { Bar } from "./nested/index.cjs";',
|
||||
'export * from "@copilotkit/channels-core";',
|
||||
'import type { OpenAI } from "openai";',
|
||||
].join("\n"),
|
||||
"foo.d.cts": "export type Foo = string;",
|
||||
"nested/index.d.cts": "export type Bar = number;",
|
||||
},
|
||||
manifest: {
|
||||
dependencies: { "@copilotkit/channels-core": "workspace:^" },
|
||||
peerDependencies: { openai: ">=5.0.0" },
|
||||
},
|
||||
installed: {
|
||||
"@copilotkit/channels-core": { types: "./dist/index.d.ts" },
|
||||
openai: { types: "./index.d.ts" },
|
||||
},
|
||||
});
|
||||
root = fixture.root;
|
||||
|
||||
expect(findImportViolations(fixture.distDir, fixture.root)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("packageNameOf", () => {
|
||||
it("keeps the scope for scoped packages and drops subpaths", () => {
|
||||
expect(packageNameOf("groq-sdk/resources/chat")).toBe("groq-sdk");
|
||||
expect(packageNameOf("@langchain/langgraph-sdk/dist/types")).toBe(
|
||||
"@langchain/langgraph-sdk",
|
||||
);
|
||||
expect(packageNameOf("cors")).toBe("cors");
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatViolations", () => {
|
||||
it("is empty when there is nothing to report", () => {
|
||||
expect(formatViolations([], "dist")).toBe("");
|
||||
});
|
||||
|
||||
it("names the file, line, specifier, and reason", () => {
|
||||
const output = formatViolations(
|
||||
[
|
||||
{
|
||||
file: "index.d.cts",
|
||||
line: 3,
|
||||
specifier: "groq-sdk",
|
||||
reason: "optional peer dependency -- consumers may not install it",
|
||||
},
|
||||
],
|
||||
"dist",
|
||||
);
|
||||
|
||||
expect(output).toContain("dist/index.d.cts:3");
|
||||
expect(output).toContain('"groq-sdk"');
|
||||
expect(output).toContain("optional peer dependency");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,273 @@
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import { builtinModules } from "node:module";
|
||||
import ts from "typescript";
|
||||
|
||||
// A published declaration file may only import modules a consumer can actually
|
||||
// resolve after installing this package and nothing else. When it imports
|
||||
// something they do not get -- a devDependency, an optional peer, a package
|
||||
// whose types live in a devDependency `@types/*`, or a bundler chunk emitted as
|
||||
// JavaScript only -- the consumer's build fails the moment they compile with
|
||||
// `skipLibCheck: false`.
|
||||
//
|
||||
// That is OSS-899: a bare `import { CopilotRuntime } from "@copilotkit/runtime"`
|
||||
// produced 81 errors under `strict` + `skipLibCheck: false`. Scaffolders all set
|
||||
// `skipLibCheck: true`, so none of it was visible to us. This validator is the
|
||||
// counterpart to validate-dts-ambient.ts: that one checks the *shape* of the
|
||||
// declarations, this one checks what they *reach for*.
|
||||
|
||||
/** Modules that must never appear in a published declaration, with the reason. */
|
||||
const FORBIDDEN: Record<string, string> = {
|
||||
"graphql-yoga":
|
||||
"the v1 GraphQL server is retired -- nothing serves GraphQL, and Yoga's " +
|
||||
"types drag lru-cache@10 into every consumer's program (5x TS2416)",
|
||||
"@graphql-yoga/plugin-defer-stream":
|
||||
"same retired v1 GraphQL server; not part of the public contract",
|
||||
};
|
||||
|
||||
const DTS_SUFFIXES = [".d.ts", ".d.mts", ".d.cts"];
|
||||
const BUILTINS = new Set([
|
||||
...builtinModules,
|
||||
...builtinModules.map((m) => `node:${m}`),
|
||||
]);
|
||||
|
||||
export interface ImportViolation {
|
||||
/** Path to the declaration file, relative to the scanned directory. */
|
||||
file: string;
|
||||
/** 1-based line number of the offending import. */
|
||||
line: number;
|
||||
/** The module specifier as written. */
|
||||
specifier: string;
|
||||
/** Why a consumer cannot resolve it. */
|
||||
reason: string;
|
||||
}
|
||||
|
||||
/** Collect every `.d.ts`/`.d.mts`/`.d.cts` file under `dir`, recursively. */
|
||||
export function listDeclarationFiles(dir: string): string[] {
|
||||
const found: string[] = [];
|
||||
const walk = (current: string) => {
|
||||
for (const entry of fs.readdirSync(current, { withFileTypes: true })) {
|
||||
const full = path.join(current, entry.name);
|
||||
if (entry.isDirectory()) walk(full);
|
||||
else if (DTS_SUFFIXES.some((s) => entry.name.endsWith(s)))
|
||||
found.push(full);
|
||||
}
|
||||
};
|
||||
walk(dir);
|
||||
return found.sort();
|
||||
}
|
||||
|
||||
/** Every module specifier a declaration file imports or re-exports, with its line. */
|
||||
function readSpecifiers(
|
||||
file: string,
|
||||
): Array<{ specifier: string; line: number }> {
|
||||
const text = fs.readFileSync(file, "utf8");
|
||||
const source = ts.createSourceFile(
|
||||
file,
|
||||
text,
|
||||
ts.ScriptTarget.Latest,
|
||||
/* setParentNodes */ false,
|
||||
ts.ScriptKind.TS,
|
||||
);
|
||||
|
||||
const found: Array<{ specifier: string; line: number }> = [];
|
||||
const record = (node: ts.Node, literal: ts.Expression | undefined) => {
|
||||
if (!literal || !ts.isStringLiteral(literal)) return;
|
||||
const { line } = source.getLineAndCharacterOfPosition(
|
||||
node.getStart(source),
|
||||
);
|
||||
found.push({ specifier: literal.text, line: line + 1 });
|
||||
};
|
||||
|
||||
for (const statement of source.statements) {
|
||||
if (ts.isImportDeclaration(statement)) {
|
||||
record(statement, statement.moduleSpecifier);
|
||||
} else if (ts.isExportDeclaration(statement)) {
|
||||
record(statement, statement.moduleSpecifier);
|
||||
}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
/** `@scope/name/sub` -> `@scope/name`; `name/sub` -> `name`. */
|
||||
export function packageNameOf(specifier: string): string {
|
||||
const parts = specifier.split("/");
|
||||
return specifier.startsWith("@") ? parts.slice(0, 2).join("/") : parts[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* Does a relative specifier point at something with declarations next to it?
|
||||
*
|
||||
* Bundlers emit shared helper chunks as JavaScript. tsdown's declaration output
|
||||
* imports those chunks by their runtime filename (`../_virtual/…/runtime.cjs`),
|
||||
* which has no sibling `.d.cts` -- a TS7016 for the consumer.
|
||||
*/
|
||||
function relativeTargetHasTypes(fromFile: string, specifier: string): boolean {
|
||||
const base = path.resolve(path.dirname(fromFile), specifier);
|
||||
const withoutExt = base.replace(/\.(c|m)?js$/, "");
|
||||
const candidates = [
|
||||
...DTS_SUFFIXES.map((s) => withoutExt + s),
|
||||
...DTS_SUFFIXES.map((s) => path.join(base, "index" + s)),
|
||||
// A specifier that already names a declaration file resolves as written.
|
||||
...(DTS_SUFFIXES.some((s) => base.endsWith(s)) ? [base] : []),
|
||||
];
|
||||
return candidates.some((c) => fs.existsSync(c));
|
||||
}
|
||||
|
||||
/** Does an installed package carry its own type declarations? */
|
||||
function shipsOwnTypes(packageDir: string): boolean {
|
||||
const manifestPath = path.join(packageDir, "package.json");
|
||||
if (!fs.existsSync(manifestPath)) return false;
|
||||
const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8"));
|
||||
if (manifest.types || manifest.typings) return true;
|
||||
if (JSON.stringify(manifest.exports ?? {}).includes('"types"')) return true;
|
||||
return fs.existsSync(path.join(packageDir, "index.d.ts"));
|
||||
}
|
||||
|
||||
/** Find an installed package by walking `node_modules` up from `start`. */
|
||||
function findInstalled(start: string, name: string): string | undefined {
|
||||
let dir = start;
|
||||
for (;;) {
|
||||
const candidate = path.join(dir, "node_modules", name);
|
||||
if (fs.existsSync(candidate)) return candidate;
|
||||
const parent = path.dirname(dir);
|
||||
if (parent === dir) return undefined;
|
||||
dir = parent;
|
||||
}
|
||||
}
|
||||
|
||||
interface Manifest {
|
||||
dependencies?: Record<string, string>;
|
||||
devDependencies?: Record<string, string>;
|
||||
peerDependencies?: Record<string, string>;
|
||||
peerDependenciesMeta?: Record<string, { optional?: boolean }>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find declaration imports a consumer of this package cannot resolve.
|
||||
*
|
||||
* @param dir Directory of built declarations to scan (e.g. a package's `dist`).
|
||||
* @param packageRoot Package root holding the `package.json` that declares deps.
|
||||
*/
|
||||
export function findImportViolations(
|
||||
dir: string,
|
||||
packageRoot: string,
|
||||
): ImportViolation[] {
|
||||
const manifest: Manifest = JSON.parse(
|
||||
fs.readFileSync(path.join(packageRoot, "package.json"), "utf8"),
|
||||
);
|
||||
const deps = manifest.dependencies ?? {};
|
||||
const devDeps = manifest.devDependencies ?? {};
|
||||
const peers = manifest.peerDependencies ?? {};
|
||||
const peerMeta = manifest.peerDependenciesMeta ?? {};
|
||||
|
||||
const violations: ImportViolation[] = [];
|
||||
|
||||
for (const file of listDeclarationFiles(dir)) {
|
||||
for (const { specifier, line } of readSpecifiers(file)) {
|
||||
const add = (reason: string) =>
|
||||
violations.push({
|
||||
file: path.relative(dir, file),
|
||||
line,
|
||||
specifier,
|
||||
reason,
|
||||
});
|
||||
|
||||
if (specifier.startsWith(".")) {
|
||||
if (!relativeTargetHasTypes(file, specifier)) {
|
||||
add("relative import has no declaration file next to it");
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (BUILTINS.has(specifier)) continue;
|
||||
|
||||
const name = packageNameOf(specifier);
|
||||
|
||||
if (FORBIDDEN[name]) {
|
||||
add(FORBIDDEN[name]);
|
||||
continue;
|
||||
}
|
||||
|
||||
const isDep = name in deps;
|
||||
const isRequiredPeer = name in peers && !peerMeta[name]?.optional;
|
||||
|
||||
if (!isDep && !isRequiredPeer) {
|
||||
if (name in peers)
|
||||
add("optional peer dependency -- consumers may not install it");
|
||||
else if (name in devDeps)
|
||||
add("devDependency -- consumers never install it");
|
||||
else add("not declared in dependencies or a required peer dependency");
|
||||
continue;
|
||||
}
|
||||
|
||||
// Declared, but a consumer still gets no types if the package ships none
|
||||
// and its `@types/*` counterpart is not itself a real dependency.
|
||||
const installed = findInstalled(packageRoot, name);
|
||||
if (installed && !shipsOwnTypes(installed)) {
|
||||
const typesName = name.startsWith("@")
|
||||
? `@types/${name.slice(1).replace("/", "__")}`
|
||||
: `@types/${name}`;
|
||||
if (!(typesName in deps)) {
|
||||
add(
|
||||
`ships no types; ${typesName} must be a dependency (it is ` +
|
||||
(typesName in devDeps ? "a devDependency" : "not declared") +
|
||||
")",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return violations;
|
||||
}
|
||||
|
||||
export function formatViolations(
|
||||
violations: ImportViolation[],
|
||||
dir: string,
|
||||
): string {
|
||||
if (violations.length === 0) return "";
|
||||
const lines = violations.map(
|
||||
(v) =>
|
||||
` ${path.join(dir, v.file)}:${v.line} "${v.specifier}" -- ${v.reason}`,
|
||||
);
|
||||
return [
|
||||
`Found ${violations.length} unresolvable import(s) in published declarations.`,
|
||||
"A consumer installs this package and nothing else. Every module its .d.ts",
|
||||
"files import must be reachable from that install, or their build breaks on",
|
||||
"skipLibCheck: false.",
|
||||
"",
|
||||
...lines,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function main(argv: string[]): number {
|
||||
const dirs = argv.length > 0 ? argv : ["dist"];
|
||||
let failed = false;
|
||||
|
||||
for (const dir of dirs) {
|
||||
const resolved = path.resolve(dir);
|
||||
if (!fs.existsSync(resolved)) {
|
||||
console.error(
|
||||
`validate-dts-imports: ${dir} does not exist -- build the package first.`,
|
||||
);
|
||||
return 1;
|
||||
}
|
||||
|
||||
const violations = findImportViolations(resolved, process.cwd());
|
||||
if (violations.length > 0) {
|
||||
console.error(formatViolations(violations, dir));
|
||||
failed = true;
|
||||
} else {
|
||||
const count = listDeclarationFiles(resolved).length;
|
||||
console.log(`validate-dts-imports: ${dir} clean (${count} files).`);
|
||||
}
|
||||
}
|
||||
|
||||
return failed ? 1 : 0;
|
||||
}
|
||||
|
||||
// Guard the CLI path so the exported helpers stay importable from tests.
|
||||
if (process.argv[1] && process.argv[1].endsWith("validate-dts-imports.ts")) {
|
||||
process.exit(main(process.argv.slice(2)));
|
||||
}
|
||||
Reference in New Issue
Block a user