From e8ccf1a53e051cf2544d1d0008c0f1453f281a05 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 20 Jul 2022 17:10:23 -0600 Subject: [PATCH] Don't blow away package manifest when updating When we read a package manifest, we need a representation of the manifest that we can make reliable assumptions about. To do this, we disregard fields that we don't care about and we fill in default values for fields that we _do_ care about but haven't been explicitly provided. The problem is that when updating the version of a package, we are using a modified version of this representation of the manifest. That isn't good, because it means that the package's manifest ends up getting fundamentally changed. This commit fixes this by storing the original representation of the manifest when it is read. This representation is then used when the version is updated instead of the "parsed" representation. This does make the parsed representation immediately out of date, but that doesn't really matter. --- package.json | 2 +- src/functional.test.ts | 57 +++++++- src/initial-parameters.ts | 2 +- src/monorepo-workflow-operations.test.ts | 127 +++++++--------- src/monorepo-workflow-operations.ts | 4 +- src/package-manifest.test.ts | 110 +++++++------- src/package-manifest.ts | 33 ++--- src/package.test.ts | 52 ++++--- src/package.ts | 16 ++- src/project.test.ts | 6 +- src/project.ts | 4 +- src/release-specification.ts | 6 +- tests/functional/helpers/local-monorepo.ts | 21 +++ .../helpers/monorepo-environment.ts | 4 + tests/unit/helpers.ts | 135 +++++++++++++----- yarn.lock | 10 +- 16 files changed, 357 insertions(+), 232 deletions(-) diff --git a/package.json b/package.json index 800cccb7..0618cf19 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ }, "dependencies": { "@metamask/action-utils": "^0.0.2", - "@metamask/utils": "^2.0.0", + "@metamask/utils": "^2.1.0", "debug": "^4.3.4", "execa": "^5.0.0", "glob": "^8.0.3", diff --git a/src/functional.test.ts b/src/functional.test.ts index 4c1bb33c..9c9dc4b9 100644 --- a/src/functional.test.ts +++ b/src/functional.test.ts @@ -44,6 +44,37 @@ describe('create-release-branch (functional)', () => { today: new Date('2022-06-24'), }, async (environment) => { + await environment.updateJsonFile('package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.updateJsonFileWithinPackage('a', 'package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.updateJsonFileWithinPackage('b', 'package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.updateJsonFileWithinPackage('c', 'package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.updateJsonFileWithinPackage('d', 'package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.updateJsonFileWithinPackage('e', 'package.json', { + scripts: { + foo: 'bar', + }, + }); + await environment.runTool({ releaseSpecification: { packages: { @@ -55,33 +86,47 @@ describe('create-release-branch (functional)', () => { }, }); - expect(await environment.readJsonFile('package.json')).toMatchObject({ + expect(await environment.readJsonFile('package.json')).toStrictEqual({ + name: '@scope/monorepo', version: '2022.6.24', + private: true, + workspaces: ['packages/*'], + scripts: { foo: 'bar' }, }); expect( await environment.readJsonFileWithinPackage('a', 'package.json'), - ).toMatchObject({ + ).toStrictEqual({ + name: '@scope/a', version: '1.0.0', + scripts: { foo: 'bar' }, }); expect( await environment.readJsonFileWithinPackage('b', 'package.json'), - ).toMatchObject({ + ).toStrictEqual({ + name: '@scope/b', version: '1.2.0', + scripts: { foo: 'bar' }, }); expect( await environment.readJsonFileWithinPackage('c', 'package.json'), - ).toMatchObject({ + ).toStrictEqual({ + name: '@scope/c', version: '2.0.14', + scripts: { foo: 'bar' }, }); expect( await environment.readJsonFileWithinPackage('d', 'package.json'), - ).toMatchObject({ + ).toStrictEqual({ + name: '@scope/d', version: '1.2.4', + scripts: { foo: 'bar' }, }); expect( await environment.readJsonFileWithinPackage('e', 'package.json'), - ).toMatchObject({ + ).toStrictEqual({ + name: '@scope/e', version: '0.0.3', + scripts: { foo: 'bar' }, }); }, ); diff --git a/src/initial-parameters.ts b/src/initial-parameters.ts index 5ccfd85f..a409b410 100644 --- a/src/initial-parameters.ts +++ b/src/initial-parameters.ts @@ -29,7 +29,7 @@ export async function determineInitialParameters( ? path.join( os.tmpdir(), 'create-release-branch', - project.rootPackage.manifest.name.replace('/', '__'), + project.rootPackage.validatedManifest.name.replace('/', '__'), ) : path.resolve(cwd, inputs.tempDirectory); diff --git a/src/monorepo-workflow-operations.test.ts b/src/monorepo-workflow-operations.test.ts index 54475f09..fd83ed14 100644 --- a/src/monorepo-workflow-operations.test.ts +++ b/src/monorepo-workflow-operations.test.ts @@ -2,13 +2,15 @@ import fs from 'fs'; import path from 'path'; import { SemVer } from 'semver'; import { withSandbox } from '../tests/helpers'; -import { buildMockPackage, buildMockProject } from '../tests/unit/helpers'; +import { + buildMockPackage, + buildMockMonorepoRootPackage, + buildMockProject, +} from '../tests/unit/helpers'; import { followMonorepoWorkflow } from './monorepo-workflow-operations'; import * as editorModule from './editor'; import * as envModule from './env'; import * as packageModule from './package'; -import type { Package } from './package'; -import type { ValidatedPackageManifest } from './package-manifest'; import type { Project } from './project'; import * as releaseSpecificationModule from './release-specification'; import * as workflowOperations from './workflow-operations'; @@ -34,29 +36,29 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), b: buildMockPackage('b', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), c: buildMockPackage('c', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), d: buildMockPackage('d', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -151,14 +153,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -244,14 +246,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -292,14 +294,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.3', { - manifest: { + validatedManifest: { private: false, }, }), @@ -340,14 +342,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -477,14 +479,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -552,14 +554,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -652,14 +654,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -705,14 +707,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.3', { - manifest: { + validatedManifest: { private: false, }, }), @@ -758,14 +760,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -904,29 +906,29 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), b: buildMockPackage('b', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), c: buildMockPackage('c', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), d: buildMockPackage('d', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1021,14 +1023,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1114,14 +1116,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1162,14 +1164,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.3', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1210,14 +1212,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1342,14 +1344,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1420,14 +1422,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1519,14 +1521,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1572,14 +1574,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.3', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1625,14 +1627,14 @@ describe('monorepo-workflow-operations', () => { await withSandbox(async (sandbox) => { const project = buildMockMonorepoProject({ rootPackage: buildMockPackage('root', '2022.1.1', { - manifest: { + validatedManifest: { private: true, workspaces: ['packages/*'], }, }), workspacePackages: { a: buildMockPackage('a', '1.0.0', { - manifest: { + validatedManifest: { private: false, }, }), @@ -1694,33 +1696,6 @@ function buildMockMonorepoProject(overrides: Partial = {}) { }); } -/** - * Builds a package for use in tests which is designed to be the root package of - * a monorepo. - * - * @param name - The name of the package. - * @param version - The version of the package, as a version string. - * @param overrides - The properties that will go into the object. - * @returns The mock Package object. - */ -function buildMockMonorepoRootPackage( - name = 'root', - version = '2022.1.1', - overrides: Omit, 'manifest'> & { - manifest?: Partial; - } = {}, -) { - const { manifest, ...rest } = overrides; - return buildMockPackage(name, version, { - manifest: { - private: true, - workspaces: ['packages/*'], - ...manifest, - }, - ...rest, - }); -} - /** * Mocks dependencies that `followMonorepoWorkflow` uses internally. * diff --git a/src/monorepo-workflow-operations.ts b/src/monorepo-workflow-operations.ts index ef2f0d54..776b4c8c 100644 --- a/src/monorepo-workflow-operations.ts +++ b/src/monorepo-workflow-operations.ts @@ -182,7 +182,7 @@ async function planRelease( ).map((packageName) => { const pkg = project.workspacePackages[packageName]; const versionSpecifier = releaseSpecification.packages[packageName]; - const currentVersion = pkg.manifest.version; + const currentVersion = pkg.validatedManifest.version; let newVersion: SemVer; if (versionSpecifier instanceof SemVer) { @@ -242,7 +242,7 @@ async function applyUpdatesToMonorepo( await Promise.all( releasePlan.packages.map(async (workspaceReleasePlan) => { debug( - `Updating package ${workspaceReleasePlan.package.manifest.name}...`, + `Updating package ${workspaceReleasePlan.package.validatedManifest.name}...`, ); await updatePackage({ project, diff --git a/src/package-manifest.test.ts b/src/package-manifest.test.ts index fc4021ef..1ea120ab 100644 --- a/src/package-manifest.test.ts +++ b/src/package-manifest.test.ts @@ -9,19 +9,21 @@ describe('package-manifest', () => { it('reads a minimal package manifest, expanding it by filling in values for optional fields', async () => { await withSandbox(async (sandbox) => { const manifestPath = path.join(sandbox.directoryPath, 'package.json'); - await fs.promises.writeFile( - manifestPath, - JSON.stringify({ - name: 'foo', - version: '1.2.3', - }), - ); - - expect(await readPackageManifest(manifestPath)).toStrictEqual({ + const unvalidated = { + name: 'foo', + version: '1.2.3', + }; + const validated = { name: 'foo', version: new SemVer('1.2.3'), workspaces: [], private: false, + }; + await fs.promises.writeFile(manifestPath, JSON.stringify(unvalidated)); + + expect(await readPackageManifest(manifestPath)).toStrictEqual({ + unvalidated, + validated, }); }); }); @@ -29,20 +31,22 @@ describe('package-manifest', () => { it('reads a package manifest where "private" is true', async () => { await withSandbox(async (sandbox) => { const manifestPath = path.join(sandbox.directoryPath, 'package.json'); - await fs.promises.writeFile( - manifestPath, - JSON.stringify({ - name: 'foo', - version: '1.2.3', - private: true, - }), - ); - - expect(await readPackageManifest(manifestPath)).toStrictEqual({ + const unvalidated = { + name: 'foo', + version: '1.2.3', + private: true, + }; + const validated = { name: 'foo', version: new SemVer('1.2.3'), workspaces: [], private: true, + }; + await fs.promises.writeFile(manifestPath, JSON.stringify(unvalidated)); + + expect(await readPackageManifest(manifestPath)).toStrictEqual({ + unvalidated, + validated, }); }); }); @@ -50,20 +54,22 @@ describe('package-manifest', () => { it('reads a package manifest where "private" is false', async () => { await withSandbox(async (sandbox) => { const manifestPath = path.join(sandbox.directoryPath, 'package.json'); - await fs.promises.writeFile( - manifestPath, - JSON.stringify({ - name: 'foo', - version: '1.2.3', - private: false, - }), - ); - - expect(await readPackageManifest(manifestPath)).toStrictEqual({ + const unvalidated = { + name: 'foo', + version: '1.2.3', + private: false, + }; + const validated = { name: 'foo', version: new SemVer('1.2.3'), workspaces: [], private: false, + }; + await fs.promises.writeFile(manifestPath, JSON.stringify(unvalidated)); + + expect(await readPackageManifest(manifestPath)).toStrictEqual({ + unvalidated, + validated, }); }); }); @@ -71,21 +77,23 @@ describe('package-manifest', () => { it('reads a package manifest where optional fields are fully provided', async () => { await withSandbox(async (sandbox) => { const manifestPath = path.join(sandbox.directoryPath, 'package.json'); - await fs.promises.writeFile( - manifestPath, - JSON.stringify({ - name: 'foo', - version: '1.2.3', - workspaces: ['packages/*'], - private: true, - }), - ); - - expect(await readPackageManifest(manifestPath)).toStrictEqual({ + const unvalidated = { + name: 'foo', + version: '1.2.3', + workspaces: ['packages/*'], + private: true, + }; + const validated = { name: 'foo', version: new SemVer('1.2.3'), workspaces: ['packages/*'], private: true, + }; + await fs.promises.writeFile(manifestPath, JSON.stringify(unvalidated)); + + expect(await readPackageManifest(manifestPath)).toStrictEqual({ + unvalidated, + validated, }); }); }); @@ -93,20 +101,22 @@ describe('package-manifest', () => { it('reads a package manifest where the "workspaces" field is provided but empty', async () => { await withSandbox(async (sandbox) => { const manifestPath = path.join(sandbox.directoryPath, 'package.json'); - await fs.promises.writeFile( - manifestPath, - JSON.stringify({ - name: 'foo', - version: '1.2.3', - workspaces: [], - }), - ); - - expect(await readPackageManifest(manifestPath)).toStrictEqual({ + const unvalidated = { + name: 'foo', + version: '1.2.3', + workspaces: [], + }; + const validated = { name: 'foo', version: new SemVer('1.2.3'), workspaces: [], private: false, + }; + await fs.promises.writeFile(manifestPath, JSON.stringify(unvalidated)); + + expect(await readPackageManifest(manifestPath)).toStrictEqual({ + unvalidated, + validated, }); }); }); diff --git a/src/package-manifest.ts b/src/package-manifest.ts index d25e858b..8a8e1ba2 100644 --- a/src/package-manifest.ts +++ b/src/package-manifest.ts @@ -24,14 +24,12 @@ export type UnvalidatedPackageManifest = Readonly>; * @property bundledDependencies - The set of packages that are expected to be * bundled when publishing the package. */ -export type ValidatedPackageManifest = { +export interface ValidatedPackageManifest { readonly [PackageManifestFieldNames.Name]: string; readonly [PackageManifestFieldNames.Version]: SemVer; readonly [PackageManifestFieldNames.Private]: boolean; readonly [PackageManifestFieldNames.Workspaces]: string[]; -} & Readonly< - Partial>> ->; +} /** * Constructs a validation error message for a field within the manifest. @@ -267,32 +265,29 @@ export function readPackageManifestPrivateField( * @throws If key data within the manifest is missing (currently `name` and * `version`) or the value of any other fields is unexpected. */ -export async function readPackageManifest( - manifestPath: string, -): Promise { - const unvalidatedPackageManifest = await readJsonObjectFile(manifestPath); +export async function readPackageManifest(manifestPath: string): Promise<{ + unvalidated: UnvalidatedPackageManifest; + validated: ValidatedPackageManifest; +}> { + const unvalidated = await readJsonObjectFile(manifestPath); const parentDirectory = path.dirname(manifestPath); - const name = readPackageManifestNameField( - unvalidatedPackageManifest, - parentDirectory, - ); - const version = readPackageManifestVersionField( - unvalidatedPackageManifest, - parentDirectory, - ); + const name = readPackageManifestNameField(unvalidated, parentDirectory); + const version = readPackageManifestVersionField(unvalidated, parentDirectory); const workspaces = readPackageManifestWorkspacesField( - unvalidatedPackageManifest, + unvalidated, parentDirectory, ); const privateValue = readPackageManifestPrivateField( - unvalidatedPackageManifest, + unvalidated, parentDirectory, ); - return { + const validated = { [PackageManifestFieldNames.Name]: name, [PackageManifestFieldNames.Version]: version, [PackageManifestFieldNames.Workspaces]: workspaces, [PackageManifestFieldNames.Private]: privateValue, }; + + return { unvalidated, validated }; } diff --git a/src/package.test.ts b/src/package.test.ts index 8398508d..6117b9db 100644 --- a/src/package.test.ts +++ b/src/package.test.ts @@ -3,7 +3,11 @@ import path from 'path'; import { when } from 'jest-when'; import * as autoChangelog from '@metamask/auto-changelog'; import { withSandbox } from '../tests/helpers'; -import { buildMockProject, buildMockManifest } from '../tests/unit/helpers'; +import { + buildMockPackage, + buildMockProject, + buildMockManifest, +} from '../tests/unit/helpers'; import * as fsModule from './fs'; import { readPackage, updatePackage } from './package'; import * as packageManifestModule from './package-manifest'; @@ -15,16 +19,22 @@ describe('package', () => { describe('readPackage', () => { it('reads information about the package located at the given directory', async () => { const packageDirectoryPath = '/path/to/package'; + const unvalidatedManifest = {}; + const validatedManifest = buildMockManifest(); jest .spyOn(packageManifestModule, 'readPackageManifest') - .mockResolvedValue(buildMockManifest()); + .mockResolvedValue({ + unvalidated: unvalidatedManifest, + validated: validatedManifest, + }); const pkg = await readPackage(packageDirectoryPath); expect(pkg).toStrictEqual({ directoryPath: packageDirectoryPath, manifestPath: path.join(packageDirectoryPath, 'package.json'), - manifest: buildMockManifest(), + unvalidatedManifest, + validatedManifest, changelogPath: path.join(packageDirectoryPath, 'CHANGELOG.md'), }); }); @@ -39,12 +49,12 @@ describe('package', () => { }; const manifestPath = path.join(sandbox.directoryPath, 'package.json'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath, - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath: path.join(sandbox.directoryPath, 'CHANGELOG.md'), - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: false, }; @@ -67,12 +77,12 @@ describe('package', () => { }); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath: path.join(sandbox.directoryPath, 'package.json'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath, - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: true, }; @@ -102,12 +112,12 @@ describe('package', () => { const project = buildMockProject(); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath: path.join(sandbox.directoryPath, 'package.json'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath, - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: true, }; @@ -124,12 +134,12 @@ describe('package', () => { const project = buildMockProject(); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath: path.join(sandbox.directoryPath, 'package.json'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath, - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: true, }; @@ -150,12 +160,12 @@ describe('package', () => { }); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath: path.join(sandbox.directoryPath, 'package.json'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath, - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: true, }; @@ -187,12 +197,12 @@ describe('package', () => { }); const changelogPath = path.join(sandbox.directoryPath, 'CHANGELOG.md'); const packageReleasePlan = { - package: { + package: buildMockPackage({ directoryPath: sandbox.directoryPath, manifestPath: path.join(sandbox.directoryPath, 'package.json'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), changelogPath, - }, + }), newVersion: '2.0.0', shouldUpdateChangelog: false, }; diff --git a/src/package.ts b/src/package.ts index 26efc852..5eb9513f 100644 --- a/src/package.ts +++ b/src/package.ts @@ -5,6 +5,7 @@ import { isErrorWithCode } from './misc-utils'; import { readFile, writeFile, writeJsonFile } from './fs'; import { readPackageManifest, + UnvalidatedPackageManifest, ValidatedPackageManifest, } from './package-manifest'; import { Project } from './project'; @@ -26,7 +27,8 @@ const CHANGELOG_FILE_NAME = 'CHANGELOG.md'; export interface Package { directoryPath: string; manifestPath: string; - manifest: ValidatedPackageManifest; + unvalidatedManifest: UnvalidatedPackageManifest; + validatedManifest: ValidatedPackageManifest; changelogPath: string; } @@ -41,12 +43,14 @@ export async function readPackage( ): Promise { const manifestPath = path.join(packageDirectoryPath, MANIFEST_FILE_NAME); const changelogPath = path.join(packageDirectoryPath, CHANGELOG_FILE_NAME); - const validatedManifest = await readPackageManifest(manifestPath); + const { unvalidated: unvalidatedManifest, validated: validatedManifest } = + await readPackageManifest(manifestPath); return { directoryPath: packageDirectoryPath, manifestPath, - manifest: validatedManifest, + validatedManifest, + unvalidatedManifest, changelogPath, }; } @@ -79,7 +83,7 @@ async function updatePackageChangelog({ } catch (error) { if (isErrorWithCode(error) && error.code === 'ENOENT') { stderr.write( - `${pkg.manifest.name} does not seem to have a changelog. Skipping.\n`, + `${pkg.validatedManifest.name} does not seem to have a changelog. Skipping.\n`, ); return; } @@ -99,7 +103,7 @@ async function updatePackageChangelog({ await writeFile(pkg.changelogPath, newChangelogContent); } else { stderr.write( - `Changelog for ${pkg.manifest.name} was not updated as there were no updates to make.`, + `Changelog for ${pkg.validatedManifest.name} was not updated as there were no updates to make.`, ); } } @@ -132,7 +136,7 @@ export async function updatePackage({ } = packageReleasePlan; await writeJsonFile(pkg.manifestPath, { - ...pkg.manifest, + ...pkg.unvalidatedManifest, version: newVersion, }); diff --git a/src/project.test.ts b/src/project.test.ts index 029f28ab..77673d70 100644 --- a/src/project.test.ts +++ b/src/project.test.ts @@ -18,14 +18,14 @@ describe('project', () => { const projectRepositoryUrl = 'https://github.com/some-org/some-repo'; const rootPackage = buildMockPackage('root', { directoryPath: projectDirectoryPath, - manifest: buildMockManifest({ + validatedManifest: buildMockManifest({ workspaces: ['packages/a', 'packages/subpackages/*'], }), }); const workspacePackages = { a: buildMockPackage('a', { directoryPath: path.join(projectDirectoryPath, 'packages', 'a'), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), }), b: buildMockPackage('b', { directoryPath: path.join( @@ -34,7 +34,7 @@ describe('project', () => { 'subpackages', 'b', ), - manifest: buildMockManifest(), + validatedManifest: buildMockManifest(), }), }; when(jest.spyOn(repoModule, 'getRepositoryHttpsUrl')) diff --git a/src/project.ts b/src/project.ts index 5976130b..7c33c3ac 100644 --- a/src/project.ts +++ b/src/project.ts @@ -47,7 +47,7 @@ export async function readProject( const workspaceDirectories = ( await Promise.all( - rootPackage.manifest[PackageManifestFieldNames.Workspaces].map( + rootPackage.validatedManifest[PackageManifestFieldNames.Workspaces].map( async (workspacePattern) => { return await promisifiedGlob(workspacePattern, { cwd: projectDirectoryPath, @@ -65,7 +65,7 @@ export async function readProject( }), ) ).reduce((obj, pkg) => { - return { ...obj, [pkg.manifest.name]: pkg }; + return { ...obj, [pkg.validatedManifest.name]: pkg }; }, {} as Record); const isMonorepo = Object.keys(workspacePackages).length > 0; diff --git a/src/release-specification.ts b/src/release-specification.ts index ef51ff02..d85295e2 100644 --- a/src/release-specification.ts +++ b/src/release-specification.ts @@ -64,7 +64,7 @@ export async function generateReleaseSpecificationTemplateForMonorepo({ # create-release-branch.`.trim(); const instructions = ` -# The following is a list of packages in ${rootPackage.manifest.name}. +# The following is a list of packages in ${rootPackage.validatedManifest.name}. # Please indicate the packages for which you want to create a new release # by updating "null" (which does nothing) to one of the following: # @@ -78,7 +78,7 @@ ${afterEditingInstructions} `.trim(); const packages = Object.values(workspacePackages).reduce((obj, pkg) => { - return { ...obj, [pkg.manifest.name]: null }; + return { ...obj, [pkg.validatedManifest.name]: null }; }, {}); return [instructions, YAML.stringify({ packages })].join('\n\n'); @@ -151,7 +151,7 @@ export async function validateReleaseSpecification( releaseSpecificationPath: string, ): Promise { const workspacePackageNames = Object.values(project.workspacePackages).map( - (pkg) => pkg.manifest.name, + (pkg) => pkg.validatedManifest.name, ); const releaseSpecificationContents = await readFile(releaseSpecificationPath); const indexOfFirstUsableLine = releaseSpecificationContents diff --git a/tests/functional/helpers/local-monorepo.ts b/tests/functional/helpers/local-monorepo.ts index 09457268..794cc56e 100644 --- a/tests/functional/helpers/local-monorepo.ts +++ b/tests/functional/helpers/local-monorepo.ts @@ -132,6 +132,27 @@ export default class LocalMonorepo< ); } + /** + * Updates a JSON file within a workspace package within the project. + * + * @param packageNickname - The nickname of the workspace package, as + * identified in the `packages` options passed to + * `withMonorepoProjectEnvironment`. + * @param partialFilePath - The path to the desired file within the package. + * @param object - The new object should be merged into the file. + */ + async updateJsonFileWithinPackage( + packageNickname: '$root$' | WorkspacePackageNickname, + partialFilePath: string, + object: Record, + ): Promise { + const packageDirectoryPath = this.#packages[packageNickname].directoryPath; + await this.updateJsonFile( + path.join(packageDirectoryPath, partialFilePath), + object, + ); + } + /** * Writes an initial package.json for the root package as well as any * workspace packages (if specified). diff --git a/tests/functional/helpers/monorepo-environment.ts b/tests/functional/helpers/monorepo-environment.ts index c6f4ebc0..12c3497a 100644 --- a/tests/functional/helpers/monorepo-environment.ts +++ b/tests/functional/helpers/monorepo-environment.ts @@ -51,6 +51,8 @@ export default class MonorepoEnvironment< readJsonFileWithinPackage: LocalMonorepo['readJsonFileWithinPackage']; + updateJsonFileWithinPackage: LocalMonorepo['updateJsonFileWithinPackage']; + #packages: MonorepoEnvironmentOptions['packages']; #today: MonorepoEnvironmentOptions['today']; @@ -70,6 +72,8 @@ export default class MonorepoEnvironment< ); this.readJsonFileWithinPackage = this.localRepo.readJsonFileWithinPackage.bind(this.localRepo); + this.updateJsonFileWithinPackage = + this.localRepo.updateJsonFileWithinPackage.bind(this.localRepo); } /** diff --git a/tests/unit/helpers.ts b/tests/unit/helpers.ts index 21a851a4..56064b27 100644 --- a/tests/unit/helpers.ts +++ b/tests/unit/helpers.ts @@ -1,10 +1,8 @@ import path from 'path'; import { SemVer } from 'semver'; +import { isPlainObject } from '@metamask/utils'; import type { Package } from '../../src/package'; -import { - PackageManifestFieldNames, - PackageManifestDependenciesFieldNames, -} from '../../src/package-manifest'; +import { PackageManifestFieldNames } from '../../src/package-manifest'; import type { ValidatedPackageManifest } from '../../src/package-manifest'; import type { Project } from '../../src/project'; @@ -16,6 +14,16 @@ type Unrequire = Omit & { [P in K]+?: T[P]; }; +type MockPackageOverrides = Omit< + Unrequire, + 'unvalidatedManifest' | 'validatedManifest' +> & { + validatedManifest?: Omit< + Partial, + PackageManifestFieldNames.Name | PackageManifestFieldNames.Version + >; +}; + /** * Builds a project object for use in tests. All properties have default * values, so you can specify only the properties you care about. @@ -34,44 +42,49 @@ export function buildMockProject(overrides: Partial = {}): Project { }; } -type MockPackageOverrides = Omit< - Unrequire, - 'manifest' -> & { - manifest?: Omit< - Partial, - PackageManifestFieldNames.Name | PackageManifestFieldNames.Version - >; -}; - /** * Builds a package object for use in tests. All properties have default * values, so you can specify only the properties you care about. * - * @param name - The name of the package. - * @param args - Either the version of the package and the properties that will - * go into the object, or just the properties. + * @param args - The name of the package (optional), the version of the package + * (optional) and the properties that will go into the object (optional). * @returns The mock Package object. */ export function buildMockPackage( - name: string, - ...args: [string | SemVer, MockPackageOverrides] | [MockPackageOverrides] | [] + ...args: + | [string, string | SemVer, MockPackageOverrides] + | [string, string | SemVer] + | [string, MockPackageOverrides] + | [string] + | [MockPackageOverrides] + | [] ): Package { - let version, overrides; + let name, version, overrides; - if (args.length === 0) { - version = '1.0.0'; - overrides = {}; - } else if (args.length === 1) { - version = '1.0.0'; - overrides = args[0]; - } else { - version = args[0]; - overrides = args[1]; + switch (args.length) { + case 0: + name = 'package'; + version = '1.0.0'; + overrides = {}; + break; + case 1: + name = isPlainObject(args[0]) ? 'package' : args[0]; + version = '1.0.0'; + overrides = isPlainObject(args[0]) ? args[0] : {}; + break; + case 2: + name = args[0]; + version = isPlainObject(args[1]) ? '1.0.0' : args[1]; + overrides = isPlainObject(args[1]) ? args[1] : {}; + break; + default: + name = args[0]; + version = args[1]; + overrides = args[2]; } const { - manifest = {}, + validatedManifest = {}, directoryPath = `/path/to/packages/${name}`, manifestPath = path.join(directoryPath, 'package.json'), changelogPath = path.join(directoryPath, 'CHANGELOG.md'), @@ -79,8 +92,9 @@ export function buildMockPackage( return { directoryPath, - manifest: buildMockManifest({ - ...manifest, + unvalidatedManifest: {}, + validatedManifest: buildMockManifest({ + ...validatedManifest, [PackageManifestFieldNames.Name]: name, [PackageManifestFieldNames.Version]: version instanceof SemVer ? version : new SemVer(version), @@ -90,6 +104,58 @@ export function buildMockPackage( }; } +/** + * Builds a package for use in tests which is designed to be the root package of + * a monorepo. + * + * @param args - The name of the package (optional), the version of the package + * (optional) and the properties that will go into the object (optional). + * @returns The mock Package object. + */ +export function buildMockMonorepoRootPackage( + ...args: + | [string, string | SemVer, MockPackageOverrides] + | [string, string | SemVer] + | [string, MockPackageOverrides] + | [string] + | [MockPackageOverrides] + | [] +): Package { + let name, version, overrides; + + switch (args.length) { + case 0: + name = 'package'; + version = '1.0.0'; + overrides = {}; + break; + case 1: + name = isPlainObject(args[0]) ? 'package' : args[0]; + version = '1.0.0'; + overrides = isPlainObject(args[0]) ? args[0] : {}; + break; + case 2: + name = args[0]; + version = isPlainObject(args[1]) ? '1.0.0' : args[1]; + overrides = isPlainObject(args[1]) ? args[1] : {}; + break; + default: + name = args[0]; + version = args[1]; + overrides = args[2]; + } + + const { validatedManifest, ...rest } = overrides; + return buildMockPackage(name, version, { + validatedManifest: { + private: true, + workspaces: ['packages/*'], + ...validatedManifest, + }, + ...rest, + }); +} + /** * Builds a manifest object for use in tests. All properties have default * values, so you can specify only the properties you care about. @@ -105,11 +171,6 @@ export function buildMockManifest( [PackageManifestFieldNames.Version]: new SemVer('1.2.3'), [PackageManifestFieldNames.Private]: false, [PackageManifestFieldNames.Workspaces]: [], - [PackageManifestDependenciesFieldNames.Bundled]: {}, - [PackageManifestDependenciesFieldNames.Production]: {}, - [PackageManifestDependenciesFieldNames.Development]: {}, - [PackageManifestDependenciesFieldNames.Optional]: {}, - [PackageManifestDependenciesFieldNames.Peer]: {}, ...overrides, }; } diff --git a/yarn.lock b/yarn.lock index f21e94dd..02499a4c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -871,7 +871,7 @@ __metadata: "@metamask/eslint-config-jest": ^9.0.0 "@metamask/eslint-config-nodejs": ^9.0.0 "@metamask/eslint-config-typescript": ^9.0.1 - "@metamask/utils": ^2.0.0 + "@metamask/utils": ^2.1.0 "@types/debug": ^4.1.7 "@types/jest": ^28.1.4 "@types/jest-when": ^3.5.2 @@ -962,12 +962,12 @@ __metadata: languageName: node linkType: hard -"@metamask/utils@npm:^2.0.0": - version: 2.0.0 - resolution: "@metamask/utils@npm:2.0.0" +"@metamask/utils@npm:^2.1.0": + version: 2.1.0 + resolution: "@metamask/utils@npm:2.1.0" dependencies: fast-deep-equal: ^3.1.3 - checksum: 517afc6724e58aee889b9962fcedc0345cb264ed8232756cd16e2d47e22b5501af276986a3d84a9ab903075d20802bf38ff4f6a70c58a158666f18cb69ff458d + checksum: 50970fe28cbf98fbc34fb4f69d9bc90f5db94929c69ab532f57b48f42163ea77fb080ab31854efd984361c3e29e67831a78d94d1211ac3bcc6b9557769c73127 languageName: node linkType: hard