Always infer the package manager unless set by an argument

README.md

@@ -130,15 +130,7 @@ Expects:
 - `yarn`
 - `pnpm`
 
-Default:
-
-<!-- todo: refine -->
-
-If the script was **not** run with `yarn`, but there is a `yarn.lock` file and `yarn` is installed, we will choose `yarn`.
-
-If the script was **not** run with `pnpm`, but there is a `pnpm-lock.yml` file and `pnpm` is installed, we will choose `pnpm`.
-
-In all other cases we will choose the package manager that executed the script or `npm`.
+Default: `yarn`, if there's a `yarn.lock` file and `yarn` is installed. `pnpm`, if there's a `pnpm-lock.yml` file and `pnpm` is installed. Otherwise the package manager that is executing the script will be used to install packages.
 
 ### --ignore-git-changes
 

src/arguments/ArgumentHandler.ts

@@ -10,7 +10,7 @@ export class ArgumentHandler {
     this.argumentDefinitions = argumentDefinitions;
   }
 
-  async parse(env: Environment): Promise<Partial<CliArguments>> {
+  async parseArgs(env: Environment): Promise<Partial<CliArguments>> {
     const details = await env.ownPackageJson.getDetails();
 
     program.name(details.name).description(details.description).version(details.version, `-v, --version`);
@@ -38,7 +38,7 @@ export class ArgumentHandler {
     return cliArgs;
   }
 
-  async promptForMissing(parsedArgs: Partial<CliArguments>): Promise<CliArguments> {
+  async resolveMissingArgs(parsedArgs: Partial<CliArguments>): Promise<CliArguments> {
     const allArgs: Partial<CliArguments> = { ...parsedArgs };
 
     for (const argumentDefinition of this.argumentDefinitions) {

src/arguments/PackageManagerArgument.ts

@@ -1,9 +1,8 @@
 import { execSync } from "child_process";
 import { Command } from "commander";
-import path from "path";
 import { Environment } from "../misc/Environment.js";
 import { Filesystem } from "../misc/Filesystem.js";
-import { inferPackageManager } from "../misc/packageManagers/index.js";
+import { getExecutingPackageManager } from "../misc/packageManagers/index.js";
 import { CliArguments, PackageManagerType, PackageManagerOptions } from "../types.js";
 import { ArgumentBase, getNormalizedCliString } from "./ArgumentBase.js";
 
@@ -39,12 +38,9 @@ export class PackageManagerArgument extends ArgumentBase<"packageManager"> {
   }
 
   getDefaultValue(existingArgs: Partial<CliArguments>): Promise<PackageManagerType> {
-    const inferred = inferPackageManager();
-
     const yarnLockFile = this.env.rel("yarn.lock");
-    const pnpmLockFile = this.env.rel("pnpm-lock.yaml");
 
-    if (inferred !== "yarn" && this.fs.exists(yarnLockFile.abs)) {
+    if (this.fs.exists(yarnLockFile.abs)) {
       try {
         execSync("yarn --version", { stdio: "ignore" });
 
@@ -53,7 +49,9 @@ export class PackageManagerArgument extends ArgumentBase<"packageManager"> {
       } catch {}
     }
 
-    if (inferred !== "pnpm" && this.fs.exists(pnpmLockFile.abs)) {
+    const pnpmLockFile = this.env.rel("pnpm-lock.yaml");
+
+    if (this.fs.exists(pnpmLockFile.abs)) {
       try {
         execSync("pnpm --version", { stdio: "ignore" });
 
@@ -62,7 +60,9 @@ export class PackageManagerArgument extends ArgumentBase<"packageManager"> {
       } catch {}
     }
 
-    return Promise.resolve(inferred);
+    const executingPackageManager = getExecutingPackageManager();
+
+    return Promise.resolve(executingPackageManager);
   }
 
   parsePackageManager(rawInput?: string): PackageManagerType | null {

src/bin.ts

@@ -16,7 +16,7 @@ import {
 } from "./arguments/index.js";
 import { BABEL_RELAY_MACRO, PACKAGE_FILE } from "./consts.js";
 import { Filesystem } from "./misc/Filesystem.js";
-import { getPackageManger, inferPackageManager } from "./misc/packageManagers/index.js";
+import { getPackageManger, getExecutingPackageManager } from "./misc/packageManagers/index.js";
 import {
   GenerateArtifactDirectoryTask,
   GenerateRelayEnvironmentTask,
@@ -50,7 +50,7 @@ const distDirectory = dirname(fileURLToPath(import.meta.url));
 const ownPackageJsonFilepath = path.join(distDirectory, "..", PACKAGE_FILE);
 
 const cwd = process.cwd();
-const pacMan = inferPackageManager();
+const pacMan = getExecutingPackageManager();
 
 const env = new Environment(cwd, ownPackageJsonFilepath, fs);
 
@@ -101,10 +101,9 @@ const isGitRepo = await git.isGitRepository(env.cwd);
 
 let userArgs: CliArguments;
 
-// Try to parse the CLI arguments
 try {
   // Get the arguments provided to the program.
-  const cliArgs = await argumentHandler.parse(env);
+  const cliArgs = await argumentHandler.parseArgs(env);
 
   if (isGitRepo && !cliArgs.ignoreGitChanges) {
     const hasUnsavedChanges = await git.hasUnsavedChanges(env.cwd);
@@ -116,7 +115,7 @@ try {
   }
 
   // Prompt for all of the missing arguments, required to execute the program.
-  userArgs = await argumentHandler.promptForMissing(cliArgs);
+  userArgs = await argumentHandler.resolveMissingArgs(cliArgs);
 
   console.log();
 } catch (error) {
@@ -155,18 +154,13 @@ const runner = new TaskRunner([
   new Next_AddRelayEnvironmentProvider(context),
 ]);
 
-// We capture whether there was an error here, in order to
-// correctly end the program with a exit code of 1 later.
-let runnerHadError = false;
-
-runner.onError = () => {
-  runnerHadError = true;
-};
-
 // Execute all of the tasks sequentially.
 try {
   await runner.run();
 } catch {
+  // The error should've already been correctly handled by the runner,
+  // we just exit the program here.
+
   console.log();
   printError("Some of the tasks failed unexpectedly.");
   exit(1);
@@ -211,9 +205,3 @@ if (context.is("next")) {
 }
 
 console.log();
-
-// We let the system output anything relevant
-// and only then fail at the end.
-if (runnerHadError) {
-  exit(1);
-}

src/misc/packageManagers/index.ts

@@ -18,11 +18,9 @@ export function getPackageManger(type: PackageManagerType, cmdRunner: CommandRun
   }
 }
 
-export function inferPackageManager(): PackageManagerType {
+export function getExecutingPackageManager(): PackageManagerType {
   const userAgent = process.env.npm_config_user_agent;
 
-  // If this script is being run by a specific manager,
-  // we use this mananger.
   if (userAgent) {
     if (userAgent.startsWith("yarn")) {
       return "yarn";

src/tasks/TaskRunner.ts

@@ -5,9 +5,9 @@ import { TaskBase, TaskSkippedError } from "./TaskBase.js";
 export class TaskRunner {
   constructor(private taskDefs: (false | TaskBase)[]) {}
 
-  onError?(): void;
-
   async run(): Promise<void> {
+    let hadError = false;
+
     for (let i = 0; i < this.taskDefs.length; i++) {
       const task = this.taskDefs[i];
 
@@ -52,10 +52,14 @@ export class TaskRunner {
           console.log();
         }
 
-        if (this.onError) {
-          this.onError();
+        if (!hadError) {
+          hadError = true;
         }
       }
     }
+
+    if (hadError) {
+      throw new Error();
+    }
   }
 }