From 9dbada8315017f1fc20e8cb5ad868fd6f37c64a2 Mon Sep 17 00:00:00 2001 From: btea <2356281422@qq.com> Date: Wed, 13 Aug 2025 23:32:43 +0800 Subject: [PATCH] refactor: combine addCatalogs to updateWorkspaceManifest (#9850) --- .changeset/good-cycles-fix.md | 5 ++ config/config-writer/src/index.ts | 4 +- .../plugin-commands-config/src/configSet.ts | 4 +- .../src/installDeps.ts | 10 ++- .../src/recursive.ts | 14 +-- workspace/manifest-writer/src/index.ts | 17 ++-- .../manifest-writer/test/addCatalogs.test.ts | 88 ++++++++++++------- .../test/updateWorkspaceManifest.test.ts | 6 +- 8 files changed, 91 insertions(+), 57 deletions(-) create mode 100644 .changeset/good-cycles-fix.md diff --git a/.changeset/good-cycles-fix.md b/.changeset/good-cycles-fix.md new file mode 100644 index 0000000000..2e36e8e79d --- /dev/null +++ b/.changeset/good-cycles-fix.md @@ -0,0 +1,5 @@ +--- +"@pnpm/workspace.manifest-writer": major +--- + +Combine the logic of the `addCatalogs` function into the `updateWorkspaceManifest` function. diff --git a/config/config-writer/src/index.ts b/config/config-writer/src/index.ts index 920006cb91..2b8052e090 100644 --- a/config/config-writer/src/index.ts +++ b/config/config-writer/src/index.ts @@ -35,5 +35,7 @@ export async function writeSettings (opts: WriteSettingsOptions): Promise return } } - await updateWorkspaceManifest(opts.workspaceDir, opts.updatedSettings) + await updateWorkspaceManifest(opts.workspaceDir, { + updatedFields: opts.updatedSettings, + }) } diff --git a/config/plugin-commands-config/src/configSet.ts b/config/plugin-commands-config/src/configSet.ts index 4413e80ea8..52810fe15c 100644 --- a/config/plugin-commands-config/src/configSet.ts +++ b/config/plugin-commands-config/src/configSet.ts @@ -36,7 +36,9 @@ export async function configSet (opts: ConfigCommandOptions, key: string, value: } key = camelCase(key) await updateWorkspaceManifest(opts.workspaceDir ?? opts.dir, { - [key]: castField(value, kebabCase(key)), + updatedFields: ({ + [key]: castField(value, kebabCase(key)), + }), }) } diff --git a/pkg-manager/plugin-commands-installation/src/installDeps.ts b/pkg-manager/plugin-commands-installation/src/installDeps.ts index 07c5a5a3e2..bfa0380461 100644 --- a/pkg-manager/plugin-commands-installation/src/installDeps.ts +++ b/pkg-manager/plugin-commands-installation/src/installDeps.ts @@ -22,7 +22,7 @@ import { } from '@pnpm/core' import { globalInfo, logger } from '@pnpm/logger' import { sequenceGraph } from '@pnpm/sort-packages' -import { addCatalogs } from '@pnpm/workspace.manifest-writer' +import { updateWorkspaceManifest } from '@pnpm/workspace.manifest-writer' import { createPkgGraph } from '@pnpm/workspace.pkgs-graph' import { updateWorkspaceState, type WorkspaceStateSettings } from '@pnpm/workspace.state' import isSubdir from 'is-subdir' @@ -319,7 +319,9 @@ when running add/update with the --workspace option') if (opts.save !== false) { await Promise.all([ writeProjectManifest(updatedProject.manifest), - updatedCatalogs && addCatalogs(opts.workspaceDir ?? opts.dir, updatedCatalogs), + updateWorkspaceManifest(opts.workspaceDir ?? opts.dir, { + updatedCatalogs, + }), ]) } if (!opts.lockfileOnly) { @@ -342,7 +344,9 @@ when running add/update with the --workspace option') if (opts.update === true && opts.save !== false) { await Promise.all([ writeProjectManifest(updatedManifest), - updatedCatalogs && addCatalogs(opts.workspaceDir ?? opts.dir, updatedCatalogs), + updateWorkspaceManifest(opts.workspaceDir ?? opts.dir, { + updatedCatalogs, + }), ]) } if (opts.strictDepBuilds && ignoredBuilds?.length) { diff --git a/pkg-manager/plugin-commands-installation/src/recursive.ts b/pkg-manager/plugin-commands-installation/src/recursive.ts index 2775709c2f..3f2226335b 100755 --- a/pkg-manager/plugin-commands-installation/src/recursive.ts +++ b/pkg-manager/plugin-commands-installation/src/recursive.ts @@ -31,7 +31,7 @@ import { type ProjectRootDir, type ProjectRootDirRealPath, } from '@pnpm/types' -import { addCatalogs } from '@pnpm/workspace.manifest-writer' +import { updateWorkspaceManifest } from '@pnpm/workspace.manifest-writer' import { addDependenciesToPackage, install, @@ -291,9 +291,9 @@ export async function recursive ( const promises: Array> = mutatedPkgs.map(async ({ originalManifest, manifest, rootDir }) => { return manifestsByPath[rootDir].writeProjectManifest(originalManifest ?? manifest) }) - if (updatedCatalogs) { - promises.push(addCatalogs(opts.workspaceDir, updatedCatalogs)) - } + promises.push(updateWorkspaceManifest(opts.workspaceDir, { + updatedCatalogs, + })) await Promise.all(promises) } if (opts.strictDepBuilds && ignoredBuilds?.length) { @@ -439,9 +439,9 @@ export async function recursive ( }) )) - if (updatedCatalogs) { - await addCatalogs(opts.workspaceDir, updatedCatalogs) - } + await updateWorkspaceManifest(opts.workspaceDir, { + updatedCatalogs, + }) if ( !opts.lockfileOnly && !opts.ignoreScripts && ( diff --git a/workspace/manifest-writer/src/index.ts b/workspace/manifest-writer/src/index.ts index 52e89bf438..42a6d9d5b1 100644 --- a/workspace/manifest-writer/src/index.ts +++ b/workspace/manifest-writer/src/index.ts @@ -22,10 +22,14 @@ async function writeManifestFile (dir: string, manifest: Partial): Promise { +export async function updateWorkspaceManifest (dir: string, opts: { + updatedFields?: Partial + updatedCatalogs?: Catalogs +}): Promise { const manifest = await readWorkspaceManifest(dir) ?? {} as WorkspaceManifest - let shouldBeUpdated = false - for (const [key, value] of Object.entries(updatedFields)) { + let shouldBeUpdated = opts.updatedCatalogs != null && addCatalogs(manifest, opts.updatedCatalogs) + + for (const [key, value] of Object.entries(opts.updatedFields ?? {})) { if (!equals(manifest[key as keyof WorkspaceManifest], value)) { shouldBeUpdated = true if (value == null) { @@ -52,8 +56,7 @@ export interface NewCatalogs { } } -export async function addCatalogs (workspaceDir: string, newCatalogs: Catalogs): Promise { - const manifest: Partial = await readWorkspaceManifest(workspaceDir) ?? {} +function addCatalogs (manifest: Partial, newCatalogs: Catalogs): boolean { let shouldBeUpdated = false for (const catalogName in newCatalogs) { @@ -85,7 +88,5 @@ export async function addCatalogs (workspaceDir: string, newCatalogs: Catalogs): } } - if (shouldBeUpdated) { - await writeManifestFile(workspaceDir, manifest) - } + return shouldBeUpdated } diff --git a/workspace/manifest-writer/test/addCatalogs.test.ts b/workspace/manifest-writer/test/addCatalogs.test.ts index c7e48ded8e..ff6d6609e7 100644 --- a/workspace/manifest-writer/test/addCatalogs.test.ts +++ b/workspace/manifest-writer/test/addCatalogs.test.ts @@ -2,22 +2,24 @@ import fs from 'fs' import path from 'path' import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants' import { tempDir } from '@pnpm/prepare-temp-dir' -import { addCatalogs } from '@pnpm/workspace.manifest-writer' +import { updateWorkspaceManifest } from '@pnpm/workspace.manifest-writer' import { sync as readYamlFile } from 'read-yaml-file' import { sync as writeYamlFile } from 'write-yaml-file' test('addCatalogs does not write new workspace manifest for empty catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) - await addCatalogs(dir, {}) + await updateWorkspaceManifest(dir, {}) expect(fs.existsSync(filePath)).toBe(false) }) test('addCatalogs does not write new workspace manifest for empty default catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) - await addCatalogs(dir, { - default: {}, + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + default: {}, + }, }) expect(fs.existsSync(filePath)).toBe(false) }) @@ -25,9 +27,11 @@ test('addCatalogs does not write new workspace manifest for empty default catalo test('addCatalogs does not write new workspace manifest for empty any-named catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) - await addCatalogs(dir, { - foo: {}, - bar: {}, + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + foo: {}, + bar: {}, + }, }) expect(fs.existsSync(filePath)).toBe(false) }) @@ -36,7 +40,9 @@ test('addCatalogs does not add empty catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, {}) - await addCatalogs(dir, {}) + await updateWorkspaceManifest(dir, { + updatedCatalogs: {}, + }) expect(readYamlFile(filePath)).toStrictEqual({}) }) @@ -44,8 +50,10 @@ test('addCatalogs does not add empty default catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, {}) - await addCatalogs(dir, { - default: {}, + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + default: {}, + }, }) expect(readYamlFile(filePath)).toStrictEqual({}) }) @@ -54,9 +62,11 @@ test('addCatalogs does not add empty any-named catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, {}) - await addCatalogs(dir, { - foo: {}, - bar: {}, + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + foo: {}, + bar: {}, + }, }) expect(readYamlFile(filePath)).toStrictEqual({}) }) @@ -64,9 +74,11 @@ test('addCatalogs does not add empty any-named catalogs', async () => { test('addCatalogs adds `default` catalogs to the `catalog` object by default', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) - await addCatalogs(dir, { - default: { - foo: '^0.1.2', + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + default: { + foo: '^0.1.2', + }, }, }) expect(readYamlFile(filePath)).toStrictEqual({ @@ -84,9 +96,11 @@ test('addCatalogs adds `default` catalogs to the `catalog` object if it exists', bar: '3.2.1', }, }) - await addCatalogs(dir, { - default: { - foo: '^0.1.2', + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + default: { + foo: '^0.1.2', + }, }, }) expect(readYamlFile(filePath)).toStrictEqual({ @@ -107,9 +121,11 @@ test('addCatalogs adds `default` catalogs to the `catalogs.default` object if it }, }, }) - await addCatalogs(dir, { - default: { - foo: '^0.1.2', + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + default: { + foo: '^0.1.2', + }, }, }) expect(readYamlFile(filePath)).toStrictEqual({ @@ -125,12 +141,14 @@ test('addCatalogs adds `default` catalogs to the `catalogs.default` object if it test('addCatalogs creates a `catalogs` object for any-named catalogs', async () => { const dir = tempDir(false) const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) - await addCatalogs(dir, { - foo: { - abc: '0.1.2', - }, - bar: { - def: '3.2.1', + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + foo: { + abc: '0.1.2', + }, + bar: { + def: '3.2.1', + }, }, }) expect(readYamlFile(filePath)).toStrictEqual({ @@ -155,12 +173,14 @@ test('addCatalogs add any-named catalogs to the `catalogs` object if it already }, }, }) - await addCatalogs(dir, { - foo: { - abc: '0.1.2', - }, - bar: { - def: '3.2.1', + await updateWorkspaceManifest(dir, { + updatedCatalogs: { + foo: { + abc: '0.1.2', + }, + bar: { + def: '3.2.1', + }, }, }) expect(readYamlFile(filePath)).toStrictEqual({ diff --git a/workspace/manifest-writer/test/updateWorkspaceManifest.test.ts b/workspace/manifest-writer/test/updateWorkspaceManifest.test.ts index f7ee6a6e01..6cc396c244 100644 --- a/workspace/manifest-writer/test/updateWorkspaceManifest.test.ts +++ b/workspace/manifest-writer/test/updateWorkspaceManifest.test.ts @@ -10,7 +10,7 @@ test('updateWorkspaceManifest adds a new setting', async () => { const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, { packages: ['*'] }) await updateWorkspaceManifest(dir, { - onlyBuiltDependencies: [], + updatedFields: { onlyBuiltDependencies: [] }, }) expect(readYamlFile(filePath)).toStrictEqual({ packages: ['*'], @@ -23,7 +23,7 @@ test('updateWorkspaceManifest removes an existing setting', async () => { const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, { packages: ['*'], overrides: { foo: '2' } }) await updateWorkspaceManifest(dir, { - overrides: undefined, + updatedFields: { overrides: undefined }, }) expect(readYamlFile(filePath)).toStrictEqual({ packages: ['*'], @@ -35,7 +35,7 @@ test('updateWorkspaceManifest updates an existing setting', async () => { const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME) writeYamlFile(filePath, { packages: ['*'], overrides: { foo: '2' } }) await updateWorkspaceManifest(dir, { - overrides: { bar: '3' }, + updatedFields: { overrides: { bar: '3' } }, }) expect(readYamlFile(filePath)).toStrictEqual({ packages: ['*'],