Publish config type updates, and performPublishConfig tests

change/beachball-b0d4d413-19f2-4277-8fe3-4a6a80933ad2.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Add type for PublishConfig",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

src/__fixtures__/packageInfos.ts

@@ -1,22 +1,33 @@
+import _ from 'lodash';
 import { BeachballOptions } from '../types/BeachballOptions';
 import { PackageInfo, PackageInfos } from '../types/PackageInfo';
 
 /**
- * Makes a properly typed PackageInfos object from a partial object, filling in the `name`,
- * `version` 1.0.0, and an empty `combinedOptions` object. (Other properties are not set, but this
- * at least makes the fixture code a bit more concise and ensures that any properties provided in
- * the override object are valid.)
+ * Makes a properly typed PackageInfos object from a partial object, filling in defaults:
+ * ```
+ * {
+ *   name: '<key>',
+ *   version: '1.0.0',
+ *   private: false,
+ *   combinedOptions: {},
+ *   packageOptions: {},
+ *   packageJsonPath: ''
+ * }
+ * ```
  */
 export function makePackageInfos(packageInfos: {
   [name: string]: Partial<Omit<PackageInfo, 'combinedOptions'>> & { combinedOptions?: Partial<BeachballOptions> };
 }): PackageInfos {
-  const result: PackageInfos = {};
-  for (const [name, info] of Object.entries(packageInfos)) {
-    result[name] = {
+  return _.mapValues(packageInfos, (info, name): PackageInfo => {
+    const { combinedOptions, ...rest } = info;
+    return {
       name,
-      combinedOptions: {} as BeachballOptions,
-      ...info,
-    } as PackageInfo;
-  }
-  return result;
+      version: '1.0.0',
+      private: false,
+      combinedOptions: { ...combinedOptions } as BeachballOptions,
+      packageOptions: {},
+      packageJsonPath: '',
+      ...rest,
+    };
+  });
 }

src/__tests__/publish/performPublishOverrides.test.ts

@@ -1,177 +1,153 @@
-import { describe, expect, it, afterEach } from '@jest/globals';
+import { describe, expect, it, afterEach, jest } from '@jest/globals';
 import * as fs from 'fs-extra';
-import * as path from 'path';
-import { tmpdir } from '../../__fixtures__/tmpdir';
+import _ from 'lodash';
 import { performPublishOverrides } from '../../publish/performPublishOverrides';
-import { PackageInfos } from '../../types/PackageInfo';
+import { PackageInfos, PackageJson, PublishConfig } from '../../types/PackageInfo';
+import { makePackageInfos } from '../../__fixtures__/packageInfos';
 
