refactor(ngcc): abstract updating package.json files behind an interface (#32427)

To persist some of its state, `ngcc` needs to update `package.json`
files (both in memory and on disk).

This refactoring abstracts these operations behind the
`PackageJsonUpdater` interface, making it easier to orchestrate them
from different contexts (e.g. when running tasks in parallel on multiple
processes).

Inspired by/Based on @alxhub's prototype: alxhub/angular@cb631bdb1

PR Close #32427

packages/compiler-cli/ngcc/src/main.ts

@@ -28,6 +28,7 @@ import {PathMappings} from './utils';
 import {FileWriter} from './writing/file_writer';
 import {InPlaceFileWriter} from './writing/in_place_file_writer';
 import {NewEntryPointFileWriter} from './writing/new_entry_point_file_writer';
+import {DirectPackageJsonUpdater, PackageJsonUpdater} from './writing/package_json_updater';
 
 /**
  * The options to configure the ngcc compiler.
@@ -85,6 +86,7 @@ export function mainNgcc(
      compileAllFormats = true, createNewEntryPointFormats = false,
      logger = new ConsoleLogger(LogLevel.info), pathMappings}: NgccOptions): void {
   const fileSystem = getFileSystem();
+  const pkgJsonUpdater = new DirectPackageJsonUpdater(fileSystem);
 
   // The function for performing the analysis.
   const analyzeEntryPoints: AnalyzeEntryPointsFn = () => {
@@ -104,8 +106,8 @@ export function mainNgcc(
     const absBasePath = absoluteFrom(basePath);
     const config = new NgccConfiguration(fileSystem, dirname(absBasePath));
     const entryPoints = getEntryPoints(
-        fileSystem, config, logger, dependencyResolver, absBasePath, targetEntryPointPath,
+        fileSystem, pkgJsonUpdater, logger, dependencyResolver, config, absBasePath,
-        pathMappings, supportedPropertiesToConsider, compileAllFormats);
+        targetEntryPointPath, pathMappings, supportedPropertiesToConsider, compileAllFormats);
 
     const processingMetadataPerEntryPoint = new Map<string, EntryPointProcessingMetadata>();
     const tasks: Task[] = [];
@@ -136,7 +138,7 @@ export function mainNgcc(
 
   // The function for creating the `compile()` function.
   const createCompileFn: CreateCompileFn = onTaskCompleted => {
-    const fileWriter = getFileWriter(fileSystem, createNewEntryPointFormats);
+    const fileWriter = getFileWriter(fileSystem, pkgJsonUpdater, createNewEntryPointFormats);
     const transformer = new Transformer(fileSystem, logger);
 
     return (task: Task) => {
@@ -207,7 +209,7 @@ export function mainNgcc(
             }
 
             markAsProcessed(
-                fileSystem, entryPoint.packageJson, packageJsonPath, propsToMarkAsProcessed);
+                pkgJsonUpdater, entryPoint.packageJson, packageJsonPath, propsToMarkAsProcessed);
           }
         });
 
@@ -261,28 +263,32 @@ function ensureSupportedProperties(properties: string[]): EntryPointJsonProperty
   return supportedProperties;
 }
 
-function getFileWriter(fs: FileSystem, createNewEntryPointFormats: boolean): FileWriter {
+function getFileWriter(
-  return createNewEntryPointFormats ? new NewEntryPointFileWriter(fs) : new InPlaceFileWriter(fs);
+    fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater,
+    createNewEntryPointFormats: boolean): FileWriter {
+  return createNewEntryPointFormats ? new NewEntryPointFileWriter(fs, pkgJsonUpdater) :
+                                      new InPlaceFileWriter(fs);
 }
 
 function getEntryPoints(
-    fs: FileSystem, config: NgccConfiguration, logger: Logger, resolver: DependencyResolver,
+    fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater, logger: Logger,
-    basePath: AbsoluteFsPath, targetEntryPointPath: string | undefined,
+    resolver: DependencyResolver, config: NgccConfiguration, basePath: AbsoluteFsPath,
-    pathMappings: PathMappings | undefined, propertiesToConsider: string[],
+    targetEntryPointPath: string | undefined, pathMappings: PathMappings | undefined,
-    compileAllFormats: boolean): EntryPoint[] {
+    propertiesToConsider: string[], compileAllFormats: boolean): EntryPoint[] {
   const {entryPoints, invalidEntryPoints} = (targetEntryPointPath !== undefined) ?
       getTargetedEntryPoints(
-          fs, config, logger, resolver, basePath, targetEntryPointPath, propertiesToConsider,
+          fs, pkgJsonUpdater, logger, resolver, config, basePath, targetEntryPointPath,
-          compileAllFormats, pathMappings) :
+          propertiesToConsider, compileAllFormats, pathMappings) :
       getAllEntryPoints(fs, config, logger, resolver, basePath, pathMappings);
   logInvalidEntryPoints(logger, invalidEntryPoints);
   return entryPoints;
 }
 
 function getTargetedEntryPoints(
-    fs: FileSystem, config: NgccConfiguration, logger: Logger, resolver: DependencyResolver,
+    fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater, logger: Logger,
-    basePath: AbsoluteFsPath, targetEntryPointPath: string, propertiesToConsider: string[],
+    resolver: DependencyResolver, config: NgccConfiguration, basePath: AbsoluteFsPath,
-    compileAllFormats: boolean, pathMappings: PathMappings | undefined): SortedEntryPointsInfo {
+    targetEntryPointPath: string, propertiesToConsider: string[], compileAllFormats: boolean,
+    pathMappings: PathMappings | undefined): SortedEntryPointsInfo {
   const absoluteTargetEntryPointPath = resolve(basePath, targetEntryPointPath);
   if (hasProcessedTargetEntryPoint(
           fs, absoluteTargetEntryPointPath, propertiesToConsider, compileAllFormats)) {
@@ -300,7 +306,7 @@ function getTargetedEntryPoints(
         invalidTarget.missingDependencies.map(dep => ` - ${dep}\n`));
   }
   if (entryPointInfo.entryPoints.length === 0) {
-    markNonAngularPackageAsProcessed(fs, absoluteTargetEntryPointPath);
+    markNonAngularPackageAsProcessed(fs, pkgJsonUpdater, absoluteTargetEntryPointPath);
   }
   return entryPointInfo;
 }
@@ -349,13 +355,14 @@ function hasProcessedTargetEntryPoint(
  * So mark all formats in this entry-point as processed so that clients of ngcc can avoid
  * triggering ngcc for this entry-point in the future.
  */
-function markNonAngularPackageAsProcessed(fs: FileSystem, path: AbsoluteFsPath) {
+function markNonAngularPackageAsProcessed(
+    fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater, path: AbsoluteFsPath) {
   const packageJsonPath = resolve(path, 'package.json');
   const packageJson = JSON.parse(fs.readFile(packageJsonPath));
 
   // Note: We are marking all supported properties as processed, even if they don't exist in the
   //       `package.json` file. While this is redundant, it is also harmless.
-  markAsProcessed(fs, packageJson, packageJsonPath, SUPPORTED_FORMAT_PROPERTIES);
+  markAsProcessed(pkgJsonUpdater, packageJson, packageJsonPath, SUPPORTED_FORMAT_PROPERTIES);
 }
 
 function logInvalidEntryPoints(logger: Logger, invalidEntryPoints: InvalidEntryPoint[]): void {

packages/compiler-cli/ngcc/src/packages/build_marker.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {AbsoluteFsPath, FileSystem, dirname} from '../../../src/ngtsc/file_system';
+import {PackageJsonUpdater} from '../writing/package_json_updater';
 import {EntryPointPackageJson, PackageJsonFormatProperties} from './entry_point';
 
 export const NGCC_VERSION = '0.0.0-PLACEHOLDER';
@@ -42,24 +43,25 @@ export function hasBeenProcessed(
  * Write a build marker for the given entry-point and format properties, to indicate that they have
  * been compiled by this version of ngcc.
  *
- * @param fs The current file-system being used.
+ * @param pkgJsonUpdater The writer to use for updating `package.json`.
  * @param packageJson The parsed contents of the `package.json` file for the entry-point.
  * @param packageJsonPath The absolute path to the `package.json` file.
  * @param properties The properties in the `package.json` of the formats for which we are writing
  *                   the marker.
  */
 export function markAsProcessed(
-    fs: FileSystem, packageJson: EntryPointPackageJson, packageJsonPath: AbsoluteFsPath,
+    pkgJsonUpdater: PackageJsonUpdater, packageJson: EntryPointPackageJson,
-    properties: PackageJsonFormatProperties[]) {
+    packageJsonPath: AbsoluteFsPath, formatProperties: PackageJsonFormatProperties[]): void {
-  const processed =
+  const update = pkgJsonUpdater.createUpdate();
-      packageJson.__processed_by_ivy_ngcc__ || (packageJson.__processed_by_ivy_ngcc__ = {});
 
-  for (const prop of properties) {
+  // Update the format properties to mark them as processed.
-    processed[prop] = NGCC_VERSION;
+  for (const prop of formatProperties) {
+    update.addChange(['__processed_by_ivy_ngcc__', prop], NGCC_VERSION);
   }
 
-  const scripts = packageJson.scripts || (packageJson.scripts = {});
+  // Update the `prepublishOnly` script (keeping a backup, if necessary) to prevent `ngcc`'d
-  const oldPrepublishOnly = scripts.prepublishOnly;
+  // packages from getting accidentally published.
+  const oldPrepublishOnly = packageJson.scripts && packageJson.scripts.prepublishOnly;
   const newPrepublishOnly = 'node --eval \"console.error(\'' +
       'ERROR: Trying to publish a package that has been compiled by NGCC. This is not allowed.\\n' +
       'Please delete and rebuild the package, without compiling with NGCC, before attempting to publish.\\n' +
@@ -68,13 +70,10 @@ export function markAsProcessed(
       '&& exit 1';
 
   if (oldPrepublishOnly && (oldPrepublishOnly !== newPrepublishOnly)) {
-    scripts.prepublishOnly__ivy_ngcc_bak = oldPrepublishOnly;
+    update.addChange(['scripts', 'prepublishOnly__ivy_ngcc_bak'], oldPrepublishOnly);
   }
 
-  scripts.prepublishOnly = newPrepublishOnly;
+  update.addChange(['scripts', 'prepublishOnly'], newPrepublishOnly);
 
-  // Just in case this package.json was synthesized due to a custom configuration
+  update.writeChanges(packageJsonPath, packageJson);
-  // we will ensure that the path to the containing folder exists before we write the file.
-  fs.ensureDir(dirname(packageJsonPath));
-  fs.writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2));
 }

packages/compiler-cli/ngcc/src/packages/entry_point.ts

@@ -38,6 +38,11 @@ export interface EntryPoint {
   compiledByAngular: boolean;
 }
 
+export type JsonPrimitive = string | number | boolean | null;
+export type JsonValue = JsonPrimitive | JsonArray | JsonObject | undefined;
+export interface JsonArray extends Array<JsonValue> {}
+export interface JsonObject { [key: string]: JsonValue; }
+
 export interface PackageJsonFormatPropertiesMap {
   fesm2015?: string;
   fesm5?: string;
@@ -55,7 +60,7 @@ export type PackageJsonFormatProperties = keyof PackageJsonFormatPropertiesMap;
 /**
  * The properties that may be loaded from the `package.json` file.
  */
-export interface EntryPointPackageJson extends PackageJsonFormatPropertiesMap {
+export interface EntryPointPackageJson extends JsonObject, PackageJsonFormatPropertiesMap {
   name: string;
   scripts?: Record<string, string>;
   __processed_by_ivy_ngcc__?: Record<string, string>;

packages/compiler-cli/ngcc/src/writing/in_place_file_writer.ts

@@ -14,7 +14,7 @@ import {FileWriter} from './file_writer';
 
 /**
  * This FileWriter overwrites the transformed file, in-place, while creating
- * a back-up of the original file with an extra `.bak` extension.
+ * a back-up of the original file with an extra `.__ivy_ngcc_bak` extension.
  */
 export class InPlaceFileWriter implements FileWriter {
   constructor(protected fs: FileSystem) {}

packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts

@@ -6,13 +6,14 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {AbsoluteFsPath, absoluteFromSourceFile, dirname, join, relative} from '../../../src/ngtsc/file_system';
+import {AbsoluteFsPath, FileSystem, absoluteFromSourceFile, dirname, join, relative} from '../../../src/ngtsc/file_system';
 import {isDtsPath} from '../../../src/ngtsc/util/src/typescript';
 import {EntryPoint, EntryPointJsonProperty} from '../packages/entry_point';
 import {EntryPointBundle} from '../packages/entry_point_bundle';
 import {FileToWrite} from '../rendering/utils';
 
 import {InPlaceFileWriter} from './in_place_file_writer';
+import {PackageJsonUpdater} from './package_json_updater';
 
 const NGCC_DIRECTORY = '__ivy_ngcc__';
 
@@ -25,6 +26,8 @@ const NGCC_DIRECTORY = '__ivy_ngcc__';
  * `InPlaceFileWriter`).
  */
 export class NewEntryPointFileWriter extends InPlaceFileWriter {
+  constructor(fs: FileSystem, private pkgJsonUpdater: PackageJsonUpdater) { super(fs); }
+
   writeBundle(
       bundle: EntryPointBundle, transformedFiles: FileToWrite[],
       formatProperties: EntryPointJsonProperty[]) {
@@ -65,16 +68,34 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
   protected updatePackageJson(
       entryPoint: EntryPoint, formatProperties: EntryPointJsonProperty[],
       ngccFolder: AbsoluteFsPath) {
-    const packageJson = entryPoint.packageJson;
+    if (formatProperties.length === 0) {
-
+      // No format properties need updating.
-    for (const formatProperty of formatProperties) {
+      return;
-      const formatPath = join(entryPoint.path, packageJson[formatProperty] !);
-      const newFormatPath = join(ngccFolder, relative(entryPoint.package, formatPath));
-      const newFormatProperty = formatProperty + '_ivy_ngcc';
-      (packageJson as any)[newFormatProperty] = relative(entryPoint.path, newFormatPath);
     }
 
-    this.fs.writeFile(
+    const packageJson = entryPoint.packageJson;
-        join(entryPoint.path, 'package.json'), `${JSON.stringify(packageJson, null, 2)}\n`);
+    const packageJsonPath = join(entryPoint.path, 'package.json');
+
+    // All format properties point to the same format-path.
+    const oldFormatProp = formatProperties[0] !;
+    const oldFormatPath = packageJson[oldFormatProp] !;
+    const oldAbsFormatPath = join(entryPoint.path, oldFormatPath);
+    const newAbsFormatPath = join(ngccFolder, relative(entryPoint.package, oldAbsFormatPath));
+    const newFormatPath = relative(entryPoint.path, newAbsFormatPath);
+
+    // Update all properties in `package.json` (both in memory and on disk).
+    const update = this.pkgJsonUpdater.createUpdate();
+
+    for (const formatProperty of formatProperties) {
+      if (packageJson[formatProperty] !== oldFormatPath) {
+        throw new Error(
+            `Unable to update '${packageJsonPath}': Format properties ` +
+            `(${formatProperties.join(', ')}) map to more than one format-path.`);
+      }
+
+      update.addChange([`${formatProperty}_ivy_ngcc`], newFormatPath);
+    }
+
+    update.writeChanges(packageJsonPath, packageJson);
   }
 }

packages/compiler-cli/ngcc/src/writing/package_json_updater.ts

@@ -0,0 +1,160 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {AbsoluteFsPath, FileSystem, dirname} from '../../../src/ngtsc/file_system';
+import {JsonObject, JsonValue} from '../packages/entry_point';
+
+
+export type PackageJsonChange = [string[], JsonValue];
+export type WritePackageJsonChangesFn =
+    (changes: PackageJsonChange[], packageJsonPath: AbsoluteFsPath, parsedJson?: JsonObject) =>
+        void;
+
+/**
+ * A utility object that can be used to safely update values in a `package.json` file.
+ *
+ * Example usage:
+ * ```ts
+ * const updatePackageJson = packageJsonUpdater
+ *     .createUpdate()
+ *     .addChange(['name'], 'package-foo')
+ *     .addChange(['scripts', 'foo'], 'echo FOOOO...')
+ *     .addChange(['dependencies', 'bar'], '1.0.0')
+ *     .writeChanges('/foo/package.json');
+ *     // or
+ *     // .writeChanges('/foo/package.json', inMemoryParsedJson);
+ * ```
+ */
+export interface PackageJsonUpdater {
+  /**
+   * Create a `PackageJsonUpdate` object, which provides a fluent API for batching updates to a
+   * `package.json` file. (Batching the updates is useful, because it avoid unnecessary I/O
+   * operations.)
+   */
+  createUpdate(): PackageJsonUpdate;
+
+  /**
+   * Write a set of changes to the specified `package.json` file and (and optionally a pre-existing,
+   * in-memory representation of it).
+   *
+   * @param changes The set of changes to apply.
+   * @param packageJsonPath The path to the `package.json` file that needs to be updated.
+   * @param parsedJson A pre-existing, in-memory representation of the `package.json` file that
+   *                   needs to be updated as well.
+   */
+  writeChanges(
+      changes: PackageJsonChange[], packageJsonPath: AbsoluteFsPath, parsedJson?: JsonObject): void;
+}
+
+/**
+ * A utility class providing a fluent API for recording multiple changes to a `package.json` file
+ * (and optionally its in-memory parsed representation).
+ *
+ * NOTE: This class should generally not be instantiated directly; instances are implicitly created
+ *       via `PackageJsonUpdater#createUpdate()`.
+ */
+export class PackageJsonUpdate {
+  private changes: PackageJsonChange[] = [];
+  private applied = false;
+
+  constructor(private writeChangesImpl: WritePackageJsonChangesFn) {}
+
+  /**
+   * Record a change to a `package.json` property. If the ancestor objects do not yet exist in the
+   * `package.json` file, they will be created.
+   *
+   * @param propertyPath The path of a (possibly nested) property to update.
+   * @param value The new value to set the property to.
+   */
+  addChange(propertyPath: string[], value: JsonValue): this {
+    this.ensureNotApplied();
+    this.changes.push([propertyPath, value]);
+    return this;
+  }
+
+  /**
+   * Write the recorded changes to the associated `package.json` file (and optionally a
+   * pre-existing, in-memory representation of it).
+   *
+   * @param packageJsonPath The path to the `package.json` file that needs to be updated.
+   * @param parsedJson A pre-existing, in-memory representation of the `package.json` file that
+   *                   needs to be updated as well.
+   */
+  writeChanges(packageJsonPath: AbsoluteFsPath, parsedJson?: JsonObject): void {
+    this.ensureNotApplied();
+    this.writeChangesImpl(this.changes, packageJsonPath, parsedJson);
+    this.applied = true;
+  }
+
+  private ensureNotApplied() {
+    if (this.applied) {
+      throw new Error('Trying to apply a `PackageJsonUpdate` that has already been applied.');
+    }
+  }
+}
+
+/** A `PackageJsonUpdater` that writes directly to the file-system. */
+export class DirectPackageJsonUpdater implements PackageJsonUpdater {
+  constructor(private fs: FileSystem) {}
+
+  createUpdate(): PackageJsonUpdate {
+    return new PackageJsonUpdate((...args) => this.writeChanges(...args));
+  }
+
+  writeChanges(
+      changes: PackageJsonChange[], packageJsonPath: AbsoluteFsPath,
+      preExistingParsedJson?: JsonObject): void {
+    if (changes.length === 0) {
+      throw new Error(`No changes to write to '${packageJsonPath}'.`);
+    }
+
+    // Read and parse the `package.json` content.
+    // NOTE: We are not using `preExistingParsedJson` (even if specified) to avoid corrupting the
+    //       content on disk in case `preExistingParsedJson` is outdated.
+    const parsedJson =
+        this.fs.exists(packageJsonPath) ? JSON.parse(this.fs.readFile(packageJsonPath)) : {};
+
+    // Apply all changes to both the canonical representation (read from disk) and any pre-existing,
+    // in-memory representation.
+    for (const [propPath, value] of changes) {
+      if (propPath.length === 0) {
+        throw new Error(`Missing property path for writing value to '${packageJsonPath}'.`);
+      }
+
+      applyChange(parsedJson, propPath, value);
+
+      if (preExistingParsedJson) {
+        applyChange(preExistingParsedJson, propPath, value);
+      }
+    }
+
+    // Ensure the containing directory exists (in case this is a synthesized `package.json` due to a
+    // custom configuration) and write the updated content to disk.
+    this.fs.ensureDir(dirname(packageJsonPath));
+    this.fs.writeFile(packageJsonPath, `${JSON.stringify(parsedJson, null, 2)}\n`);
+  }
+}
+
+// Helpers
+export function applyChange(ctx: JsonObject, propPath: string[], value: JsonValue): void {
+  const lastPropIdx = propPath.length - 1;
+  const lastProp = propPath[lastPropIdx];
+
+  for (let i = 0; i < lastPropIdx; i++) {
+    const key = propPath[i];
+    const newCtx = ctx.hasOwnProperty(key) ? ctx[key] : (ctx[key] = {});
+
+    if ((typeof newCtx !== 'object') || (newCtx === null) || Array.isArray(newCtx)) {
+      throw new Error(`Property path '${propPath.join('.')}' does not point to an object.`);
+    }
+
+    ctx = newCtx;
+  }
+
+  ctx[lastProp] = value;
+}

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -11,6 +11,7 @@ import {loadStandardTestFiles, loadTestFiles} from '../../../test/helpers';
 import {mainNgcc} from '../../src/main';
 import {markAsProcessed} from '../../src/packages/build_marker';
 import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point';
+import {DirectPackageJsonUpdater, PackageJsonUpdater} from '../../src/writing/package_json_updater';
 import {MockLogger} from '../helpers/mock_logger';
 
 const testFiles = loadStandardTestFiles({fakeCore: false, rxjs: true});
@@ -19,10 +20,12 @@ runInEachFileSystem(() => {
   describe('ngcc main()', () => {
     let _: typeof absoluteFrom;
     let fs: FileSystem;
+    let pkgJsonUpdater: PackageJsonUpdater;
 
     beforeEach(() => {
       _ = absoluteFrom;
       fs = getFileSystem();
+      pkgJsonUpdater = new DirectPackageJsonUpdater(fs);
       initMockFileSystem(fs, testFiles);
     });
 
@@ -195,7 +198,8 @@ runInEachFileSystem(() => {
       const basePath = _('/node_modules');
       const targetPackageJsonPath = join(basePath, packagePath, 'package.json');
       const targetPackage = loadPackage(packagePath);
-      markAsProcessed(fs, targetPackage, targetPackageJsonPath, ['typings', ...properties]);
+      markAsProcessed(
+          pkgJsonUpdater, targetPackage, targetPackageJsonPath, ['typings', ...properties]);
     }
 
 

packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts

@@ -9,6 +9,7 @@ import {AbsoluteFsPath, absoluteFrom, getFileSystem} from '../../../src/ngtsc/fi
 import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
 import {loadTestFiles} from '../../../test/helpers';
 import {hasBeenProcessed, markAsProcessed} from '../../src/packages/build_marker';
+import {DirectPackageJsonUpdater} from '../../src/writing/package_json_updater';
 
 runInEachFileSystem(() => {
   describe('Marker files', () => {
@@ -82,11 +83,12 @@ runInEachFileSystem(() => {
       it('should write properties in the package.json containing the version placeholder', () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
+        const pkgUpdater = new DirectPackageJsonUpdater(fs);
         let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__).toBeUndefined();
         expect(pkg.scripts).toBeUndefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
@@ -94,7 +96,7 @@ runInEachFileSystem(() => {
         expect(pkg.__processed_by_ivy_ngcc__.esm5).toBeUndefined();
         expect(pkg.scripts.prepublishOnly).toBeDefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
@@ -106,18 +108,19 @@ runInEachFileSystem(() => {
       it('should update the packageJson object in-place', () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
+        const pkgUpdater = new DirectPackageJsonUpdater(fs);
         const pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__).toBeUndefined();
         expect(pkg.scripts).toBeUndefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
         expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBeUndefined();
         expect(pkg.__processed_by_ivy_ngcc__.esm5).toBeUndefined();
         expect(pkg.scripts.prepublishOnly).toBeDefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
         expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
         expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBe('0.0.0-PLACEHOLDER');
@@ -128,21 +131,24 @@ runInEachFileSystem(() => {
       it('should one perform one write operation for all updated properties', () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
+        const pkgUpdater = new DirectPackageJsonUpdater(fs);
         const writeFileSpy = spyOn(fs, 'writeFile');
         let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5', 'esm2015', 'esm5']);
+        markAsProcessed(
+            pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5', 'esm2015', 'esm5']);
         expect(writeFileSpy).toHaveBeenCalledTimes(1);
       });
 
       it(`should keep backup of existing 'prepublishOnly' script`, () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
+        const pkgUpdater = new DirectPackageJsonUpdater(fs);
         const prepublishOnly = 'existing script';
         let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         pkg.scripts = {prepublishOnly};
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.scripts.prepublishOnly).toContain('This is not allowed');
         expect(pkg.scripts.prepublishOnly__ivy_ngcc_bak).toBe(prepublishOnly);
@@ -151,9 +157,10 @@ runInEachFileSystem(() => {
       it(`should not keep backup of overwritten 'prepublishOnly' script`, () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
+        const pkgUpdater = new DirectPackageJsonUpdater(fs);
         let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
 
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.scripts.prepublishOnly).toContain('This is not allowed');
@@ -161,7 +168,7 @@ runInEachFileSystem(() => {
 
         // Running again, now that there is `prepublishOnly` script (created by `ngcc`), it should
         // still not back it up as `prepublishOnly__ivy_ngcc_bak`.
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
+        markAsProcessed(pkgUpdater, pkg, COMMON_PACKAGE_PATH, ['fesm2015']);
 
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.scripts.prepublishOnly).toContain('This is not allowed');

packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts

@@ -13,6 +13,7 @@ import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointInfo}
 import {EntryPointBundle, makeEntryPointBundle} from '../../src/packages/entry_point_bundle';
 import {FileWriter} from '../../src/writing/file_writer';
 import {NewEntryPointFileWriter} from '../../src/writing/new_entry_point_file_writer';
+import {DirectPackageJsonUpdater} from '../../src/writing/package_json_updater';
 import {MockLogger} from '../helpers/mock_logger';
 import {loadPackageJson} from '../packages/entry_point_spec';
 
@@ -100,7 +101,7 @@ runInEachFileSystem(() => {
     describe('writeBundle() [primary entry-point]', () => {
       beforeEach(() => {
         fs = getFileSystem();
-        fileWriter = new NewEntryPointFileWriter(fs);
+        fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs));
         const config = new NgccConfiguration(fs, _('/'));
         entryPoint = getEntryPointInfo(
             fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test')) !;
@@ -236,7 +237,7 @@ runInEachFileSystem(() => {
     describe('writeBundle() [secondary entry-point]', () => {
       beforeEach(() => {
         fs = getFileSystem();
-        fileWriter = new NewEntryPointFileWriter(fs);
+        fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs));
         const config = new NgccConfiguration(fs, _('/'));
         entryPoint = getEntryPointInfo(
             fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test/a')) !;
@@ -361,7 +362,7 @@ runInEachFileSystem(() => {
     describe('writeBundle() [entry-point (with files placed outside entry-point folder)]', () => {
       beforeEach(() => {
         fs = getFileSystem();
-        fileWriter = new NewEntryPointFileWriter(fs);
+        fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs));
         const config = new NgccConfiguration(fs, _('/'));
         entryPoint = getEntryPointInfo(
             fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test/b')) !;

packages/compiler-cli/ngcc/test/writing/package_json_updater_spec.ts

@@ -0,0 +1,155 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system';
+import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
+import {loadTestFiles} from '../../../test/helpers';
+import {DirectPackageJsonUpdater, PackageJsonUpdater} from '../../src/writing/package_json_updater';
+
+runInEachFileSystem(() => {
+  describe('DirectPackageJsonUpdater', () => {
+    let _: typeof absoluteFrom;
+    let fs: FileSystem;
+    let updater: PackageJsonUpdater;
+
+    // Helpers
+    const readJson = (path: AbsoluteFsPath) => JSON.parse(fs.readFile(path));
+
+    beforeEach(() => {
+      _ = absoluteFrom;
+      fs = getFileSystem();
+      updater = new DirectPackageJsonUpdater(fs);
+    });
+
+    it('should update a `package.json` file on disk', () => {
+      const jsonPath = _('/foo/package.json');
+      loadTestFiles([
+        {name: jsonPath, contents: '{"foo": true, "bar": {"baz": "OK"}}'},
+      ]);
+
+      const update = updater.createUpdate().addChange(['foo'], false).addChange(['bar', 'baz'], 42);
+
+      // Not updated yet.
+      expect(readJson(jsonPath)).toEqual({
+        foo: true,
+        bar: {baz: 'OK'},
+      });
+
+      update.writeChanges(jsonPath);
+
+      // Updated now.
+      expect(readJson(jsonPath)).toEqual({
+        foo: false,
+        bar: {baz: 42},
+      });
+    });
+
+    it('should update an in-memory representation (if provided)', () => {
+      const jsonPath = _('/foo/package.json');
+      loadTestFiles([
+        {name: jsonPath, contents: '{"foo": true, "bar": {"baz": "OK"}}'},
+      ]);
+
+      const pkg = readJson(jsonPath);
+      const update = updater.createUpdate().addChange(['foo'], false).addChange(['bar', 'baz'], 42);
+
+      // Not updated yet.
+      expect(pkg).toEqual({
+        foo: true,
+        bar: {baz: 'OK'},
+      });
+
+      update.writeChanges(jsonPath, pkg);
+
+      // Updated now.
+      expect(pkg).toEqual({
+        foo: false,
+        bar: {baz: 42},
+      });
+    });
+
+    it('should create the `package.json` file, if it does not exist', () => {
+      const jsonPath = _('/foo/package.json');
+      expect(fs.exists(jsonPath)).toBe(false);
+
+      updater.createUpdate()
+          .addChange(['foo'], false)
+          .addChange(['bar', 'baz'], 42)
+          .writeChanges(jsonPath);
+
+      expect(fs.exists(jsonPath)).toBe(true);
+      expect(readJson(jsonPath)).toEqual({
+        foo: false,
+        bar: {baz: 42},
+      });
+    });
+
+    it('should create any missing ancestor objects', () => {
+      const jsonPath = _('/foo/package.json');
+      loadTestFiles([
+        {name: jsonPath, contents: '{"foo": {}}'},
+      ]);
+
+      const pkg = readJson(jsonPath);
+      updater.createUpdate()
+          .addChange(['foo', 'bar', 'baz', 'qux'], 'updated')
+          .writeChanges(jsonPath, pkg);
+
+      expect(readJson(jsonPath)).toEqual(pkg);
+      expect(pkg).toEqual({
+        foo: {
+          bar: {
+            baz: {
+              qux: 'updated',
+            },
+          },
+        },
+      });
+    });
+
+    it('should throw, if no changes have been recorded', () => {
+      const jsonPath = _('/foo/package.json');
+
+      expect(() => updater.createUpdate().writeChanges(jsonPath))
+          .toThrowError(`No changes to write to '${jsonPath}'.`);
+    });
+
+    it('should throw, if a property-path is empty', () => {
+      const jsonPath = _('/foo/package.json');
+
+      expect(() => updater.createUpdate().addChange([], 'missing').writeChanges(jsonPath))
+          .toThrowError(`Missing property path for writing value to '${jsonPath}'.`);
+    });
+
+    it('should throw, if a property-path points to a non-object intermediate value', () => {
+      const jsonPath = _('/foo/package.json');
+      loadTestFiles([
+        {name: jsonPath, contents: '{"foo": null, "bar": 42, "baz": {"qux": []}}'},
+      ]);
+
+      const writeToProp = (propPath: string[]) =>
+          updater.createUpdate().addChange(propPath, 'updated').writeChanges(jsonPath);
+
+      expect(() => writeToProp(['foo', 'child']))
+          .toThrowError('Property path \'foo.child\' does not point to an object.');
+      expect(() => writeToProp(['bar', 'child']))
+          .toThrowError('Property path \'bar.child\' does not point to an object.');
+      expect(() => writeToProp(['baz', 'qux', 'child']))
+          .toThrowError('Property path \'baz.qux.child\' does not point to an object.');
+    });
+
+    it('should throw, if trying to re-apply an already applied update', () => {
+      const update = updater.createUpdate().addChange(['foo'], 'updated');
+
+      expect(() => update.writeChanges(_('/foo/package.json'))).not.toThrow();
+      expect(() => update.writeChanges(_('/foo/package.json')))
+          .toThrowError('Trying to apply a `PackageJsonUpdate` that has already been applied.');
+      expect(() => update.writeChanges(_('/bar/package.json')))
+          .toThrowError('Trying to apply a `PackageJsonUpdate` that has already been applied.');
+    });
+  });
+});