diff --git a/src/analyze/dependencies.ts b/src/analyze/dependencies.ts index ae124c4..7a90b70 100644 --- a/src/analyze/dependencies.ts +++ b/src/analyze/dependencies.ts @@ -4,14 +4,10 @@ import type { Stats, 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; @@ -23,37 +19,6 @@ 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); - 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; - } - } - } - - if (packageMatch) { - await traverse(packageMatch, pathInTree + ' > ' + depName); - } - } - } - - // Start traversal from root - await traverse('/package.json', 'root'); - const stats: Partial = { name: pkg.name, version: pkg.version, diff --git a/src/local-file-system.ts b/src/local-file-system.ts index c2d987a..0443831 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 ScanResult { + packageFiles: string[]; + installSize: number; +} + export class LocalFileSystem implements FileSystem { #root: string; #logger = fileSystemLogger; + #scanResult: ScanResult | undefined; constructor(root: string) { this.#root = root; @@ -18,47 +24,28 @@ 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 #scanFiles(): Promise { + if (this.#scanResult) { + return this.#scanResult; } - } - async readFile(filePath: string): Promise { - return await readFile(path.join(this.#root, filePath), 'utf8'); - } - - 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 +57,21 @@ export class LocalFileSystem implements FileSystem { this.#logger.debug('No node_modules directory found'); } + this.#scanResult = {packageFiles, installSize}; + return this.#scanResult; + } + + async listPackageFiles(): Promise { + const {packageFiles} = await this.#scanFiles(); + return packageFiles; + } + + async readFile(filePath: string): Promise { + return await readFile(path.join(this.#root, filePath), 'utf8'); + } + + async getInstallSize(): Promise { + const {installSize} = await this.#scanFiles(); 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 = {