Skip to content

Commit cd52bad

Browse files
GordonSmithGordonSmith
committed
fix: Improve type-leaks test
Was missing some node deps still in devDeps Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
1 parent 0dc9787 commit cd52bad

3 files changed

Lines changed: 258 additions & 32 deletions

File tree

‎packages/comms/package.json‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@
7575
"@hpcc-js/util": "^3.3.2",
7676
"@xmldom/xmldom": "0.9.8",
7777
"abort-controller": "3.0.0",
78+
"d3-array": "^1",
79+
"d3-format": "^1",
80+
"d3-time-format": "^2",
7881
"node-fetch": "3.3.2",
82+
"safe-buffer": "5.2.1",
83+
"tmp": "0.2.3",
7984
"undici": "6.21.2"
8085
},
8186
"devDependencies": {
@@ -86,12 +91,7 @@
8691
"@types/d3-time-format": "2.3.4",
8792
"@types/node": "^18",
8893
"@types/xmldom": "0.1.34",
89-
"d3-array": "^1",
90-
"d3-format": "^1",
91-
"d3-time-format": "^2",
9294
"data-uri-to-buffer": "6.0.2",
93-
"safe-buffer": "5.2.1",
94-
"tmp": "0.2.3",
9595
"soap": "1.1.10",
9696
"typescript-formatter": "^7.2.2"
9797
},

