Update logic for getting package infos and options (#1022)

Author: ecraig12345

change/beachball-1290455d-876a-4e72-821c-6e0bc0750219.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "getPackageInfos should only get repo and CLI options once. Also clarify in types and logic that changeFilePrompt can't be specified at package level.",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

docs/overview/configuration.md

@@ -13,7 +13,7 @@ There are two types of configurations:
 1. repository config
 2. package config
 
-## Configuration files
+## Repository config
 
 `beachball` uses [`cosmiconfig`](https://github.com/davidtheclark/cosmiconfig) to read its configuration, so you can specify configuration in several ways (in addition to CLI arguments).
 
@@ -22,37 +22,48 @@ There are two types of configurations:
 - `.beachballrc.json`
 - `beachball.config.js` (CJS or ESM depending on your project setup; explicit `.cjs` or `.mjs` is also supported)
 
-### `beachball.config.js`
+It's most common to use a JavaScript file for the repo-level config, since it's the most flexible and allows comments. Usually this file is at the repo root.
 
-In many cases, you'll want to use a JavaScript config file, since this is the most flexible and allows comments. The example below uses JSDoc type annotations to enable intellisense in some editors (these are optional).
+The `beachball.config.js` example below uses JSDoc type annotations to enable intellisense in some editors (these are optional).
 
 ```js
 // @ts-check
 /** @type {import('beachball').BeachallConfig} */
 const config = {
-  key: value,
-  key2: value2
-  key3: value3
+  disallowedChangeTypes: ['major'],
+  changehint: 'Run "yarn change" to generate a change file',
+  groupChanges: true,
 };
 module.exports = config;
 ```
 
-Config files can be placed in either the root of a repo and/or within individual packages (package config overrides the repo config where applicable). For example:
+## Package config
+
+Package-level configuration is currently only supported under the `beachball` key in `package.json`.
+
+For example, suppose the repo config above is at `beachball.config.js` at the repo root, and there are these other files:
 
 ```
 packages/
   foo/
-    src/
     package.json
-    beachball.config.js
   bar/
-    src/
     package.json
-package.json
 beachball.config.js
+package.json
 ```
 
-It's also common to have a repo-level `beachball.config.js` and any individual package overrides (if they're simple) in the `"beachball"` key in the package's `package.json`.
+To change the `disallowedChangeTypes` for package `foo`, you could add the following to `packages/foo/package.json`:
+
+```json
+{
+  "name": "foo",
+  "version": "1.0.0",
+  "beachball": {
+    "disallowedChangeTypes": null
+  }
+}
+```
 
 ## Options
 

src/__e2e__/publishE2E.test.ts

@@ -483,7 +483,7 @@ describe('publish command (e2e)', () => {
     // All git results should still have previous information
     expect(repo.getCurrentTags()).toEqual(['foo_v1.1.0']);
     const manifestJson = fs.readFileSync(repo.pathTo('foo.txt'));
-    expect(manifestJson.toString()).toMatchInlineSnapshot(`"foo"`);
+    expect(manifestJson.toString()).toEqual('foo');
   });
 
   it('publishes multiple packages concurrently respecting the concurrency limit', async () => {

src/__fixtures__/registry.ts

@@ -4,7 +4,7 @@ import execa from 'execa';
 import fs from 'fs-extra';
 import getPort from 'get-port';
 import path from 'path';
-import { tmpdir } from './tmpdir';
+import { removeTempDir, tmpdir } from './tmpdir';
 
 const verdaccioUser = {
   username: 'fake',
@@ -86,13 +86,8 @@ export class Registry {
 
   /** Delete the temp directory used for the config file. */
   public cleanUp(): void {
-    try {
-      this.tempRoot && fs.removeSync(this.tempRoot);
-      this.tempRoot = undefined;
-    } catch {
-      // This can fail on Windows with EBUSY (likely due to the server not being fully shut down
-      // or all handles released or something). Just ignore it.
-    }
+    this.tempRoot && removeTempDir(this.tempRoot);
+    this.tempRoot = undefined;
   }
 
   private async startWithPort(port: number): Promise<void> {

src/__functional__/monorepo/getPackageInfos.test.ts

@@ -92,67 +92,63 @@ describe('getPackageInfos', () => {
     const repo = singleFactory.cloneRepository();
     let packageInfos = getPackageInfos(repo.rootPath);
     packageInfos = cleanPackageInfos(repo.rootPath, packageInfos);
-    expect(packageInfos).toMatchInlineSnapshot(`
-      {
-        "foo": {
-          "dependencies": {
-            "bar": "1.0.0",
-            "baz": "1.0.0",
-          },
-          "name": "foo",
-          "packageJsonPath": "package.json",
-          "private": false,
-          "version": "1.0.0",
+    expect(packageInfos).toEqual({
+      foo: {
+        dependencies: {
+          bar: '1.0.0',
+          baz: '1.0.0',
         },
-      }
-    `);
+        name: 'foo',
+        packageJsonPath: 'package.json',
+        private: false,
+        version: '1.0.0',
+      },
+    });
   });
 
   // both yarn and npm define "workspaces" in package.json
   it('works in yarn/npm monorepo', () => {
     const repo = monorepoFactory.cloneRepository();
     let packageInfos = getPackageInfos(repo.rootPath);
     packageInfos = cleanPackageInfos(repo.rootPath, packageInfos);
-    expect(packageInfos).toMatchInlineSnapshot(`
-      {
-        "a": {
-          "name": "a",
-          "packageJsonPath": "packages/grouped/a/package.json",
-          "private": false,
-          "version": "3.1.2",
-        },
-        "b": {
-          "name": "b",
-          "packageJsonPath": "packages/grouped/b/package.json",
-          "private": false,
-          "version": "3.1.2",
+    expect(packageInfos).toEqual({
+      a: {
+        name: 'a',
+        packageJsonPath: 'packages/grouped/a/package.json',
+        private: false,
+        version: '3.1.2',
+      },
+      b: {
+        name: 'b',
+        packageJsonPath: 'packages/grouped/b/package.json',
+        private: false,
+        version: '3.1.2',
+      },
+      bar: {
+        dependencies: {
+          baz: '^1.3.4',
         },
-        "bar": {
-          "dependencies": {
-            "baz": "^1.3.4",
-          },
-          "name": "bar",
-          "packageJsonPath": "packages/bar/package.json",
-          "private": false,
-          "version": "1.3.4",
+        name: 'bar',
+        packageJsonPath: 'packages/bar/package.json',
+        private: false,
+        version: '1.3.4',
+      },
+      baz: {
+        name: 'baz',
+        packageJsonPath: 'packages/baz/package.json',
+        private: false,
+        version: '1.3.4',
+      },
+      foo: {
+        dependencies: {
+          bar: '^1.3.4',
         },
-        "baz": {
-          "name": "baz",
-          "packageJsonPath": "packages/baz/package.json",
-          "private": false,
-          "version": "1.3.4",
-        },
-        "foo": {
-          "dependencies": {
-            "bar": "^1.3.4",
-          },
-          "name": "foo",
-          "packageJsonPath": "packages/foo/package.json",
-          "private": false,
-          "version": "1.0.0",
-        },
-      }
-    `);
+        name: 'foo',
+        packageJsonPath: 'packages/foo/package.json',
+        private: false,
+        version: '1.0.0',
+      },
+    });
   });
 
   it('works in pnpm monorepo', () => {
@@ -162,16 +158,14 @@ describe('getPackageInfos', () => {
     fs.writeFileSync(repo.pathTo('pnpm-workspace.yaml'), 'packages: ["packages/*", "packages/grouped/*"]');
 
     const rootPackageInfos = getPackageInfos(repo.rootPath);
-    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
-      {
-        "a": "packages/grouped/a/package.json",
-        "b": "packages/grouped/b/package.json",
-        "bar": "packages/bar/package.json",
-        "baz": "packages/baz/package.json",
-        "foo": "packages/foo/package.json",
-        "pnpm-monorepo": "package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toEqual({
+      a: 'packages/grouped/a/package.json',
+      b: 'packages/grouped/b/package.json',
+      bar: 'packages/bar/package.json',
+      baz: 'packages/baz/package.json',
+      foo: 'packages/foo/package.json',
+      'pnpm-monorepo': 'package.json',
+    });
   });
 
   it('works in rush monorepo', () => {
@@ -182,16 +176,14 @@ describe('getPackageInfos', () => {
     });
 
     const rootPackageInfos = getPackageInfos(repo.rootPath);
-    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
-      {
-        "a": "packages/grouped/a/package.json",
-        "b": "packages/grouped/b/package.json",
-        "bar": "packages/bar/package.json",
-        "baz": "packages/baz/package.json",
-        "foo": "packages/foo/package.json",
-        "rush-monorepo": "package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toEqual({
+      a: 'packages/grouped/a/package.json',
+      b: 'packages/grouped/b/package.json',
+      bar: 'packages/bar/package.json',
+      baz: 'packages/baz/package.json',
+      foo: 'packages/foo/package.json',
+      'rush-monorepo': 'package.json',
+    });
   });
 
   it('works in lerna monorepo', () => {
@@ -200,62 +192,54 @@ describe('getPackageInfos', () => {
     fs.writeJSONSync(repo.pathTo('lerna.json'), { packages: ['packages/*', 'packages/grouped/*'] });
 
     const rootPackageInfos = getPackageInfos(repo.rootPath);
-    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
-      {
-        "a": "packages/grouped/a/package.json",
-        "b": "packages/grouped/b/package.json",
-        "bar": "packages/bar/package.json",
-        "baz": "packages/baz/package.json",
-        "foo": "packages/foo/package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toEqual({
+      a: 'packages/grouped/a/package.json',
+      b: 'packages/grouped/b/package.json',
+      bar: 'packages/bar/package.json',
+      baz: 'packages/baz/package.json',
+      foo: 'packages/foo/package.json',
+    });
   });
 
   it('works multi-workspace monorepo', () => {
     const repo = multiWorkspaceFactory.cloneRepository();
 
     // For this test, only snapshot the package names and paths
     const rootPackageInfos = getPackageInfos(repo.rootPath);
-    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
-      {
-        "@workspace-a/a": "workspace-a/packages/grouped/a/package.json",
-        "@workspace-a/b": "workspace-a/packages/grouped/b/package.json",
-        "@workspace-a/bar": "workspace-a/packages/bar/package.json",
-        "@workspace-a/baz": "workspace-a/packages/baz/package.json",
-        "@workspace-a/foo": "workspace-a/packages/foo/package.json",
-        "@workspace-a/monorepo-fixture": "workspace-a/package.json",
-        "@workspace-b/a": "workspace-b/packages/grouped/a/package.json",
-        "@workspace-b/b": "workspace-b/packages/grouped/b/package.json",
-        "@workspace-b/bar": "workspace-b/packages/bar/package.json",
-        "@workspace-b/baz": "workspace-b/packages/baz/package.json",
-        "@workspace-b/foo": "workspace-b/packages/foo/package.json",
-        "@workspace-b/monorepo-fixture": "workspace-b/package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toEqual({
+      '@workspace-a/a': 'workspace-a/packages/grouped/a/package.json',
+      '@workspace-a/b': 'workspace-a/packages/grouped/b/package.json',
+      '@workspace-a/bar': 'workspace-a/packages/bar/package.json',
+      '@workspace-a/baz': 'workspace-a/packages/baz/package.json',
+      '@workspace-a/foo': 'workspace-a/packages/foo/package.json',
+      '@workspace-a/monorepo-fixture': 'workspace-a/package.json',
+      '@workspace-b/a': 'workspace-b/packages/grouped/a/package.json',
+      '@workspace-b/b': 'workspace-b/packages/grouped/b/package.json',
+      '@workspace-b/bar': 'workspace-b/packages/bar/package.json',
+      '@workspace-b/baz': 'workspace-b/packages/baz/package.json',
+      '@workspace-b/foo': 'workspace-b/packages/foo/package.json',
+      '@workspace-b/monorepo-fixture': 'workspace-b/package.json',
+    });
 
     const workspaceARoot = repo.pathTo('workspace-a');
     const packageInfosA = getPackageInfos(workspaceARoot);
-    expect(getPackageNamesAndPaths(workspaceARoot, packageInfosA)).toMatchInlineSnapshot(`
-      {
-        "@workspace-a/a": "packages/grouped/a/package.json",
-        "@workspace-a/b": "packages/grouped/b/package.json",
-        "@workspace-a/bar": "packages/bar/package.json",
-        "@workspace-a/baz": "packages/baz/package.json",
-        "@workspace-a/foo": "packages/foo/package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(workspaceARoot, packageInfosA)).toEqual({
+      '@workspace-a/a': 'packages/grouped/a/package.json',
+      '@workspace-a/b': 'packages/grouped/b/package.json',
+      '@workspace-a/bar': 'packages/bar/package.json',
+      '@workspace-a/baz': 'packages/baz/package.json',
+      '@workspace-a/foo': 'packages/foo/package.json',
+    });
 
     const workspaceBRoot = repo.pathTo('workspace-b');
     const packageInfosB = getPackageInfos(workspaceBRoot);
-    expect(getPackageNamesAndPaths(workspaceBRoot, packageInfosB)).toMatchInlineSnapshot(`
-      {
-        "@workspace-b/a": "packages/grouped/a/package.json",
-        "@workspace-b/b": "packages/grouped/b/package.json",
-        "@workspace-b/bar": "packages/bar/package.json",
-        "@workspace-b/baz": "packages/baz/package.json",
-        "@workspace-b/foo": "packages/foo/package.json",
-      }
-    `);
+    expect(getPackageNamesAndPaths(workspaceBRoot, packageInfosB)).toEqual({
+      '@workspace-b/a': 'packages/grouped/a/package.json',
+      '@workspace-b/b': 'packages/grouped/b/package.json',
+      '@workspace-b/bar': 'packages/bar/package.json',
+      '@workspace-b/baz': 'packages/baz/package.json',
+      '@workspace-b/foo': 'packages/foo/package.json',
+    });
   });
 
   it('throws if multiple packages have the same name in multi-workspace monorepo', () => {

src/__tests__/changefile/getQuestionsForPackage.test.ts

@@ -150,7 +150,7 @@ describe('getQuestionsForPackage', () => {
 
     const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
-      packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { changeFilePrompt: { changePrompt } } } }),
+      options: { ...defaultQuestionsParams.options, changeFilePrompt: { changePrompt } },
     });
 
     expect(questions).toEqual(customQuestions);

src/__tests__/changefile/promptForChange.test.ts

@@ -135,11 +135,8 @@ describe('promptForChange', () => {
     };
     const changeFilesPromise = promptForChange({
       ...defaultParams(),
+      options: { message: 'message', changeFilePrompt },
       changedPackages: ['foo', 'bar'],
-      packageInfos: makePackageInfos({
-        foo: { combinedOptions: { changeFilePrompt } },
-        bar: { combinedOptions: { changeFilePrompt } },
-      }),
     });
     await waitForPrompt();