[rush] Fix an issue where rush add always adds the latest version if ensureConsistentVersions is set in rush.json.

common/changes/@microsoft/rush/fix-rush-add_2024-07-10-19-56.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Fix an issue where `rush add` would ignore the `ensureConsistentVersions` option if that option was set in `rush.json` instead of in `common/config/rush/common-versions.json`.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

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

@@ -120,12 +120,12 @@ export type CobuildLockProviderFactory = (cobuildJson: ICobuildJson) => ICobuild
 // @public
 export class CommonVersionsConfiguration {
     readonly allowedAlternativeVersions: Map<string, ReadonlyArray<string>>;
-    readonly ensureConsistentVersions: boolean | undefined;
+    readonly ensureConsistentVersions: boolean;
     readonly filePath: string;
     getAllPreferredVersions(): Map<string, string>;
     getPreferredVersionsHash(): string;
     readonly implicitlyPreferredVersions: boolean | undefined;
-    static loadFromFile(jsonFilename: string): CommonVersionsConfiguration;
+    static loadFromFile(jsonFilename: string, rushConfiguration?: RushConfiguration): CommonVersionsConfiguration;
     readonly preferredVersions: Map<string, string>;
     save(): boolean;
 }
@@ -1169,6 +1169,8 @@ export class RushConfiguration {
     get defaultSubspace(): Subspace;
     // @deprecated
     readonly ensureConsistentVersions: boolean;
+    // @internal
+    readonly _ensureConsistentVersionsJsonValue: boolean | undefined;
     // @beta
     readonly eventHooks: EventHooks;
     // @beta

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

@@ -14,6 +14,8 @@ import {
 
 import { PackageNameParsers } from './PackageNameParsers';
 import { JsonSchemaUrls } from '../logic/JsonSchemaUrls';
+import type { RushConfiguration } from './RushConfiguration';
+import { RushConstants } from '../logic/RushConstants';
 import schemaJson from '../schemas/common-versions.schema.json';
 
 /**
@@ -84,7 +86,7 @@ export class CommonVersionsConfiguration {
    * If true, then consistent version specifiers for dependencies will be enforced.
    * I.e. "rush check" is run before some commands.
    */
-  public readonly ensureConsistentVersions: boolean | undefined;
+  public readonly ensureConsistentVersions: boolean;
 
   /**
    * A table that specifies a "preferred version" for a given NPM package.  This feature is typically used
@@ -116,7 +118,11 @@ export class CommonVersionsConfiguration {
    */
   public readonly allowedAlternativeVersions: Map<string, ReadonlyArray<string>>;
 
-  private constructor(commonVersionsJson: ICommonVersionsJson | undefined, filePath: string) {
+  private constructor(
+    commonVersionsJson: ICommonVersionsJson | undefined,
+    filePath: string,
+    rushConfiguration: RushConfiguration | undefined
+  ) {
     this._preferredVersions = new ProtectableMap<string, string>({
       onSet: this._onSetPreferredVersions.bind(this)
     });
@@ -132,7 +138,30 @@ export class CommonVersionsConfiguration {
       onSet: this._onSetAllowedAlternativeVersions.bind(this)
     });
     this.allowedAlternativeVersions = this._allowedAlternativeVersions.protectedView;
-    this.ensureConsistentVersions = commonVersionsJson?.ensureConsistentVersions;
+
+    const subspacesFeatureEnabled: boolean | undefined = rushConfiguration?.subspacesFeatureEnabled;
+    const rushJsonEnsureConsistentVersions: boolean | undefined =
+      rushConfiguration?._ensureConsistentVersionsJsonValue;
+    const commonVersionsEnsureConsistentVersions: boolean | undefined =
+      commonVersionsJson?.ensureConsistentVersions;
+    if (subspacesFeatureEnabled && rushJsonEnsureConsistentVersions !== undefined) {
+      throw new Error(
+        `When using subspaces, the ensureConsistentVersions config is now defined in the ${RushConstants.commonVersionsFilename} file, ` +
+          `you must remove the old setting "ensureConsistentVersions" from ${RushConstants.rushJsonFilename}`
+      );
+    } else if (
+      !subspacesFeatureEnabled &&
+      rushJsonEnsureConsistentVersions !== undefined &&
+      commonVersionsEnsureConsistentVersions !== undefined
+    ) {
+      throw new Error(
+        `When the ensureConsistentVersions config is defined in the ${RushConstants.rushJsonFilename} file, ` +
+          `it cannot also be defined in the ${RushConstants.commonVersionsFilename} file`
+      );
+    }
+
+    this.ensureConsistentVersions =
+      commonVersionsEnsureConsistentVersions ?? rushJsonEnsureConsistentVersions ?? false;
 
     if (commonVersionsJson) {
       try {
@@ -155,14 +184,17 @@ 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.
    */
-  public static loadFromFile(jsonFilename: string): CommonVersionsConfiguration {
+  public static loadFromFile(
+    jsonFilename: string,
+    rushConfiguration?: RushConfiguration
+  ): CommonVersionsConfiguration {
     let commonVersionsJson: ICommonVersionsJson | undefined = undefined;
 
     if (FileSystem.exists(jsonFilename)) {
       commonVersionsJson = JsonFile.loadAndValidate(jsonFilename, CommonVersionsConfiguration._jsonSchema);
     }
 
-    return new CommonVersionsConfiguration(commonVersionsJson, jsonFilename);
+    return new CommonVersionsConfiguration(commonVersionsJson, jsonFilename, rushConfiguration);
   }
 
   private static _deserializeTable<TValue>(

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

@@ -470,6 +470,13 @@ export class RushConfiguration {
    */
   public readonly suppressNodeLtsWarning: boolean;
 
+  /**
+   * The raw value of `ensureConsistentVersions` from the `rush.json` file.
+   *
+   * @internal
+   */
+  public readonly _ensureConsistentVersionsJsonValue: boolean | undefined;
+
   /**
    * If true, then consistent version specifiers for dependencies will be enforced.
    * I.e. "rush check" is run before some commands.
@@ -610,6 +617,7 @@ export class RushConfiguration {
 
     this.suppressNodeLtsWarning = !!rushConfigurationJson.suppressNodeLtsWarning;
 
+    this._ensureConsistentVersionsJsonValue = rushConfigurationJson.ensureConsistentVersions;
     this.ensureConsistentVersions = !!rushConfigurationJson.ensureConsistentVersions;
 
     // Try getting a subspace configuration
@@ -819,22 +827,6 @@ export class RushConfiguration {
     this._hasVariantsField = !!rushConfigurationJson.variants;
 
     this._pathTrees = new Map();
-
-    if (this.subspacesFeatureEnabled && rushConfigurationJson.ensureConsistentVersions !== undefined) {
-      throw new Error(
-        `When using subspaces, the ensureConsistentVersions config is now defined in the ${RushConstants.commonVersionsFilename} file, ` +
-          `you must remove the old setting "ensureConsistentVersions" from ${RushConstants.rushJsonFilename}`
-      );
-    } else if (
-      !this.subspacesFeatureEnabled &&
-      rushConfigurationJson.ensureConsistentVersions !== undefined &&
-      this.defaultSubspace.getCommonVersions().ensureConsistentVersions !== undefined
-    ) {
-      throw new Error(
-        `When the ensureConsistentVersions config is defined in the ${RushConstants.rushJsonFilename} file, ` +
-          `it cannot also be defined in the ${RushConstants.commonVersionsFilename} file`
-      );
-    }
   }
 
   private _initializeAndValidateLocalProjects(): void {

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

@@ -267,7 +267,10 @@ export class Subspace {
   public getCommonVersions(): CommonVersionsConfiguration {
     const commonVersionsFilename: string = this.getCommonVersionsFilePath();
     if (!this._commonVersionsConfiguration) {
-      this._commonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(commonVersionsFilename);
+      this._commonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(
+        commonVersionsFilename,
+        this._rushConfiguration
+      );
     }
     return this._commonVersionsConfiguration;
   }

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

@@ -2,13 +2,47 @@
 // See LICENSE in the project root for license information.
 
 import { CommonVersionsConfiguration } from '../CommonVersionsConfiguration';
+import type { RushConfiguration } from '../RushConfiguration';
 
 describe(CommonVersionsConfiguration.name, () => {
   it('can load the file', () => {
     const filename: string = `${__dirname}/jsonFiles/common-versions.json`;
-    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(filename);
+    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(
+      filename,
+      {} as RushConfiguration
+    );
 
     expect(configuration.preferredVersions.get('@scope/library-1')).toEqual('~3.2.1');
     expect(configuration.allowedAlternativeVersions.get('library-3')).toEqual(['^1.2.3']);
   });
+
+  it('gets `ensureConsistentVersions` from the file if it provides that value', () => {
+    const filename: string = `${__dirname}/jsonFiles/common-versions-with-ensureConsistentVersionsTrue.json`;
+    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(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", () => {
+    const filename: string = `${__dirname}/jsonFiles/common-versions.json`;
+    const configuration: CommonVersionsConfiguration = CommonVersionsConfiguration.loadFromFile(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', () => {
+    const filename: string = `${__dirname}/jsonFiles/common-versions-with-ensureConsistentVersionsTrue.json`;
+    expect(() =>
+      CommonVersionsConfiguration.loadFromFile(filename, {
+        _ensureConsistentVersionsJsonValue: false,
+        ensureConsistentVersions: false
+      } as RushConfiguration)
+    ).toThrowErrorMatchingSnapshot();
+  });
 });

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

@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`CommonVersionsConfiguration Does not allow \`ensureConsistentVersions\` to be set in both rush.json and common-versions.json 1`] = `"When the ensureConsistentVersions config is defined in the rush.json file, it cannot also be defined in the common-versions.json file"`;

libraries/rush-lib/src/api/test/jsonFiles/common-versions-with-ensureConsistentVersionsTrue.json

@@ -0,0 +1,9 @@
+{
+  "ensureConsistentVersions": true,
+  "preferredVersions": {
+    "@scope/library-1": "~3.2.1"
+  },
+  "allowedAlternativeVersions": {
+    "library-3": ["^1.2.3"]
+  }
+}