‎packages/markdown-it-plugins/package.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@
5858
"@types/markdown-it": "14.1.2"
5959
},
6060
"peerDependencies": {
61-
"markdown-it": "14.1.0"
61+
"apache-arrow": "19.0.1",
62+
"markdown-it": "14.1.0",
63+
"shiki": "2.5.0"
6264
},
6365
"devDependencies": {
6466
"@hpcc-js/esbuild-plugins": "^1.4.2",
65-
"apache-arrow": "19.0.1",
6667
"d3-dsv": "3.0.1",
6768
"d3-fetch": "3.0.1",
6869
"dotenv": "16.4.7",
69-
"shiki": "2.5.0",
7070
"tsx": "4.19.3"
7171
},
7272
"repository": {

‎tests/type-leaks/type-dependencies.spec.ts‎

Lines changed: 250 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { describe, it, expect } from "vitest";
22
import { existsSync, readFileSync, readdirSync, statSync } from "fs";
3-
import { join, resolve } from "path";
3+
import { join, resolve, dirname } from "path";
4+
import { fileURLToPath } from "url";
45
import { glob } from "glob";
56
import { parse } from "@typescript-eslint/typescript-estree";
67

7-
const PACKAGES_DIR = resolve("../../packages");
8+
const __filename = fileURLToPath(import.meta.url);
9+
const __dirname = dirname(__filename);
10+
const PACKAGES_DIR = resolve(__dirname, "../../packages");
811

912
interface PackageInfo {
1013
name: string;
@@ -88,6 +91,19 @@ function extractTypeReferences(filePath: string): Set<string> {
8891
}
8992
}
9093

94+
// Export declarations (for re-exports like `export * from "package"`)
95+
if (node.type === "ExportAllDeclaration" || node.type === "ExportNamedDeclaration") {
96+
if (node.source && node.source.value) {
97+
const exportSource = node.source.value;
98+
if (!exportSource.startsWith(".") && !exportSource.startsWith("/")) {
99+
const packageName = extractPackageName(exportSource);
100+
if (packageName) {
101+
typeReferences.add(packageName);
102+
}
103+
}
104+
}
105+
}
106+
91107
// Dynamic imports
92108
if (node.type === "ImportExpression" && node.source?.value) {
93109
const importSource = node.source.value;
@@ -117,20 +133,26 @@ function extractTypeReferences(filePath: string): Set<string> {
117133
// Ignore parse errors for now - we'll still catch obvious import statements
118134
console.warn(`Failed to parse ${filePath}:`, error);
119135

120-
// Fallback: use regex to find import statements
136+
// Fallback: use regex to find import and export statements
121137
const content = readFileSync(filePath, "utf8");
122-
const importRegex = /(?:import|from|require\()\s*["\']([^"\']+)["\']/g;
123-
let match;
124-
125-
while ((match = importRegex.exec(content)) !== null) {
126-
const importSource = match[1];
127-
if (!importSource.startsWith(".") && !importSource.startsWith("/")) {
128-
const packageName = extractPackageName(importSource);
129-
if (packageName) {
130-
typeReferences.add(packageName);
138+
const patterns = [
139+
/(?:import|from|require\()\s*["\']([^"\']+)["\']/g,
140+
/export\s+\*\s+from\s+["\']([^"\']+)["\']/g,
141+
/export\s+.*?\s+from\s+["\']([^"\']+)["\']/g
142+
];
143+
144+
patterns.forEach(pattern => {
145+
let match;
146+
while ((match = pattern.exec(content)) !== null) {
147+
const importSource = match[1];
148+
if (!importSource.startsWith(".") && !importSource.startsWith("/")) {
149+
const packageName = extractPackageName(importSource);
150+
if (packageName) {
151+
typeReferences.add(packageName);
152+
}
131153
}
132154
}
133-
}
155+
});
134156
}
135157

136158
return typeReferences;
@@ -176,24 +198,214 @@ function isBuiltInModule(packageName: string): boolean {
176198
packageName.startsWith("node:");
177199
}
178200

201+
/**
202+
* Extract runtime dependency references from bundled JavaScript files
203+
*/
204+
function extractRuntimeReferences(filePath: string): Set<string> {
205+
const runtimeReferences = new Set<string>();
206+
207+
try {
208+
const content = readFileSync(filePath, "utf8");
209+
210+
// More precise regex patterns to match import/require statements in bundled files
211+
const patterns = [
212+
// ES6 imports: import ... from "package" (not template literals)
213+
/\bimport\s+[^'"]*\s+from\s+["']([^"'${}]+)["']/g,
214+
// CommonJS require: require("package") (not template literals)
215+
/\brequire\s*\(\s*["']([^"'${}]+)["']\s*\)/g,
216+
// Dynamic imports: import("package") (not template literals)
217+
/\bimport\s*\(\s*["']([^"'${}]+)["']\s*\)/g,
218+
];
219+
220+
// AMD/UMD define dependencies need special handling
221+
const defineRegex = /define\s*\(\s*\[([^\]]+)\]/g;
222+
let defineMatch;
223+
while ((defineMatch = defineRegex.exec(content)) !== null) {
224+
const depList = defineMatch[1];
225+
// Split by comma but be careful about quoted strings
226+
const deps = depList.match(/["'][^"']*["']/g) || [];
227+
for (const quotedDep of deps) {
228+
const dep = quotedDep.slice(1, -1); // Remove quotes
229+
if (dep && !dep.startsWith(".") && !dep.startsWith("/") &&
230+
!dep.includes("${") && !dep.includes("'") && !dep.includes('"') &&
231+
!dep.includes(" ") && dep.length < 50 && // Avoid false positives from long code snippets
232+
/^[a-zA-Z0-9@/_-]+$/.test(dep)) { // Only valid package name characters
233+
const packageName = extractPackageName(dep);
234+
if (packageName && packageName !== "exports" && packageName !== "module" &&
235+
!["value", "dojo.parser"].includes(packageName)) {
236+
runtimeReferences.add(packageName);
237+
}
238+
}
239+
}
240+
}
241+
242+
// Process other patterns
243+
for (const pattern of patterns) {
244+
let match;
245+
while ((match = pattern.exec(content)) !== null) {
246+
const importSource = match[1];
247+
if (importSource && !importSource.startsWith(".") && !importSource.startsWith("/") &&
248+
!importSource.includes("${") && importSource.length < 100) {
249+
const packageName = extractPackageName(importSource);
250+
if (packageName && packageName !== "exports" && packageName !== "module") {
251+
runtimeReferences.add(packageName);
252+
}
253+
}
254+
}
255+
}
256+
} catch (error) {
257+
console.warn(`Failed to read bundled file ${filePath}:`, error);
258+
}
259+
260+
return runtimeReferences;
261+
}
262+
263+
describe("Package Dependencies", () => {
264+
const packages = getPackages();
265+
266+
it("should have valid package.json files", () => {
267+
expect(packages.length).toBeGreaterThan(0);
268+
packages.forEach(pkg => {
269+
expect(pkg.name).toBeTruthy();
270+
expect(pkg.packageJson).toBeTruthy();
271+
});
272+
});
273+
274+
it("should not have type dependencies that are not declared as dependencies or devDependencies", () => {
275+
const errors: string[] = [];
276+
277+
packages.forEach(pkg => {
278+
const srcDir = join(pkg.path, "src");
279+
if (!existsSync(srcDir)) return;
280+
281+
const tsFiles = glob.sync("**/*.ts", {
282+
cwd: srcDir,
283+
absolute: true,
284+
ignore: ["**/*.d.ts", "**/*.spec.ts", "**/*.test.ts"]
285+
});
286+
287+
const allTypeReferences = new Set<string>();
288+
tsFiles.forEach(file => {
289+
const refs = extractTypeReferences(file);
290+
refs.forEach(ref => allTypeReferences.add(ref));
291+
});
292+
293+
// Filter out built-in modules and internal @hpcc-js packages
294+
const externalReferences = Array.from(allTypeReferences).filter(ref =>
295+
!isBuiltInModule(ref) && !ref.startsWith("@hpcc-js/")
296+
);
297+
298+
externalReferences.forEach(ref => {
299+
if (!pkg.dependencies.has(ref) &&
300+
!pkg.devDependencies.has(ref) &&
301+
!pkg.peerDependencies.has(ref)) {
302+
errors.push(`${pkg.name}: Type reference '${ref}' not found in dependencies, devDependencies, or peerDependencies`);
303+
}
304+
});
305+
});
306+
307+
if (errors.length > 0) {
308+
throw new Error(`Type dependency violations found:\n${errors.join("\n")}`);
309+
}
310+
});
311+
312+
it("should not reference third-party runtime dependencies unless they are in dependencies", () => {
313+
const errors: string[] = [];
314+
315+
// Known build-time/dev-only packages that should be ignored
316+
const knownBuildTimeDeps = new Set([
317+
"tslib", "regenerator-runtime", "core-js", "@babel/runtime",
318+
"rollup", "webpack", "vite", "esbuild", "terser", "uglify-js",
319+
"typescript", "tsc", "@typescript-eslint", "eslint", "prettier",
320+
// Additional false positives from bundled files
321+
"exports", "module", "this", "arguments", "function", "return",
322+
"require", "define", "import", "global", "window", "document",
323+
// Template literal placeholders
324+
"${", "}", "${url}", "${e}", "${name}", "${path}",
325+
// AMD loader internals and false positives
326+
"some", "tap", "type", "listener", "addListener", "dontFix",
327+
"target", "matchesTarget", "touch.press", "touchListener",
328+
"value", "dojo.parser", "s on() method, so it can handle it"
329+
]);
330+
331+
packages.forEach(pkg => {
332+
// Check for bundled files in common output directories
333+
const bundleDirs = ["dist"];
334+
const bundledFiles: string[] = [];
335+
336+
bundleDirs.forEach(dir => {
337+
const bundleDir = join(pkg.path, dir);
338+
if (existsSync(bundleDir)) {
339+
const jsFiles = glob.sync("**/*.{js,mjs,cjs}", {
340+
cwd: bundleDir,
341+
absolute: true,
342+
ignore: ["**/*.min.js", "**/*.map", "**/*.d.ts"]
343+
});
344+
bundledFiles.push(...jsFiles);
345+
}
346+
});
347+
348+
if (bundledFiles.length === 0) return;
349+
350+
const allRuntimeReferences = new Set<string>();
351+
bundledFiles.forEach(file => {
352+
const refs = extractRuntimeReferences(file);
353+
refs.forEach(ref => allRuntimeReferences.add(ref));
354+
});
355+
356+
// Filter out built-in modules, internal @hpcc-js packages, and known build-time deps
357+
const externalReferences = Array.from(allRuntimeReferences).filter(ref =>
358+
!isBuiltInModule(ref) &&
359+
!ref.startsWith("@hpcc-js/") &&
360+
!knownBuildTimeDeps.has(ref)
361+
);
362+
363+
externalReferences.forEach(ref => {
364+
// Special handling for shim packages - they legitimately bundle dependencies
365+
// that are declared in devDependencies rather than runtime dependencies
366+
const isShimPackage = pkg.name.includes("-shim");
367+
const isInDevDeps = pkg.devDependencies.has(ref);
368+
369+
if (!pkg.dependencies.has(ref) && !pkg.peerDependencies.has(ref)) {
370+
// For shim packages, allow dependencies that are in devDependencies
371+
if (isShimPackage && isInDevDeps) {
372+
// This is expected for shim packages - skip the error
373+
return;
374+
}
375+
376+
errors.push(`${pkg.name}: Runtime reference '${ref}' not found in dependencies or peerDependencies (found in bundled files)`);
377+
}
378+
});
379+
});
380+
381+
if (errors.length > 0) {
382+
throw new Error(`Runtime dependency violations found:\n${errors.join("\n")}`);
383+
}
384+
});
385+
});
386+
179387
describe("Package Type Dependencies", () => {
180388
const packages = getPackages();
181389

182390
for (const pkg of packages) {
183391
describe(`${pkg.name}`, () => {
184392
it("should not reference third-party types unless they are in dependencies", async () => {
185-
// Get all TypeScript files in the package
186-
const tsFiles = ["types/index.d.ts", "types/index.browser.d.ts", "types/index.node.d.ts"];
393+
// Get all TypeScript declaration files in the package
394+
const typesDir = join(pkg.path, "types");
395+
if (!existsSync(typesDir)) return;
396+
397+
const tsFiles = glob.sync("**/index*.d.ts", {
398+
cwd: typesDir,
399+
absolute: true,
400+
ignore: ["**/*.spec.d.ts", "**/*.test.d.ts"]
401+
});
187402

188403
const allTypeReferences = new Set<string>();
189404

190-
// Extract type references from all TypeScript files
405+
// Extract type references from all TypeScript declaration files
191406
for (const tsFile of tsFiles) {
192-
const filePath = join(pkg.path, tsFile);
193-
if (existsSync(filePath)) {
194-
const typeRefs = extractTypeReferences(filePath);
195-
typeRefs.forEach(ref => allTypeReferences.add(ref));
196-
}
407+
const typeRefs = extractTypeReferences(tsFile);
408+
typeRefs.forEach(ref => allTypeReferences.add(ref));
197409
}
198410

199411
// Check each type reference
@@ -215,15 +427,27 @@ describe("Package Type Dependencies", () => {
215427
continue;
216428
}
217429

218-
// Skip if it's in dependencies
430+
// Skip if it's in peerDependencies
219431
if (pkg.peerDependencies.has(typeRef)) {
220432
continue;
221433
}
222434

223-
// Skip if it's a @types/ package and the base package is in dependencies
435+
// Skip if it's a @types/ package and the base package is in dependencies/peerDependencies
224436
if (typeRef.startsWith("@types/")) {
225437
const basePackage = typeRef.replace("@types/", "");
226-
if (pkg.dependencies.has(basePackage)) {
438+
if (pkg.dependencies.has(basePackage) || pkg.peerDependencies.has(basePackage)) {
439+
continue;
440+
}
441+
// Also allow if the @types package itself is in dependencies or peerDependencies
442+
if (pkg.dependencies.has(typeRef) || pkg.peerDependencies.has(typeRef)) {
443+
continue;
444+
}
445+
}
446+
447+
// Skip if it's a regular package but we have its @types/ equivalent in dependencies
448+
if (!typeRef.startsWith("@types/")) {
449+
const typesPackage = `@types/${typeRef}`;
450+
if (pkg.dependencies.has(typesPackage) || pkg.peerDependencies.has(typesPackage)) {
227451
continue;
228452
}
229453
}
@@ -250,6 +474,7 @@ describe("Package Type Dependencies", () => {
250474
if (violations.length > 0) {
251475
console.error(`\n${pkg.name} type violations:`);
252476
console.error(`Dependencies: ${Array.from(pkg.dependencies).join(", ") || "(none)"}`);
477+
console.error(`PeerDependencies: ${Array.from(pkg.peerDependencies).join(", ") || "(none)"}`);
253478
console.error(`Violations: ${violations.join(", ")}`);
254479
}
255480

@@ -258,3 +483,4 @@ describe("Package Type Dependencies", () => {
258483
});
259484
}
260485
});
486+

0 commit comments

Comments
 (0)