From ceba107425229361014d56902d9d96d8cdaa3274 Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Thu, 21 Dec 2023 13:19:52 +0200 Subject: [PATCH] feat: add lockfile direct/transitive dep affected detection (#24) * feat: add lockfile direct dep affected detection * feat: use package manager list to get direct deps by transitive deps yarn is not supported atm :( --- .cspell.json | 1 + libs/core/src/assets.spec.ts | 95 +++++++++ libs/core/src/assets.ts | 63 ++++++ libs/core/src/find-direct-deps.ts | 71 +++++++ libs/core/src/find-drect-deps.spec.ts | 268 ++++++++++++++++++++++++ libs/core/src/git.spec.ts | 32 ++- libs/core/src/git.ts | 28 ++- libs/core/src/lock-files.spec.ts | 287 ++++++++++++++++++++++++++ libs/core/src/lock-files.ts | 95 +++++++++ libs/core/src/true-affected.spec.ts | 30 +++ libs/core/src/true-affected.ts | 32 ++- libs/core/src/utils.spec.ts | 100 +-------- libs/core/src/utils.ts | 65 ------ package-lock.json | 6 + package.json | 1 + 15 files changed, 999 insertions(+), 175 deletions(-) create mode 100644 libs/core/src/assets.spec.ts create mode 100644 libs/core/src/assets.ts create mode 100644 libs/core/src/find-direct-deps.ts create mode 100644 libs/core/src/find-drect-deps.spec.ts create mode 100644 libs/core/src/lock-files.spec.ts create mode 100644 libs/core/src/lock-files.ts diff --git a/.cspell.json b/.cspell.json index 5199a26..124b2ac 100644 --- a/.cspell.json +++ b/.cspell.json @@ -3,6 +3,7 @@ "language": "en", "words": [ "iife", + "microdiff", "nxignore", "traf", "turborepo" diff --git a/libs/core/src/assets.spec.ts b/libs/core/src/assets.spec.ts new file mode 100644 index 0000000..80906f3 --- /dev/null +++ b/libs/core/src/assets.spec.ts @@ -0,0 +1,95 @@ +import { fastFindInFiles } from 'fast-find-in-files'; +import { existsSync } from 'fs'; +import * as path from 'path'; +import { findNonSourceAffectedFiles } from './assets'; + +jest.mock('fast-find-in-files'); +jest.mock('fs'); + +describe('findNonSourceAffectedFiles', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('should return relevant files', () => { + const cwd = '/project'; + const changedFilePath = '/project/src/file.ts'; + const excludeFolderPaths = ['node_modules', 'dist', '.git']; + + (fastFindInFiles as jest.Mock).mockReturnValue([ + { + filePath: '/project/src/file.ts', + queryHits: [{ lineNumber: 1, line: `"file.ts"` }], + }, + ]); + (existsSync as jest.Mock).mockReturnValue(true); + + const result = findNonSourceAffectedFiles( + cwd, + changedFilePath, + excludeFolderPaths + ); + + expect(result).toEqual([{ filePath: 'src/file.ts', changedLines: [1] }]); + expect(fastFindInFiles).toHaveBeenCalledWith({ + directory: cwd, + needle: path.basename(changedFilePath), + excludeFolderPaths: excludeFolderPaths.map((folder) => + path.join(cwd, folder) + ), + }); + }); + + it('should return empty array if no relevant files found', () => { + const cwd = '/project'; + const changedFilePath = '/project/src/file.ts'; + const excludeFolderPaths = ['node_modules', 'dist', '.git']; + + (fastFindInFiles as jest.Mock).mockReturnValue([]); + (existsSync as jest.Mock).mockReturnValue(true); + + const result = findNonSourceAffectedFiles( + cwd, + changedFilePath, + excludeFolderPaths + ); + + expect(result).toEqual([]); + expect(fastFindInFiles).toHaveBeenCalledWith({ + directory: cwd, + needle: path.basename(changedFilePath), + excludeFolderPaths: excludeFolderPaths.map((folder) => + path.join(cwd, folder) + ), + }); + }); + + it("should still work even if found file didn't have a match", () => { + const cwd = '/project'; + const changedFilePath = '/project/src/file.ts'; + const excludeFolderPaths = ['node_modules', 'dist', '.git']; + + (fastFindInFiles as jest.Mock).mockReturnValue([ + { + filePath: '/project/src/file.ts', + queryHits: [{ lineNumber: 1, line: `console.log('hi')` }], + }, + ]); + (existsSync as jest.Mock).mockReturnValue(true); + + const result = findNonSourceAffectedFiles( + cwd, + changedFilePath, + excludeFolderPaths + ); + + expect(result).toEqual([]); + expect(fastFindInFiles).toHaveBeenCalledWith({ + directory: cwd, + needle: path.basename(changedFilePath), + excludeFolderPaths: excludeFolderPaths.map((folder) => + path.join(cwd, folder) + ), + }); + }); +}); diff --git a/libs/core/src/assets.ts b/libs/core/src/assets.ts new file mode 100644 index 0000000..5579365 --- /dev/null +++ b/libs/core/src/assets.ts @@ -0,0 +1,63 @@ +import { basename, dirname, join, relative, resolve } from 'path'; +import { ChangedFiles } from './git'; +import { FastFindInFiles, fastFindInFiles } from 'fast-find-in-files'; +import { existsSync } from 'fs'; + +export function findNonSourceAffectedFiles( + cwd: string, + changedFilePath: string, + excludeFolderPaths: string[] +): ChangedFiles[] { + const fileName = basename(changedFilePath); + + const files = fastFindInFiles({ + directory: cwd, + needle: fileName, + excludeFolderPaths: excludeFolderPaths.map((path) => join(cwd, path)), + }); + + const relevantFiles = filterRelevantFiles(cwd, files, changedFilePath); + + return relevantFiles; +} + +function filterRelevantFiles( + cwd: string, + files: FastFindInFiles[], + changedFilePath: string +): ChangedFiles[] { + const fileName = basename(changedFilePath); + const regExp = new RegExp(`['"\`](?.*${fileName})['"\`]`); + + return files + .map(({ filePath: foundFilePath, queryHits }) => ({ + filePath: relative(cwd, foundFilePath), + changedLines: queryHits + .filter(({ line }) => + isRelevantLine(line, regExp, cwd, foundFilePath, changedFilePath) + ) + .map(({ lineNumber }) => lineNumber), + })) + .filter(({ changedLines }) => changedLines.length > 0); +} + +function isRelevantLine( + line: string, + regExp: RegExp, + cwd: string, + foundFilePath: string, + changedFilePath: string +): boolean { + const match = regExp.exec(line); + const { relFilePath } = match?.groups ?? {}; + + if (relFilePath == null) return false; + + const changedFile = resolve(cwd, changedFilePath); + const relatedFilePath = resolve( + cwd, + relative(cwd, join(dirname(foundFilePath), relFilePath)) + ); + + return relatedFilePath === changedFile && existsSync(relatedFilePath); +} diff --git a/libs/core/src/find-direct-deps.ts b/libs/core/src/find-direct-deps.ts new file mode 100644 index 0000000..6becc69 --- /dev/null +++ b/libs/core/src/find-direct-deps.ts @@ -0,0 +1,71 @@ +import { execSync } from 'node:child_process'; +import { readModulePackageJson } from 'nx/src/utils/package-json.js'; +import type { PackageManager } from 'nx/src/utils/package-manager.js'; + +interface NpmListJson { + dependencies: Record; +} + +export function npmFindDirectDeps(cwd: string, packages: string[]): string[] { + const pattern = packages.length > 1 ? `{${packages.join(',')}}` : packages; + const result = execSync(`npm list -a --json --package-lock-only ${pattern}`, { + cwd, + encoding: 'utf-8', + }); + const { dependencies = {} } = JSON.parse(result) as NpmListJson; + + return Object.keys(dependencies); +} + +export function yarnFindDirectDeps(cwd: string, packages: string[]): string[] { + const pkg = readModulePackageJson(cwd).packageJson; + const deps = [ + ...Object.keys(pkg.dependencies || {}), + ...Object.keys(pkg.devDependencies || {}), + ]; + + const direct = deps.filter((dep) => packages.includes(dep)); + const transitive = deps.filter((dep) => !packages.includes(dep)); + + if (transitive.length > 0) { + console.warn( + 'INFO: detected yarn & affected transitive deps. unfortunately yarn list does not return direct dependencies from transitive dependencies. only top level dependencies are returned atm. PRs are welcome!' + ); + } + + return direct; +} + +interface PnpmListJson { + dependencies: Record; + devDependencies: Record; +} + +export function pnpmFindDirectDeps(cwd: string, packages: string[]): string[] { + // pnpm ls {fast-glob,loader-utils} --depth Infinity --json + const pattern = packages.length > 1 ? `{${packages.join(',')}}` : packages; + const result = execSync(`pnpm ls ${pattern} --depth Infinity --json`, { + cwd, + encoding: 'utf-8', + }); + const [{ dependencies = {}, devDependencies = {} }] = JSON.parse( + result + ) as PnpmListJson[]; + + return [...Object.keys(dependencies), ...Object.keys(devDependencies)]; +} + +export function findDirectDeps( + packageManager: PackageManager, + cwd: string, + packages: string[] +): string[] { + switch (packageManager) { + case 'npm': + return npmFindDirectDeps(cwd, packages); + case 'yarn': + return yarnFindDirectDeps(cwd, packages); + case 'pnpm': + return pnpmFindDirectDeps(cwd, packages); + } +} diff --git a/libs/core/src/find-drect-deps.spec.ts b/libs/core/src/find-drect-deps.spec.ts new file mode 100644 index 0000000..e6fb6d2 --- /dev/null +++ b/libs/core/src/find-drect-deps.spec.ts @@ -0,0 +1,268 @@ +import * as cp from 'node:child_process'; +import * as packageJsonUtils from 'nx/src/utils/package-json.js'; +import { + pnpmFindDirectDeps, + npmFindDirectDeps, + yarnFindDirectDeps, + findDirectDeps, +} from './find-direct-deps'; + +describe('pnpmFindDirectDeps', () => { + const cwd = '/project'; + const packages = ['nested-dep1', 'nested-dep2']; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should execute pnpm ls command with correct pattern and options', () => { + const spy = jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify([ + { + dependencies: { dep1: '1.0.0' }, + devDependencies: { dep2: '2.0.0' }, + }, + ]) + ); + + pnpmFindDirectDeps(cwd, packages); + + expect(spy).toHaveBeenCalledWith( + `pnpm ls {nested-dep1,nested-dep2} --depth Infinity --json`, + { + cwd, + encoding: 'utf-8', + } + ); + }); + + it('should execute pnpm ls command with correct pattern when there is only one package', () => { + const spy = jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify([ + { + dependencies: { dep1: '1.0.0' }, + devDependencies: { dep2: '2.0.0' }, + }, + ]) + ); + + pnpmFindDirectDeps(cwd, ['nested-dep1']); + + expect(spy).toHaveBeenCalledWith( + `pnpm ls nested-dep1 --depth Infinity --json`, + { + cwd, + encoding: 'utf-8', + } + ); + }); + + it('should return direct dependencies from the pnpm ls result', () => { + jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify([ + { + dependencies: { dep1: '1.0.0' }, + devDependencies: { dep2: '2.0.0' }, + }, + ]) + ); + + const result = pnpmFindDirectDeps(cwd, packages); + + expect(result).toEqual(['dep1', 'dep2']); + }); + + it('should return an empty array if pnpm ls result is empty', () => { + jest.spyOn(cp, 'execSync').mockReturnValueOnce(JSON.stringify([{}])); + + const result = pnpmFindDirectDeps(cwd, packages); + + expect(result).toEqual([]); + }); +}); + +describe('npmFindDirectDeps', () => { + const cwd = '/project'; + const packages = ['nested-dep1', 'nested-dep2']; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should execute npm list command with correct pattern and options', () => { + const spy = jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify({ + dependencies: { dep1: '1.0.0', dep2: '2.0.0' }, + }) + ); + + npmFindDirectDeps(cwd, packages); + + expect(spy).toHaveBeenCalledWith( + `npm list -a --json --package-lock-only {nested-dep1,nested-dep2}`, + { + cwd, + encoding: 'utf-8', + } + ); + }); + + it('should execute npm list command with correct pattern when there is only one package', () => { + const spy = jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify({ + dependencies: { dep1: '1.0.0', dep2: '2.0.0' }, + }) + ); + + npmFindDirectDeps(cwd, ['nested-dep1']); + + expect(spy).toHaveBeenCalledWith( + `npm list -a --json --package-lock-only nested-dep1`, + { + cwd, + encoding: 'utf-8', + } + ); + }); + + it('should return direct dependencies from the npm list result', () => { + jest.spyOn(cp, 'execSync').mockReturnValueOnce( + JSON.stringify({ + dependencies: { dep1: '1.0.0', dep2: '2.0.0' }, + }) + ); + + const result = npmFindDirectDeps(cwd, packages); + + expect(result).toEqual(['dep1', 'dep2']); + }); + + it('should return an empty array if npm list result is empty', () => { + jest.spyOn(cp, 'execSync').mockReturnValueOnce(JSON.stringify({})); + + const result = npmFindDirectDeps(cwd, packages); + + expect(result).toEqual([]); + }); +}); + +describe('yarnFindDirectDeps', () => { + const cwd = '/project'; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return direct dependencies from package.json if they were affected', () => { + jest.spyOn(packageJsonUtils, 'readModulePackageJson').mockReturnValueOnce({ + path: '/project/package.json', + packageJson: { + name: 'project', + version: '1.0.0', + dependencies: { + dep1: '1.0.0', + }, + devDependencies: { + dep2: '2.0.0', + }, + }, + }); + const result = yarnFindDirectDeps(cwd, ['dep1', 'dep2']); + + expect(result).toEqual(['dep1', 'dep2']); + }); + + it('should return an empty array if affected packages are transitive', () => { + jest.spyOn(packageJsonUtils, 'readModulePackageJson').mockReturnValueOnce({ + path: '/project/package.json', + packageJson: { + name: 'project', + version: '1.0.0', + dependencies: { + dep1: '1.0.0', + }, + }, + }); + + const result = yarnFindDirectDeps(cwd, ['nested-dep1', 'nested-dep2']); + + expect(result).toEqual([]); + }); + + it('should warn if found transitive dependencies', () => { + jest.spyOn(packageJsonUtils, 'readModulePackageJson').mockReturnValueOnce({ + path: '/project/package.json', + packageJson: { + name: 'project', + version: '1.0.0', + devDependencies: { + dep2: '2.0.0', + }, + }, + }); + + const warnSpy = jest + .spyOn(console, 'warn') + .mockImplementationOnce(() => {}); + yarnFindDirectDeps(cwd, ['nested-dep1', 'nested-dep2']); + + expect(warnSpy).toHaveBeenCalledWith( + 'INFO: detected yarn & affected transitive deps. unfortunately yarn list does not return direct dependencies from transitive dependencies. only top level dependencies are returned atm. PRs are welcome!' + ); + }); +}); + +describe('findDirectDeps', () => { + const cwd = '/project'; + + it('should call pnpmFindDirectDeps if packageManager is pnpm', () => { + const execSpy = jest + .spyOn(cp, 'execSync') + .mockImplementationOnce(() => '[{}]'); + + findDirectDeps('pnpm', cwd, ['dep1', 'dep2']); + + expect(execSpy).toHaveBeenLastCalledWith( + expect.stringContaining('pnpm ls'), + expect.objectContaining({ + cwd, + encoding: 'utf-8', + }) + ); + }); + + it('should call npmFindDirectDeps if packageManager is npm', () => { + const execSpy = jest + .spyOn(cp, 'execSync') + .mockImplementationOnce(() => '{}'); + + findDirectDeps('npm', cwd, ['dep1', 'dep2']); + + expect(execSpy).toHaveBeenLastCalledWith( + expect.stringContaining('npm list'), + expect.objectContaining({ + cwd, + encoding: 'utf-8', + }) + ); + }); + + it('should call yarnFindDirectDeps if packageManager is yarn', () => { + const readModulePackageJsonSpy = jest + .spyOn(packageJsonUtils, 'readModulePackageJson') + .mockReturnValueOnce({ + path: '/project/package.json', + packageJson: { + name: 'project', + version: '1.0.0', + dependencies: { + dep1: '1.0.0', + }, + }, + }); + + findDirectDeps('yarn', cwd, ['dep1', 'dep2']); + + expect(readModulePackageJsonSpy).toHaveBeenCalledWith(cwd); + }); +}); diff --git a/libs/core/src/git.spec.ts b/libs/core/src/git.spec.ts index 592805b..37176af 100644 --- a/libs/core/src/git.spec.ts +++ b/libs/core/src/git.spec.ts @@ -1,4 +1,9 @@ -import { getChangedFiles, getMergeBase, getDiff } from './git'; +import { + getChangedFiles, + getMergeBase, + getDiff, + getFileFromRevision, +} from './git'; import { resolve } from 'path'; import { readFile, writeFile } from 'fs/promises'; import * as childProcess from 'node:child_process'; @@ -184,4 +189,29 @@ describe('git', () => { expect(changedFiles).toEqual([]); }); }); + + describe('getFileFromRevision', () => { + it('should return the file content from the specified revision', () => { + const fileContent = getFileFromRevision({ + base: branch, + filePath: './index.ts', + cwd, + }); + + expect(fileContent).toEqual(expect.any(String)); + }); + + it('should throw an error if unable to get the file content', () => { + const filePath = './missing.ts'; + expect(() => + getFileFromRevision({ + base: branch, + filePath, + cwd, + }) + ).toThrow( + `Unable to get file "${filePath}" for base: "${branch}". are you using the correct base?` + ); + }); + }); }); diff --git a/libs/core/src/git.ts b/libs/core/src/git.ts index 4576d3d..4a1f958 100644 --- a/libs/core/src/git.ts +++ b/libs/core/src/git.ts @@ -61,7 +61,31 @@ export function getDiff({ base, cwd }: BaseGitActionArgs): string { } } -export interface GetChangedFiles { +interface FileFromRevisionArgs extends BaseGitActionArgs { + filePath: string; +} + +export function getFileFromRevision({ + base, + filePath, + cwd, +}: FileFromRevisionArgs): string { + try { + return execSync(`git show ${base}:${filePath}`, { + maxBuffer: TEN_MEGABYTES, + cwd, + stdio: 'pipe', + }) + .toString() + .trim(); + } catch (e) { + throw new Error( + `Unable to get file "${filePath}" for base: "${base}". are you using the correct base?` + ); + } +} + +export interface ChangedFiles { filePath: string; changedLines: number[]; } @@ -69,7 +93,7 @@ export interface GetChangedFiles { export function getChangedFiles({ base, cwd, -}: BaseGitActionArgs): GetChangedFiles[] { +}: BaseGitActionArgs): ChangedFiles[] { const mergeBase = getMergeBase({ base, cwd }); const diff = getDiff({ base: mergeBase, cwd }); diff --git a/libs/core/src/lock-files.spec.ts b/libs/core/src/lock-files.spec.ts new file mode 100644 index 0000000..2b20a9b --- /dev/null +++ b/libs/core/src/lock-files.spec.ts @@ -0,0 +1,287 @@ +import { readFileSync } from 'fs'; +import { getLockFileNodes } from 'nx/src/plugins/js/lock-file/lock-file.js'; +import { readModulePackageJson } from 'nx/src/utils/package-json.js'; +import { fastFindInFiles } from 'fast-find-in-files'; +import { + hasLockfileChanged, + findAffectedModules, + findAffectedFilesByLockfile, +} from './lock-files'; +import { getFileFromRevision } from './git'; +import { findDirectDeps } from './find-direct-deps'; + +jest.mock('nx/src/plugins/js/lock-file/lock-file.js', () => ({ + getLockFileName: jest.fn().mockReturnValue('yarn.lock'), + getLockFileNodes: jest.fn(), +})); +jest.mock('fs'); +jest.mock('nx/src/utils/package-json.js'); +jest.mock('fast-find-in-files'); +jest.mock('./git'); +jest.mock('./find-direct-deps'); + +describe('hasLockfileChanged', () => { + it('should return true if lockfile has changed', () => { + const hasChanged = hasLockfileChanged([ + { filePath: 'yarn.lock', changedLines: [] }, + ]); + + expect(hasChanged).toBe(true); + }); + + it('should return false if lockfile has not changed', () => { + const hasChanged = hasLockfileChanged([ + { filePath: 'index.ts', changedLines: [] }, + ]); + + expect(hasChanged).toBe(false); + }); +}); + +describe('findAffectedModules', () => { + beforeEach(() => { + (readFileSync as jest.Mock).mockReturnValueOnce('{}'); + (readModulePackageJson as jest.Mock).mockReturnValueOnce({ + packageJson: { + dependencies: { + dep: '1.0.0', + '@scope/dep': '1.0.0', + }, + }, + }); + }); + + it('should return empty array if lockfile has not changed', () => { + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockReturnValueOnce({}); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining([]) + ); + }); + + it('should return empty array if package.json has no dependencies', () => { + (readModulePackageJson as jest.Mock).mockReturnValueOnce({ + packageJson: {}, + }); + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockReturnValueOnce({}); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining([]) + ); + }); + + it('should still work when getFileFromRevision throws (no previous version of lock file)', () => { + (getFileFromRevision as jest.Mock).mockImplementation(() => { + throw new Error(); + }); + (getLockFileNodes as jest.Mock).mockReturnValueOnce({}); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining([]) + ); + }); + + it('should return changed modules if lockfile has not changed', () => { + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockImplementation((manager, file, key) => { + if (key === 'lock') { + return { + [`npm:dep`]: '1.0.0', + }; + } else { + return { + [`npm:dep`]: '2.0.0', + }; + } + }); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining(['dep']) + ); + }); + + it('should support scoped packages', () => { + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockImplementation((manager, file, key) => { + if (key === 'lock') { + return { + [`npm:@scope/dep`]: '1.0.0', + }; + } else { + return { + [`npm:@scope/dep`]: '2.0.0', + }; + } + }); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining(['@scope/dep']) + ); + }); + + it('should support scoped packages with different versions', () => { + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockImplementation((manager, file, key) => { + if (key === 'lock') { + return { + [`npm:@scope/dep@1.0.0`]: '1.0.0', + }; + } else { + return { + [`npm:@scope/dep@1.0.0`]: '2.0.0', + }; + } + }); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining(['@scope/dep']) + ); + }); + + it('should return module name even if it does not start with npm:', () => { + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockImplementation((manager, file, key) => { + if (key === 'lock') { + return { + [`dep`]: '1.0.0', + }; + } else { + return { + [`dep`]: '2.0.0', + }; + } + }); + + findAffectedModules('./', 'main'); + + expect(findDirectDeps).toHaveBeenCalledWith( + 'npm', + './', + expect.arrayContaining(['dep']) + ); + }); +}); + +describe('findAffectedFilesByLockfile', () => { + const cwd = '/project'; + const excludeFolderPaths = ['node_modules', 'dist', '.git']; + + beforeEach(() => { + (readFileSync as jest.Mock).mockReturnValueOnce('{}'); + (readModulePackageJson as jest.Mock).mockReturnValueOnce({ + packageJson: { + dependencies: { + '@scope/dep': '1.0.0', + }, + }, + }); + (getFileFromRevision as jest.Mock).mockReturnValueOnce('{}'); + (getLockFileNodes as jest.Mock).mockImplementation((manager, file, key) => { + if (key === 'lock') { + return { + [`npm:@scope/dep`]: '1.0.0', + }; + } else { + return { + [`npm:@scope/dep`]: '2.0.0', + }; + } + }); + (findDirectDeps as jest.Mock).mockReturnValue(['@scope/dep']); + }); + + it('should return relevant files', () => { + (fastFindInFiles as jest.Mock).mockReturnValue([ + { + filePath: '/project/src/file.ts', + queryHits: [{ lineNumber: 1, line: `import dep from '@scope/dep';` }], + }, + { + filePath: '/project/src/file.ts', + queryHits: [ + { lineNumber: 2, line: `const a = import(() => '@scope/dep');` }, + ], + }, + { + filePath: '/project/src/file.ts', + queryHits: [ + { lineNumber: 3, line: `const a = require('@scope/dep');` }, + ], + }, + ]); + + const result = findAffectedFilesByLockfile(cwd, 'main', excludeFolderPaths); + + expect(result).toEqual([ + { filePath: 'src/file.ts', changedLines: [1] }, + { filePath: 'src/file.ts', changedLines: [2] }, + { filePath: 'src/file.ts', changedLines: [3] }, + ]); + }); + + it('should return relevant files with multiple hits', () => { + (fastFindInFiles as jest.Mock).mockReturnValue([ + { + filePath: '/project/src/file.ts', + queryHits: [ + { lineNumber: 1, line: `import dep from '@scope/dep';` }, + { lineNumber: 2, line: `const a = import(() => '@scope/dep');` }, + { lineNumber: 3, line: `const a = require('@scope/dep');` }, + ], + }, + ]); + + const result = findAffectedFilesByLockfile(cwd, 'main', excludeFolderPaths); + + expect(result).toEqual([ + { filePath: 'src/file.ts', changedLines: [1, 2, 3] }, + ]); + }); + + it('should return empty array if no relevant files found', () => { + (fastFindInFiles as jest.Mock).mockReturnValue([]); + + const result = findAffectedFilesByLockfile(cwd, 'main', excludeFolderPaths); + + expect(result).toEqual([]); + }); + + it("should still work even if found file didn't have a match", () => { + (fastFindInFiles as jest.Mock).mockReturnValue([ + { + filePath: '/project/src/file.ts', + queryHits: [{ lineNumber: 1, line: `console.log('hi')` }], + }, + ]); + + const result = findAffectedFilesByLockfile(cwd, 'main', excludeFolderPaths); + + expect(result).toEqual([]); + }); +}); diff --git a/libs/core/src/lock-files.ts b/libs/core/src/lock-files.ts new file mode 100644 index 0000000..343143d --- /dev/null +++ b/libs/core/src/lock-files.ts @@ -0,0 +1,95 @@ +import { readFileSync } from 'fs'; +import diff from 'microdiff'; +import { FastFindInFiles, fastFindInFiles } from 'fast-find-in-files'; +import { join, relative } from 'path'; +import { + getLockFileName, + getLockFileNodes, +} from 'nx/src/plugins/js/lock-file/lock-file.js'; +import { detectPackageManager } from 'nx/src/utils/package-manager.js'; +import { ChangedFiles, getFileFromRevision } from './git'; +import { findDirectDeps } from './find-direct-deps'; + +const packageManager = detectPackageManager(); +export const lockFileName = getLockFileName(packageManager); + +export function findAffectedModules(cwd: string, base: string): string[] { + const lock = readFileSync(lockFileName, 'utf-8'); + let prevLock = '{}'; + + try { + prevLock = getFileFromRevision({ + base, + filePath: lockFileName, + cwd, + }); + } catch (e) { + // ignore + } + + const nodes = getLockFileNodes(packageManager, lock, 'lock'); + const prevNodes = getLockFileNodes(packageManager, prevLock, 'prevLock'); + const changes = diff(prevNodes, nodes); + + const captureModuleName = new RegExp(/npm:(@?[\w-/]+)/); + const changedModules = Array.from( + new Set( + changes.map( + ({ path }) => + captureModuleName.exec(path[0].toString())?.[1] ?? path[0].toString() + ) + ) + ); + + return findDirectDeps(packageManager, cwd, changedModules); +} + +export function hasLockfileChanged(changedFiles: ChangedFiles[]): boolean { + return changedFiles.some(({ filePath }) => filePath === lockFileName); +} + +export function findAffectedFilesByLockfile( + cwd: string, + base: string, + excludePaths: string[] +): ChangedFiles[] { + const dependencies = findAffectedModules(cwd, base); + const excludeFolderPaths = excludePaths.map((path) => join(cwd, path)); + + // fastFindInFiles supports regex but fails with `@` in the regex + const files = dependencies.flatMap((dep) => + fastFindInFiles({ + directory: cwd, + needle: dep, + excludeFolderPaths, + }) + ); + + const relevantFiles = filterRelevantFiles(cwd, files, dependencies.join('|')); + + return relevantFiles; +} + +function filterRelevantFiles( + cwd: string, + files: FastFindInFiles[], + libName: string +): ChangedFiles[] { + const regExp = new RegExp(`['"\`](?${libName})(?:/.*)?['"\`]`); + + return files + .map(({ filePath: foundFilePath, queryHits }) => ({ + filePath: relative(cwd, foundFilePath), + changedLines: queryHits + .filter(({ line }) => isRelevantLine(line, regExp)) + .map(({ lineNumber }) => lineNumber), + })) + .filter(({ changedLines }) => changedLines.length > 0); +} + +function isRelevantLine(line: string, regExp: RegExp): boolean { + const match = regExp.exec(line); + const { lib } = match?.groups ?? {}; + + return lib != null; +} diff --git a/libs/core/src/true-affected.spec.ts b/libs/core/src/true-affected.spec.ts index 8ec89a6..64cddca 100644 --- a/libs/core/src/true-affected.spec.ts +++ b/libs/core/src/true-affected.spec.ts @@ -1,5 +1,6 @@ import { trueAffected } from './true-affected'; import * as git from './git'; +import * as lockFiles from './lock-files'; describe('trueAffected', () => { const cwd = 'libs/core/src/__fixtures__/monorepo'; @@ -240,6 +241,35 @@ describe('trueAffected', () => { expect(affected).toEqual(['angular-component']); }); + it('should find fils that are related to changed modules from lockfile', async () => { + jest.spyOn(git, 'getChangedFiles').mockReturnValue([ + { + filePath: 'package-lock.json', + changedLines: [2], + }, + ]); + jest.spyOn(lockFiles, 'hasLockfileChanged').mockReturnValue(true); + jest.spyOn(lockFiles, 'findAffectedFilesByLockfile').mockReturnValue([ + { + filePath: 'proj1/index.ts', + changedLines: [2], + }, + ]); + + const affected = await trueAffected({ + cwd, + base: 'main', + projects: [ + { + name: 'proj1', + sourceRoot: 'proj1/', + }, + ], + }); + + expect(affected).toEqual(['proj1']); + }); + it("should ignore files when can't find the changed line", async () => { jest.spyOn(git, 'getChangedFiles').mockReturnValue([ { diff --git a/libs/core/src/true-affected.ts b/libs/core/src/true-affected.ts index 21367cb..08cc111 100644 --- a/libs/core/src/true-affected.ts +++ b/libs/core/src/true-affected.ts @@ -1,13 +1,15 @@ import { existsSync } from 'fs'; import { join, resolve } from 'path'; import { Project, Node, ts, SyntaxKind } from 'ts-morph'; -import { GetChangedFiles, getChangedFiles } from './git'; -import { - findNonSourceAffectedFiles, - findRootNode, - getPackageNameByPath, -} from './utils'; +import { ChangedFiles, getChangedFiles } from './git'; +import { findRootNode, getPackageNameByPath } from './utils'; import { TrueAffected, TrueAffectedProject } from './types'; +import { findNonSourceAffectedFiles } from './assets'; +import { + findAffectedFilesByLockfile, + hasLockfileChanged, + lockFileName, +} from './lock-files'; const ignoredRootNodeTypes = [ SyntaxKind.ImportDeclaration, @@ -67,25 +69,38 @@ export const trueAffected = async ({ cwd, }); - const sourceChangedFiles: GetChangedFiles[] = changedFiles.filter( + const sourceChangedFiles = changedFiles.filter( ({ filePath }) => project.getSourceFile(resolve(cwd, filePath)) != null ); const ignoredPaths = ['./node_modules', './dist', './.git']; - const nonSourceChangedFiles: GetChangedFiles[] = changedFiles + const nonSourceChangedFiles = changedFiles .filter( ({ filePath }) => !filePath.match(/.*\.(ts|js)x?$/g) && + !filePath.endsWith(lockFileName) && project.getSourceFile(resolve(cwd, filePath)) == null ) .flatMap(({ filePath: changedFilePath }) => findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths) ); + let changedFilesByLockfile: ChangedFiles[] = []; + if (hasLockfileChanged(changedFiles)) { + changedFilesByLockfile = findAffectedFilesByLockfile( + cwd, + base, + ignoredPaths + ).filter( + ({ filePath }) => project.getSourceFile(resolve(cwd, filePath)) != null + ); + } + const filteredChangedFiles = [ ...sourceChangedFiles, ...nonSourceChangedFiles, + ...changedFilesByLockfile, ]; const changedIncludedFilesPackages = changedFiles @@ -140,6 +155,7 @@ export const trueAffected = async ({ filteredChangedFiles.forEach(({ filePath, changedLines }) => { const sourceFile = project.getSourceFile(resolve(cwd, filePath)); + /* istanbul ignore next */ if (sourceFile == null) return; changedLines.forEach((line) => { diff --git a/libs/core/src/utils.spec.ts b/libs/core/src/utils.spec.ts index 53af4db..0c910bc 100644 --- a/libs/core/src/utils.spec.ts +++ b/libs/core/src/utils.spec.ts @@ -1,104 +1,6 @@ -import { - findNonSourceAffectedFiles, - findRootNode, - getPackageNameByPath, -} from './utils'; -import * as path from 'path'; -import { fastFindInFiles } from 'fast-find-in-files'; -import { existsSync } from 'fs'; +import { findRootNode, getPackageNameByPath } from './utils'; import { Project, SyntaxKind } from 'ts-morph'; -jest.mock('fast-find-in-files'); -jest.mock('fs'); - -describe('findNonSourceAffectedFiles', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - - it('should return relevant files', () => { - const cwd = '/project'; - const changedFilePath = '/project/src/file.ts'; - const excludeFolderPaths = ['node_modules', 'dist', '.git']; - - (fastFindInFiles as jest.Mock).mockReturnValue([ - { - filePath: '/project/src/file.ts', - queryHits: [{ lineNumber: 1, line: `"file.ts"` }], - }, - ]); - (existsSync as jest.Mock).mockReturnValue(true); - - const result = findNonSourceAffectedFiles( - cwd, - changedFilePath, - excludeFolderPaths - ); - - expect(result).toEqual([{ filePath: 'src/file.ts', changedLines: [1] }]); - expect(fastFindInFiles).toHaveBeenCalledWith({ - directory: cwd, - needle: path.basename(changedFilePath), - excludeFolderPaths: excludeFolderPaths.map((folder) => - path.join(cwd, folder) - ), - }); - }); - - it('should return empty array if no relevant files found', () => { - const cwd = '/project'; - const changedFilePath = '/project/src/file.ts'; - const excludeFolderPaths = ['node_modules', 'dist', '.git']; - - (fastFindInFiles as jest.Mock).mockReturnValue([]); - (existsSync as jest.Mock).mockReturnValue(true); - - const result = findNonSourceAffectedFiles( - cwd, - changedFilePath, - excludeFolderPaths - ); - - expect(result).toEqual([]); - expect(fastFindInFiles).toHaveBeenCalledWith({ - directory: cwd, - needle: path.basename(changedFilePath), - excludeFolderPaths: excludeFolderPaths.map((folder) => - path.join(cwd, folder) - ), - }); - }); - - it("should still work even if found file didn't have a match", () => { - const cwd = '/project'; - const changedFilePath = '/project/src/file.ts'; - const excludeFolderPaths = ['node_modules', 'dist', '.git']; - - (fastFindInFiles as jest.Mock).mockReturnValue([ - { - filePath: '/project/src/file.ts', - queryHits: [{ lineNumber: 1, line: `console.log('hi')` }], - }, - ]); - (existsSync as jest.Mock).mockReturnValue(true); - - const result = findNonSourceAffectedFiles( - cwd, - changedFilePath, - excludeFolderPaths - ); - - expect(result).toEqual([]); - expect(fastFindInFiles).toHaveBeenCalledWith({ - directory: cwd, - needle: path.basename(changedFilePath), - excludeFolderPaths: excludeFolderPaths.map((folder) => - path.join(cwd, folder) - ), - }); - }); -}); - describe('findRootNode', () => { it('should find the root node', () => { const project = new Project({ useInMemoryFileSystem: true }); diff --git a/libs/core/src/utils.ts b/libs/core/src/utils.ts index 04c3ea3..3a201c7 100644 --- a/libs/core/src/utils.ts +++ b/libs/core/src/utils.ts @@ -1,7 +1,3 @@ -import { basename, dirname, join, relative, resolve } from 'path'; -import { GetChangedFiles } from './git'; -import { FastFindInFiles, fastFindInFiles } from 'fast-find-in-files'; -import { existsSync } from 'fs'; import { ts, SyntaxKind, Node } from 'ts-morph'; import { TrueAffectedProject } from './types'; @@ -20,64 +16,3 @@ export const getPackageNameByPath = ( ): string | undefined => { return projects.find(({ sourceRoot }) => path.includes(sourceRoot))?.name; }; - -export function findNonSourceAffectedFiles( - cwd: string, - changedFilePath: string, - excludeFolderPaths: string[] -): GetChangedFiles[] { - const fileName = basename(changedFilePath); - - const files = fastFindInFiles({ - directory: cwd, - needle: fileName, - excludeFolderPaths: excludeFolderPaths.map((path) => join(cwd, path)), - }); - - const relevantFiles = filterRelevantFiles(cwd, files, changedFilePath); - - return relevantFiles; -} - -function filterRelevantFiles( - cwd: string, - files: FastFindInFiles[], - changedFilePath: string -): GetChangedFiles[] { - const fileName = basename(changedFilePath); - const regExp = new RegExp(`['"\`](?.*${fileName})['"\`]`); - - return files - .map(({ filePath: foundFilePath, queryHits }) => ({ - filePath: relative(cwd, foundFilePath), - changedLines: queryHits - .filter(({ line }) => - isRelevantLine(line, regExp, cwd, foundFilePath, changedFilePath) - ) - .map(({ lineNumber }) => lineNumber), - })) - .filter(({ changedLines }) => changedLines.length > 0); -} - -function isRelevantLine( - line: string, - regExp: RegExp, - cwd: string, - foundFilePath: string, - changedFilePath: string -): boolean { - const match = regExp.exec(line); - const { relFilePath } = match?.groups ?? {}; - - if (relFilePath == null) return false; - - const foundFileDir = resolve(dirname(foundFilePath)); - const changedFileDir = resolve(cwd, dirname(changedFilePath)); - - const relatedFilePath = resolve( - cwd, - relative(cwd, join(dirname(foundFilePath), relFilePath)) - ); - - return foundFileDir === changedFileDir && existsSync(relatedFilePath); -} diff --git a/package-lock.json b/package-lock.json index 32fb5fa..89d116b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "chalk": "^5.2.0", "fast-find-in-files": "^1.0.1", "globby": "^13.1.4", + "microdiff": "^1.3.2", "ts-morph": "^18.0.0", "tslib": "^2.3.0", "yaml": "^2.3.1", @@ -10147,6 +10148,11 @@ "node": ">= 8" } }, + "node_modules/microdiff": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/microdiff/-/microdiff-1.3.2.tgz", + "integrity": "sha512-pKy60S2febliZIbwdfEQKTtL5bLNxOyiRRmD400gueYl9XcHyNGxzHSlJWn9IMHwYXT0yohPYL08+bGozVk8cQ==" + }, "node_modules/micromatch": { "version": "4.0.5", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", diff --git a/package.json b/package.json index a0c3897..c79cbff 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "chalk": "^5.2.0", "fast-find-in-files": "^1.0.1", "globby": "^13.1.4", + "microdiff": "^1.3.2", "ts-morph": "^18.0.0", "tslib": "^2.3.0", "yaml": "^2.3.1",