From a813e6c063a76f60490b1f676f5860e635c4e2ab Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Tue, 11 Jun 2024 11:22:55 +0300 Subject: [PATCH] feat: DEBUG flag and project.json validation (#31) * feat: support verbose logging * feat(nx): project.json runtime validation & verbose * use DEBUG env var instead of verbose mode --- libs/core/src/true-affected.spec.ts | 74 ++++++++++++ libs/core/src/true-affected.ts | 129 +++++++++++++++++---- libs/core/src/types.ts | 6 +- libs/nx/src/cli.spec.ts | 18 ++- libs/nx/src/cli.ts | 31 ++++- libs/nx/src/nx.spec.ts | 171 ++++++++++++++++++++++++++++ libs/nx/src/nx.ts | 80 ++++++++++--- 7 files changed, 470 insertions(+), 39 deletions(-) diff --git a/libs/core/src/true-affected.spec.ts b/libs/core/src/true-affected.spec.ts index 73ed654..a3110bb 100644 --- a/libs/core/src/true-affected.spec.ts +++ b/libs/core/src/true-affected.spec.ts @@ -2,6 +2,12 @@ import { trueAffected } from './true-affected'; import * as git from './git'; import * as lockFiles from './lock-files'; +jest.mock('chalk', () => ({ + default: { + bold: (str: string) => str, + }, +})); + describe('trueAffected', () => { const cwd = 'libs/core/src/__fixtures__/monorepo'; @@ -406,4 +412,72 @@ describe('trueAffected', () => { expect(affected).toEqual(expected); }); + + it('should log the progress', async () => { + const changedFiles = [ + { + filePath: 'proj1/index.ts', + changedLines: [2], + }, + ]; + jest.spyOn(git, 'getChangedFiles').mockReturnValue(changedFiles); + + const debug = jest.fn(); + await trueAffected({ + cwd, + base: 'main', + rootTsConfig: 'tsconfig.json', + projects: [ + { + name: 'proj1', + sourceRoot: 'proj1/', + tsConfig: 'proj1/tsconfig.json', + }, + { + name: 'proj2', + sourceRoot: 'proj2/', + tsConfig: 'proj2/tsconfig.json', + }, + { + name: 'proj3', + sourceRoot: 'proj3/', + tsConfig: 'proj3/tsconfig.json', + implicitDependencies: ['proj1'], + }, + ], + logger: { + debug, + } as unknown as Console, + }); + + expect(debug).toHaveBeenCalledWith('Getting affected projects'); + expect(debug).toHaveBeenCalledWith( + expect.stringContaining('Creating project with root tsconfig from') + ); + expect(debug).toHaveBeenCalledWith( + expect.stringContaining('Adding source files for project proj1') + ); + expect(debug).toHaveBeenCalledWith( + expect.stringContaining('Adding source files for project proj2') + ); + expect(debug).toHaveBeenCalledWith( + expect.stringContaining( + 'Could not find a tsconfig for project proj3, adding source files paths' + ) + ); + expect(debug).toHaveBeenCalledWith( + `Found ${changedFiles.length} changed files` + ); + expect(debug).toHaveBeenCalledWith( + `Added package proj1 to affected packages for changed line ${changedFiles[0].changedLines[0]} in ${changedFiles[0].filePath}` + ); + expect(debug).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`^Found identifier .* in .*${changedFiles[0].filePath}$`) + ) + ); + expect(debug).toHaveBeenCalledWith( + 'Added package proj2 to affected packages' + ); + }); }); diff --git a/libs/core/src/true-affected.ts b/libs/core/src/true-affected.ts index 0ca9d10..c17c907 100644 --- a/libs/core/src/true-affected.ts +++ b/libs/core/src/true-affected.ts @@ -1,6 +1,7 @@ import { existsSync } from 'fs'; import { join, resolve } from 'path'; import { Project, Node, ts, SyntaxKind } from 'ts-morph'; +import chalk from 'chalk'; import { ChangedFiles, getChangedFiles } from './git'; import { findRootNode, getPackageNameByPath } from './utils'; import { TrueAffected, TrueAffectedProject } from './types'; @@ -21,14 +22,29 @@ const ignoredRootNodeTypes = [ export const DEFAULT_INCLUDE_TEST_FILES = /\.(spec|test)\.(ts|js)x?/; +const DEFAULT_LOGGER = { + ...console, + debug: process.env['DEBUG'] === 'true' ? console.debug : () => {}, +}; + export const trueAffected = async ({ cwd, rootTsConfig, base = 'origin/main', projects, include = [DEFAULT_INCLUDE_TEST_FILES], + logger = DEFAULT_LOGGER, __experimentalLockfileCheck = false, }: TrueAffected) => { + logger.debug('Getting affected projects'); + if (rootTsConfig != null) { + logger.debug( + `Creating project with root tsconfig from ${chalk.bold( + resolve(cwd, rootTsConfig) + )}` + ); + } + const project = new Project({ compilerOptions: { allowJs: true, @@ -41,23 +57,27 @@ export const trueAffected = async ({ }), }); - const implicitDeps = ( - projects.filter( - ({ implicitDependencies = [] }) => implicitDependencies.length > 0 - ) as Required[] - ).reduce( - (acc, { name, implicitDependencies }) => - acc.set(name, implicitDependencies), - new Map() - ); - projects.forEach( - ({ sourceRoot, tsConfig = join(sourceRoot, 'tsconfig.json') }) => { + ({ name, sourceRoot, tsConfig = join(sourceRoot, 'tsconfig.json') }) => { const tsConfigPath = resolve(cwd, tsConfig); if (existsSync(tsConfigPath)) { + logger.debug( + `Adding source files for project ${chalk.bold( + name + )} from tsconfig at ${chalk.bold(tsConfigPath)}` + ); + project.addSourceFilesFromTsConfig(tsConfigPath); } else { + logger.debug( + `Could not find a tsconfig for project ${chalk.bold( + name + )}, adding source files paths in ${chalk.bold( + resolve(cwd, sourceRoot) + )}` + ); + project.addSourceFilesAtPaths( join(resolve(cwd, sourceRoot), '**/*.{ts,js}') ); @@ -70,11 +90,13 @@ export const trueAffected = async ({ cwd, }); + logger.debug(`Found ${chalk.bold(changedFiles.length)} changed files`); + const sourceChangedFiles = changedFiles.filter( ({ filePath }) => project.getSourceFile(resolve(cwd, filePath)) != null ); - const ignoredPaths = ['./node_modules', './dist', './.git']; + const ignoredPaths = ['./node_modules', './build', './dist', './.git']; const nonSourceChangedFiles = changedFiles .filter( @@ -83,12 +105,26 @@ export const trueAffected = async ({ !filePath.endsWith(lockFileName) && project.getSourceFile(resolve(cwd, filePath)) == null ) - .flatMap(({ filePath: changedFilePath }) => - findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths) + .flatMap(({ filePath: changedFilePath }) => { + logger.debug( + `Finding non-source affected files for ${chalk.bold(changedFilePath)}` + ); + + return findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths); + }); + + if (nonSourceChangedFiles.length > 0) { + logger.debug( + `Found ${chalk.bold( + nonSourceChangedFiles.length + )} non-source affected files` ); + } let changedFilesByLockfile: ChangedFiles[] = []; if (__experimentalLockfileCheck && hasLockfileChanged(changedFiles)) { + logger.debug('Lockfile has changed, finding affected files'); + changedFilesByLockfile = findAffectedFilesByLockfile( cwd, base, @@ -115,6 +151,14 @@ export const trueAffected = async ({ .map(({ filePath }) => getPackageNameByPath(filePath, projects)) .filter((v): v is string => v != null); + if (changedIncludedFilesPackages.length > 0) { + logger.debug( + `Found ${chalk.bold( + changedIncludedFilesPackages.length + )} affected packages from included files` + ); + } + const affectedPackages = new Set(changedIncludedFilesPackages); const visitedIdentifiers = new Map(); @@ -137,17 +181,36 @@ export const trueAffected = async ({ const identifierName = identifier.getText(); const path = rootNode.getSourceFile().getFilePath(); + logger.debug( + `Found identifier ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); + if (identifierName && path) { - const visited = visitedIdentifiers.get(identifierName) ?? []; - if (visited.includes(path)) return; - visitedIdentifiers.set(identifierName, [...visited, path]); + const visited = visitedIdentifiers.get(path) ?? []; + if (visited.includes(identifierName)) { + logger.debug( + `Already visited ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); + + return; + } + + visitedIdentifiers.set(path, [...visited, identifierName]); + + logger.debug( + `Visiting ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); } refs.forEach((node) => { const sourceFile = node.getSourceFile(); const pkg = getPackageNameByPath(sourceFile.getFilePath(), projects); - if (pkg) affectedPackages.add(pkg); + if (pkg) { + affectedPackages.add(pkg); + + logger.debug(`Added package ${chalk.bold(pkg)} to affected packages`); + } findReferencesLibs(node); }); @@ -170,7 +233,17 @@ export const trueAffected = async ({ const pkg = getPackageNameByPath(sourceFile.getFilePath(), projects); - if (pkg) affectedPackages.add(pkg); + if (pkg) { + affectedPackages.add(pkg); + + logger.debug( + `Added package ${chalk.bold( + pkg + )} to affected packages for changed line ${chalk.bold( + line + )} in ${chalk.bold(filePath)}` + ); + } findReferencesLibs(changedNode); } catch { @@ -179,12 +252,30 @@ export const trueAffected = async ({ }); }); + const implicitDeps = ( + projects.filter( + ({ implicitDependencies = [] }) => implicitDependencies.length > 0 + ) as Required[] + ).reduce( + (acc, { name, implicitDependencies }) => + acc.set(name, implicitDependencies), + new Map() + ); + // add implicit deps affectedPackages.forEach((pkg) => { const deps = Array.from(implicitDeps.entries()) .filter(([, deps]) => deps.includes(pkg)) .map(([name]) => name); + if (deps.length > 0) { + logger.debug( + `Adding implicit dependencies ${chalk.bold( + deps.join(', ') + )} to ${chalk.bold(pkg)}` + ); + } + deps.forEach((dep) => affectedPackages.add(dep)); }); diff --git a/libs/core/src/types.ts b/libs/core/src/types.ts index dee5f83..06787f0 100644 --- a/libs/core/src/types.ts +++ b/libs/core/src/types.ts @@ -6,7 +6,11 @@ export interface TrueAffectedProject { targets?: string[]; } -export interface TrueAffected { +export interface TrueAffectedLogging { + logger?: Console; +} + +export interface TrueAffected extends TrueAffectedLogging { cwd: string; rootTsConfig?: string; base?: string; diff --git a/libs/nx/src/cli.spec.ts b/libs/nx/src/cli.spec.ts index 0c1fe85..7c7f1bc 100644 --- a/libs/nx/src/cli.spec.ts +++ b/libs/nx/src/cli.spec.ts @@ -1,14 +1,18 @@ import { resolve } from 'path'; +import type { TrueAffectedProject } from '@traf/core'; + import * as cli from './cli'; import * as nx from './nx'; import { workspaceCwd } from './mocks'; -import { TrueAffectedProject } from '@traf/core'; jest.mock('chalk', () => ({ - hex: jest.fn().mockReturnValue(jest.fn()), + hex: () => jest.fn(), bgHex: jest.fn().mockReturnValue({ bold: jest.fn(), }), + bgGray: { + bold: jest.fn(), + }, chalk: jest.fn(), })); @@ -88,7 +92,7 @@ describe('cli', () => { ]); expect(affectedActionSpy).toBeCalledWith({ action: 'build', - all: 'true', + all: true, base: 'master', cwd: resolve(process.cwd(), workspaceCwd), includeFiles: ['package.json', 'jest.setup.js'], @@ -110,7 +114,8 @@ describe('cli', () => { beforeEach(() => { getNxTrueAffectedProjectsSpy = jest .spyOn(nx, 'getNxTrueAffectedProjects') - .mockImplementation(); + .mockImplementation() + .mockResolvedValue([]); logSpy = jest.spyOn(cli, 'log').mockImplementation(); }); @@ -132,13 +137,16 @@ describe('cli', () => { target: [], }); - expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd()); + expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd(), { + logger: expect.any(Object), + }); expect(trafSpy).toHaveBeenCalledWith({ cwd: process.cwd(), rootTsConfig: 'tsconfig.base.json', base: 'origin/main', projects: [], include: [], + logger: expect.any(Object), }); }); diff --git a/libs/nx/src/cli.ts b/libs/nx/src/cli.ts index ec48c12..ed00a26 100644 --- a/libs/nx/src/cli.ts +++ b/libs/nx/src/cli.ts @@ -14,6 +14,23 @@ export const log = (message: string) => ` ${chalk.hex(color)('>')} ${chalk.bgHex(color).bold(' TRAF ')} ${message}` ); +const getLogger = (namespace: string) => ({ + ...console, + debug: (message: string) => + process.env['DEBUG'] === 'true' && + console.debug( + ` ${chalk.hex(color)('>')} ${chalk.bgGray.bold( + ` ${namespace} ` + )} ${message}` + ), + warn: (message: string) => + console.warn( + ` ${chalk.yellowBright('⚠️')} ${chalk.bgGray.bold( + ` ${namespace} ` + )} ${chalk.yellow(message)}` + ), +}); + export const affectedAction = async ({ cwd, action = 'log', @@ -26,7 +43,15 @@ export const affectedAction = async ({ target, experimentalLockfileCheck, }: AffectedOptions) => { - let projects = await getNxTrueAffectedProjects(cwd); + const nxLogger = getLogger('NX'); + + nxLogger.debug('Getting nx projects'); + + let projects = await getNxTrueAffectedProjects(cwd, { + logger: nxLogger, + }); + + nxLogger.debug(`Found ${projects.length} projects`); if (target.length) { projects = projects.filter( @@ -45,6 +70,7 @@ export const affectedAction = async ({ base, projects, include: [...includeFiles, DEFAULT_INCLUDE_TEST_FILES], + logger: getLogger('CORE'), __experimentalLockfileCheck: experimentalLockfileCheck, }); @@ -116,6 +142,7 @@ const affectedCommand: CommandModule = { all: { desc: 'Outputs all available projects regardless of changes', default: false, + coerce: Boolean, }, base: { desc: 'Base branch to compare against', @@ -124,6 +151,7 @@ const affectedCommand: CommandModule = { json: { desc: 'Output affected projects as JSON', default: false, + coerce: Boolean, }, includeFiles: { desc: 'Comma separated list of files to include', @@ -145,6 +173,7 @@ const affectedCommand: CommandModule = { desc: 'Experimental lockfile check', type: 'boolean', default: false, + coerce: Boolean, }, }, handler: async ({ diff --git a/libs/nx/src/nx.spec.ts b/libs/nx/src/nx.spec.ts index 554694b..4d87973 100644 --- a/libs/nx/src/nx.spec.ts +++ b/libs/nx/src/nx.spec.ts @@ -8,6 +8,10 @@ import { workspaceWithPathsCwd, } from './mocks'; +jest.mock('chalk', () => ({ + bold: jest.fn().mockImplementation((text) => text), +})); + jest.mock('globby', () => ({ globby: jest.fn(), })); @@ -481,5 +485,172 @@ describe('nx', () => { ); }); }); + + describe('invalid project config', () => { + it('should warn about invalid project config and fallback to use project path', async () => { + const logger = { + warn: jest.fn(), + debug: jest.fn(), + } as unknown as Console; + + jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { + const path = pathLike.toString(); + + if (path.endsWith('proj1/project.json')) { + return Promise.resolve(JSON.stringify({})); + } + + return Promise.reject('File not found'); + }); + + const cwd = 'libs/nx/src/__fixtures__/nx-project'; + const projects = await getNxTrueAffectedProjects(cwd, { logger }); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp( + `Project at .*/proj1/project.json does not have a name property. Using project.json directory name proj1.` + ) + ) + ); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp( + `Project at .*/proj1/project.json does not have a sourceRoot property. Using project.json directory.` + ) + ) + ); + + expect(projects).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'proj1', + sourceRoot: expect.stringContaining('proj1'), + }), + ]) + ); + }); + }); + + describe('debug', () => { + it('should log found projects', async () => { + const logger = { + log: jest.fn(), + debug: jest.fn(), + } as unknown as Console; + + jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { + const path = pathLike.toString(); + + if (path.endsWith('proj1/project.json')) { + return Promise.resolve( + JSON.stringify({ + name: 'proj1', + sourceRoot: 'proj1/src', + }) + ); + } + + return Promise.reject('File not found'); + }); + + const cwd = 'libs/nx/src/__fixtures__/nx-project'; + await getNxTrueAffectedProjects(cwd, { logger }); + + expect(logger.debug).toHaveBeenCalledWith('Found 1 nx projects'); + }); + + it('should notify about missing tsconfig and fallback to sourceRoot', async () => { + const logger = { + warn: jest.fn(), + log: jest.fn(), + debug: jest.fn(), + } as unknown as Console; + + jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { + const path = pathLike.toString(); + + if (path.endsWith('proj1/project.json')) { + return Promise.resolve(JSON.stringify({ sourceRoot: 'proj1/src' })); + } + + return Promise.reject('File not found'); + }); + + const cwd = 'libs/nx/src/__fixtures__/nx-project'; + await getNxTrueAffectedProjects(cwd, { logger }); + + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp( + "Project at .*/proj1/project.json does not have a tsConfig property under 'targets.build.options.tsConfig'. Trying to use 'sourceRoot'" + ) + ) + ); + }); + + it('should notify about missing tsconfig and missing sourceRoot', async () => { + const logger = { + warn: jest.fn(), + log: jest.fn(), + debug: jest.fn(), + } as unknown as Console; + + jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { + const path = pathLike.toString(); + + if (path.endsWith('proj1/project.json')) { + return Promise.resolve(JSON.stringify({})); + } + + return Promise.reject('File not found'); + }); + + const cwd = 'libs/nx/src/__fixtures__/nx-project'; + await getNxTrueAffectedProjects(cwd, { logger }); + + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp( + "Project at .*/proj1/project.json does not have a tsConfig property under 'targets.build.options.tsConfig'. Using project.json directory." + ) + ) + ); + }); + + it('should notify which tsconfig is going to be used', async () => { + const logger = { + log: jest.fn(), + debug: jest.fn(), + } as unknown as Console; + + jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { + const path = pathLike.toString(); + + if (path.endsWith('proj1/project.json')) { + return Promise.resolve( + JSON.stringify({ + name: 'proj1', + sourceRoot: 'proj1/src', + }) + ); + } + + return Promise.reject('File not found'); + }); + + const cwd = 'libs/nx/src/__fixtures__/nx-project'; + await getNxTrueAffectedProjects(cwd, { logger }); + + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp( + `Using tsconfig at proj1/tsconfig.app.json for project proj1` + ) + ) + ); + }); + }); }); }); diff --git a/libs/nx/src/nx.ts b/libs/nx/src/nx.ts index ff2b6ae..8df38fa 100644 --- a/libs/nx/src/nx.ts +++ b/libs/nx/src/nx.ts @@ -1,12 +1,13 @@ -import { join, resolve } from 'path'; +import { basename, dirname, join, resolve } from 'path'; import { readFile } from 'fs/promises'; import { globby } from 'globby'; -import { TrueAffectedProject } from '@traf/core'; +import { TrueAffectedLogging, TrueAffectedProject } from '@traf/core'; import { existsSync } from 'fs'; +import chalk from 'chalk'; interface NxProject { - name: string; - sourceRoot: string; + name?: string; + sourceRoot?: string; projectType: 'application' | 'library'; implicitDependencies?: string[]; targets?: { @@ -23,7 +24,8 @@ interface WorkspaceJsonConfiguration { } interface WorkspaceProject { - name: string; + name?: string; + path: string; project: NxProject; } @@ -51,6 +53,7 @@ async function getNxWorkspaceProjects( .filter(([, project]) => typeof project === 'object') .map(([name, project]) => ({ name, + path, project: project as NxProject, })); } catch (e) { @@ -82,11 +85,11 @@ async function getNxProjectJsonProjects( const projectFiles = []; for (const file of files) { - const project = JSON.parse( - await readFile(resolve(cwd, file), 'utf-8') - ) as NxProject; + const path = resolve(cwd, file); + const project = JSON.parse(await readFile(path, 'utf-8')) as NxProject; projectFiles.push({ name: project.name, + path, project, }); } @@ -97,16 +100,61 @@ async function getNxProjectJsonProjects( } } +type GetNxTrueAffectedProjectsOptions = TrueAffectedLogging; + export async function getNxTrueAffectedProjects( - cwd: string + cwd: string, + { logger = console }: GetNxTrueAffectedProjectsOptions = {} ): Promise { const projects = await getNxProjects(cwd); - return projects.map(({ name, project }) => { + logger.debug(`Found ${chalk.bold(projects.length)} nx projects`); + + return projects.map(({ name, path, project }) => { let tsConfig = project.targets?.build?.options?.tsConfig; + const projectPathDir = dirname(path); + const projectName = name ?? basename(projectPathDir); + + if (!name) { + logger.warn( + `Project at ${chalk.bold( + path + )} does not have a name property. Using project.json directory name ${chalk.bold( + projectName + )}.` + ); + } + + if (!project.sourceRoot) { + logger.warn( + `Project at ${chalk.bold( + path + )} does not have a sourceRoot property. Using project.json directory.` + ); + } if (!tsConfig) { - const projectRoot = join(project.sourceRoot, '..'); + if (project.sourceRoot) { + logger.debug( + `Project at ${chalk.bold( + path + )} does not have a tsConfig property under '${chalk.bold( + 'targets.build.options.tsConfig' + )}'. Trying to use '${chalk.bold('sourceRoot')}' ` + ); + } else { + logger.debug( + `Project at ${chalk.bold( + path + )} does not have a tsConfig property under '${chalk.bold( + 'targets.build.options.tsConfig' + )}'. Using project.json directory.` + ); + } + + const projectRoot = project.sourceRoot + ? join(project.sourceRoot, '..') + : projectPathDir; if (project.projectType === 'library') { tsConfig = join(projectRoot, 'tsconfig.lib.json'); @@ -119,9 +167,15 @@ export async function getNxTrueAffectedProjects( } } + logger.debug( + `Using tsconfig at ${chalk.bold(tsConfig)} for project ${chalk.bold( + projectName + )}` + ); + return { - name, - sourceRoot: project.sourceRoot, + name: projectName, + sourceRoot: project.sourceRoot ?? projectPathDir, implicitDependencies: project.implicitDependencies ?? [], tsConfig, targets: Object.keys(project.targets ?? {}),