Simplify getRepoOptions logic

Author: ecraig12345

change/beachball-ec545b44-2909-47a7-b1b2-40c4406eb1ba.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Simplify getRepoOptions logic",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

src/__functional__/options/getOptions.test.ts

@@ -1,44 +1,87 @@
-import { describe, expect, it, beforeAll, afterEach, afterAll } from '@jest/globals';
+import { describe, expect, it, beforeAll, afterAll, jest } from '@jest/globals';
 import fs from 'fs-extra';
-import type { Repository } from '../../__fixtures__/repository';
 import { RepositoryFactory } from '../../__fixtures__/repositoryFactory';
 import { getOptions } from '../../options/getOptions';
+import type { RepoOptions } from '../../types/BeachballOptions';
 
-const baseArgv = ['node.exe', 'bin.js'];
+// Return a new object each time since getRepoOptions caches the result based on object identity.
+const baseArgv = () => ['node.exe', 'bin.js'];
 
 describe('getOptions', () => {
   let repositoryFactory: RepositoryFactory;
-  let repo: Repository;
+  // Don't reuse a repo in these tests! If multiple tests load beachball.config.js from the same path,
+  // it will use the version from the require cache, which will have outdated contents.
 
   beforeAll(() => {
     repositoryFactory = new RepositoryFactory('single');
-    repo = repositoryFactory.cloneRepository();
-  });
-
-  afterEach(() => {
-    repo.git(['clean', '-fdx']);
+    jest.spyOn(console, 'log').mockImplementation(() => {});
   });
 
   afterAll(() => {
     repositoryFactory.cleanUp();
+    jest.restoreAllMocks();
   });
 
   it('uses the branch name defined in beachball.config.js', () => {
+    const repo = repositoryFactory.cloneRepository();
     const config = inDirectory(repo.rootPath, () => {
       fs.writeFileSync('beachball.config.js', 'module.exports = { branch: "origin/foo" };');
-      return getOptions(baseArgv);
+      return getOptions(baseArgv());
+    });
+    expect(config.branch).toEqual('origin/foo');
+  });
+
+  it('reads config from package.json', () => {
+    const repo = repositoryFactory.cloneRepository();
+    const config = inDirectory(repo.rootPath, () => {
+      fs.writeJsonSync('package.json', { beachball: { branch: 'origin/foo' } });
+      return getOptions(baseArgv());
+    });
+    expect(config.branch).toEqual('origin/foo');
+  });
+
+  it('finds a .beachballrc.json file', () => {
+    const repo = repositoryFactory.cloneRepository();
+    const config = inDirectory(repo.rootPath, () => {
+      fs.writeJsonSync('.beachballrc.json', { branch: 'origin/foo' });
+      return getOptions(baseArgv());
     });
     expect(config.branch).toEqual('origin/foo');
   });
 
   it('--config overrides configuration path', () => {
+    const repo = repositoryFactory.cloneRepository();
     const config = inDirectory(repo.rootPath, () => {
       fs.writeFileSync('beachball.config.js', 'module.exports = { branch: "origin/main" };');
       fs.writeFileSync('alternate.config.js', 'module.exports = { branch: "origin/foo" };');
-      return getOptions([...baseArgv, '--config', 'alternate.config.js']);
+      return getOptions([...baseArgv(), '--config', 'alternate.config.js']);
     });
     expect(config.branch).toEqual('origin/foo');
   });
+
+  it('merges options including objects', () => {
+    const repo = repositoryFactory.cloneRepository();
+    // Ideally this test should include nested objects from multiple sources, but as of writing,
+    // the only place that can have nested objects is the repo options.
+    const repoOptions: Partial<RepoOptions> = {
+      access: 'public',
+      publish: false,
+      disallowedChangeTypes: null,
+      changelog: {
+        groups: [{ masterPackageName: 'foo', include: ['foo'], changelogPath: '.' }],
+      },
+    };
+    const config = inDirectory(repo.rootPath, () => {
+      fs.writeFileSync('beachball.config.js', `module.exports = ${JSON.stringify(repoOptions)};`);
+      return getOptions([...baseArgv(), '--disallowed-change-types', 'patch']);
+    });
+    expect(config).toMatchObject({
+      access: 'public',
+      publish: false,
+      disallowedChangeTypes: ['patch'],
+    });
+    expect(config.changelog).toEqual(repoOptions.changelog);
+  });
 });
 
 const inDirectory = <T>(directory: string, cb: () => T): T => {

src/options/getRepoOptions.ts

@@ -3,23 +3,26 @@ import { getDefaultRemoteBranch } from 'workspace-tools';
 import { env } from '../env';
 import type { RepoOptions, CliOptions, BeachballOptions } from '../types/BeachballOptions';
 
-const cachedRepoOptions = new Map<CliOptions, RepoOptions>();
+const cachedRepoOptions = new Map<CliOptions, Partial<RepoOptions>>();
 
-export function getRepoOptions(cliOptions: CliOptions): RepoOptions {
+export function getRepoOptions(cliOptions: CliOptions): Partial<RepoOptions> {
   const { configPath, path: cwd, branch } = cliOptions;
   if (!env.beachballDisableCache && cachedRepoOptions.has(cliOptions)) {
     return cachedRepoOptions.get(cliOptions)!;
   }
 
-  let repoOptions: RepoOptions | null;
+  let repoOptions: Partial<RepoOptions> | null | undefined;
+
+  const configExplorer = cosmiconfigSync('beachball', { cache: false });
+
   if (configPath) {
-    repoOptions = tryLoadConfig(configPath);
+    repoOptions = configExplorer.load(configPath)?.config as Partial<RepoOptions> | undefined;
     if (!repoOptions) {
       console.error(`Config file "${configPath}" could not be loaded`);
       process.exit(1);
     }
   } else {
-    repoOptions = trySearchConfig() || ({} as RepoOptions);
+    repoOptions = (configExplorer.search()?.config as Partial<RepoOptions> | undefined) || {};
   }
 
   // Only if the branch isn't specified in cliOptions (which takes precedence), fix it up or add it
@@ -41,15 +44,3 @@ export function getRepoOptions(cliOptions: CliOptions): RepoOptions {
 
   return repoOptions;
 }
-
-function tryLoadConfig(configPath: string): RepoOptions | null {
-  const configExplorer = cosmiconfigSync('beachball');
-  const loadResults = configExplorer.load(configPath);
-  return (loadResults?.config as RepoOptions) || null;
-}
-
-function trySearchConfig(): RepoOptions | null {
-  const configExplorer = cosmiconfigSync('beachball');
-  const searchResults = configExplorer.search();
-  return (searchResults?.config as RepoOptions) || null;
-}