feat: preserve comments when updating pnpm-workspace.yaml (#10402)

Cherry-pick of 2b14c742e from main, adapted for v10:
- Use CJS module type for @pnpm/yaml.document-sync
- Use ramda/src/equals import style (v10 CJS)
- Remove GLOBAL_CONFIG_YAML_FILENAME (v11-only)
- Replace write-yaml-file with yaml + write-file-atomic + patchDocument

Co-Authored-By: Brandon Cheng <gluxon@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

.changeset/sharp-insects-joke.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/workspace.read-manifest": minor
+---
+
+The `validateWorkspaceManifest` function is now exported and can be used to validate whether a workspace manifest object's schema is correct.

.changeset/wet-rabbits-reply.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/workspace.manifest-writer": minor
+pnpm: minor
+---
+
+When pnpm updates the `pnpm-workspace.yaml`, comments, string formatting, and whitespace will be preserved.

pkg-manager/plugin-commands-installation/test/link.ts

@@ -289,6 +289,7 @@ test('logger warns about peer dependencies when linking', async () => {
     ...DEFAULT_OPTS,
     dir: process.cwd(),
     globalPkgDir: globalDir,
+    rootProjectManifestDir: globalDir,
   }, ['linked-with-peer-deps'])
 
   expect(warnMock).toHaveBeenCalledWith(expect.objectContaining({

pnpm-lock.yaml

@@ -8913,12 +8913,18 @@ importers:
       '@pnpm/workspace.read-manifest':
         specifier: workspace:*
         version: link:../read-manifest
+      '@pnpm/yaml.document-sync':
+        specifier: workspace:*
+        version: link:../../yaml/document-sync
       ramda:
         specifier: 'catalog:'
         version: '@pnpm/ramda@0.28.1'
-      write-yaml-file:
+      write-file-atomic:
         specifier: 'catalog:'
-        version: 5.0.0
+        version: 5.0.1
+      yaml:
+        specifier: 'catalog:'
+        version: 2.8.1
     devDependencies:
       '@pnpm/fs.find-packages':
         specifier: workspace:*
@@ -8935,9 +8941,15 @@ importers:
       '@types/ramda':
         specifier: 'catalog:'
         version: 0.29.12
+      '@types/write-file-atomic':
+        specifier: 'catalog:'
+        version: 4.0.3
       read-yaml-file:
         specifier: 'catalog:'
         version: 2.1.0
+      write-yaml-file:
+        specifier: 'catalog:'
+        version: 5.0.0
 
   workspace/pkgs-graph:
     dependencies:

workspace/manifest-writer/package.json

@@ -37,8 +37,10 @@
     "@pnpm/object.key-sorting": "workspace:*",
     "@pnpm/types": "workspace:*",
     "@pnpm/workspace.read-manifest": "workspace:*",
+    "@pnpm/yaml.document-sync": "workspace:*",
     "ramda": "catalog:",
-    "write-yaml-file": "catalog:"
+    "write-file-atomic": "catalog:",
+    "yaml": "catalog:"
   },
   "devDependencies": {
     "@pnpm/fs.find-packages": "workspace:*",
@@ -46,7 +48,9 @@
     "@pnpm/prepare-temp-dir": "workspace:*",
     "@pnpm/workspace.manifest-writer": "workspace:*",
     "@types/ramda": "catalog:",
-    "read-yaml-file": "catalog:"
+    "@types/write-file-atomic": "catalog:",
+    "read-yaml-file": "catalog:",
+    "write-yaml-file": "catalog:"
   },
   "engines": {
     "node": ">=18.12"

workspace/manifest-writer/src/index.ts

@@ -1,28 +1,37 @@
 import fs from 'fs'
 import path from 'path'
+import util from 'util'
 import { type Catalogs } from '@pnpm/catalogs.types'
 import { type ResolvedCatalogEntry } from '@pnpm/lockfile.types'
-import { readWorkspaceManifest, type WorkspaceManifest } from '@pnpm/workspace.read-manifest'
+import { validateWorkspaceManifest, type WorkspaceManifest } from '@pnpm/workspace.read-manifest'
 import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants'
-import writeYamlFile from 'write-yaml-file'
+import { patchDocument } from '@pnpm/yaml.document-sync'
 import equals from 'ramda/src/equals'
+import yaml from 'yaml'
+import writeFileAtomic from 'write-file-atomic'
 import { sortKeysByPriority } from '@pnpm/object.key-sorting'
 import {
   type Project,
 } from '@pnpm/types'
 
-async function writeManifestFile (dir: string, manifest: Partial<WorkspaceManifest>): Promise<void> {
-  manifest = sortKeysByPriority({
-    priority: { packages: 0 },
-    deep: true,
-  }, manifest)
-  return writeYamlFile(path.join(dir, WORKSPACE_MANIFEST_FILENAME), manifest, {
-    lineWidth: -1, // This is setting line width to never wrap
-    blankLines: true,
-    noCompatMode: true,
-    noRefs: true,
-    sortKeys: false,
+async function writeManifestFile (dir: string, manifest: yaml.Document): Promise<void> {
+  const manifestStr = manifest.toString({
+    lineWidth: 0, // This is setting line width to never wrap
+    singleQuote: true, // Prefer single quotes over double quotes
   })
+  await fs.promises.mkdir(dir, { recursive: true })
+  await writeFileAtomic(path.join(dir, WORKSPACE_MANIFEST_FILENAME), manifestStr)
+}
+
+async function readManifestRaw (file: string): Promise<string | undefined> {
+  try {
+    return (await fs.promises.readFile(file)).toString()
+  } catch (err) {
+    if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') {
+      return undefined
+    }
+    throw err
+  }
 }
 
 export async function updateWorkspaceManifest (dir: string, opts: {
@@ -32,7 +41,16 @@ export async function updateWorkspaceManifest (dir: string, opts: {
   cleanupUnusedCatalogs?: boolean
   allProjects?: Project[]
 }): Promise<void> {
-  const manifest = await readWorkspaceManifest(dir) ?? {} as WorkspaceManifest
+  const workspaceManifestStr = await readManifestRaw(path.join(dir, WORKSPACE_MANIFEST_FILENAME))
+
+  const document = workspaceManifestStr != null
+    ? yaml.parseDocument(workspaceManifestStr)
+    : new yaml.Document()
+
+  let manifest = document.toJSON()
+  validateWorkspaceManifest(manifest)
+  manifest ??= {}
+
   let shouldBeUpdated = opts.updatedCatalogs != null && addCatalogs(manifest, opts.updatedCatalogs)
   if (opts.cleanupUnusedCatalogs) {
     shouldBeUpdated = removePackagesFromWorkspaceCatalog(manifest, opts.allProjects ?? []) || shouldBeUpdated
@@ -71,7 +89,6 @@ export async function updateWorkspaceManifest (dir: string, opts: {
       if (value == null) {
         delete manifest[key as keyof WorkspaceManifest]
       } else {
-        // @ts-expect-error
         manifest[key as keyof WorkspaceManifest] = value
       }
     }
@@ -92,7 +109,15 @@ export async function updateWorkspaceManifest (dir: string, opts: {
     await fs.promises.rm(path.join(dir, WORKSPACE_MANIFEST_FILENAME))
     return
   }
-  await writeManifestFile(dir, manifest)
+
+  manifest = sortKeysByPriority({
+    priority: { packages: 0 },
+    deep: true,
+  }, manifest)
+
+  patchDocument(document, manifest)
+
+  await writeManifestFile(dir, document)
 }
 
 export interface NewCatalogs {

workspace/manifest-writer/test/updateWorkspaceManifest.test.ts

@@ -44,6 +44,45 @@ test('updateWorkspaceManifest updates an existing setting', async () => {
   })
 })
 
+// This test is intentionally minimal and doesn't exhaustively cover every case
+// of comment preservation in pnpm-workspace.yaml.
+//
+// The tests in @pnpm/yaml.document-sync should cover more cases and be
+// sufficient. It's likely not necessary to duplicate the tests in that package.
+test('updateWorkspaceManifest preserves comments', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+
+  const manifest = `\
+packages:
+  - '*'
+
+overrides:
+  bar: '2'
+  # This comment on foo should be preserved
+  foo: '3'
+`
+
+  const expected = `\
+packages:
+  - '*'
+
+overrides:
+  bar: '3'
+  baz: '1'
+  # This comment on foo should be preserved
+  foo: '2'
+`
+
+  fs.writeFileSync(filePath, manifest)
+
+  await updateWorkspaceManifest(dir, {
+    updatedFields: { overrides: { foo: '2', bar: '3', baz: '1' } },
+  })
+
+  expect(fs.readFileSync(filePath).toString()).toStrictEqual(expected)
+})
+
 test('updateWorkspaceManifest updates allowBuilds', async () => {
   const dir = tempDir(false)
   const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)

workspace/manifest-writer/tsconfig.json

@@ -33,6 +33,9 @@
     {
       "path": "../../packages/types"
     },
+    {
+      "path": "../../yaml/document-sync"
+    },
     {
       "path": "../read-manifest"
     }

workspace/read-manifest/src/index.ts

@@ -48,7 +48,7 @@ async function readManifestRaw (dir: string): Promise<unknown> {
   }
 }
 
-function validateWorkspaceManifest (manifest: unknown): asserts manifest is WorkspaceManifest | undefined {
+export function validateWorkspaceManifest (manifest: unknown): asserts manifest is WorkspaceManifest | undefined {
   if (manifest === undefined || manifest === null) {
     // Empty or null manifest is ok
     return

yaml/document-sync/package.json

@@ -4,7 +4,7 @@
   "description": "Update a YAML document to match the contents of an in-memory object.",
   "keywords": [
     "pnpm",
-    "pnpm11",
+    "pnpm10",
     "patch",
     "yaml"
   ],
@@ -15,7 +15,7 @@
   "bugs": {
     "url": "https://github.com/pnpm/pnpm/issues"
   },
-  "type": "module",
+  "type": "commonjs",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",
   "exports": {
@@ -30,7 +30,7 @@
     "compile": "tsc --build && pnpm run lint --fix",
     "lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"",
     "prepublishOnly": "pnpm run compile",
-    "_test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\" jest"
+    "_test": "jest"
   },
   "dependencies": {
     "yaml": "catalog:"
@@ -39,7 +39,7 @@
     "@pnpm/yaml.document-sync": "workspace:*"
   },
   "engines": {
-    "node": ">=20.19"
+    "node": ">=18.12"
   },
   "jest": {
     "preset": "@pnpm/jest-config"

yaml/document-sync/test/tsconfig.json

@@ -2,9 +2,8 @@
   "extends": "../tsconfig.json",
   "compilerOptions": {
     "noEmit": false,
-    "outDir": "../node_modules/.test.lib",
-    "rootDir": "..",
-    "isolatedModules": true
+    "outDir": "../test.lib",
+    "rootDir": "."
   },
   "include": [
     "**/*.ts",