-describe('perform publishConfig overrides', () => {
-  let tmpDir: string | undefined;
+jest.mock('fs-extra', () => ({
+  readJSONSync: jest.fn(),
+  writeJSONSync: jest.fn(),
+}));
+
+describe('performPublishOverrides', () => {
+  const readJSONSync = fs.readJSONSync as jest.MockedFunction<typeof fs.readJSONSync>;
+  const writeJSONSync = fs.writeJSONSync as jest.MockedFunction<typeof fs.writeJSONSync>;
 
   afterEach(() => {
-    if (tmpDir) {
-      fs.removeSync(tmpDir);
-      tmpDir = undefined;
-    }
+    jest.restoreAllMocks();
   });
 
-  function createFixture(publishConfig: any = {}) {
-    tmpDir = tmpdir({ prefix: 'beachball-publishConfig-' });
-    const fixturePackageJson = {
-      name: 'foo',
-      version: '1.0.0',
+  function createFixture(partialPackageJsons: Record<string, Partial<PackageJson>>): {
+    packageInfos: PackageInfos;
+    packageJsons: Record<string, PackageJson>;
+  } {
+    const packageInfos = makePackageInfos(
+      _.mapValues(partialPackageJsons, (json, name) => ({
+        packageJsonPath: `packages/${name}/package.json`,
+        version: json.version || '1.0.0',
+        dependencies: json.dependencies || {},
+      }))
+    );
+    const packageJsons: Record<string, PackageJson> = _.mapValues(partialPackageJsons, (json, name) => ({
+      name,
+      version: packageInfos[name].version,
+      // these values can potentially be overridden by publishConfig
       main: 'src/index.ts',
-      bin: {
-        'foo-bin': 'src/foo-bin.js',
-      },
-      publishConfig,
-    };
-
-    const packageInfos: PackageInfos = {
-      foo: {
-        combinedOptions: {
-          defaultNpmTag: 'latest',
-          disallowedChangeTypes: [],
-          gitTags: true,
-          tag: 'latest',
-        },
-        name: 'foo',
-        packageJsonPath: path.join(tmpDir, 'package.json'),
-        packageOptions: {},
-        private: false,
-        version: '1.0.0',
-      },
-    };
-
-    fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify(fixturePackageJson));
+      bin: 'src/foo-bin.ts',
+      ...json,
+    }));
+
+    readJSONSync.mockImplementation((path: string) => {
+      for (const pkg of Object.values(packageInfos)) {
+        if (path === pkg.packageJsonPath) {
+          // performPublishConfigOverrides mutates the packageJson, so we need to clone it to
+          // simulate reading the file from the disk and avoid mutating original fixtures.
+          // This is also just safer in general for tests that use this method for before/after comparisons.
+          return _.cloneDeep(packageJsons[pkg.name]);
+        }
+      }
+      throw new Error('not found: ' + path);
+    });
 
-    return packageInfos;
+    return { packageInfos, packageJsons };
   }
 
-  it('overrides accepted keys', () => {
-    const packageInfos = createFixture({
+  it('overrides accepted publishConfig keys and preserves values not specified', () => {
+    const publishConfig: PublishConfig = {
       main: 'lib/index.js',
       types: 'lib/index.d.ts',
-    });
-
-    const original = JSON.parse(fs.readFileSync(packageInfos['foo'].packageJsonPath, 'utf-8'));
-
-    expect(original.main).toBe('src/index.ts');
-    expect(original.types).toBeUndefined();
+    };
+    const { packageInfos, packageJsons } = createFixture({ foo: { publishConfig } });
+    expect(packageJsons.foo).not.toMatchObject(publishConfig);
 
     performPublishOverrides(['foo'], packageInfos);
 
-    const modified = JSON.parse(fs.readFileSync(packageInfos['foo'].packageJsonPath, 'utf-8'));
-
-    expect(modified.main).toBe('lib/index.js');
-    expect(modified.types).toBe('lib/index.d.ts');
-    expect(modified.publishConfig.main).toBeUndefined();
-    expect(modified.publishConfig.types).toBeUndefined();
-  });
-
-  it('uses values on packageJson root as fallback values when present', () => {
-    const packageInfos = createFixture({
+    expect(writeJSONSync).toHaveBeenCalledTimes(1);
+    expect(publishConfig).toEqual({
       main: 'lib/index.js',
+      types: 'lib/index.d.ts',
     });
+    expect(writeJSONSync).toHaveBeenCalledWith(
+      packageInfos.foo.packageJsonPath,
+      // package.json data with publishConfig values promoted to root,
+      // and any original values not specified in publishConfig preserved
+      {
+        ...packageJsons.foo,
+        ...publishConfig,
+        publishConfig: {},
+      },
+      // JSON stringify options
+      expect.anything()
+    );
+  });
 
-    const original = JSON.parse(fs.readFileSync(packageInfos['foo'].packageJsonPath, 'utf-8'));
-
-    expect(original.main).toBe('src/index.ts');
-    expect(original.bin).toStrictEqual({ 'foo-bin': 'src/foo-bin.js' });
-    expect(original.files).toBeUndefined();
+  it('does not override non-accepted publishConfig keys', () => {
+    const publishConfig = { version: '1.2.3', extra: 'nope' } as unknown as PublishConfig;
+    const { packageInfos, packageJsons } = createFixture({ foo: { publishConfig } });
+    expect(packageJsons.foo).not.toMatchObject(publishConfig);
 
     performPublishOverrides(['foo'], packageInfos);
 
-    const modified = JSON.parse(fs.readFileSync(packageInfos['foo'].packageJsonPath, 'utf-8'));
-
-    expect(modified.main).toBe('lib/index.js');
-    expect(modified.bin).toStrictEqual({ 'foo-bin': 'src/foo-bin.js' });
-    expect(modified.files).toBeUndefined();
-    expect(modified.publishConfig.main).toBeUndefined();
-    expect(modified.publishConfig.bin).toBeUndefined();
-    expect(modified.publishConfig.files).toBeUndefined();
-  });
-});
-
-describe('perform workspace version overrides', () => {
-  let tmpDir: string | undefined;
-
-  afterEach(() => {
-    if (tmpDir) {
-      fs.removeSync(tmpDir);
-      tmpDir = undefined;
-    }
+    expect(writeJSONSync).toHaveBeenCalledTimes(1);
+    expect(writeJSONSync).toHaveBeenCalledWith(packageInfos.foo.packageJsonPath, packageJsons.foo, expect.anything());
   });
 
-  function createFixture(dependencyVersion: string) {
-    tmpDir = tmpdir({ prefix: 'beachball-publishConfig-' });
-    fs.mkdirSync(path.join(tmpDir, 'foo'));
-    fs.mkdirSync(path.join(tmpDir, 'bar'));
-
-    const fooPackageJson = {
-      name: 'foo',
-      version: '1.0.0',
-    };
-
-    const barPackageJson = {
-      name: 'bar',
-      version: '2.0.0',
-      dependencies: {
-        foo: dependencyVersion,
-      },
-    };
-
-    fs.writeFileSync(path.join(tmpDir, 'foo', 'package.json'), JSON.stringify(fooPackageJson));
-    fs.writeFileSync(path.join(tmpDir, 'bar', 'package.json'), JSON.stringify(barPackageJson));
-
-    const packageInfos: PackageInfos = {
-      foo: {
-        combinedOptions: {
-          defaultNpmTag: 'latest',
-          disallowedChangeTypes: [],
-          gitTags: true,
-          tag: 'latest',
-        },
-        name: 'foo',
-        packageJsonPath: path.join(tmpDir, 'foo', 'package.json'),
-        packageOptions: {},
-        private: false,
-        version: '1.0.0',
+  it('performs publish overrides for multiple packages', () => {
+    const { packageInfos, packageJsons } = createFixture({
+      foo: { publishConfig: { main: 'lib/index.js' } },
+      bar: { publishConfig: { types: 'lib/index.d.ts' } },
+    });
+    const originalFoo = packageJsons.foo;
+    const originalBar = packageJsons.bar;
+    expect(originalFoo).not.toMatchObject(originalFoo.publishConfig!);
+    expect(originalBar).not.toMatchObject(originalBar.publishConfig!);
+
+    performPublishOverrides(['foo', 'bar'], packageInfos);
+
+    expect(writeJSONSync).toHaveBeenCalledTimes(2);
+    expect(writeJSONSync).toHaveBeenCalledWith(
+      packageInfos.foo.packageJsonPath,
+      {
+        ...originalFoo,
+        ...originalFoo.publishConfig,
+        publishConfig: {},
       },
-      bar: {
-        combinedOptions: {
-          defaultNpmTag: 'latest',
-          disallowedChangeTypes: [],
-          gitTags: true,
-          tag: 'latest',
-        },
-        name: 'bar',
-        packageJsonPath: path.join(tmpDir, 'bar', 'package.json'),
-        packageOptions: {},
-        private: false,
-        dependencies: { foo: dependencyVersion },
-        version: '2.0.0',
+      expect.anything()
+    );
+    expect(writeJSONSync).toHaveBeenCalledWith(
+      packageInfos.bar.packageJsonPath,
+      {
+        ...originalBar,
+        ...originalBar.publishConfig,
+        publishConfig: {},
       },
-    };
-
-    return packageInfos;
-  }
+      expect.anything()
+    );
+  });
 
   it.each([
     ['workspace:*', '1.0.0'],
     ['workspace:~', '~1.0.0'],
     ['workspace:^', '^1.0.0'],
     ['workspace:~1.0.0', '~1.0.0'],
     ['workspace:^1.0.0', '^1.0.0'],
-  ])('overrides %s dependency versions during publishing', (dependencyVersion, expectedPublishVersion) => {
-    const packageInfos = createFixture(dependencyVersion);
-
-    const original = JSON.parse(fs.readFileSync(packageInfos['bar'].packageJsonPath, 'utf-8'));
-    expect(original.dependencies.foo).toBe(dependencyVersion);
+  ])('overrides %s dependency versions', (dependencyVersion, expectedPublishVersion) => {
+    const { packageInfos, packageJsons } = createFixture({
+      foo: { version: '1.0.0' },
+      bar: { version: '2.0.0', dependencies: { foo: dependencyVersion } },
+    });
+    expect(packageJsons.bar.dependencies!.foo).toBe(dependencyVersion);
 
     performPublishOverrides(['bar'], packageInfos);
 
-    const modified = JSON.parse(fs.readFileSync(packageInfos['bar'].packageJsonPath, 'utf-8'));
-    expect(modified.dependencies.foo).toBe(expectedPublishVersion);
+    expect(writeJSONSync).toHaveBeenCalledTimes(1);
+    expect(writeJSONSync).toHaveBeenCalledWith(
+      packageInfos.bar.packageJsonPath,
+      expect.objectContaining({
+        dependencies: { foo: expectedPublishVersion },
+      }),
+      expect.anything()
+    );
   });
 });

src/publish/performPublishOverrides.ts

@@ -1,7 +1,7 @@
-import { PackageInfos, PackageJson } from '../types/PackageInfo';
+import { PackageInfos, PackageJson, PublishConfig } from '../types/PackageInfo';
 import * as fs from 'fs-extra';
 
-const acceptedKeys = [
+const acceptedKeys: (keyof PublishConfig)[] = [
   'types',
   'typings',
   'main',
@@ -11,7 +11,7 @@ const acceptedKeys = [
   'bin',
   'browser',
   'files',
-] as const;
+];
 const workspacePrefix = 'workspace:';
 
 export function performPublishOverrides(packagesToPublish: string[], packageInfos: PackageInfos): void {

src/types/PackageInfo.ts

@@ -5,6 +5,15 @@ export interface PackageDeps {
   [dep: string]: string;
 }
 
+/**
+ * The `publishConfig` field in package.json.
+ * (If modifying this, be sure to update `acceptedKeys` in src/publish/performPublishOverrides.ts.)
+ */
+export type PublishConfig = Pick<
+  PackageJson,
+  'types' | 'typings' | 'main' | 'module' | 'exports' | 'repository' | 'bin' | 'browser' | 'files'
+>;
+
 export interface PackageJson {
   name: string;
   version: string;
@@ -24,10 +33,7 @@ export interface PackageJson {
   scripts?: Record<string, string>;
   beachball?: BeachballOptions;
   /** Overrides applied during publishing */
-  publishConfig?: Pick<
-    PackageJson,
-    'types' | 'typings' | 'main' | 'module' | 'exports' | 'repository' | 'bin' | 'browser' | 'files'
-  >;
+  publishConfig?: PublishConfig;
 }
 
 export interface PackageInfo {