From 3d9dd6df0e61c5a9a47ac78d2e4590df065852f2 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 12 Aug 2019 18:15:24 +0300 Subject: [PATCH] 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 | 43 +++-- .../ngcc/src/packages/build_marker.ts | 29 ++-- .../ngcc/src/packages/entry_point.ts | 7 +- .../ngcc/src/writing/in_place_file_writer.ts | 2 +- .../writing/new_entry_point_file_writer.ts | 41 +++-- .../ngcc/src/writing/package_json_updater.ts | 160 ++++++++++++++++++ .../ngcc/test/integration/ngcc_spec.ts | 6 +- .../ngcc/test/packages/build_marker_spec.ts | 23 ++- .../new_entry_point_file_writer_spec.ts | 7 +- .../test/writing/package_json_updater_spec.ts | 155 +++++++++++++++++ 10 files changed, 416 insertions(+), 57 deletions(-) create mode 100644 packages/compiler-cli/ngcc/src/writing/package_json_updater.ts create mode 100644 packages/compiler-cli/ngcc/test/writing/package_json_updater_spec.ts diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 54b093bab1..9f76ad0990 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/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, - pathMappings, supportedPropertiesToConsider, compileAllFormats); + fileSystem, pkgJsonUpdater, logger, dependencyResolver, config, absBasePath, + targetEntryPointPath, pathMappings, supportedPropertiesToConsider, compileAllFormats); const processingMetadataPerEntryPoint = new Map(); 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 { - return createNewEntryPointFormats ? new NewEntryPointFileWriter(fs) : new InPlaceFileWriter(fs); +function getFileWriter( + 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, - basePath: AbsoluteFsPath, targetEntryPointPath: string | undefined, - pathMappings: PathMappings | undefined, propertiesToConsider: string[], - compileAllFormats: boolean): EntryPoint[] { + fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater, logger: Logger, + resolver: DependencyResolver, config: NgccConfiguration, basePath: AbsoluteFsPath, + targetEntryPointPath: string | undefined, pathMappings: PathMappings | undefined, + propertiesToConsider: string[], compileAllFormats: boolean): EntryPoint[] { const {entryPoints, invalidEntryPoints} = (targetEntryPointPath !== undefined) ? getTargetedEntryPoints( - fs, config, logger, resolver, basePath, targetEntryPointPath, propertiesToConsider, - compileAllFormats, pathMappings) : + fs, pkgJsonUpdater, logger, resolver, config, basePath, targetEntryPointPath, + 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, - basePath: AbsoluteFsPath, targetEntryPointPath: string, propertiesToConsider: string[], - compileAllFormats: boolean, pathMappings: PathMappings | undefined): SortedEntryPointsInfo { + fs: FileSystem, pkgJsonUpdater: PackageJsonUpdater, logger: Logger, + resolver: DependencyResolver, config: NgccConfiguration, basePath: AbsoluteFsPath, + 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 { diff --git a/packages/compiler-cli/ngcc/src/packages/build_marker.ts b/packages/compiler-cli/ngcc/src/packages/build_marker.ts index d9b99ff651..0238afa17c 100644 --- a/packages/compiler-cli/ngcc/src/packages/build_marker.ts +++ b/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, - properties: PackageJsonFormatProperties[]) { - const processed = - packageJson.__processed_by_ivy_ngcc__ || (packageJson.__processed_by_ivy_ngcc__ = {}); + pkgJsonUpdater: PackageJsonUpdater, packageJson: EntryPointPackageJson, + packageJsonPath: AbsoluteFsPath, formatProperties: PackageJsonFormatProperties[]): void { + const update = pkgJsonUpdater.createUpdate(); - for (const prop of properties) { - processed[prop] = NGCC_VERSION; + // Update the format properties to mark them as processed. + for (const prop of formatProperties) { + update.addChange(['__processed_by_ivy_ngcc__', prop], NGCC_VERSION); } - const scripts = packageJson.scripts || (packageJson.scripts = {}); - const oldPrepublishOnly = scripts.prepublishOnly; + // Update the `prepublishOnly` script (keeping a backup, if necessary) to prevent `ngcc`'d + // 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 - // 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)); + update.writeChanges(packageJsonPath, packageJson); } diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 4132f42992..4fe88198de 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/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 {} +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; __processed_by_ivy_ngcc__?: Record; diff --git a/packages/compiler-cli/ngcc/src/writing/in_place_file_writer.ts b/packages/compiler-cli/ngcc/src/writing/in_place_file_writer.ts index c6fc47ce43..118f256fed 100644 --- a/packages/compiler-cli/ngcc/src/writing/in_place_file_writer.ts +++ b/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) {} diff --git a/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts b/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts index 389f4d3b14..f21d64b049 100644 --- a/packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts +++ b/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; - - for (const formatProperty of formatProperties) { - 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); + if (formatProperties.length === 0) { + // No format properties need updating. + return; } - this.fs.writeFile( - join(entryPoint.path, 'package.json'), `${JSON.stringify(packageJson, null, 2)}\n`); + const packageJson = entryPoint.packageJson; + 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); } } diff --git a/packages/compiler-cli/ngcc/src/writing/package_json_updater.ts b/packages/compiler-cli/ngcc/src/writing/package_json_updater.ts new file mode 100644 index 0000000000..9295191efd --- /dev/null +++ b/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; +} diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index aa789fb069..31351e81c6 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/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]); } diff --git a/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts b/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts index 1eeb377d1b..630591a0f2 100644 --- a/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts +++ b/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'); diff --git a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts b/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts index caff700c55..f3772d8727 100644 --- a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts +++ b/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')) !; diff --git a/packages/compiler-cli/ngcc/test/writing/package_json_updater_spec.ts b/packages/compiler-cli/ngcc/test/writing/package_json_updater_spec.ts new file mode 100644 index 0000000000..1a3c5dd346 --- /dev/null +++ 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.'); + }); + }); +});