fixup! Refactor PackageJsonEditor.

Author: iclanton

common/reviews/api/rush-lib.api.md

@@ -137,9 +137,13 @@ export class CommonVersionsConfiguration {
     getAllPreferredVersions(): Map<string, string>;
     getPreferredVersionsHash(): string;
     readonly implicitlyPreferredVersions: boolean | undefined;
+    // @deprecated (undocumented)
     static loadFromFile(jsonFilePath: string, rushConfiguration?: RushConfiguration): CommonVersionsConfiguration;
+    static loadFromFileAsync(jsonFilePath: string, rushConfiguration?: RushConfiguration): Promise<CommonVersionsConfiguration>;
     readonly preferredVersions: Map<string, string>;
+    // @deprecated (undocumented)
     save(): boolean;
+    saveAsync(): Promise<boolean>;
 }
 
 export { CredentialCache }
@@ -1097,15 +1101,19 @@ export class PackageJsonEditor {
     readonly filePath: string;
     // (undocumented)
     static fromObject(object: IPackageJson, filename: string): PackageJsonEditor;
-    // (undocumented)
+    // @deprecated (undocumented)
     static load(filePath: string): PackageJsonEditor;
     // (undocumented)
+    static loadAsync(filePath: string): Promise<PackageJsonEditor>;
+    // (undocumented)
     get name(): string;
     // (undocumented)
     removeDependency(packageName: string, dependencyType: DependencyType): void;
     get resolutionsList(): ReadonlyArray<PackageJsonDependency>;
-    // (undocumented)
+    // @deprecated (undocumented)
     saveIfModified(): boolean;
+    // (undocumented)
+    saveIfModifiedAsync(): Promise<boolean>;
     saveToObject(): IPackageJson;
     // (undocumented)
     tryGetDependency(packageName: string): PackageJsonDependency | undefined;
@@ -1648,8 +1656,12 @@ export abstract class VersionPolicy {
     // @internal
     static load(versionPolicyJson: IVersionPolicyJson): VersionPolicy | undefined;
     get policyName(): string;
+    // @deprecated (undocumented)
     setDependenciesBeforeCommit(packageName: string, configuration: RushConfiguration): void;
+    setDependenciesBeforeCommitAsync(packageName: string, configuration: RushConfiguration): Promise<void>;
+    // @deprecated (undocumented)
     setDependenciesBeforePublish(packageName: string, configuration: RushConfiguration): void;
+    setDependenciesBeforePublishAsync(packageName: string, configuration: RushConfiguration): Promise<void>;
     abstract validate(versionString: string, packageName: string): void;
 }
 

libraries/rush-lib/src/api/CommonVersionsConfiguration.ts

@@ -186,17 +186,42 @@ export class CommonVersionsConfiguration {
   }
 
   /**
-   * Loads the common-versions.json data from the specified file path.
-   * If the file has not been created yet, then an empty object is returned.
+   * @deprecated Use {@link CommonVersionsConfiguration.loadFromFileAsync} method instead.
    */
   public static loadFromFile(
     jsonFilePath: string,
     rushConfiguration?: RushConfiguration
   ): CommonVersionsConfiguration {
     let commonVersionsJson: ICommonVersionsJson | undefined = undefined;
-
-    if (FileSystem.exists(jsonFilePath)) {
+    try {
       commonVersionsJson = JsonFile.loadAndValidate(jsonFilePath, CommonVersionsConfiguration._jsonSchema);
+    } catch (error) {
+      if (!FileSystem.isNotExistError(error)) {
+        throw error;
+      }
+    }
+
+    return new CommonVersionsConfiguration(commonVersionsJson, jsonFilePath, rushConfiguration);
+  }
+
+  /**
+   * Loads the common-versions.json data from the specified file path.
+   * If the file has not been created yet, then an empty object is returned.
+   */
+  public static async loadFromFileAsync(
+    jsonFilePath: string,
+    rushConfiguration?: RushConfiguration
+  ): Promise<CommonVersionsConfiguration> {
+    let commonVersionsJson: ICommonVersionsJson | undefined = undefined;
+    try {
+      commonVersionsJson = await JsonFile.loadAndValidateAsync(
+        jsonFilePath,
+        CommonVersionsConfiguration._jsonSchema
+      );
+    } catch (error) {
+      if (!FileSystem.isNotExistError(error)) {
+        throw error;
+      }
     }
 
     return new CommonVersionsConfiguration(commonVersionsJson, jsonFilePath, rushConfiguration);
@@ -242,7 +267,7 @@ export class CommonVersionsConfiguration {
   }
 
   /**
-   * Writes the "common-versions.json" file to disk, using the filename that was passed to loadFromFile().
+   * @deprecated Use {@link CommonVersionsConfiguration.saveAsync} method instead.
    */
   public save(): boolean {
     if (this._modified) {
@@ -257,6 +282,22 @@ export class CommonVersionsConfiguration {
     return false;
   }
 
+  /**
+   * Writes the "common-versions.json" file to disk, using the filename that was passed to loadFromFile().
+   */
+  public async saveAsync(): Promise<boolean> {
+    if (this._modified) {
+      await JsonFile.saveAsync(this._serialize(), this.filePath, {
+        updateExistingFile: true,
+        ignoreUndefinedValues: true
+      });
+      this._modified = false;
+      return true;
+    }
+
+    return false;
+  }
+
   /**
    * Returns preferredVersions.
    */

libraries/rush-lib/src/api/PackageJsonEditor.ts

@@ -189,6 +189,9 @@ export class PackageJsonEditor {
     }
   }
 
+  /**
+   * @deprecated Use {@link PackageJsonEditor.loadAsync} method instead.
+   */
   public static load(filePath: string): PackageJsonEditor {
     const packageJson: IPackageJson = JsonFile.load(filePath);
     return new PackageJsonEditor(filePath, packageJson);
@@ -318,6 +321,9 @@ export class PackageJsonEditor {
     this._modified = true;
   }
 
+  /**
+   * @deprecated Use {@link PackageJsonEditor.saveIfModifiedAsync} method instead.
+   */
   public saveIfModified(): boolean {
     if (this._modified) {
       this._modified = false;

libraries/rush-lib/src/api/SaveCallbackPackageJsonEditor.ts

@@ -24,8 +24,8 @@ export class SaveCallbackPackageJsonEditor extends PackageJsonEditor {
     return new SaveCallbackPackageJsonEditor(options);
   }
 
-  public saveIfModified(): boolean {
-    const modified: boolean = super.saveIfModified();
+  public async saveIfModifiedAsync(): Promise<boolean> {
+    const modified: boolean = await super.saveIfModifiedAsync();
     if (this._onSaved) {
       this._onSaved(this.saveToObject());
     }

libraries/rush-lib/src/api/Subspace.ts

@@ -325,6 +325,7 @@ export class Subspace {
         this._rushConfiguration
       );
     }
+
     return this._commonVersionsConfiguration;
   }
 

libraries/rush-lib/src/api/VersionPolicy.ts

@@ -52,6 +52,66 @@ export enum VersionPolicyDefinitionName {
   'individualVersion'
 }
 
+/**
+ * Updates the dependencies in the package json editor to values used for publishing, if needed.
+ *
+ * @returns the updated package json editor if the version format for publish is 'exact', otherwise undefined.
+ */
+function updateDependenciesBeforePublish(
+  packageName: string,
+  configuration: RushConfiguration,
+  versionFormatForPublish: VersionFormatForPublish
+): PackageJsonEditor | undefined {
+  if (versionFormatForPublish === 'exact') {
+    const project: RushConfigurationProject = configuration.getProjectByName(packageName)!;
+
+    const packageJsonEditor: PackageJsonEditor = project.packageJsonEditor;
+
+    for (const dependency of packageJsonEditor.dependencyList) {
+      const rushDependencyProject: RushConfigurationProject | undefined = configuration.getProjectByName(
+        dependency.name
+      );
+
+      if (rushDependencyProject) {
+        const dependencyVersion: string = rushDependencyProject.packageJson.version;
+
+        dependency.setVersion(dependencyVersion);
+      }
+    }
+
+    return packageJsonEditor;
+  }
+}
+
+/**
+ * Updates the dependencies in the package json editor to values used for checked-in source, if needed.
+ *
+ * @returns the updated package json editor if the version format for commit is 'wildcard', otherwise undefined.
+ */
+function updateDependenciesBeforeCommit(
+  packageName: string,
+  configuration: RushConfiguration,
+  versionFormatForCommit: VersionFormatForCommit
+): PackageJsonEditor | undefined {
+  if (versionFormatForCommit === 'wildcard') {
+    const project: RushConfigurationProject = configuration.getProjectByName(packageName)!;
+
+    const packageJsonEditor: PackageJsonEditor = project.packageJsonEditor;
+
+    for (const dependency of packageJsonEditor.dependencyList) {
+      const rushDependencyProject: RushConfigurationProject | undefined = configuration.getProjectByName(
+        dependency.name
+      );
+
+      if (rushDependencyProject) {
+        dependency.setVersion('*');
+      }
+    }
+
+    return packageJsonEditor;
+  }
+}
+
 /**
  * This is the base class for version policy which controls how versions get bumped.
  * @public
@@ -161,53 +221,63 @@ export abstract class VersionPolicy {
   public abstract validate(versionString: string, packageName: string): void;
 
   /**
-   * Tells the version policy to modify any dependencies in the target package
-   * to values used for publishing.
+   * @deprecated Use {@link VersionPolicy.setDependenciesBeforePublishAsync} method instead.
    */
   public setDependenciesBeforePublish(packageName: string, configuration: RushConfiguration): void {
-    if (this._versionFormatForPublish === 'exact') {
-      const project: RushConfigurationProject = configuration.getProjectByName(packageName)!;
+    const packageJsonEditor: PackageJsonEditor | undefined = updateDependenciesBeforePublish(
+      packageName,
+      configuration,
+      this._versionFormatForPublish
+    );
 
-      const packageJsonEditor: PackageJsonEditor = project.packageJsonEditor;
+    packageJsonEditor?.saveIfModified();
+  }
 
-      for (const dependency of packageJsonEditor.dependencyList) {
-        const rushDependencyProject: RushConfigurationProject | undefined = configuration.getProjectByName(
-          dependency.name
-        );
+  /**
+   * Tells the version policy to modify any dependencies in the target package
+   * to values used for publishing.
+   */
+  public async setDependenciesBeforePublishAsync(
+    packageName: string,
+    configuration: RushConfiguration
+  ): Promise<void> {
+    const packageJsonEditor: PackageJsonEditor | undefined = updateDependenciesBeforePublish(
+      packageName,
+      configuration,
+      this._versionFormatForPublish
+    );
 
-        if (rushDependencyProject) {
-          const dependencyVersion: string = rushDependencyProject.packageJson.version;
+    await packageJsonEditor?.saveIfModifiedAsync();
+  }
 
-          dependency.setVersion(dependencyVersion);
-        }
-      }
+  /**
+   * @deprecated Use {@link VersionPolicy.setDependenciesBeforeCommitAsync} method instead.
+   */
+  public setDependenciesBeforeCommit(packageName: string, configuration: RushConfiguration): void {
+    const packageJsonEditor: PackageJsonEditor | undefined = updateDependenciesBeforeCommit(
+      packageName,
+      configuration,
+      this._versionFormatForCommit
+    );
 
-      packageJsonEditor.saveIfModified();
-    }
+    packageJsonEditor?.saveIfModified();
   }
 
   /**
    * Tells the version policy to modify any dependencies in the target package
    * to values used for checked-in source.
    */
-  public setDependenciesBeforeCommit(packageName: string, configuration: RushConfiguration): void {
-    if (this._versionFormatForCommit === 'wildcard') {
-      const project: RushConfigurationProject = configuration.getProjectByName(packageName)!;
-
-      const packageJsonEditor: PackageJsonEditor = project.packageJsonEditor;
-
-      for (const dependency of packageJsonEditor.dependencyList) {
-        const rushDependencyProject: RushConfigurationProject | undefined = configuration.getProjectByName(
-          dependency.name
-        );
-
-        if (rushDependencyProject) {
-          dependency.setVersion('*');
-        }
-      }
+  public async setDependenciesBeforeCommitAsync(
+    packageName: string,
+    configuration: RushConfiguration
+  ): Promise<void> {
+    const packageJsonEditor: PackageJsonEditor | undefined = updateDependenciesBeforeCommit(
+      packageName,
+      configuration,
+      this._versionFormatForCommit
+    );
 
-      packageJsonEditor.saveIfModified();
-    }
+    await packageJsonEditor?.saveIfModifiedAsync();
   }
 }
 

libraries/rush-lib/src/api/test/CommonVersionsConfiguration.test.ts

@@ -5,9 +5,9 @@ import { CommonVersionsConfiguration } from '../CommonVersionsConfiguration';
 import type { RushConfiguration } from '../RushConfiguration';
 
 describe(CommonVersionsConfiguration.name, () => {
-  it('can load the file', () => {
+  it('can load the file', async () => {
     const filename: string = `${__dirname}/jsonFiles/common-versions.json`;
-    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(
+    const configuration: CommonVersionsConfiguration = await CommonVersionsConfiguration.loadFromFileAsync(
       filename,
       {} as RushConfiguration
     );
@@ -16,33 +16,39 @@ describe(CommonVersionsConfiguration.name, () => {
     expect(configuration.allowedAlternativeVersions.get('library-3')).toEqual(['^1.2.3']);
   });
 
-  it('gets `ensureConsistentVersions` from the file if it provides that value', () => {
+  it('gets `ensureConsistentVersions` from the file if it provides that value', async () => {
     const filename: string = `${__dirname}/jsonFiles/common-versions-with-ensureConsistentVersionsTrue.json`;
-    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(filename, {
-      _ensureConsistentVersionsJsonValue: undefined,
-      ensureConsistentVersions: false
-    } as RushConfiguration);
+    const configuration: CommonVersionsConfiguration = await CommonVersionsConfiguration.loadFromFileAsync(
+      filename,
+      {
+        _ensureConsistentVersionsJsonValue: undefined,
+        ensureConsistentVersions: false
+      } as RushConfiguration
+    );
 
     expect(configuration.ensureConsistentVersions).toBe(true);
   });
 
-  it("gets `ensureConsistentVersions` from the rush configuration if common-versions.json doesn't provide that value", () => {
+  it("gets `ensureConsistentVersions` from the rush configuration if common-versions.json doesn't provide that value", async () => {
     const filename: string = `${__dirname}/jsonFiles/common-versions.json`;
-    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(filename, {
-      _ensureConsistentVersionsJsonValue: false,
-      ensureConsistentVersions: false
-    } as RushConfiguration);
+    const configuration: CommonVersionsConfiguration = await CommonVersionsConfiguration.loadFromFileAsync(
+      filename,
+      {
+        _ensureConsistentVersionsJsonValue: false,
+        ensureConsistentVersions: false
+      } as RushConfiguration
+    );
 
     expect(configuration.ensureConsistentVersions).toBe(false);
   });
 
-  it('Does not allow `ensureConsistentVersions` to be set in both rush.json and common-versions.json', () => {
+  it('Does not allow `ensureConsistentVersions` to be set in both rush.json and common-versions.json', async () => {
     const filename: string = `${__dirname}/jsonFiles/common-versions-with-ensureConsistentVersionsTrue.json`;
-    expect(() =>
-      CommonVersionsConfiguration.loadFromFile(filename, {
+    await expect(() =>
+      CommonVersionsConfiguration.loadFromFileAsync(filename, {
         _ensureConsistentVersionsJsonValue: false,
         ensureConsistentVersions: false
       } as RushConfiguration)
-    ).toThrowErrorMatchingSnapshot();
+    ).rejects.toThrowErrorMatchingSnapshot();
   });
 });