Minor improvements to path include/exclude logic (#915)

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": "[email protected]",
+  "dependentChangeType": "patch"
+}

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();
+  });
 });

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

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;

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;

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;