From 6e39d3698dd39b998d96fd3f315515d266a5ee10 Mon Sep 17 00:00:00 2001 From: Paul Valladares <85648028+dreyfus92@users.noreply.github.com> Date: Fri, 20 Feb 2026 00:18:35 -0600 Subject: [PATCH 1/2] fix: prevent OOM in analyze by adding cycle detection and caching --- src/analyze/dependencies.ts | 56 ++++++++++++++++---------- src/local-file-system.ts | 58 +++++++++++++-------------- src/test/analyze/dependencies.test.ts | 47 ++++++++++++++++++++++ 3 files changed, 110 insertions(+), 51 deletions(-) diff --git a/src/analyze/dependencies.ts b/src/analyze/dependencies.ts index ae124c4..26784c2 100644 --- a/src/analyze/dependencies.ts +++ b/src/analyze/dependencies.ts @@ -2,7 +2,8 @@ import type { ReportPluginResult, Message, Stats, - AnalysisContext + AnalysisContext, + PackageJsonLike } from '../types.js'; import {normalizePath} from '../utils/path.js'; import {getPackageJson} from '../utils/package-json.js'; @@ -23,36 +24,47 @@ export async function runDependencyAnalysis( const prodDependencies = Object.keys(pkg.dependencies || {}).length; const devDependencies = Object.keys(pkg.devDependencies || {}).length; - // Recursively traverse dependencies - async function traverse(packagePath: string, pathInTree: string) { - const depPkg = await getPackageJson(context.fs, packagePath); + const packageFilesByName = new Map(); + for (const file of packageFiles) { + const normalized = normalizePath(file); + const match = normalized.match( + /\/node_modules\/((?:@[^/]+\/)?[^/]+)\/package\.json$/ + ); + if (match) { + packageFilesByName.set(match[1], file); + } + } + + const pkgCache = new Map(); + + async function getCachedPackageJson(pkgPath: string) { + if (pkgCache.has(pkgPath)) { + return pkgCache.get(pkgPath); + } + const result = await getPackageJson(context.fs, pkgPath); + pkgCache.set(pkgPath, result); + return result; + } + + const visited = new Set(); + + async function traverse(packagePath: string) { + if (visited.has(packagePath)) return; + visited.add(packagePath); + + const depPkg = await getCachedPackageJson(packagePath); if (!depPkg || !depPkg.name) return; for (const depName of Object.keys(depPkg.dependencies || {})) { - let packageMatch = packageFiles.find((packageFile) => - normalizePath(packageFile).endsWith( - `/node_modules/${depName}/package.json` - ) - ); - - if (!packageMatch) { - for (const packageFile of packageFiles) { - const depPkg = await getPackageJson(context.fs, packageFile); - if (depPkg !== null && depPkg.name === depName) { - packageMatch = packageFile; - break; - } - } - } + const packageMatch = packageFilesByName.get(depName); if (packageMatch) { - await traverse(packageMatch, pathInTree + ' > ' + depName); + await traverse(packageMatch); } } } - // Start traversal from root - await traverse('/package.json', 'root'); + await traverse('/package.json'); const stats: Partial = { name: pkg.name, diff --git a/src/local-file-system.ts b/src/local-file-system.ts index c2d987a..fa6cdb4 100644 --- a/src/local-file-system.ts +++ b/src/local-file-system.ts @@ -6,9 +6,15 @@ import {fdir} from 'fdir'; import {readFile, stat} from 'node:fs/promises'; import {normalizePath} from './utils/path.js'; +interface CrawlResult { + packageFiles: string[]; + installSize: number; +} + export class LocalFileSystem implements FileSystem { #root: string; #logger = fileSystemLogger; + #crawlResult: CrawlResult | undefined; constructor(root: string) { this.#root = root; @@ -18,47 +24,26 @@ export class LocalFileSystem implements FileSystem { return this.#root; } - async listPackageFiles(): Promise { - const nodeModulesPath = path.join(this.#root, 'node_modules'); - - try { - await fs.access(nodeModulesPath); - const crawler = new fdir() - .withFullPaths() - .withSymlinks() - .filter((filePath) => normalizePath(filePath).endsWith('/package.json')) - .crawl(nodeModulesPath); - const files = await crawler.withPromise(); - return files.map((file) => { - const relativePath = path.relative(this.#root, file); - return '/' + normalizePath(relativePath); - }); - } catch { - return []; - } - } - - async readFile(filePath: string): Promise { - return await readFile(path.join(this.#root, filePath), 'utf8'); - } + async #crawlNodeModules(): Promise { + if (this.#crawlResult) return this.#crawlResult; - async getInstallSize(): Promise { const nodeModulesPath = path.join(this.#root, 'node_modules'); - + const packageFiles: string[] = []; let installSize = 0; try { await fs.access(nodeModulesPath); - - // TODO (43081j): we traverse the file system twice. Once here, - // and once when finding the package.json files. It may be worth - // caching some things one day so we only traverse once. const crawler = new fdir() .withFullPaths() .withSymlinks() .crawl(nodeModulesPath); const files = await crawler.withPromise(); + for (const filePath of files) { + if (normalizePath(filePath).endsWith('/package.json')) { + const relativePath = path.relative(this.#root, filePath); + packageFiles.push('/' + normalizePath(relativePath)); + } try { const stats = await stat(filePath); installSize += stats.size; @@ -70,6 +55,21 @@ export class LocalFileSystem implements FileSystem { this.#logger.debug('No node_modules directory found'); } + this.#crawlResult = {packageFiles, installSize}; + return this.#crawlResult; + } + + async listPackageFiles(): Promise { + const {packageFiles} = await this.#crawlNodeModules(); + return packageFiles; + } + + async readFile(filePath: string): Promise { + return await readFile(path.join(this.#root, filePath), 'utf8'); + } + + async getInstallSize(): Promise { + const {installSize} = await this.#crawlNodeModules(); return installSize; } diff --git a/src/test/analyze/dependencies.test.ts b/src/test/analyze/dependencies.test.ts index 2eff450..3616734 100644 --- a/src/test/analyze/dependencies.test.ts +++ b/src/test/analyze/dependencies.test.ts @@ -153,6 +153,53 @@ describe('analyzeDependencies (local)', () => { expect(stats).toMatchSnapshot(); }); + it('should handle circular dependencies without hanging', async () => { + context.packageFile.dependencies = { + 'package-a': '1.0.0' + }; + + await createTestPackage( + tempDir, + { + name: 'test-package', + version: '1.0.0', + dependencies: { + 'package-a': '1.0.0' + } + }, + {createNodeModules: true} + ); + + const nodeModules = path.join(tempDir, 'node_modules'); + + // package-a depends on package-b + const pkgADir = path.join(nodeModules, 'package-a'); + await fs.mkdir(pkgADir, {recursive: true}); + await fs.writeFile( + path.join(pkgADir, 'package.json'), + JSON.stringify({ + name: 'package-a', + version: '1.0.0', + dependencies: {'package-b': '1.0.0'} + }) + ); + + // package-b depends on package-a (circular) + const pkgBDir = path.join(nodeModules, 'package-b'); + await fs.mkdir(pkgBDir, {recursive: true}); + await fs.writeFile( + path.join(pkgBDir, 'package.json'), + JSON.stringify({ + name: 'package-b', + version: '1.0.0', + dependencies: {'package-a': '1.0.0'} + }) + ); + + const result = await runDependencyAnalysis(context); + expect(result.stats?.dependencyCount?.production).toBe(1); + }); + it('should handle missing node_modules', async () => { //update package json on context context.packageFile.dependencies = { From 464c9140e521f5055baa3ffd8df820c20917bfbc Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Sat, 21 Feb 2026 14:48:51 +0000 Subject: [PATCH 2/2] chore: we don't even use it It does nothing! --- src/analyze/dependencies.ts | 49 +------------------------------------ 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/src/analyze/dependencies.ts b/src/analyze/dependencies.ts index 26784c2..7a90b70 100644 --- a/src/analyze/dependencies.ts +++ b/src/analyze/dependencies.ts @@ -2,17 +2,12 @@ import type { ReportPluginResult, Message, Stats, - AnalysisContext, - PackageJsonLike + AnalysisContext } from '../types.js'; -import {normalizePath} from '../utils/path.js'; -import {getPackageJson} from '../utils/package-json.js'; export async function runDependencyAnalysis( context: AnalysisContext ): Promise { - const packageFiles = await context.fs.listPackageFiles(); - const messages: Message[] = []; const pkg = context.packageFile; @@ -24,48 +19,6 @@ export async function runDependencyAnalysis( const prodDependencies = Object.keys(pkg.dependencies || {}).length; const devDependencies = Object.keys(pkg.devDependencies || {}).length; - const packageFilesByName = new Map(); - for (const file of packageFiles) { - const normalized = normalizePath(file); - const match = normalized.match( - /\/node_modules\/((?:@[^/]+\/)?[^/]+)\/package\.json$/ - ); - if (match) { - packageFilesByName.set(match[1], file); - } - } - - const pkgCache = new Map(); - - async function getCachedPackageJson(pkgPath: string) { - if (pkgCache.has(pkgPath)) { - return pkgCache.get(pkgPath); - } - const result = await getPackageJson(context.fs, pkgPath); - pkgCache.set(pkgPath, result); - return result; - } - - const visited = new Set(); - - async function traverse(packagePath: string) { - if (visited.has(packagePath)) return; - visited.add(packagePath); - - const depPkg = await getCachedPackageJson(packagePath); - if (!depPkg || !depPkg.name) return; - - for (const depName of Object.keys(depPkg.dependencies || {})) { - const packageMatch = packageFilesByName.get(depName); - - if (packageMatch) { - await traverse(packageMatch); - } - } - } - - await traverse('/package.json'); - const stats: Partial = { name: pkg.name, version: pkg.version,