diff --git a/change/beachball-ec545b44-2909-47a7-b1b2-40c4406eb1ba.json b/change/beachball-ec545b44-2909-47a7-b1b2-40c4406eb1ba.json new file mode 100644 index 00000000..e2ab0523 --- /dev/null +++ b/change/beachball-ec545b44-2909-47a7-b1b2-40c4406eb1ba.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Simplify getRepoOptions logic", + "packageName": "beachball", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/src/__functional__/options/getOptions.test.ts b/src/__functional__/options/getOptions.test.ts index e65cd270..a6fbed7f 100644 --- a/src/__functional__/options/getOptions.test.ts +++ b/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 = { + 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 = (directory: string, cb: () => T): T => { diff --git a/src/options/getRepoOptions.ts b/src/options/getRepoOptions.ts index 2bb865b0..64aaeedd 100644 --- a/src/options/getRepoOptions.ts +++ b/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(); +const cachedRepoOptions = new Map>(); -export function getRepoOptions(cliOptions: CliOptions): RepoOptions { +export function getRepoOptions(cliOptions: CliOptions): Partial { const { configPath, path: cwd, branch } = cliOptions; if (!env.beachballDisableCache && cachedRepoOptions.has(cliOptions)) { return cachedRepoOptions.get(cliOptions)!; } - let repoOptions: RepoOptions | null; + let repoOptions: Partial | null | undefined; + + const configExplorer = cosmiconfigSync('beachball', { cache: false }); + if (configPath) { - repoOptions = tryLoadConfig(configPath); + repoOptions = configExplorer.load(configPath)?.config as Partial | 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 | 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; -}