From e214255904b1af6841c04e89cdfc3d2c838c1068 Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Mon, 13 May 2024 15:09:15 +0300 Subject: [PATCH 1/3] feat: support verbose logging --- libs/core/src/true-affected.spec.ts | 78 +++++++++++++++ libs/core/src/true-affected.ts | 145 ++++++++++++++++++++++++---- libs/core/src/types.ts | 7 +- libs/nx/src/cli.spec.ts | 15 ++- libs/nx/src/cli.ts | 32 ++++++ 5 files changed, 254 insertions(+), 23 deletions(-) diff --git a/libs/core/src/true-affected.spec.ts b/libs/core/src/true-affected.spec.ts index 73ed654..4a73b3b 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'; @@ -213,6 +219,7 @@ describe('trueAffected', () => { cwd, base: 'main', rootTsConfig: 'tsconfig.json', + verbose: true, projects: [ { name: 'angular-component', @@ -261,6 +268,7 @@ describe('trueAffected', () => { cwd, base: 'main', __experimentalLockfileCheck: true, + verbose: true, projects: [ { name: 'proj1', @@ -402,8 +410,78 @@ describe('trueAffected', () => { }, ], include: filePatterns, + verbose: true, }); 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 log = 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: { + log, + } as unknown as Console, + verbose: true, + }); + + expect(log).toHaveBeenCalledWith('Getting affected projects'); + expect(log).toHaveBeenCalledWith( + expect.stringContaining('Creating project with root tsconfig from') + ); + expect(log).toHaveBeenCalledWith( + expect.stringContaining('Adding source files for project proj1') + ); + expect(log).toHaveBeenCalledWith( + expect.stringContaining('Adding source files for project proj2') + ); + expect(log).toHaveBeenCalledWith( + expect.stringContaining( + 'Could not find a tsconfig for project proj3, adding source files paths' + ) + ); + expect(log).toHaveBeenCalledWith( + `Found ${changedFiles.length} changed files` + ); + expect(log).toHaveBeenCalledWith( + `Added package proj1 to affected packages for changed line ${changedFiles[0].changedLines[0]} in ${changedFiles[0].filePath}` + ); + expect(log).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`^Found identifier .* in .*${changedFiles[0].filePath}$`) + ) + ); + expect(log).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..fc424f8 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'; @@ -27,8 +28,21 @@ export const trueAffected = async ({ base = 'origin/main', projects, include = [DEFAULT_INCLUDE_TEST_FILES], + verbose = false, + logger = console, __experimentalLockfileCheck = false, }: TrueAffected) => { + if (verbose) { + logger.log('Getting affected projects'); + if (rootTsConfig != null) { + logger.log( + `Creating project with root tsconfig from ${chalk.bold( + resolve(cwd, rootTsConfig) + )}` + ); + } + } + const project = new Project({ compilerOptions: { allowJs: true, @@ -41,23 +55,29 @@ 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)) { + if (verbose) { + logger.log( + `Adding source files for project ${chalk.bold( + name + )} from tsconfig at ${chalk.bold(tsConfigPath)}` + ); + } project.addSourceFilesFromTsConfig(tsConfigPath); } else { + if (verbose) { + logger.log( + `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,15 @@ export const trueAffected = async ({ cwd, }); + if (verbose) { + logger.log(`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 +107,30 @@ export const trueAffected = async ({ !filePath.endsWith(lockFileName) && project.getSourceFile(resolve(cwd, filePath)) == null ) - .flatMap(({ filePath: changedFilePath }) => - findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths) + .flatMap(({ filePath: changedFilePath }) => { + if (verbose) { + logger.log( + `Finding non-source affected files for ${chalk.bold(changedFilePath)}` + ); + } + + return findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths); + }); + + if (verbose && nonSourceChangedFiles.length > 0) { + logger.log( + `Found ${chalk.bold( + nonSourceChangedFiles.length + )} non-source affected files` ); + } let changedFilesByLockfile: ChangedFiles[] = []; if (__experimentalLockfileCheck && hasLockfileChanged(changedFiles)) { + if (verbose) { + logger.log('Lockfile has changed, finding affected files'); + } + changedFilesByLockfile = findAffectedFilesByLockfile( cwd, base, @@ -115,6 +157,14 @@ export const trueAffected = async ({ .map(({ filePath }) => getPackageNameByPath(filePath, projects)) .filter((v): v is string => v != null); + if (verbose && changedIncludedFilesPackages.length > 0) { + logger.log( + `Found ${chalk.bold( + changedIncludedFilesPackages.length + )} affected packages from included files` + ); + } + const affectedPackages = new Set(changedIncludedFilesPackages); const visitedIdentifiers = new Map(); @@ -137,17 +187,45 @@ export const trueAffected = async ({ const identifierName = identifier.getText(); const path = rootNode.getSourceFile().getFilePath(); + if (verbose) { + logger.log( + `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)) { + if (verbose) { + logger.log( + `Already visited ${chalk.bold(identifierName)} in ${chalk.bold( + path + )}` + ); + } + + return; + } + + visitedIdentifiers.set(path, [...visited, identifierName]); + + if (verbose) { + logger.log( + `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); + if (verbose) { + logger.log(`Added package ${chalk.bold(pkg)} to affected packages`); + } + } findReferencesLibs(node); }); @@ -170,7 +248,18 @@ export const trueAffected = async ({ const pkg = getPackageNameByPath(sourceFile.getFilePath(), projects); - if (pkg) affectedPackages.add(pkg); + if (pkg) { + affectedPackages.add(pkg); + if (verbose) { + logger.log( + `Added package ${chalk.bold( + pkg + )} to affected packages for changed line ${chalk.bold( + line + )} in ${chalk.bold(filePath)}` + ); + } + } findReferencesLibs(changedNode); } catch { @@ -179,12 +268,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 (verbose && deps.length > 0) { + logger.log( + `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..56e46ea 100644 --- a/libs/core/src/types.ts +++ b/libs/core/src/types.ts @@ -6,7 +6,12 @@ export interface TrueAffectedProject { targets?: string[]; } -export interface TrueAffected { +export interface TrueAffectedLogging { + verbose?: boolean; + 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..1857c2c 100644 --- a/libs/nx/src/cli.spec.ts +++ b/libs/nx/src/cli.spec.ts @@ -1,8 +1,9 @@ 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()), @@ -73,6 +74,7 @@ describe('cli', () => { tsConfigFilePath: 'tsconfig.base.json', target: [], experimentalLockfileCheck: false, + verbose: false, }); }); @@ -85,10 +87,11 @@ describe('cli', () => { '--tsConfigFilePath=tsconfig.json', `--cwd=${workspaceCwd}`, '--includeFiles=package.json,jest.setup.js', + '--verbose=true', ]); expect(affectedActionSpy).toBeCalledWith({ action: 'build', - all: 'true', + all: true, base: 'master', cwd: resolve(process.cwd(), workspaceCwd), includeFiles: ['package.json', 'jest.setup.js'], @@ -97,6 +100,7 @@ describe('cli', () => { tsConfigFilePath: 'tsconfig.json', target: [], experimentalLockfileCheck: false, + verbose: true, }); }); }); @@ -132,13 +136,18 @@ describe('cli', () => { target: [], }); - expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd()); + expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd(), { + verbose: false, + logger: expect.any(Object), + }); expect(trafSpy).toHaveBeenCalledWith({ cwd: process.cwd(), rootTsConfig: 'tsconfig.base.json', base: 'origin/main', projects: [], include: [], + verbose: false, + logger: expect.any(Object), }); }); diff --git a/libs/nx/src/cli.ts b/libs/nx/src/cli.ts index ec48c12..59976ba 100644 --- a/libs/nx/src/cli.ts +++ b/libs/nx/src/cli.ts @@ -14,6 +14,16 @@ export const log = (message: string) => ` ${chalk.hex(color)('>')} ${chalk.bgHex(color).bold(' TRAF ')} ${message}` ); +const getLogger = (namespace: string) => ({ + ...console, + log: (message: string) => + console.log( + ` ${chalk.hex(color)('>')} ${chalk.bgGray.bold( + ` ${namespace} ` + )} ${message}` + ), +}); + export const affectedAction = async ({ cwd, action = 'log', @@ -25,9 +35,18 @@ export const affectedAction = async ({ includeFiles, target, experimentalLockfileCheck, + verbose = false, }: AffectedOptions) => { + const nxLogger = getLogger('NX'); + if (verbose) { + nxLogger.log('Getting nx projects'); + } let projects = await getNxTrueAffectedProjects(cwd); + if (verbose) { + nxLogger.log(`Found ${projects.length} projects`); + } + if (target.length) { projects = projects.filter( (project) => @@ -45,6 +64,8 @@ export const affectedAction = async ({ base, projects, include: [...includeFiles, DEFAULT_INCLUDE_TEST_FILES], + verbose, + logger: getLogger('CORE'), __experimentalLockfileCheck: experimentalLockfileCheck, }); @@ -94,6 +115,7 @@ interface AffectedOptions { restArgs: string[]; target: string[]; experimentalLockfileCheck?: boolean; + verbose?: boolean; } const affectedCommand: CommandModule = { @@ -116,6 +138,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 +147,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 +169,12 @@ const affectedCommand: CommandModule = { desc: 'Experimental lockfile check', type: 'boolean', default: false, + coerce: Boolean, + }, + verbose: { + desc: 'Verbose output', + default: false, + coerce: Boolean, }, }, handler: async ({ @@ -157,6 +187,7 @@ const affectedCommand: CommandModule = { includeFiles, target, experimentalLockfileCheck, + verbose, // eslint-disable-next-line @typescript-eslint/no-unused-vars $0, // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -173,6 +204,7 @@ const affectedCommand: CommandModule = { includeFiles, target, experimentalLockfileCheck, + verbose, restArgs: Object.entries(rest).map( /* istanbul ignore next */ ([key, value]) => `--${key}=${value}` From d5099d31b5523a8de320a8ff98bef212555b02f7 Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Mon, 13 May 2024 17:00:38 +0300 Subject: [PATCH 2/3] feat(nx): project.json runtime validation & verbose --- libs/nx/src/cli.spec.ts | 10 ++- libs/nx/src/cli.ts | 11 ++- libs/nx/src/nx.spec.ts | 166 ++++++++++++++++++++++++++++++++++++++++ libs/nx/src/nx.ts | 85 ++++++++++++++++---- 4 files changed, 255 insertions(+), 17 deletions(-) diff --git a/libs/nx/src/cli.spec.ts b/libs/nx/src/cli.spec.ts index 1857c2c..af56872 100644 --- a/libs/nx/src/cli.spec.ts +++ b/libs/nx/src/cli.spec.ts @@ -6,10 +6,13 @@ import * as nx from './nx'; import { workspaceCwd } from './mocks'; 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(), })); @@ -134,10 +137,11 @@ describe('cli', () => { restArgs: [], tsConfigFilePath: 'tsconfig.base.json', target: [], + verbose: true, }); expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd(), { - verbose: false, + verbose: true, logger: expect.any(Object), }); expect(trafSpy).toHaveBeenCalledWith({ @@ -146,7 +150,7 @@ describe('cli', () => { base: 'origin/main', projects: [], include: [], - verbose: false, + verbose: true, logger: expect.any(Object), }); }); diff --git a/libs/nx/src/cli.ts b/libs/nx/src/cli.ts index 59976ba..a001276 100644 --- a/libs/nx/src/cli.ts +++ b/libs/nx/src/cli.ts @@ -22,6 +22,12 @@ const getLogger = (namespace: string) => ({ ` ${namespace} ` )} ${message}` ), + warn: (message: string) => + console.warn( + ` ${chalk.yellowBright('⚠️')} ${chalk.bgGray.bold( + ` ${namespace} ` + )} ${chalk.yellow(message)}` + ), }); export const affectedAction = async ({ @@ -41,7 +47,10 @@ export const affectedAction = async ({ if (verbose) { nxLogger.log('Getting nx projects'); } - let projects = await getNxTrueAffectedProjects(cwd); + let projects = await getNxTrueAffectedProjects(cwd, { + verbose, + logger: nxLogger, + }); if (verbose) { nxLogger.log(`Found ${projects.length} projects`); diff --git a/libs/nx/src/nx.spec.ts b/libs/nx/src/nx.spec.ts index 554694b..7aaf7a2 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,167 @@ 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(), + } 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('verbose', () => { + it('should log found projects', async () => { + const logger = { + log: 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, { verbose: true, logger }); + + expect(logger.log).toHaveBeenCalledWith('Found 1 nx projects'); + }); + + it('should notify about missing tsconfig and fallback to sourceRoot', async () => { + const logger = { + warn: jest.fn(), + log: 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, { verbose: true, logger }); + + expect(logger.log).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(), + } 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, { verbose: true, logger }); + + expect(logger.log).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(), + } 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, { verbose: true, logger }); + + expect(logger.log).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..47496be 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,64 @@ async function getNxProjectJsonProjects( } } +type GetNxTrueAffectedProjectsOptions = TrueAffectedLogging; + export async function getNxTrueAffectedProjects( - cwd: string + cwd: string, + { verbose = false, logger = console }: GetNxTrueAffectedProjectsOptions = {} ): Promise { const projects = await getNxProjects(cwd); - return projects.map(({ name, project }) => { + if (verbose) { + logger.log(`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 (verbose) { + if (project.sourceRoot) { + logger.log( + `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.log( + `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 +170,17 @@ export async function getNxTrueAffectedProjects( } } + if (verbose) { + logger.log( + `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 ?? {}), From 48cf1d6abd690a8a1fb369f962768a31f82ef7d1 Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Sun, 9 Jun 2024 18:44:36 +0300 Subject: [PATCH 3/3] use DEBUG env var instead of verbose mode --- libs/core/src/true-affected.spec.ts | 26 +++--- libs/core/src/true-affected.ts | 130 ++++++++++++---------------- libs/core/src/types.ts | 1 - libs/nx/src/cli.spec.ts | 9 +- libs/nx/src/cli.ts | 26 ++---- libs/nx/src/nx.spec.ts | 23 +++-- libs/nx/src/nx.ts | 53 +++++------- 7 files changed, 115 insertions(+), 153 deletions(-) diff --git a/libs/core/src/true-affected.spec.ts b/libs/core/src/true-affected.spec.ts index 4a73b3b..a3110bb 100644 --- a/libs/core/src/true-affected.spec.ts +++ b/libs/core/src/true-affected.spec.ts @@ -219,7 +219,6 @@ describe('trueAffected', () => { cwd, base: 'main', rootTsConfig: 'tsconfig.json', - verbose: true, projects: [ { name: 'angular-component', @@ -268,7 +267,6 @@ describe('trueAffected', () => { cwd, base: 'main', __experimentalLockfileCheck: true, - verbose: true, projects: [ { name: 'proj1', @@ -410,7 +408,6 @@ describe('trueAffected', () => { }, ], include: filePatterns, - verbose: true, }); expect(affected).toEqual(expected); @@ -425,7 +422,7 @@ describe('trueAffected', () => { ]; jest.spyOn(git, 'getChangedFiles').mockReturnValue(changedFiles); - const log = jest.fn(); + const debug = jest.fn(); await trueAffected({ cwd, base: 'main', @@ -449,38 +446,37 @@ describe('trueAffected', () => { }, ], logger: { - log, + debug, } as unknown as Console, - verbose: true, }); - expect(log).toHaveBeenCalledWith('Getting affected projects'); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith('Getting affected projects'); + expect(debug).toHaveBeenCalledWith( expect.stringContaining('Creating project with root tsconfig from') ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( expect.stringContaining('Adding source files for project proj1') ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( expect.stringContaining('Adding source files for project proj2') ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( expect.stringContaining( 'Could not find a tsconfig for project proj3, adding source files paths' ) ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( `Found ${changedFiles.length} changed files` ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( `Added package proj1 to affected packages for changed line ${changedFiles[0].changedLines[0]} in ${changedFiles[0].filePath}` ); - expect(log).toHaveBeenCalledWith( + expect(debug).toHaveBeenCalledWith( expect.stringMatching( new RegExp(`^Found identifier .* in .*${changedFiles[0].filePath}$`) ) ); - expect(log).toHaveBeenCalledWith( + 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 fc424f8..c17c907 100644 --- a/libs/core/src/true-affected.ts +++ b/libs/core/src/true-affected.ts @@ -22,25 +22,27 @@ 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], - verbose = false, - logger = console, + logger = DEFAULT_LOGGER, __experimentalLockfileCheck = false, }: TrueAffected) => { - if (verbose) { - logger.log('Getting affected projects'); - if (rootTsConfig != null) { - logger.log( - `Creating project with root tsconfig from ${chalk.bold( - resolve(cwd, rootTsConfig) - )}` - ); - } + 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({ @@ -60,24 +62,22 @@ export const trueAffected = async ({ const tsConfigPath = resolve(cwd, tsConfig); if (existsSync(tsConfigPath)) { - if (verbose) { - logger.log( - `Adding source files for project ${chalk.bold( - name - )} from tsconfig at ${chalk.bold(tsConfigPath)}` - ); - } + logger.debug( + `Adding source files for project ${chalk.bold( + name + )} from tsconfig at ${chalk.bold(tsConfigPath)}` + ); + project.addSourceFilesFromTsConfig(tsConfigPath); } else { - if (verbose) { - logger.log( - `Could not find a tsconfig for project ${chalk.bold( - name - )}, adding source files paths in ${chalk.bold( - resolve(cwd, sourceRoot) - )}` - ); - } + 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}') ); @@ -90,9 +90,7 @@ export const trueAffected = async ({ cwd, }); - if (verbose) { - logger.log(`Found ${chalk.bold(changedFiles.length)} changed files`); - } + logger.debug(`Found ${chalk.bold(changedFiles.length)} changed files`); const sourceChangedFiles = changedFiles.filter( ({ filePath }) => project.getSourceFile(resolve(cwd, filePath)) != null @@ -108,17 +106,15 @@ export const trueAffected = async ({ project.getSourceFile(resolve(cwd, filePath)) == null ) .flatMap(({ filePath: changedFilePath }) => { - if (verbose) { - logger.log( - `Finding non-source affected files for ${chalk.bold(changedFilePath)}` - ); - } + logger.debug( + `Finding non-source affected files for ${chalk.bold(changedFilePath)}` + ); return findNonSourceAffectedFiles(cwd, changedFilePath, ignoredPaths); }); - if (verbose && nonSourceChangedFiles.length > 0) { - logger.log( + if (nonSourceChangedFiles.length > 0) { + logger.debug( `Found ${chalk.bold( nonSourceChangedFiles.length )} non-source affected files` @@ -127,9 +123,7 @@ export const trueAffected = async ({ let changedFilesByLockfile: ChangedFiles[] = []; if (__experimentalLockfileCheck && hasLockfileChanged(changedFiles)) { - if (verbose) { - logger.log('Lockfile has changed, finding affected files'); - } + logger.debug('Lockfile has changed, finding affected files'); changedFilesByLockfile = findAffectedFilesByLockfile( cwd, @@ -157,8 +151,8 @@ export const trueAffected = async ({ .map(({ filePath }) => getPackageNameByPath(filePath, projects)) .filter((v): v is string => v != null); - if (verbose && changedIncludedFilesPackages.length > 0) { - logger.log( + if (changedIncludedFilesPackages.length > 0) { + logger.debug( `Found ${chalk.bold( changedIncludedFilesPackages.length )} affected packages from included files` @@ -187,33 +181,25 @@ export const trueAffected = async ({ const identifierName = identifier.getText(); const path = rootNode.getSourceFile().getFilePath(); - if (verbose) { - logger.log( - `Found identifier ${chalk.bold(identifierName)} in ${chalk.bold(path)}` - ); - } + logger.debug( + `Found identifier ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); if (identifierName && path) { const visited = visitedIdentifiers.get(path) ?? []; if (visited.includes(identifierName)) { - if (verbose) { - logger.log( - `Already visited ${chalk.bold(identifierName)} in ${chalk.bold( - path - )}` - ); - } + logger.debug( + `Already visited ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); return; } visitedIdentifiers.set(path, [...visited, identifierName]); - if (verbose) { - logger.log( - `Visiting ${chalk.bold(identifierName)} in ${chalk.bold(path)}` - ); - } + logger.debug( + `Visiting ${chalk.bold(identifierName)} in ${chalk.bold(path)}` + ); } refs.forEach((node) => { @@ -222,9 +208,8 @@ export const trueAffected = async ({ if (pkg) { affectedPackages.add(pkg); - if (verbose) { - logger.log(`Added package ${chalk.bold(pkg)} to affected packages`); - } + + logger.debug(`Added package ${chalk.bold(pkg)} to affected packages`); } findReferencesLibs(node); @@ -250,15 +235,14 @@ export const trueAffected = async ({ if (pkg) { affectedPackages.add(pkg); - if (verbose) { - logger.log( - `Added package ${chalk.bold( - pkg - )} to affected packages for changed line ${chalk.bold( - line - )} in ${chalk.bold(filePath)}` - ); - } + + logger.debug( + `Added package ${chalk.bold( + pkg + )} to affected packages for changed line ${chalk.bold( + line + )} in ${chalk.bold(filePath)}` + ); } findReferencesLibs(changedNode); @@ -284,8 +268,8 @@ export const trueAffected = async ({ .filter(([, deps]) => deps.includes(pkg)) .map(([name]) => name); - if (verbose && deps.length > 0) { - logger.log( + if (deps.length > 0) { + logger.debug( `Adding implicit dependencies ${chalk.bold( deps.join(', ') )} to ${chalk.bold(pkg)}` diff --git a/libs/core/src/types.ts b/libs/core/src/types.ts index 56e46ea..06787f0 100644 --- a/libs/core/src/types.ts +++ b/libs/core/src/types.ts @@ -7,7 +7,6 @@ export interface TrueAffectedProject { } export interface TrueAffectedLogging { - verbose?: boolean; logger?: Console; } diff --git a/libs/nx/src/cli.spec.ts b/libs/nx/src/cli.spec.ts index af56872..7c7f1bc 100644 --- a/libs/nx/src/cli.spec.ts +++ b/libs/nx/src/cli.spec.ts @@ -77,7 +77,6 @@ describe('cli', () => { tsConfigFilePath: 'tsconfig.base.json', target: [], experimentalLockfileCheck: false, - verbose: false, }); }); @@ -90,7 +89,6 @@ describe('cli', () => { '--tsConfigFilePath=tsconfig.json', `--cwd=${workspaceCwd}`, '--includeFiles=package.json,jest.setup.js', - '--verbose=true', ]); expect(affectedActionSpy).toBeCalledWith({ action: 'build', @@ -103,7 +101,6 @@ describe('cli', () => { tsConfigFilePath: 'tsconfig.json', target: [], experimentalLockfileCheck: false, - verbose: true, }); }); }); @@ -117,7 +114,8 @@ describe('cli', () => { beforeEach(() => { getNxTrueAffectedProjectsSpy = jest .spyOn(nx, 'getNxTrueAffectedProjects') - .mockImplementation(); + .mockImplementation() + .mockResolvedValue([]); logSpy = jest.spyOn(cli, 'log').mockImplementation(); }); @@ -137,11 +135,9 @@ describe('cli', () => { restArgs: [], tsConfigFilePath: 'tsconfig.base.json', target: [], - verbose: true, }); expect(getNxTrueAffectedProjectsSpy).toBeCalledWith(process.cwd(), { - verbose: true, logger: expect.any(Object), }); expect(trafSpy).toHaveBeenCalledWith({ @@ -150,7 +146,6 @@ describe('cli', () => { base: 'origin/main', projects: [], include: [], - verbose: true, logger: expect.any(Object), }); }); diff --git a/libs/nx/src/cli.ts b/libs/nx/src/cli.ts index a001276..ed00a26 100644 --- a/libs/nx/src/cli.ts +++ b/libs/nx/src/cli.ts @@ -16,8 +16,9 @@ export const log = (message: string) => const getLogger = (namespace: string) => ({ ...console, - log: (message: string) => - console.log( + debug: (message: string) => + process.env['DEBUG'] === 'true' && + console.debug( ` ${chalk.hex(color)('>')} ${chalk.bgGray.bold( ` ${namespace} ` )} ${message}` @@ -41,20 +42,16 @@ export const affectedAction = async ({ includeFiles, target, experimentalLockfileCheck, - verbose = false, }: AffectedOptions) => { const nxLogger = getLogger('NX'); - if (verbose) { - nxLogger.log('Getting nx projects'); - } + + nxLogger.debug('Getting nx projects'); + let projects = await getNxTrueAffectedProjects(cwd, { - verbose, logger: nxLogger, }); - if (verbose) { - nxLogger.log(`Found ${projects.length} projects`); - } + nxLogger.debug(`Found ${projects.length} projects`); if (target.length) { projects = projects.filter( @@ -73,7 +70,6 @@ export const affectedAction = async ({ base, projects, include: [...includeFiles, DEFAULT_INCLUDE_TEST_FILES], - verbose, logger: getLogger('CORE'), __experimentalLockfileCheck: experimentalLockfileCheck, }); @@ -124,7 +120,6 @@ interface AffectedOptions { restArgs: string[]; target: string[]; experimentalLockfileCheck?: boolean; - verbose?: boolean; } const affectedCommand: CommandModule = { @@ -180,11 +175,6 @@ const affectedCommand: CommandModule = { default: false, coerce: Boolean, }, - verbose: { - desc: 'Verbose output', - default: false, - coerce: Boolean, - }, }, handler: async ({ cwd, @@ -196,7 +186,6 @@ const affectedCommand: CommandModule = { includeFiles, target, experimentalLockfileCheck, - verbose, // eslint-disable-next-line @typescript-eslint/no-unused-vars $0, // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -213,7 +202,6 @@ const affectedCommand: CommandModule = { includeFiles, target, experimentalLockfileCheck, - verbose, restArgs: Object.entries(rest).map( /* istanbul ignore next */ ([key, value]) => `--${key}=${value}` diff --git a/libs/nx/src/nx.spec.ts b/libs/nx/src/nx.spec.ts index 7aaf7a2..4d87973 100644 --- a/libs/nx/src/nx.spec.ts +++ b/libs/nx/src/nx.spec.ts @@ -490,6 +490,7 @@ describe('nx', () => { 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) => { @@ -532,10 +533,11 @@ describe('nx', () => { }); }); - describe('verbose', () => { + 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) => { @@ -554,15 +556,16 @@ describe('nx', () => { }); const cwd = 'libs/nx/src/__fixtures__/nx-project'; - await getNxTrueAffectedProjects(cwd, { verbose: true, logger }); + await getNxTrueAffectedProjects(cwd, { logger }); - expect(logger.log).toHaveBeenCalledWith('Found 1 nx projects'); + 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) => { @@ -576,9 +579,9 @@ describe('nx', () => { }); const cwd = 'libs/nx/src/__fixtures__/nx-project'; - await getNxTrueAffectedProjects(cwd, { verbose: true, logger }); + await getNxTrueAffectedProjects(cwd, { logger }); - expect(logger.log).toHaveBeenCalledWith( + 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'" @@ -591,6 +594,7 @@ describe('nx', () => { const logger = { warn: jest.fn(), log: jest.fn(), + debug: jest.fn(), } as unknown as Console; jest.spyOn(fs.promises, 'readFile').mockImplementation((pathLike) => { @@ -604,9 +608,9 @@ describe('nx', () => { }); const cwd = 'libs/nx/src/__fixtures__/nx-project'; - await getNxTrueAffectedProjects(cwd, { verbose: true, logger }); + await getNxTrueAffectedProjects(cwd, { logger }); - expect(logger.log).toHaveBeenCalledWith( + 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." @@ -618,6 +622,7 @@ describe('nx', () => { 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) => { @@ -636,9 +641,9 @@ describe('nx', () => { }); const cwd = 'libs/nx/src/__fixtures__/nx-project'; - await getNxTrueAffectedProjects(cwd, { verbose: true, logger }); + await getNxTrueAffectedProjects(cwd, { logger }); - expect(logger.log).toHaveBeenCalledWith( + 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 47496be..8df38fa 100644 --- a/libs/nx/src/nx.ts +++ b/libs/nx/src/nx.ts @@ -104,13 +104,11 @@ type GetNxTrueAffectedProjectsOptions = TrueAffectedLogging; export async function getNxTrueAffectedProjects( cwd: string, - { verbose = false, logger = console }: GetNxTrueAffectedProjectsOptions = {} + { logger = console }: GetNxTrueAffectedProjectsOptions = {} ): Promise { const projects = await getNxProjects(cwd); - if (verbose) { - logger.log(`Found ${chalk.bold(projects.length)} nx projects`); - } + logger.debug(`Found ${chalk.bold(projects.length)} nx projects`); return projects.map(({ name, path, project }) => { let tsConfig = project.targets?.build?.options?.tsConfig; @@ -136,25 +134,24 @@ export async function getNxTrueAffectedProjects( } if (!tsConfig) { - if (verbose) { - if (project.sourceRoot) { - logger.log( - `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.log( - `Project at ${chalk.bold( - path - )} does not have a tsConfig property under '${chalk.bold( - 'targets.build.options.tsConfig' - )}'. Using project.json directory.` - ); - } + 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; @@ -170,13 +167,11 @@ export async function getNxTrueAffectedProjects( } } - if (verbose) { - logger.log( - `Using tsconfig at ${chalk.bold(tsConfig)} for project ${chalk.bold( - projectName - )}` - ); - } + logger.debug( + `Using tsconfig at ${chalk.bold(tsConfig)} for project ${chalk.bold( + projectName + )}` + ); return { name: projectName,