From 398d276762f1486e1b22bb21bbba4107dd14cff5 Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Wed, 5 Jun 2024 13:30:07 -0400 Subject: [PATCH] chore(core): refactor -> minimatch --- .../webpack-browser/webpack-browser.impl.ts | 11 +- .../nx/src/tasks-runner/create-task-graph.ts | 15 +- packages/nx/src/tasks-runner/utils.spec.ts | 123 +++++++++--- packages/nx/src/tasks-runner/utils.ts | 177 ++++++++++++------ .../nx/src/utils/find-matching-projects.ts | 9 +- 5 files changed, 247 insertions(+), 88 deletions(-) diff --git a/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts b/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts index 17051b7aab7c26..2593475c2e871b 100644 --- a/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts +++ b/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts @@ -25,10 +25,19 @@ function shouldSkipInitialTargetRun( project: string, target: string ): boolean { + const allTargetNames = new Set(); + for (const projectName in projectGraph.nodes) { + const project = projectGraph.nodes[projectName]; + for (const targetName in project.data.targets ?? {}) { + allTargetNames.add(targetName); + } + } + const projectDependencyConfigs = getDependencyConfigs( { project, target }, {}, - projectGraph + projectGraph, + Array.from(allTargetNames) ); // if the task runner already ran the target, skip the initial run diff --git a/packages/nx/src/tasks-runner/create-task-graph.ts b/packages/nx/src/tasks-runner/create-task-graph.ts index 9035be7940b7fa..443eddde322fd0 100644 --- a/packages/nx/src/tasks-runner/create-task-graph.ts +++ b/packages/nx/src/tasks-runner/create-task-graph.ts @@ -14,11 +14,21 @@ export class ProcessTasks { private readonly seen = new Set(); readonly tasks: { [id: string]: Task } = {}; readonly dependencies: { [k: string]: string[] } = {}; + private readonly allTargetNames: string[]; constructor( private readonly extraTargetDependencies: TargetDependencies, private readonly projectGraph: ProjectGraph - ) {} + ) { + const allTargetNames = new Set(); + for (const projectName in projectGraph.nodes) { + const project = projectGraph.nodes[projectName]; + for (const targetName in project.data.targets ?? {}) { + allTargetNames.add(targetName); + } + } + this.allTargetNames = Array.from(allTargetNames); + } processTasks( projectNames: string[], @@ -100,7 +110,8 @@ export class ProcessTasks { const dependencyConfigs = getDependencyConfigs( { project: task.target.project, target: task.target.target }, this.extraTargetDependencies, - this.projectGraph + this.projectGraph, + this.allTargetNames ); for (const dependencyConfig of dependencyConfigs) { const taskOverrides = diff --git a/packages/nx/src/tasks-runner/utils.spec.ts b/packages/nx/src/tasks-runner/utils.spec.ts index 4c945e9dcd9bfa..b3ed7b0af4440d 100644 --- a/packages/nx/src/tasks-runner/utils.spec.ts +++ b/packages/nx/src/tasks-runner/utils.spec.ts @@ -1,5 +1,6 @@ import { expandDependencyConfigSyntaxSugar, + getDependencyConfigs, getOutputsForTargetAndConfiguration, interpolate, transformLegacyOutputs, @@ -443,7 +444,7 @@ describe('utils', () => { describe('expandDependencyConfigSyntaxSugar', () => { it('should expand syntax for simple target names', () => { - const result = expandDependencyConfigSyntaxSugar('build', 'any', { + const result = expandDependencyConfigSyntaxSugar('build', { dependencies: {}, nodes: {}, }); @@ -455,7 +456,7 @@ describe('utils', () => { }); it('should assume target of self if simple target also matches project name', () => { - const result = expandDependencyConfigSyntaxSugar('build', 'any', { + const result = expandDependencyConfigSyntaxSugar('build', { dependencies: {}, nodes: { build: { @@ -475,7 +476,7 @@ describe('utils', () => { }); it('should expand syntax for simple target names targetting dependencies', () => { - const result = expandDependencyConfigSyntaxSugar('^build', 'any', { + const result = expandDependencyConfigSyntaxSugar('^build', { dependencies: {}, nodes: {}, }); @@ -488,7 +489,7 @@ describe('utils', () => { }); it('should expand syntax for strings like project:target if project is a valid project', () => { - const result = expandDependencyConfigSyntaxSugar('project:build', 'any', { + const result = expandDependencyConfigSyntaxSugar('project:build', { dependencies: {}, nodes: { project: { @@ -509,14 +510,10 @@ describe('utils', () => { }); it('should expand syntax for strings like target:with:colons', () => { - const result = expandDependencyConfigSyntaxSugar( - 'target:with:colons', - 'any', - { - dependencies: {}, - nodes: {}, - } - ); + const result = expandDependencyConfigSyntaxSugar('target:with:colons', { + dependencies: {}, + nodes: {}, + }); expect(result).toEqual([ { target: 'target:with:colons', @@ -525,24 +522,31 @@ describe('utils', () => { }); it('supports wildcards in targets', () => { - const result = expandDependencyConfigSyntaxSugar('build-*', 'project', { - dependencies: {}, - nodes: { - project: { - name: 'project', - type: 'app', - data: { - root: 'libs/project', - targets: { - build: {}, - 'build-css': {}, - 'build-js': {}, - 'then-build-something-else': {}, + const result = getDependencyConfigs( + { project: 'project', target: 'build' }, + {}, + { + dependencies: {}, + nodes: { + project: { + name: 'project', + type: 'app', + data: { + root: 'libs/project', + targets: { + build: { + dependsOn: ['build-*'], + }, + 'build-css': {}, + 'build-js': {}, + 'then-build-something-else': {}, + }, }, }, }, }, - }); + ['build', 'build-css', 'build-js', 'then-build-something-else'] + ); expect(result).toEqual([ { target: 'build-css', @@ -554,6 +558,73 @@ describe('utils', () => { }, ]); }); + + it('should support wildcards with dependencies', () => { + const result = getDependencyConfigs( + { project: 'project', target: 'build' }, + {}, + { + dependencies: {}, + nodes: { + project: { + name: 'project', + type: 'app', + data: { + root: 'libs/project', + targets: { + build: { + dependsOn: ['^build-*'], + }, + 'then-build-something-else': {}, + }, + }, + }, + dep1: { + name: 'dep1', + type: 'lib', + data: { + root: 'libs/dep1', + targets: { + 'build-css': {}, + 'build-js': {}, + }, + }, + }, + dep2: { + name: 'dep2', + type: 'lib', + data: { + root: 'libs/dep2', + targets: { + 'build-python': {}, + }, + }, + }, + }, + }, + [ + 'build', + 'build-css', + 'build-js', + 'then-build-something-else', + 'build-python', + ] + ); + expect(result).toEqual([ + { + target: 'build-css', + dependencies: true, + }, + { + target: 'build-js', + dependencies: true, + }, + { + target: 'build-python', + dependencies: true, + }, + ]); + }); }); describe('validateOutputs', () => { diff --git a/packages/nx/src/tasks-runner/utils.ts b/packages/nx/src/tasks-runner/utils.ts index bd3721e418aa59..e347215c6c9788 100644 --- a/packages/nx/src/tasks-runner/utils.ts +++ b/packages/nx/src/tasks-runner/utils.ts @@ -15,41 +15,81 @@ import { splitByColons } from '../utils/split-target'; import { getExecutorInformation } from '../command-line/run/executor-utils'; import { CustomHasher, ExecutorConfig } from '../config/misc-interfaces'; import { readProjectsConfigurationFromProjectGraph } from '../project-graph/project-graph'; +import { + GLOB_CHARACTERS, + findMatchingProjects, +} from '../utils/find-matching-projects'; +import { minimatch } from 'minimatch'; + +export type NormalizedTargetDependencyConfig = TargetDependencyConfig & { + projects: string[]; +}; export function getDependencyConfigs( { project, target }: { project: string; target: string }, extraTargetDependencies: Record, - projectGraph: ProjectGraph -): TargetDependencyConfig[] | undefined { + projectGraph: ProjectGraph, + allTargetNames: string[] +): NormalizedTargetDependencyConfig[] | undefined { const dependencyConfigs = ( projectGraph.nodes[project].data?.targets[target]?.dependsOn ?? // This is passed into `run-command` from programmatic invocations extraTargetDependencies[target] ?? [] ).flatMap((config) => - typeof config === 'string' - ? expandDependencyConfigSyntaxSugar(config, project, projectGraph) - : [config] + normalizeDependencyConfigDefinition( + config, + project, + projectGraph, + allTargetNames + ) ); - for (const dependencyConfig of dependencyConfigs) { - if (dependencyConfig.projects && dependencyConfig.dependencies) { - output.error({ - title: `dependsOn is improperly configured for ${project}:${target}`, - bodyLines: [ - `dependsOn.projects and dependsOn.dependencies cannot be used together.`, - ], - }); - process.exit(1); - } - } return dependencyConfigs; } -export function expandDependencyConfigSyntaxSugar( - dependencyConfigString: string, +export function normalizeDependencyConfigDefinition( + definition: string | TargetDependencyConfig, + currentProject: string, + graph: ProjectGraph, + allTargetNames: string[] +): NormalizedTargetDependencyConfig[] { + return expandWildcardTargetConfiguration( + normalizeDependencyConfigProjects( + expandDependencyConfigSyntaxSugar(definition, graph), + currentProject, + graph + ), + allTargetNames + ); +} + +export function normalizeDependencyConfigProjects( + dependencyConfig: TargetDependencyConfig, currentProject: string, graph: ProjectGraph -): TargetDependencyConfig[] { +): NormalizedTargetDependencyConfig { + const noStringConfig = + normalizeTargetDependencyWithStringProjects(dependencyConfig); + + if (noStringConfig.projects) { + dependencyConfig.projects = findMatchingProjects( + noStringConfig.projects, + graph.nodes + ); + } else if (!noStringConfig.dependencies) { + dependencyConfig.projects = [currentProject]; + } + return dependencyConfig as NormalizedTargetDependencyConfig; +} + +export function expandDependencyConfigSyntaxSugar( + dependencyConfigString: string | TargetDependencyConfig, + graph: ProjectGraph +): TargetDependencyConfig { + if (typeof dependencyConfigString !== 'string') { + return dependencyConfigString; + } + const [dependencies, targetString] = dependencyConfigString.startsWith('^') ? [true, dependencyConfigString.substring(1)] : [false, dependencyConfigString]; @@ -57,56 +97,53 @@ export function expandDependencyConfigSyntaxSugar( // Support for `project:target` syntax doesn't make sense for // dependencies, so we only support `target` syntax for dependencies. if (dependencies) { - return [ - { - target: targetString, - dependencies: true, - }, - ]; + return { + target: targetString, + dependencies: true, + }; } + const { projects, target } = readProjectAndTargetFromTargetString( + targetString, + graph.nodes + ); + + return projects ? { projects, target } : { target }; +} + +export function expandWildcardTargetConfiguration( + dependencyConfig: NormalizedTargetDependencyConfig, + allTargetNames: string[] +): NormalizedTargetDependencyConfig[] { + if (!GLOB_CHARACTERS.some((char) => dependencyConfig.target.includes(char))) { + return [dependencyConfig]; + } + const matcher = minimatch.filter(dependencyConfig.target); + + const matchingTargets = allTargetNames.filter((t) => matcher(t)); + + return matchingTargets.map((t) => ({ ...dependencyConfig, target: t })); +} + +export function readProjectAndTargetFromTargetString( + targetString: string, + projects: Record +): { projects?: string[]; target: string } { // Support for both `project:target` and `target:with:colons` syntax const [maybeProject, ...segments] = splitByColons(targetString); - let target, projects; if (!segments.length) { // if no additional segments are provided, then the string references // a target of the same project - target = maybeProject; - } else if (maybeProject in graph.nodes) { + return { target: maybeProject }; + } else if (maybeProject in projects) { // Only the first segment could be a project. If it is, the rest is a target. // If its not, then the whole targetString was a target with colons in its name. - target = segments.join(':'); - projects = [maybeProject]; + return { projects: [maybeProject], target: segments.join(':') }; } else { // If the first segment is a project, then we have a specific project. Otherwise, we don't. - target = targetString; + return { target: targetString }; } - - // handle target wildcards - if (target.indexOf('*') >= 0) { - const matches: TargetDependencyConfig[] = []; - const targetMatch = new RegExp('^' + target.replaceAll('*', '.*') + '$'); - const projectsToCheck = projects ? projects : [currentProject]; - for (const project of projectsToCheck) { - const projectTargets = graph.nodes[project].data?.targets; - if (projectTargets) { - for (const target in projectTargets) { - if (target.match(targetMatch)) { - matches.push({ target, projects: [project] }); - } - } - } - } - return matches; - } - - return [ - { - target, - projects, - }, - ]; } export function getOutputs( @@ -121,6 +158,34 @@ export function getOutputs( ); } +export function normalizeTargetDependencyWithStringProjects( + dependencyConfig: TargetDependencyConfig +): Omit & { projects: string[] } { + if (typeof dependencyConfig.projects === 'string') { + /** LERNA SUPPORT START - Remove in v20 */ + // Lerna uses `dependencies` in `prepNxOptions`, so we need to maintain + // support for it until lerna can be updated to use the syntax. + // + // This should have been removed in v17, but the updates to lerna had not + // been made yet. + // + // TODO(@agentender): Remove this part in v20 + if (dependencyConfig.projects === 'self') { + delete dependencyConfig.projects; + } else if (dependencyConfig.projects === 'dependencies') { + dependencyConfig.dependencies = true; + delete dependencyConfig.projects; + return; + /** LERNA SUPPORT END - Remove in v20 */ + } else { + dependencyConfig.projects = [dependencyConfig.projects]; + } + } + return dependencyConfig as Omit & { + projects: string[]; + }; +} + class InvalidOutputsError extends Error { constructor(public outputs: string[], public invalidOutputs: Set) { super(InvalidOutputsError.createMessage(invalidOutputs)); diff --git a/packages/nx/src/utils/find-matching-projects.ts b/packages/nx/src/utils/find-matching-projects.ts index 497e2ab13ee1a4..de76ec1e39294f 100644 --- a/packages/nx/src/utils/find-matching-projects.ts +++ b/packages/nx/src/utils/find-matching-projects.ts @@ -18,7 +18,10 @@ interface ProjectPattern { value: string; } -const globCharacters = ['*', '|', '{', '}', '(', ')']; +/** + * The presence of these characters in a string indicates that it might be a glob pattern. + */ +export const GLOB_CHARACTERS = ['*', '|', '{', '}', '(', ')']; /** * Find matching project names given a list of potential project names or globs. @@ -166,7 +169,7 @@ function addMatchingProjectsByName( return; } - if (!globCharacters.some((c) => pattern.value.includes(c))) { + if (!GLOB_CHARACTERS.some((c) => pattern.value.includes(c))) { return; } @@ -201,7 +204,7 @@ function addMatchingProjectsByTag( continue; } - if (!globCharacters.some((c) => pattern.value.includes(c))) { + if (!GLOB_CHARACTERS.some((c) => pattern.value.includes(c))) { continue; }