From 527bf1a662fd2e27072c48a59901e466864d3ced Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Thu, 31 Aug 2023 20:20:59 -0700 Subject: [PATCH] Minor improvements to path include/exclude logic (#915) --- ...-4182291c-ee3b-455f-8d64-bf4213a49c76.json | 7 ++++ src/__tests__/monorepo/isPathIncluded.test.ts | 24 +++++++++---- src/monorepo/getScopedPackages.ts | 24 ++++++------- src/monorepo/isPathIncluded.ts | 36 +++++++++++++------ src/types/BeachballOptions.ts | 13 +++++-- src/types/ChangelogOptions.ts | 13 +++++-- 6 files changed, 80 insertions(+), 37 deletions(-) create mode 100644 change/beachball-4182291c-ee3b-455f-8d64-bf4213a49c76.json diff --git a/change/beachball-4182291c-ee3b-455f-8d64-bf4213a49c76.json b/change/beachball-4182291c-ee3b-455f-8d64-bf4213a49c76.json new file mode 100644 index 000000000..e3bc5ed33 --- /dev/null +++ b/change/beachball-4182291c-ee3b-455f-8d64-bf4213a49c76.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Minor improvements to path include/exclude logic", + "packageName": "beachball", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/src/__tests__/monorepo/isPathIncluded.test.ts b/src/__tests__/monorepo/isPathIncluded.test.ts index ebe653bd6..4dd37fba4 100644 --- a/src/__tests__/monorepo/isPathIncluded.test.ts +++ b/src/__tests__/monorepo/isPathIncluded.test.ts @@ -2,23 +2,35 @@ import { describe, expect, it } from '@jest/globals'; import { isPathIncluded } from '../../monorepo/isPathIncluded'; describe('isPathIncluded', () => { - it('should return true if path is included with single include path', () => { + it('returns true if path is included (single include path)', () => { expect(isPathIncluded('packages/a', 'packages/*')).toBeTruthy(); }); - it('should return false if path is excluded with single exclude path', () => { + it('returns false if path is excluded (single exclude path)', () => { expect(isPathIncluded('packages/a', 'packages/*', '!packages/a')).toBeFalsy(); }); - it('should return true if path is included with multiple include paths', () => { + it('returns true if path is included (multiple include paths)', () => { expect(isPathIncluded('packages/a', ['packages/b', 'packages/a'], ['!packages/b'])).toBeTruthy(); }); - it('should return false if path is excluded with multiple exclude paths', () => { - expect(isPathIncluded('packages/a', ['packages/*'], ['!packages/a'])).toBeFalsy(); + it('returns false if path is excluded (multiple exclude paths)', () => { + expect(isPathIncluded('packages/a', ['packages/*'], ['!packages/a', '!packages/b'])).toBeFalsy(); }); - it('should return false if include path is empty', () => { + it('returns true if include is true (no exclude paths)', () => { + expect(isPathIncluded('packages/a', true)).toBeTruthy(); + }); + + it('returns false if include is true and path is excluded', () => { + expect(isPathIncluded('packages/a', true, '!packages/a')).toBeFalsy(); + }); + + it('returns false if include path is empty', () => { expect(isPathIncluded('packages/a', '')).toBeFalsy(); }); + + it('ignores empty exclude path array', () => { + expect(isPathIncluded('packages/a', 'packages/*', [])).toBeTruthy(); + }); }); diff --git a/src/monorepo/getScopedPackages.ts b/src/monorepo/getScopedPackages.ts index 206c9dd1a..cc741669b 100644 --- a/src/monorepo/getScopedPackages.ts +++ b/src/monorepo/getScopedPackages.ts @@ -4,24 +4,20 @@ import path from 'path'; import { isPathIncluded } from './isPathIncluded'; export function getScopedPackages(options: BeachballOptions, packageInfos: PackageInfos): string[] { - if (!options.scope) { + const { scope, path: cwd } = options; + if (!scope) { return Object.keys(packageInfos); } - let includeScopes = options.scope!.filter(s => !s.startsWith('!')); - includeScopes = includeScopes.length > 0 ? includeScopes : ['**/*', '', '*']; - const excludeScopes = options.scope!.filter(s => s.startsWith('!')); + let includeScopes: string[] | true = scope.filter(s => !s.startsWith('!')); + // If there were no include scopes, include all paths by default + includeScopes = includeScopes.length ? includeScopes : true; - const scopedPackages: string[] = []; + const excludeScopes = scope.filter(s => s.startsWith('!')); - for (let [pkgName, info] of Object.entries(packageInfos)) { - const relativePath = path.relative(options.path, path.dirname(info.packageJsonPath)); + return Object.keys(packageInfos).filter(pkgName => { + const packagePath = path.dirname(packageInfos[pkgName].packageJsonPath); - const shouldInclude = isPathIncluded(relativePath, includeScopes, excludeScopes); - if (shouldInclude) { - scopedPackages.push(pkgName); - } - } - - return scopedPackages; + return isPathIncluded(path.relative(cwd, packagePath), includeScopes, excludeScopes); + }); } diff --git a/src/monorepo/isPathIncluded.ts b/src/monorepo/isPathIncluded.ts index 7503a4fe4..c7e5a7c67 100644 --- a/src/monorepo/isPathIncluded.ts +++ b/src/monorepo/isPathIncluded.ts @@ -2,20 +2,34 @@ import minimatch from 'minimatch'; /** * Check if a relative path should be included given include and exclude patterns using minimatch. + * @param relativePath Relative path to check. + * @param include Include pattern(s). If `true`, include all paths except those excluded. + * @param exclude Exclude pattern(s). Currently these must be **negated** patterns: + * e.g. if you want to exclude `packages/foo`, you must specify `exclude` as `!packages/foo`. + * (This will be fixed in a future major version.) */ -export function isPathIncluded(relativePath: string, include: string | string[], exclude?: string | string[]): boolean { - const includePatterns = typeof include === 'string' ? [include] : include; - let shouldInclude = includePatterns.reduce( - (included, pattern) => included || minimatch(relativePath, pattern), - false - ); +export function isPathIncluded( + relativePath: string, + include: string | string[] | true, + exclude?: string | string[] +): boolean { + let shouldInclude: boolean; + if (include === true) { + shouldInclude = true; + } else { + const includePatterns = typeof include === 'string' ? [include] : include; + shouldInclude = includePatterns.some(pattern => minimatch(relativePath, pattern)); + } - if (exclude) { + if (exclude?.length && shouldInclude) { + // TODO: this is weird/buggy--it assumes that exclude patterns are always negated, + // which intuitively (or comparing to other tools) is not how it should work. + // If this is fixed, updates will be needed in: + // - getScopedPackages() + // - ChangelogGroupOptions + // - VersionGroupOptions const excludePatterns = typeof exclude === 'string' ? [exclude] : exclude; - shouldInclude = excludePatterns.reduce( - (excluded: boolean, pattern: string) => excluded && minimatch(relativePath, pattern), - shouldInclude - ); + shouldInclude = excludePatterns.every(pattern => minimatch(relativePath, pattern)); } return shouldInclude; diff --git a/src/types/BeachballOptions.ts b/src/types/BeachballOptions.ts index 7290aca24..def99276c 100644 --- a/src/types/BeachballOptions.ts +++ b/src/types/BeachballOptions.ts @@ -179,10 +179,17 @@ export interface PackageOptions { * Options for bumping package versions together. */ export interface VersionGroupOptions { - /** minimatch pattern (or array of minimatch) to detect which packages should be included in this group */ - include: string | string[]; + /** + * minimatch pattern (or array of minimatch) to detect which packages should be included in this group. + * If `true`, include all packages except those excluded by `exclude`. + */ + include: string | string[] | true; - /** minimatch pattern (or array of minimatch) to detect which packages should be excluded in this group */ + /** + * minimatch pattern (or array of minimatch) to detect which packages should be excluded in this group. + * Currently this must use **negated patterns only**: e.g. if you want to exclude `packages/foo`, + * you must specify `exclude` as `!packages/foo`. (This will be fixed in a future major version.) + */ exclude?: string | string[]; disallowedChangeTypes: ChangeType[] | null; diff --git a/src/types/ChangelogOptions.ts b/src/types/ChangelogOptions.ts index 11d8d6b4e..8857a6cff 100644 --- a/src/types/ChangelogOptions.ts +++ b/src/types/ChangelogOptions.ts @@ -36,10 +36,17 @@ export interface ChangelogGroupOptions { */ masterPackageName: string; - /** minimatch pattern (or array of minimatch) to detect which packages should be included in this group */ - include: string | string[]; + /** + * minimatch pattern (or array of minimatch) to detect which packages should be included in this group. + * If `true`, include all packages except those excluded by `exclude`. + */ + include: string | string[] | true; - /** minimatch pattern (or array of minimatch) to detect which packages should be excluded in this group */ + /** + * minimatch pattern (or array of minimatch) to detect which packages should be excluded in this group. + * Currently this must use **negated patterns only**: e.g. if you want to exclude `packages/foo`, + * you must specify `exclude` as `!packages/foo`. (This will be fixed in a future major version.) + */ exclude?: string | string[]; changelogPath: string;