Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: DEBUG flag and project.json validation #31

Merged
merged 3 commits into from
Jun 11, 2024
Merged

feat: DEBUG flag and project.json validation #31

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e214255
feat: support verbose logging
EladBezalel May 13, 2024
d5099d3
feat(nx): project.json runtime validation & verbose
EladBezalel May 13, 2024
48cf1d6
use DEBUG env var instead of verbose mode
EladBezalel Jun 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions libs/core/src/true-affected.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -213,6 +219,7 @@ describe('trueAffected', () => {
cwd,
base: 'main',
rootTsConfig: 'tsconfig.json',
verbose: true,
projects: [
{
name: 'angular-component',
Expand Down Expand Up @@ -261,6 +268,7 @@ describe('trueAffected', () => {
cwd,
base: 'main',
__experimentalLockfileCheck: true,
verbose: true,
projects: [
{
name: 'proj1',
Expand Down Expand Up @@ -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'
);
});
});
145 changes: 126 additions & 19 deletions libs/core/src/true-affected.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -41,23 +55,29 @@ export const trueAffected = async ({
}),
});

const implicitDeps = (
projects.filter(
({ implicitDependencies = [] }) => implicitDependencies.length > 0
) as Required<TrueAffectedProject>[]
).reduce(
(acc, { name, implicitDependencies }) =>
acc.set(name, implicitDependencies),
new Map<string, string[]>()
);

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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EladBezalel maybe a change in approach, maybe we can start using logger.debug and then we can set the debug level or the DEBUG env var and this way u won't need to do the if (verbose) part and the logger should know how to handle log levels.

wdyt? the benefit here is that this will reduce the amount of branching in code and will make it a bit nicer and u won't have to add additional logic for params

`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}')
);
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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<string>(changedIncludedFilesPackages);
const visitedIdentifiers = new Map<string, string[]>();

Expand All @@ -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);
});
Expand All @@ -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 {
Expand All @@ -179,12 +268,30 @@ export const trueAffected = async ({
});
});

const implicitDeps = (
projects.filter(
({ implicitDependencies = [] }) => implicitDependencies.length > 0
) as Required<TrueAffectedProject>[]
).reduce(
(acc, { name, implicitDependencies }) =>
acc.set(name, implicitDependencies),
new Map<string, string[]>()
);

// 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));
});

Expand Down
7 changes: 6 additions & 1 deletion libs/core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading