[rush] Fix an issue that rush-pnpm patch-commit can not generate co…

…rrect pnpm-config.json file in subspace case (#4838) * fix: rush-pnpm patch-commit * chore: rush change * chore: address PR comments * fix: handle case where the pnpm-config.json not exists * chore: add comments * Improve error message wording * Prepare to publish a PATCH release --------- Co-authored-by: Pete Gonzalez <[email protected]>
microsoft · Jul 19, 2024 · 6c6ed43 · 6c6ed43
1 parent f0c756b
commit 6c6ed43
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 5 deletions.
diff --git a/common/changes/@microsoft/rush/chao-fix-rush-pnpm-patch_2024-07-17-20-42.json b/common/changes/@microsoft/rush/chao-fix-rush-pnpm-patch_2024-07-17-20-42.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Fix an issue where `rush-pnpm patch-commit` did not work correctly when subspaces are enabled.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}
diff --git a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts
@@ -446,17 +446,29 @@ export class RushPnpmCommandLineParser {
 
     switch (commandName) {
       case 'patch-commit': {
+        // why need to throw error when pnpm-config.json not exists?
+        // 1. pnpm-config.json is required for `rush-pnpm patch-commit`. Rush writes the patched dependency to the pnpm-config.json when finishes.
+        // 2. we can not fallback to use Monorepo config folder (common/config/rush) due to that this command is intended to apply to input subspace only.
+        //    It will produce unexpected behavior if we use the fallback.
+        if (this._subspace.getPnpmOptions() === undefined) {
+          this._terminal.writeErrorLine(
+            `The "rush-pnpm patch-commit" command cannot proceed without a pnpm-config.json file.` +
+              `  Create one in this folder: ${this._subspace.getSubspaceConfigFolder()}`
+          );
+          break;
+        }
+
         // Example: "C:\MyRepo\common\temp\package.json"
         const commonPackageJsonFilename: string = `${subspaceTempFolder}/${FileConstants.PackageJson}`;
         const commonPackageJson: JsonObject = JsonFile.load(commonPackageJsonFilename);
         const newGlobalPatchedDependencies: Record<string, string> | undefined =
           commonPackageJson?.pnpm?.patchedDependencies;
         const currentGlobalPatchedDependencies: Record<string, string> | undefined =
-          this._rushConfiguration.pnpmOptions.globalPatchedDependencies;
+          this._subspace.getPnpmOptions()?.globalPatchedDependencies;
 
         if (!objectsAreDeepEqual(currentGlobalPatchedDependencies, newGlobalPatchedDependencies)) {
           const commonTempPnpmPatchesFolder: string = `${subspaceTempFolder}/${RushConstants.pnpmPatchesFolderName}`;
-          const rushPnpmPatchesFolder: string = `${this._rushConfiguration.commonFolder}/${RushConstants.pnpmPatchesCommonFolderName}`;
+          const rushPnpmPatchesFolder: string = `${subspaceTempFolder}/${RushConstants.pnpmPatchesCommonFolderName}`;
           // Copy (or delete) common\temp\patches\ --> common\pnpm-patches\
           if (FileSystem.exists(commonTempPnpmPatchesFolder)) {
             FileSystem.ensureEmptyFolder(rushPnpmPatchesFolder);
@@ -477,14 +489,14 @@ export class RushPnpmCommandLineParser {
           }
 
           // Update patchedDependencies to pnpm configuration file
-          this._rushConfiguration.pnpmOptions.updateGlobalPatchedDependencies(newGlobalPatchedDependencies);
+          this._subspace.getPnpmOptions()?.updateGlobalPatchedDependencies(newGlobalPatchedDependencies);
 
           // Rerun installation to update
           await this._doRushUpdateAsync();
 
           this._terminal.writeWarningLine(
             `Rush refreshed the ${RushConstants.pnpmConfigFilename}, shrinkwrap file and patch files under the ` +
-              `"${RushConstants.commonFolderName}/${RushConstants.pnpmPatchesCommonFolderName}" folder.\n` +
+              `"${commonTempPnpmPatchesFolder}" folder.\n` +
               '  Please commit this change to Git.'
           );
         }

diff --git a/libraries/rush-lib/src/logic/RepoStateFile.ts b/libraries/rush-lib/src/logic/RepoStateFile.ts
@@ -213,7 +213,7 @@ export class RepoStateFile {
         // means users may turn off the injected installation
         // so we will need to remove unused fields in repo-state.json as well
         this._packageJsonInjectedDependenciesHash = undefined;
-        this._modified = true
+        this._modified = true;
       }
     }