From 081155f91f1c382a667399d6298d0cbe9bb53712 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 | 68 ++---- packages/nx/src/tasks-runner/utils.spec.ts | 163 +++++++++----- packages/nx/src/tasks-runner/utils.ts | 199 +++++++++++++----- .../nx/src/utils/find-matching-projects.ts | 9 +- 5 files changed, 291 insertions(+), 159 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 17051b7aab7c2..2593475c2e871 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 9035be7940b7f..8989476b4b584 100644 --- a/packages/nx/src/tasks-runner/create-task-graph.ts +++ b/packages/nx/src/tasks-runner/create-task-graph.ts @@ -7,18 +7,27 @@ import { import { Task, TaskGraph } from '../config/task-graph'; import { TargetDefaults, TargetDependencies } from '../config/nx-json'; import { TargetDependencyConfig } from '../devkit-exports'; -import { findMatchingProjects } from '../utils/find-matching-projects'; import { output } from '../utils/output'; 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 +109,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 = @@ -108,40 +118,7 @@ export class ProcessTasks { ? overrides : { __overrides_unparsed__: [] }; if (dependencyConfig.projects) { - /** 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 (typeof dependencyConfig.projects === 'string') { - if (dependencyConfig.projects === 'self') { - this.processTasksForSingleProject( - task, - task.target.project, - dependencyConfig, - configuration, - taskOverrides, - overrides - ); - continue; - } else if (dependencyConfig.projects === 'dependencies') { - this.processTasksForDependencies( - projectUsedToDeriveDependencies, - dependencyConfig, - configuration, - task, - taskOverrides, - overrides - ); - continue; - } - } - /** LERNA SUPPORT END - Remove in v17 */ - - this.processTasksForMatchingProjects( + this.processTasksForMultipleProjects( dependencyConfig, configuration, task, @@ -170,31 +147,22 @@ export class ProcessTasks { } } - private processTasksForMatchingProjects( + private processTasksForMultipleProjects( dependencyConfig: TargetDependencyConfig, configuration: string, task: Task, taskOverrides: Object | { __overrides_unparsed__: any[] }, overrides: Object ) { - const targetProjectSpecifiers = - typeof dependencyConfig.projects === 'string' - ? [dependencyConfig.projects] - : dependencyConfig.projects; - const matchingProjects = findMatchingProjects( - targetProjectSpecifiers, - this.projectGraph.nodes - ); - - if (matchingProjects.length === 0) { + if (dependencyConfig.projects.length === 0) { output.warn({ title: `\`dependsOn\` is misconfigured for ${task.target.project}:${task.target.target}`, bodyLines: [ - `Project patterns "${targetProjectSpecifiers}" does not match any projects.`, + `Project patterns "${dependencyConfig.projects}" does not match any projects.`, ], }); } - for (const projectName of matchingProjects) { + for (const projectName of dependencyConfig.projects) { this.processTasksForSingleProject( task, projectName, diff --git a/packages/nx/src/tasks-runner/utils.spec.ts b/packages/nx/src/tasks-runner/utils.spec.ts index 4c945e9dcd9bf..9c69104c6667b 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, @@ -408,6 +409,7 @@ describe('utils', () => { const result = transformLegacyOutputs('myapp', e); expect(result).toEqual(['{workspaceRoot}/dist']); } + expect.assertions(1); }); it('should prefix unix-absolute paths with {workspaceRoot}', () => { @@ -418,6 +420,7 @@ describe('utils', () => { const result = transformLegacyOutputs('myapp', e); expect(result).toEqual(['{workspaceRoot}/dist']); } + expect.assertions(1); }); }); @@ -429,6 +432,7 @@ describe('utils', () => { const result = transformLegacyOutputs('myapp', e); expect(result).toEqual(['{workspaceRoot}/dist']); } + expect.assertions(1); }); it('should prefix paths within the project with {projectRoot}', () => { @@ -439,23 +443,22 @@ describe('utils', () => { const result = transformLegacyOutputs('myapp', e); expect(result).toEqual(['{projectRoot}/dist']); } + expect.assertions(1); }); describe('expandDependencyConfigSyntaxSugar', () => { it('should expand syntax for simple target names', () => { - const result = expandDependencyConfigSyntaxSugar('build', 'any', { + const result = expandDependencyConfigSyntaxSugar('build', { dependencies: {}, nodes: {}, }); - expect(result).toEqual([ - { - target: 'build', - }, - ]); + expect(result).toEqual({ + target: 'build', + }); }); 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: { @@ -467,28 +470,24 @@ describe('utils', () => { }, }, }); - expect(result).toEqual([ - { - target: 'build', - }, - ]); + expect(result).toEqual({ + target: 'build', + }); }); it('should expand syntax for simple target names targetting dependencies', () => { - const result = expandDependencyConfigSyntaxSugar('^build', 'any', { + const result = expandDependencyConfigSyntaxSugar('^build', { dependencies: {}, nodes: {}, }); - expect(result).toEqual([ - { - target: 'build', - dependencies: true, - }, - ]); + expect(result).toEqual({ + target: 'build', + dependencies: true, + }); }); 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: { @@ -500,57 +499,123 @@ describe('utils', () => { }, }, }); - expect(result).toEqual([ - { - target: 'build', - projects: ['project'], - }, - ]); + expect(result).toEqual({ + target: 'build', + projects: ['project'], + }); }); it('should expand syntax for strings like target:with:colons', () => { - const result = expandDependencyConfigSyntaxSugar( - 'target:with:colons', - 'any', + const result = expandDependencyConfigSyntaxSugar('target:with:colons', { + dependencies: {}, + nodes: {}, + }); + expect(result).toEqual({ + target: 'target:with:colons', + }); + }); + + it('supports wildcards in targets', () => { + const result = getDependencyConfigs( + { project: 'project', target: 'build' }, + {}, { dependencies: {}, - nodes: {}, - } + 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: 'target:with:colons', + target: 'build-css', + projects: ['project'], + }, + { + target: 'build-js', + projects: ['project'], }, ]); }); - 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': {}, + 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', - projects: ['project'], + dependencies: true, }, { target: 'build-js', - projects: ['project'], + dependencies: true, + }, + { + target: 'build-python', + dependencies: true, }, ]); }); diff --git a/packages/nx/src/tasks-runner/utils.ts b/packages/nx/src/tasks-runner/utils.ts index f59b0c0746cf7..2d7534cdc1191 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,75 @@ 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 }; +} + +// Weakmap let's the cache get cleared by garbage collector if allTargetNames is no longer used +const patternResultCache = new WeakMap< + string[], + // Map< Pattern, Dependency Configs > + Map +>(); + +export function expandWildcardTargetConfiguration( + dependencyConfig: NormalizedTargetDependencyConfig, + allTargetNames: string[] +): NormalizedTargetDependencyConfig[] { + if (!GLOB_CHARACTERS.some((char) => dependencyConfig.target.includes(char))) { + return [dependencyConfig]; + } + let cache = patternResultCache.get(allTargetNames); + if (!cache) { + cache = new Map(); + patternResultCache.set(allTargetNames, cache); + } + const cachedResult = cache.get(dependencyConfig.target); + if (cachedResult) { + return cachedResult; + } + + const matcher = minimatch.filter(dependencyConfig.target); + + const matchingTargets = allTargetNames.filter((t) => matcher(t)); + + const result = matchingTargets.map((t) => ({ + ...dependencyConfig, + target: t, + })); + cache.set(dependencyConfig.target, result); + return result; +} + +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; - } - - // 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: targetString }; } - - return [ - { - target, - projects, - }, - ]; } export function getOutputs( @@ -121,6 +180,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 497e2ab13ee1a..de76ec1e39294 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; }