From d2ffdb6ca361841c6117eed0df2cd7d59b0d3bca Mon Sep 17 00:00:00 2001 From: AgentEnder Date: Wed, 5 Apr 2023 14:30:52 -0400 Subject: [PATCH] chore(misc): apply review feedback to clean up findMatchingProjects --- docs/generated/cli/run-many.md | 2 +- .../angular/generators/application.json | 2 +- .../packages/nx/documents/run-many.md | 2 +- .../react/generators/application.json | 2 +- .../packages/web/generators/application.json | 2 +- .../recipes/generators/generator-options.md | 2 +- packages/nx/src/command-line/examples.ts | 2 +- .../implicit-project-dependencies.ts | 4 +- .../implict-project-dependencies.spec.ts | 16 +- .../build-nodes/workspace-projects.ts | 16 +- .../nx/src/utils/assert-workspace-validity.ts | 6 +- .../src/utils/find-matching-projects.spec.ts | 7 +- .../nx/src/utils/find-matching-projects.ts | 169 ++++++++++++------ 13 files changed, 147 insertions(+), 85 deletions(-) diff --git a/docs/generated/cli/run-many.md b/docs/generated/cli/run-many.md index b9e5a21bf53e8..91620395d8bac 100644 --- a/docs/generated/cli/run-many.md +++ b/docs/generated/cli/run-many.md @@ -47,7 +47,7 @@ Test all projects ending with `*-app` except `excluded-app`. Note: your shell ma nx run-many --target=test --projects=*-app --exclude=excluded-app ``` -Test all projects with tags starting with `api-*`. Note: your shell may require you to escape the `*` like this: `\*`: +Test all projects with tags starting with `api-`. Note: your shell may require you to escape the `*` like this: `\*`: ```shell nx run-many --target=test --projects=tag:api-* diff --git a/docs/generated/packages/angular/generators/application.json b/docs/generated/packages/angular/generators/application.json index ddc27ec116055..549762f8b30d0 100644 --- a/docs/generated/packages/angular/generators/application.json +++ b/docs/generated/packages/angular/generators/application.json @@ -165,7 +165,7 @@ }, "additionalProperties": false, "required": ["name"], - "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/angular:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory and style extension\" %}\n\nCreate an application named `my-app` in the `my-dir` directory and use `scss` for styles:\n\n```bash\nnx g @nrwl/angular:app my-app --directory=my-dir --style=scss\n```\n\n{% /tab %}\n\n{% tab label=\"Single File Components application\" %}\n\nCreate an application with Single File Components (inline styles and inline templates):\n\n```bash\nnx g @nrwl/angular:app my-app --inlineStyle --inlineTemplate\n```\n\n{% /tab %}\n\n{% tab label=\"Standalone Components application\" %}\n\nCreate an application that is setup to use standalone components:\n\n```bash\nnx g @nrwl/angular:app my-app --standalone\n```\n\n{% /tab %}\n\n{% tab label=\"Set custom prefix and tags\" %}\n\nSet the prefix to apply to generated selectors and add tags to the application.\n\n```bash\nnx g @nrwl/angular:app my-app --prefix=admin --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", + "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/angular:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory and style extension\" %}\n\nCreate an application named `my-app` in the `my-dir` directory and use `scss` for styles:\n\n```bash\nnx g @nrwl/angular:app my-app --directory=my-dir --style=scss\n```\n\n{% /tab %}\n\n{% tab label=\"Single File Components application\" %}\n\nCreate an application with Single File Components (inline styles and inline templates):\n\n```bash\nnx g @nrwl/angular:app my-app --inlineStyle --inlineTemplate\n```\n\n{% /tab %}\n\n{% tab label=\"Standalone Components application\" %}\n\nCreate an application that is setup to use standalone components:\n\n```bash\nnx g @nrwl/angular:app my-app --standalone\n```\n\n{% /tab %}\n\n{% tab label=\"Set custom prefix and tags\" %}\n\nSet the prefix to apply to generated selectors and add tags to the application (used for linting).\n\n```bash\nnx g @nrwl/angular:app my-app --prefix=admin --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", "presets": [] }, "aliases": ["app"], diff --git a/docs/generated/packages/nx/documents/run-many.md b/docs/generated/packages/nx/documents/run-many.md index b9e5a21bf53e8..91620395d8bac 100644 --- a/docs/generated/packages/nx/documents/run-many.md +++ b/docs/generated/packages/nx/documents/run-many.md @@ -47,7 +47,7 @@ Test all projects ending with `*-app` except `excluded-app`. Note: your shell ma nx run-many --target=test --projects=*-app --exclude=excluded-app ``` -Test all projects with tags starting with `api-*`. Note: your shell may require you to escape the `*` like this: `\*`: +Test all projects with tags starting with `api-`. Note: your shell may require you to escape the `*` like this: `\*`: ```shell nx run-many --target=test --projects=tag:api-* diff --git a/docs/generated/packages/react/generators/application.json b/docs/generated/packages/react/generators/application.json index e58345ab6d24c..c9813e4946715 100644 --- a/docs/generated/packages/react/generators/application.json +++ b/docs/generated/packages/react/generators/application.json @@ -192,7 +192,7 @@ } }, "required": [], - "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/react:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Application using Vite as bundler\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/react:app my-app --bundler=vite\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory and style extension\" %}\n\nCreate an application named `my-app` in the `my-dir` directory and use `scss` for styles:\n\n```bash\nnx g @nrwl/react:app my-app --directory=my-dir --style=scss\n```\n\n{% /tab %}\n\n{% tab label=\"Add tags\" %}\n\nAdd tags to the application.\n\n```bash\nnx g @nrwl/react:app my-app --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", + "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/react:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Application using Vite as bundler\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/react:app my-app --bundler=vite\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory and style extension\" %}\n\nCreate an application named `my-app` in the `my-dir` directory and use `scss` for styles:\n\n```bash\nnx g @nrwl/react:app my-app --directory=my-dir --style=scss\n```\n\n{% /tab %}\n\n{% tab label=\"Add tags\" %}\n\nAdd tags to the application (used for linting).\n\n```bash\nnx g @nrwl/react:app my-app --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", "presets": [] }, "aliases": ["app"], diff --git a/docs/generated/packages/web/generators/application.json b/docs/generated/packages/web/generators/application.json index 490f73fe8e317..5b2bc487bbd4e 100644 --- a/docs/generated/packages/web/generators/application.json +++ b/docs/generated/packages/web/generators/application.json @@ -104,7 +104,7 @@ } }, "required": ["name"], - "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/web:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Application using Vite as bundler\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/web:app my-app --bundler=vite\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory\" %}\n\nCreate an application named `my-app` in the `my-dir` directory:\n\n```bash\nnx g @nrwl/web:app my-app --directory=my-dir\n```\n\n{% /tab %}\n\n{% tab label=\"Add tags\" %}\n\nAdd tags to the application.\n\n```bash\nnx g @nrwl/web:app my-app --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", + "examplesFile": "## Examples\n\n{% tabs %}\n{% tab label=\"Simple Application\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/web:application my-app\n```\n\n{% /tab %}\n\n{% tab label=\"Application using Vite as bundler\" %}\n\nCreate an application named `my-app`:\n\n```bash\nnx g @nrwl/web:app my-app --bundler=vite\n```\n\n{% /tab %}\n\n{% tab label=\"Specify directory\" %}\n\nCreate an application named `my-app` in the `my-dir` directory:\n\n```bash\nnx g @nrwl/web:app my-app --directory=my-dir\n```\n\n{% /tab %}\n\n{% tab label=\"Add tags\" %}\n\nAdd tags to the application (used for linting).\n\n```bash\nnx g @nrwl/web:app my-app --tags=scope:admin,type:ui\n```\n\n{% /tab %}\n{% /tabs %}\n", "presets": [] }, "aliases": ["app"], diff --git a/docs/shared/recipes/generators/generator-options.md b/docs/shared/recipes/generators/generator-options.md index 29c6ba95a6f9c..09d0df483a5c3 100644 --- a/docs/shared/recipes/generators/generator-options.md +++ b/docs/shared/recipes/generators/generator-options.md @@ -533,7 +533,7 @@ The alias of this property. Example: { "tags": { "type": "string", - "description": "Add tags to the project", + "description": "Add tags to the project (used for linting)", "alias": "t" }, "directory": { diff --git a/packages/nx/src/command-line/examples.ts b/packages/nx/src/command-line/examples.ts index 9cf5538abb250..68977fc89d460 100644 --- a/packages/nx/src/command-line/examples.ts +++ b/packages/nx/src/command-line/examples.ts @@ -312,7 +312,7 @@ export const examples: Record = { { command: 'run-many --target=test --projects=tag:api-*', description: - 'Test all projects with tags starting with `api-*`. Note: your shell may require you to escape the `*` like this: `\\*`', + 'Test all projects with tags starting with `api-`. Note: your shell may require you to escape the `*` like this: `\\*`', }, { command: 'run-many --targets=lint,test,build --all', diff --git a/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts b/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts index 97e6409d725e5..371189c68f4e0 100644 --- a/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts +++ b/packages/nx/src/project-graph/build-dependencies/implicit-project-dependencies.ts @@ -5,8 +5,8 @@ export function buildImplicitProjectDependencies( ctx: ProjectGraphProcessorContext, builder: ProjectGraphBuilder ) { - Object.keys(ctx.workspace.projects).forEach((source) => { - const p = ctx.workspace.projects[source]; + Object.keys(ctx.projectsConfigurations.projects).forEach((source) => { + const p = ctx.projectsConfigurations.projects[source]; if (p.implicitDependencies && p.implicitDependencies.length > 0) { p.implicitDependencies.forEach((target) => { if (target.startsWith('!')) { diff --git a/packages/nx/src/project-graph/build-dependencies/implict-project-dependencies.spec.ts b/packages/nx/src/project-graph/build-dependencies/implict-project-dependencies.spec.ts index a63f2a9aa8cfa..c93cb5166a85e 100644 --- a/packages/nx/src/project-graph/build-dependencies/implict-project-dependencies.spec.ts +++ b/packages/nx/src/project-graph/build-dependencies/implict-project-dependencies.spec.ts @@ -8,8 +8,6 @@ jest.mock('nx/src/utils/workspace-root', () => ({ })); describe('explicit project dependencies', () => { - let ctx: ProjectGraphProcessorContext; - it(`should add implicit deps`, () => { const builder = new ProjectGraphBuilder(); builder.addNode({ @@ -25,12 +23,13 @@ describe('explicit project dependencies', () => { { filesToProcess: {}, fileMap: {}, - workspace: { + projectsConfigurations: { + version: 2, projects: { - proj1: { implicitDependencies: ['proj2'] }, + proj1: { root: '', implicitDependencies: ['proj2'] }, }, }, - } as any, + } as Partial as ProjectGraphProcessorContext, builder ); @@ -59,12 +58,13 @@ describe('explicit project dependencies', () => { { filesToProcess: {}, fileMap: {}, - workspace: { + projectsConfigurations: { + version: 2, projects: { - proj1: { implicitDependencies: ['!proj2'] }, + proj1: { root: '', implicitDependencies: ['!proj2'] }, }, }, - } as any, + } as Partial as ProjectGraphProcessorContext, builder ); diff --git a/packages/nx/src/project-graph/build-nodes/workspace-projects.ts b/packages/nx/src/project-graph/build-nodes/workspace-projects.ts index 595a94aa6a060..20aea0afa65f5 100644 --- a/packages/nx/src/project-graph/build-nodes/workspace-projects.ts +++ b/packages/nx/src/project-graph/build-nodes/workspace-projects.ts @@ -28,9 +28,11 @@ export async function buildWorkspaceProjectNodes( nxJson: NxJsonConfiguration ) { const toAdd = []; - const projects = Object.keys(ctx.workspace.projects); - const projectsGraph = projects.reduce((graph, project) => { - const projectConfiguration = ctx.workspace.projects[project]; + const projects = Object.keys(ctx.projectsConfigurations.projects); + + // Used for expanding implicit dependencies (e.g. `@proj/*` or `tag:foo`) + const partialProjectGraphNodes = projects.reduce((graph, project) => { + const projectConfiguration = ctx.projectsConfigurations.projects[project]; graph[project] = { name: project, type: projectConfiguration.projectType === 'library' ? 'lib' : 'app', // missing fallback to `e2e` @@ -43,7 +45,7 @@ export async function buildWorkspaceProjectNodes( }, {} as Record); for (const key of projects) { - const p = ctx.workspace.projects[key]; + const p = ctx.projectsConfigurations.projects[key]; const projectRoot = join(workspaceRoot, p.root); if (existsSync(join(projectRoot, 'package.json'))) { @@ -73,13 +75,13 @@ export async function buildWorkspaceProjectNodes( p.implicitDependencies = normalizeImplicitDependencies( key, p.implicitDependencies, - projectsGraph + partialProjectGraphNodes ); p.targets = mergePluginTargetsWithNxTargets( p.root, p.targets, - await loadNxPlugins(ctx.workspace.plugins) + await loadNxPlugins(ctx.nxJsonConfiguration.plugins) ); p.targets = normalizeProjectTargets(p, nxJson.targetDefaults, key); @@ -91,7 +93,7 @@ export async function buildWorkspaceProjectNodes( ? 'e2e' : 'app' : 'lib'; - const tags = ctx.workspace.projects?.[key]?.tags || []; + const tags = ctx.projectsConfigurations.projects?.[key]?.tags || []; toAdd.push({ name: key, diff --git a/packages/nx/src/utils/assert-workspace-validity.ts b/packages/nx/src/utils/assert-workspace-validity.ts index 7c5c947690cb5..cb57461a015ea 100644 --- a/packages/nx/src/utils/assert-workspace-validity.ts +++ b/packages/nx/src/utils/assert-workspace-validity.ts @@ -1,7 +1,7 @@ import { ProjectsConfigurations } from '../config/workspace-json-project-json'; import { NxJsonConfiguration } from '../config/nx-json'; import { findMatchingProjects } from './find-matching-projects'; -import {output} from './output'; +import { output } from './output'; import { ProjectGraphProjectNode } from '../config/project-graph'; export function assertWorkspaceValidity( @@ -9,7 +9,7 @@ export function assertWorkspaceValidity( nxJson: NxJsonConfiguration ) { const projectNames = Object.keys(projectsConfigurations.projects); - const projectsGraph = projectNames.reduce((graph, project) => { + const projectGraphNodes = projectNames.reduce((graph, project) => { const projectConfiguration = projectsConfigurations.projects[project]; graph[project] = { name: project, @@ -51,7 +51,7 @@ export function assertWorkspaceValidity( projectName, project.implicitDependencies, projects, - projectsGraph + projectGraphNodes ); return map; }, invalidImplicitDependencies); diff --git a/packages/nx/src/utils/find-matching-projects.spec.ts b/packages/nx/src/utils/find-matching-projects.spec.ts index 2794d091a06a8..ebfeb7af2c878 100644 --- a/packages/nx/src/utils/find-matching-projects.spec.ts +++ b/packages/nx/src/utils/find-matching-projects.spec.ts @@ -1,5 +1,5 @@ import { findMatchingProjects } from './find-matching-projects'; -import { type ProjectGraphProjectNode } from '../config/project-graph'; +import type { ProjectGraphProjectNode } from '../config/project-graph'; describe('findMatchingProjects', () => { let projectGraph: Record = { @@ -94,7 +94,7 @@ describe('findMatchingProjects', () => { }); it('should expand "*" for tags', () => { - expect(findMatchingProjects(['tags:*'], projectGraph)).toEqual([ + expect(findMatchingProjects(['tag:*'], projectGraph)).toEqual([ 'test-project', 'a', 'b', @@ -103,9 +103,6 @@ describe('findMatchingProjects', () => { }); it('should support negation "!" for tags', () => { - expect(findMatchingProjects(['*', 'tag:!api'], projectGraph)).toEqual([ - 'b', - ]); expect(findMatchingProjects(['*', '!tag:api'], projectGraph)).toEqual([ 'b', ]); diff --git a/packages/nx/src/utils/find-matching-projects.ts b/packages/nx/src/utils/find-matching-projects.ts index 846e8607bf92a..686762fb24aa4 100644 --- a/packages/nx/src/utils/find-matching-projects.ts +++ b/packages/nx/src/utils/find-matching-projects.ts @@ -1,8 +1,23 @@ import minimatch = require('minimatch'); -import { type ProjectGraphProjectNode } from '../config/project-graph'; +import type { ProjectGraphProjectNode } from '../config/project-graph'; const globCharacters = ['*', '|', '{', '}', '(', ')']; +const validPatternTypes = [ + 'name', // Pattern is based on the project's name + 'tag', // Pattern is based on the project's tags +] as const; +type ProjectPatternType = typeof validPatternTypes[number]; + +interface ProjectPattern { + // If true, the pattern is an exclude pattern + exclude: boolean; + // The type of pattern to match against + type: ProjectPatternType; + // The pattern to match against + value: string; +} + /** * Find matching project names given a list of potential project names or globs. * @@ -16,81 +31,72 @@ export function findMatchingProjects( | Record | Map ): string[] { - const projectObject = - projects instanceof Map ? Object.fromEntries(projects) : projects; - const projectNames = Object.keys(projectObject); - const patternObjects = patterns.map((pattern) => { - let isExclude = false; - if (pattern.startsWith('!')) { - isExclude = true; - pattern = pattern.substring(1); - } - let [value, type] = pattern.split(':').reverse(); - if (value.startsWith('!')) { - isExclude ||= true; - value = value.substring(1); - } - return { - not: isExclude, - type: type, - value, - }; - }); + const projectNames = keys(projects); + + const patternObjects: ProjectPattern[] = patterns.map((p) => + parseStringPattern(p, projects) + ); const selectedProjects: Set = new Set(); const excludedProjects: Set = new Set(); - for (const patternObject of patternObjects) { - if (patternObject.value === '*') { - projectNames.every((projectName) => - (patternObject.not ? excludedProjects : selectedProjects).add( - projectName - ) - ); - if (patternObjects.length === 1) continue; + for (const pattern of patternObjects) { + // Handle wildcard with short-circuit, as its a common case with potentially + // large project sets and we can avoid the more expensive glob matching. + if (pattern.value === '*') { + for (const projectName of projectNames) { + if (pattern.exclude) { + excludedProjects.add(projectName); + } else { + selectedProjects.add(projectName); + } + } + continue; } - if (patternObject.type === 'tag') { + if (pattern.type === 'tag') { for (const projectName of projectNames) { - const tags = projectObject[projectName].data.tags || []; + const tags = + getItemInMapOrRecord(projects, projectName).data.tags || []; - if (tags.includes(patternObject.value)) { - (patternObject.not ? excludedProjects : selectedProjects).add( + if (tags.includes(pattern.value)) { + (pattern.exclude ? excludedProjects : selectedProjects).add( projectName ); continue; } - if (!globCharacters.some((c) => patternObject.value.includes(c))) { + if (!globCharacters.some((c) => pattern.value.includes(c))) { continue; } - if (minimatch.match(tags, patternObject.value).length) - (patternObject.not ? excludedProjects : selectedProjects).add( + if (minimatch.match(tags, pattern.value).length) + (pattern.exclude ? excludedProjects : selectedProjects).add( projectName ); } continue; - } + } else if (pattern.type === 'name') { + if (hasKey(projects, pattern.value)) { + (pattern.exclude ? excludedProjects : selectedProjects).add( + pattern.value + ); + continue; + } - if (projectNames.includes(patternObject.value)) { - (patternObject.not ? excludedProjects : selectedProjects).add( - patternObject.value - ); - continue; - } + if (!globCharacters.some((c) => pattern.value.includes(c))) { + continue; + } - if (!globCharacters.some((c) => patternObject.value.includes(c))) { - continue; + const matchedProjectNames = minimatch.match(projectNames, pattern.value); + for (const projectName of matchedProjectNames) { + if (pattern.exclude) { + excludedProjects.add(projectName); + } else { + selectedProjects.add(projectName); + } + } } - - const matchedProjectNames = minimatch.match( - projectNames, - patternObject.value - ); - matchedProjectNames.every((projectName) => - (patternObject.not ? excludedProjects : selectedProjects).add(projectName) - ); } for (const project of excludedProjects) { @@ -99,3 +105,60 @@ export function findMatchingProjects( return Array.from(selectedProjects); } + +function keys( + object: Record | Map +): string[] { + return object instanceof Map ? [...object.keys()] : Object.keys(object); +} + +function hasKey( + object: Record | Map, + key: string +) { + return object instanceof Map ? object.has(key) : key in object; +} + +function getItemInMapOrRecord( + object: Record | Map, + key: string +): T { + return object instanceof Map ? object.get(key) : object[key]; +} + +function parseStringPattern( + pattern: string, + projects: + | Map + | Record +): ProjectPattern { + let type: ProjectPatternType; + let value: string; + const isExclude = pattern.startsWith('!'); + + // Support for things like: `!{type}:value` + if (isExclude) { + pattern = pattern.substring(1); + } + + const indexOfFirstPotentialSeparator = pattern.indexOf(':'); + if (indexOfFirstPotentialSeparator === -1 || hasKey(projects, pattern)) { + type = 'name'; + value = pattern; + } else { + const potentialType = pattern.substring(0, indexOfFirstPotentialSeparator); + if (isValidPatternType(potentialType)) { + type = potentialType; + value = pattern.substring(indexOfFirstPotentialSeparator + 1); + } else { + type = 'name'; + value = pattern; + } + } + + return { type, value, exclude: isExclude }; +} + +function isValidPatternType(type: string): type is ProjectPatternType { + return validPatternTypes.includes(type as ProjectPatternType); +}