[breaking] Fix behavior of VersionGroupOptions.exclude (see #916)

change/beachball-9c18f781-7c73-4b14-adb7-a5fc6930628a.json

@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "[BREAKING] Fix behavior of `VersionGroupOptions.exclude`: if a package path **matches** any `exclude` pattern, it's excluded, rather than requiring negation. (To migrate, just remove the leading `!` from your `exclude` patterns.)",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

src/__tests__/monorepo/isPathIncluded.test.ts

@@ -3,34 +3,43 @@ import { isPathIncluded } from '../../monorepo/isPathIncluded';
 
 describe('isPathIncluded', () => {
   it('returns true if path is included (single include path)', () => {
-    expect(isPathIncluded('packages/a', 'packages/*')).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*' })).toBeTruthy();
   });
 
-  it('returns false if path is excluded (single exclude path)', () => {
-    expect(isPathIncluded('packages/a', 'packages/*', '!packages/a')).toBeFalsy();
+  it('returns false if path is not included, with single include path', () => {
+    expect(isPathIncluded({ relativePath: 'stuff/b', include: 'packages/*' })).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/b', include: 'packages/!(b)' })).toBeFalsy();
   });
 
-  it('returns true if path is included (multiple include paths)', () => {
-    expect(isPathIncluded('packages/a', ['packages/b', 'packages/a'], ['!packages/b'])).toBeTruthy();
+  it('returns false if path is excluded, with single exclude path', () => {
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*', exclude: 'packages/a' })).toBeFalsy();
   });
 
-  it('returns false if path is excluded (multiple exclude paths)', () => {
-    expect(isPathIncluded('packages/a', ['packages/*'], ['!packages/a', '!packages/b'])).toBeFalsy();
+  it('returns true if path is included, with multiple include paths', () => {
+    expect(
+      isPathIncluded({ relativePath: 'packages/a', include: ['packages/b', 'packages/a'], exclude: ['packages/b'] })
+    ).toBeTruthy();
+  });
+
+  it('returns false if path is excluded, with multiple exclude paths', () => {
+    expect(
+      isPathIncluded({ relativePath: 'packages/a', include: ['packages/*'], exclude: ['packages/a'] })
+    ).toBeFalsy();
   });
 
   it('returns true if include is true (no exclude paths)', () => {
-    expect(isPathIncluded('packages/a', true)).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: true })).toBeTruthy();
   });
 
   it('returns false if include is true and path is excluded', () => {
-    expect(isPathIncluded('packages/a', true, '!packages/a')).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: true, exclude: 'packages/a' })).toBeFalsy();
   });
 
   it('returns false if include path is empty', () => {
-    expect(isPathIncluded('packages/a', '')).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: '' })).toBeFalsy();
   });
 
   it('ignores empty exclude path array', () => {
-    expect(isPathIncluded('packages/a', 'packages/*', [])).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*', exclude: [] })).toBeTruthy();
   });
 });

src/changelog/writeChangelog.ts

@@ -85,7 +85,7 @@ async function writeGroupedChangelog(
     const relativePath = path.relative(options.path, packagePath);
 
     for (const group of changelogGroups) {
-      const isInGroup = isPathIncluded(relativePath, group.include, group.exclude);
+      const isInGroup = isPathIncluded({ relativePath, include: group.include, exclude: group.exclude });
       if (isInGroup) {
         groupedChangelogs[group.changelogAbsDir].changelogs.push(changelogs[pkg]);
       }

src/monorepo/getPackageGroups.ts

@@ -23,7 +23,9 @@ export function getPackageGroups(
     const packagePath = path.dirname(info.packageJsonPath);
     const relativePath = path.relative(root, packagePath);
 
-    const groupsForPkg = groups.filter(group => isPathIncluded(relativePath, group.include, group.exclude));
+    const groupsForPkg = groups.filter(group =>
+      isPathIncluded({ relativePath, include: group.include, exclude: group.exclude })
+    );
     if (groupsForPkg.length > 1) {
       // Keep going after this error to ensure we report all errors
       errorPackages[pkgName] = groupsForPkg;

src/monorepo/getScopedPackages.ts

@@ -16,11 +16,15 @@ export function getScopedPackages(
   // If there were no include scopes, include all paths by default
   includeScopes = includeScopes.length ? includeScopes : true;
 
-  const excludeScopes = scope.filter(s => s.startsWith('!'));
+  const excludeScopes = scope.filter(s => s.startsWith('!')).map(s => s.slice(1));
 
   return Object.keys(packageInfos).filter(pkgName => {
     const packagePath = path.dirname(packageInfos[pkgName].packageJsonPath);
 
-    return isPathIncluded(path.relative(cwd, packagePath), includeScopes, excludeScopes);
+    return isPathIncluded({
+      relativePath: path.relative(cwd, packagePath),
+      include: includeScopes,
+      exclude: excludeScopes,
+    });
   });
 }

src/monorepo/isPathIncluded.ts

@@ -8,11 +8,13 @@ import minimatch from 'minimatch';
  * 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[] | true,
-  exclude?: string | string[]
-): boolean {
+export function isPathIncluded(params: {
+  relativePath: string;
+  include: string | string[] | true;
+  exclude?: string | string[];
+}): boolean {
+  const { relativePath, include, exclude } = params;
+
   let shouldInclude: boolean;
   if (include === true) {
     shouldInclude = true;
@@ -22,14 +24,8 @@ export function isPathIncluded(
   }
 
   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.every(pattern => minimatch(relativePath, pattern));
+    shouldInclude = !excludePatterns.some(pattern => minimatch(relativePath, pattern));
   }
 
   return shouldInclude;

src/types/BeachballOptions.ts

@@ -196,15 +196,13 @@ export interface PackageOptions {
  */
 export interface VersionGroupOptions {
   /**
-   * 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`.
+   * minimatch pattern (or array of minimatch) for packages names to include in this group.
+   * If `true`, include all packages except those matching `exclude`.
    */
   include: string | string[] | true;
 
   /**
-   * 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.)
+   * minimatch pattern (or array of minimatch) for packages names to exclude from this group.
    */
   exclude?: string | string[];
 

src/types/ChangelogOptions.ts

@@ -66,15 +66,13 @@ export interface ChangelogGroupOptions {
   masterPackageName: 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`.
+   * minimatch pattern (or array of minimatch) for packages names to include in this group.
+   * If `true`, include all packages except those matching `exclude`.
    */
   include: string | string[] | true;
 
   /**
-   * 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.)
+   * minimatch pattern (or array of minimatch) for packages names to exclude from this group.
    */
   exclude?: string | string[];