refactor(ng-dev): remove pnpm version checking

Since pnpm now automatically uses the version defined in the package.json file we don't need to extract it and run the specific version manually anymore

Author: josephperrott

ng-dev/release/publish/actions.ts

@@ -101,8 +101,6 @@ export abstract class ReleaseAction {
    */
   abstract perform(): Promise<void>;
 
-  protected pnpmVersioning = new PnpmVersioning();
-
   constructor(
     protected active: ActiveReleaseTrains,
     protected git: AuthenticatedGitClient,
@@ -388,7 +386,7 @@ export abstract class ReleaseAction {
 
   /** Installs all Yarn dependencies in the current branch. */
   protected async installDependenciesForCurrentBranch() {
-    if (await this.pnpmVersioning.isUsingPnpm(this.projectDir)) {
+    if (PnpmVersioning.isUsingPnpm(this.projectDir)) {
       // Note: We delate all contents of `node_modules` before installing dependencies. We do
       // this because if a pnpm workspace package exists at one ref and not another, it can
       // cause the pnpm install from within Bazel to errantly attempt to install a package that
@@ -439,14 +437,8 @@ export abstract class ReleaseAction {
     // publish branch. e.g. consider we publish patch version and a new package has been
     // created in the `next` branch. The new package would not be part of the patch branch,
     // so we cannot build and publish it.
-    const builtPackages = await ExternalCommands.invokeReleaseBuild(
-      this.projectDir,
-      this.pnpmVersioning,
-    );
-    const releaseInfo = await ExternalCommands.invokeReleaseInfo(
-      this.projectDir,
-      this.pnpmVersioning,
-    );
+    const builtPackages = await ExternalCommands.invokeReleaseBuild(this.projectDir);
+    const releaseInfo = await ExternalCommands.invokeReleaseInfo(this.projectDir);
 
     // Extend the built packages with their disk hash and NPM package information. This is
     // helpful later for verifying integrity and filtering out e.g. experimental packages.
@@ -511,7 +503,6 @@ export abstract class ReleaseAction {
       this.projectDir,
       newVersion,
       builtPackagesWithInfo,
-      this.pnpmVersioning,
     );
 
     // Verify the packages built are the correct version.

ng-dev/release/publish/actions/cut-stable.ts

@@ -86,7 +86,6 @@ export class CutStableAction extends ReleaseAction {
       await ExternalCommands.invokeDeleteNpmDistTag(
         this.projectDir,
         'do-not-use-exceptional-minor',
-        this.pnpmVersioning,
       );
     }
 
@@ -109,7 +108,6 @@ export class CutStableAction extends ReleaseAction {
         this.projectDir,
         ltsTagForPatch,
         previousPatch.version,
-        this.pnpmVersioning,
         {
           // We do not intend to tag experimental NPM packages as LTS.
           skipExperimentalPackages: true,

ng-dev/release/publish/actions/tag-recent-major-as-latest.ts

@@ -34,12 +34,7 @@ export class TagRecentMajorAsLatest extends ReleaseAction {
     await this.updateGithubReleaseEntryToStable(this.active.latest.version);
     await this.checkoutUpstreamBranch(this.active.latest.branchName);
     await this.installDependenciesForCurrentBranch();
-    await ExternalCommands.invokeSetNpmDist(
-      this.projectDir,
-      'latest',
-      this.active.latest.version,
-      this.pnpmVersioning,
-    );
+    await ExternalCommands.invokeSetNpmDist(this.projectDir, 'latest', this.active.latest.version);
   }
 
   /**

ng-dev/release/publish/external-commands.ts

@@ -18,8 +18,8 @@ import {ReleaseInfoJsonStdout} from '../info/cli.js';
 import {ReleasePrecheckJsonStdin} from '../precheck/cli.js';
 import {BuiltPackageWithInfo} from '../config/index.js';
 import {green, Log} from '../../utils/logging.js';
-import {PnpmVersioning} from './pnpm-versioning.js';
 import {getBazelBin} from '../../utils/bazel-bin.js';
+import {PnpmVersioning} from './pnpm-versioning.js';
 
 /*
  * ###############################################################
@@ -51,7 +51,6 @@ export abstract class ExternalCommands {
     projectDir: string,
     npmDistTag: NpmDistTag,
     version: semver.SemVer,
-    pnpmVersioning: PnpmVersioning,
     options: {skipExperimentalPackages: boolean} = {skipExperimentalPackages: false},
   ) {
     try {
@@ -66,7 +65,6 @@ export abstract class ExternalCommands {
           `--skip-experimental-packages=${options.skipExperimentalPackages}`,
         ],
         projectDir,
-        pnpmVersioning,
       );
 
       Log.info(green(`  ✓   Set "${npmDistTag}" NPM dist tag for all packages to v${version}.`));
@@ -81,17 +79,12 @@ export abstract class ExternalCommands {
    * Invokes the `ng-dev release npm-dist-tag delete` command in order to delete the
    * NPM dist tag for all packages in the checked-out version branch.
    */
-  static async invokeDeleteNpmDistTag(
-    projectDir: string,
-    npmDistTag: NpmDistTag,
-    pnpmVersioning: PnpmVersioning,
-  ) {
+  static async invokeDeleteNpmDistTag(projectDir: string, npmDistTag: NpmDistTag) {
     try {
       // Note: No progress indicator needed as that is the responsibility of the command.
       await this._spawnNpmScript(
         ['ng-dev', 'release', 'npm-dist-tag', 'delete', npmDistTag],
         projectDir,
-        pnpmVersioning,
       );
 
       Log.info(green(`  ✓   Deleted "${npmDistTag}" NPM dist tag for all packages.`));
@@ -106,10 +99,7 @@ export abstract class ExternalCommands {
    * Invokes the `ng-dev release build` command in order to build the release
    * packages for the currently checked out branch.
    */
-  static async invokeReleaseBuild(
-    projectDir: string,
-    pnpmVersioning: PnpmVersioning,
-  ): Promise<ReleaseBuildJsonStdout> {
+  static async invokeReleaseBuild(projectDir: string): Promise<ReleaseBuildJsonStdout> {
     // Note: We explicitly mention that this can take a few minutes, so that it's obvious
     // to caretakers that it can take longer than just a few seconds.
     const spinner = new Spinner('Building release output. This can take a few minutes.');
@@ -118,7 +108,6 @@ export abstract class ExternalCommands {
       const {stdout} = await this._spawnNpmScript(
         ['ng-dev', 'release', 'build', '--json'],
         projectDir,
-        pnpmVersioning,
         {
           mode: 'silent',
         },
@@ -144,15 +133,11 @@ export abstract class ExternalCommands {
    * This is useful to e.g. determine whether a built package is currently
    * denoted as experimental or not.
    */
-  static async invokeReleaseInfo(
-    projectDir: string,
-    pnpmVersioning: PnpmVersioning,
-  ): Promise<ReleaseInfoJsonStdout> {
+  static async invokeReleaseInfo(projectDir: string): Promise<ReleaseInfoJsonStdout> {
     try {
       const {stdout} = await this._spawnNpmScript(
         ['ng-dev', 'release', 'info', '--json'],
         projectDir,
-        pnpmVersioning,
         {mode: 'silent'},
       );
 
@@ -180,15 +165,14 @@ export abstract class ExternalCommands {
     projectDir: string,
     newVersion: semver.SemVer,
     builtPackagesWithInfo: BuiltPackageWithInfo[],
-    pnpmVersioning: PnpmVersioning,
   ): Promise<void> {
     const precheckStdin: ReleasePrecheckJsonStdin = {
       builtPackagesWithInfo,
       newVersion: newVersion.format(),
     };
 
     try {
-      await this._spawnNpmScript(['ng-dev', 'release', 'precheck'], projectDir, pnpmVersioning, {
+      await this._spawnNpmScript(['ng-dev', 'release', 'precheck'], projectDir, {
         // Note: We pass the precheck information to the command through `stdin`
         // because command line arguments are less reliable and have length limits.
         input: JSON.stringify(precheckStdin),
@@ -269,12 +253,10 @@ export abstract class ExternalCommands {
   private static async _spawnNpmScript(
     args: string[],
     projectDir: string,
-    pnpmVersioning: PnpmVersioning,
     spawnOptions: SpawnOptions = {},
   ): Promise<SpawnResult> {
-    if (await pnpmVersioning.isUsingPnpm(projectDir)) {
-      const pnpmSpec = await pnpmVersioning.getPackageSpec(projectDir);
-      return ChildProcess.spawn('npx', ['--yes', pnpmSpec, '-s', 'run', ...args], {
+    if (PnpmVersioning.isUsingPnpm(projectDir)) {
+      return ChildProcess.spawn('npx', ['--yes', 'pnpm', '-s', 'run', ...args], {
         ...spawnOptions,
         cwd: projectDir,
       });

ng-dev/release/publish/pnpm-versioning.ts

@@ -6,32 +6,15 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {readFile} from 'node:fs/promises';
 import {join} from 'node:path';
 import {existsSync} from 'node:fs';
 
-/**
- * Class that exposes helpers for fetching and using pnpm
- * based on a currently-checked out revision.
- *
- * This is useful as there is no vendoring/checking-in of specific
- * pnpm versions, so we need to automatically fetch the proper pnpm
- * version when executing commands in version branches. Keep in mind that
- * version branches may have different pnpm version ranges, and the release
- * tool should automatically be able to satisfy those.
- */
+// TODO(josephperrott): Remove this version check once we no longer support v19
+
 export class PnpmVersioning {
-  async isUsingPnpm(repoPath: string) {
+  static isUsingPnpm(repoPath: string) {
     // If there is only a pnpm lock file at the workspace root, we assume pnpm
     // is the primary package manager. We can remove such checks in the future.
     return existsSync(join(repoPath, 'pnpm-lock.yaml')) && !existsSync(join(repoPath, 'yarn.lock'));
   }
-
-  async getPackageSpec(repoPath: string) {
-    const packageJsonRaw = await readFile(join(repoPath, 'package.json'), 'utf8');
-    const packageJson = JSON.parse(packageJsonRaw) as {engines?: Record<string, string>};
-
-    const pnpmAllowedRange = packageJson?.engines?.['pnpm'] ?? 'latest';
-    return `pnpm@${pnpmAllowedRange}`;
-  }
 }

ng-dev/release/publish/test/cut-stable.spec.ts

@@ -26,7 +26,6 @@ import {
 } from '../../versioning/index.js';
 import {ReleaseNotes} from '../../notes/release-notes.js';
 import {workspaceRelativePackageJsonPath} from '../../../utils/constants.js';
-import {PnpmVersioning} from '../pnpm-versioning.js';
 
 describe('cut stable action', () => {
   it('should not activate if a feature-freeze release-train is active', async () => {
@@ -163,7 +162,6 @@ describe('cut stable action', () => {
       action.projectDir,
       'v10-lts',
       matchesVersion('10.0.3'),
-      new PnpmVersioning(),
       // Experimental packages are expected to be not tagged as LTS.
       {skipExperimentalPackages: true},
     );

ng-dev/release/publish/test/tag-recent-major-as-latest.spec.ts

@@ -11,7 +11,6 @@ import {ActiveReleaseTrains} from '../../versioning/index.js';
 import {ReleaseTrain} from '../../versioning/release-trains.js';
 import {TagRecentMajorAsLatest} from '../actions/tag-recent-major-as-latest.js';
 import {ExternalCommands} from '../external-commands.js';
-import {PnpmVersioning} from '../pnpm-versioning.js';
 import {getTestConfigurationsForAction} from './test-utils/action-mocks.js';
 import {
   fakeNpmPackageQueryRequest,
@@ -149,7 +148,6 @@ describe('tag recent major as latest action', () => {
       projectDir,
       'latest',
       matchesVersion('10.0.0'),
-      new PnpmVersioning(),
     );
   });
 });