fix!: call executePackageManagerRequest directly (#430)

* fix!: call `executePackageManagerRequest` directly * Apply suggestions from code review Co-authored-by: Antoine du Hamel <[email protected]> * test: update assertions --------- Co-authored-by: Antoine du Hamel <[email protected]>
nodejs · Apr 20, 2024 · 0f9b748 · 0f9b748
1 parent df1b773
commit 0f9b748
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 88 deletions.
diff --git a/sources/main.ts b/sources/main.ts
@@ -1,23 +1,23 @@
-import {BaseContext, Builtins, Cli, Command, Option} from 'clipanion';
-
-import {version as corepackVersion}  from '../package.json';
-
-import {Engine, PackageManagerRequest}  from './Engine';
-import {CacheCommand}  from './commands/Cache';
-import {DisableCommand}  from './commands/Disable';
-import {EnableCommand}  from './commands/Enable';
-import {InstallGlobalCommand}  from './commands/InstallGlobal';
-import {InstallLocalCommand}  from './commands/InstallLocal';
-import {PackCommand}  from './commands/Pack';
-import {UpCommand}  from './commands/Up';
-import {UseCommand}  from './commands/Use';
-import {HydrateCommand}  from './commands/deprecated/Hydrate';
-import {PrepareCommand}  from './commands/deprecated/Prepare';
+import {BaseContext, Builtins, Cli}  from 'clipanion';
+
+import {version as corepackVersion} from '../package.json';
+
+import {Engine, PackageManagerRequest} from './Engine';
+import {CacheCommand} from './commands/Cache';
+import {DisableCommand} from './commands/Disable';
+import {EnableCommand} from './commands/Enable';
+import {InstallGlobalCommand} from './commands/InstallGlobal';
+import {InstallLocalCommand} from './commands/InstallLocal';
+import {PackCommand} from './commands/Pack';
+import {UpCommand} from './commands/Up';
+import {UseCommand} from './commands/Use';
+import {HydrateCommand} from './commands/deprecated/Hydrate';
+import {PrepareCommand} from './commands/deprecated/Prepare';
 
 export type CustomContext = {cwd: string, engine: Engine};
 export type Context = BaseContext & CustomContext;
 
-function getPackageManagerRequestFromCli(parameter: string | undefined, context: CustomContext & Partial<Context>): PackageManagerRequest | null {
+function getPackageManagerRequestFromCli(parameter: string | undefined, engine: Engine): PackageManagerRequest | null {
  if (!parameter)
  return null;
 
@@ -26,7 +26,7 @@ function getPackageManagerRequestFromCli(parameter: string | undefined, context:
  return null;
 
  const [, binaryName, binaryVersion] = match;
- const packageManager = context.engine.getPackageManagerFor(binaryName)!;
+ const packageManager = engine.getPackageManagerFor(binaryName)!;
 
  if (packageManager == null && binaryVersion == null) return null;
 
@@ -38,17 +38,10 @@ function getPackageManagerRequestFromCli(parameter: string | undefined, context:
 }
 
 export async function runMain(argv: Array<string>) {
- // Because we load the binaries in the same process, we don't support custom contexts.
- const context = {
- ...Cli.defaultContext,
- cwd: process.cwd(),
- engine: new Engine(),
- };
+ const engine = new Engine();
 
  const [firstArg, ...restArgs] = argv;
- const request = getPackageManagerRequestFromCli(firstArg, context);
-
- let code: number;
+ const request = getPackageManagerRequestFromCli(firstArg, engine);
 
  if (!request) {
  // If the first argument doesn't match any supported package manager, we fallback to the standard Corepack CLI
@@ -74,29 +67,21 @@ export async function runMain(argv: Array<string>) {
  cli.register(HydrateCommand);
  cli.register(PrepareCommand);
 
- code = await cli.run(argv, context);
- } else {
- // Otherwise, we create a single-command CLI to run the specified package manager (we still use Clipanion in order to pretty-print usage errors).
- const cli = new Cli({
- binaryLabel: `'${request.binaryName}', via Corepack`,
- binaryName: request.binaryName,
- binaryVersion: `corepack/${corepackVersion}`,
- });
-
- cli.register(class BinaryCommand extends Command<Context> {
- proxy = Option.Proxy();
- async execute() {
- return this.context.engine.executePackageManagerRequest(request, {
- cwd: this.context.cwd,
- args: this.proxy,
- });
- }
- });
+ const context = {
+ ...Cli.defaultContext,
+ cwd: process.cwd(),
+ engine,
+ };
 
- code = await cli.run(restArgs, context);
- }
+ const code = await cli.run(argv, context);
 
- if (code !== 0) {
- process.exitCode ??= code;
+ if (code !== 0) {
+ process.exitCode ??= code;
+ }
+ } else {
+ await engine.executePackageManagerRequest(request, {
+ cwd: process.cwd(),
+ args: restArgs,
+ });
  }
 }
diff --git a/tests/main.test.ts b/tests/main.test.ts
@@ -24,8 +24,8 @@ it(`should refuse to download a package manager if the hash doesn't match`, asyn
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /Mismatch hashes/,
+ stderr: /Mismatch hashes/,
+ stdout: ``,
  });
  });
 });
@@ -35,8 +35,8 @@ it(`should refuse to download a known package manager from a URL`, async () => {
  // Package managers known by Corepack cannot be loaded from a URL.
  await expect(runCli(cwd, [`yarn@https://registry.npmjs.com/yarn/-/yarn-1.22.21.tgz`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /Illegal use of URL for known package manager/,
+ stderr: /Illegal use of URL for known package manager/,
+ stdout: ``,
  });
 
  // Unknown package managers can be loaded from a URL.
@@ -57,8 +57,8 @@ it.failing(`should refuse to download a known package manager from a URL in pack
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /Illegal use of URL for known package manager/,
+ stderr: /Illegal use of URL for known package manager/,
+ stdout: ``,
  });
 
  // Unknown package managers can be loaded from a URL.
@@ -82,8 +82,8 @@ it(`should require a version to be specified`, async () => {
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /expected a semver version/,
+ stderr: /expected a semver version/,
+ stdout: ``,
  });
 
  await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
@@ -92,8 +92,8 @@ it(`should require a version to be specified`, async () => {
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /expected a semver version/,
+ stderr: /expected a semver version/,
+ stdout: ``,
  });
 
  await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
@@ -102,8 +102,8 @@ it(`should require a version to be specified`, async () => {
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
- stdout: /expected a semver version/,
+ stderr: /expected a semver version/,
+ stdout: ``,
  });
  });
 });
@@ -272,7 +272,7 @@ it(`shouldn't allow using regular Yarn commands on npm-configured projects`, asy
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
  exitCode: 1,
- stderr: ``,
+ stderr: /This project is configured to use npm/,
  });
  });
 });
@@ -419,9 +419,10 @@ it(`should refuse to run a different package manager within a configured project
  process.env.FORCE_COLOR = `0`;
 
  await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
- stdout: `Usage Error: This project is configured to use yarn because ${
+ stdout: ``,
+ stderr: expect.stringContaining(`This project is configured to use yarn because ${
  npath.fromPortablePath(ppath.join(cwd, `package.json` as Filename))
- } has a "packageManager" field\n\n$ pnpm ...\n`,
+ } has a "packageManager" field`),
  exitCode: 1,
  });
 
@@ -471,8 +472,8 @@ it(`should support disabling the network accesses from the environment`, async (
  });
 
  await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
- stdout: expect.stringContaining(`Network access disabled by the environment`),
- stderr: ``,
+ stdout: ``,
+ stderr: /Network access disabled by the environment/,
  exitCode: 1,
  });
  });
@@ -998,13 +999,13 @@ describe(`handle integrity checks`, () => {
  await xfs.mktempPromise(async cwd => {
  await expect(runCli(cwd, [`[email protected]`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`yarn@stable`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  });
  });
@@ -1014,19 +1015,19 @@ describe(`handle integrity checks`, () => {
  await xfs.mktempPromise(async cwd => {
  await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
- stderr: ``,
+ stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
+ stdout: ``,
  });
  // A second time to validate the invalid version was not cached.
  await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
- stderr: ``,
+ stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`yarn`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
- stderr: ``,
+ stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({
  exitCode: 1,
@@ -1041,19 +1042,19 @@ describe(`handle integrity checks`, () => {
  await xfs.mktempPromise(async cwd => {
  await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  // A second time to validate the invalid version was not cached.
  await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`yarn`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`use`, `pnpm`], true)).resolves.toMatchObject({
  exitCode: 1,
@@ -1068,8 +1069,8 @@ describe(`handle integrity checks`, () => {
  await xfs.mktempPromise(async cwd => {
  await expect(runCli(cwd, [`[email protected]`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Signature does not match/,
- stderr: ``,
+ stderr: /Signature does not match/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`use`, `[email protected]`], true)).resolves.toMatchObject({
  exitCode: 1,
@@ -1084,8 +1085,8 @@ describe(`handle integrity checks`, () => {
  await xfs.mktempPromise(async cwd => {
  await expect(runCli(cwd, [`[email protected]`, `--version`], true)).resolves.toMatchObject({
  exitCode: 1,
- stdout: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
- stderr: ``,
+ stderr: /Mismatch hashes. Expected [a-f0-9]{128}, got [a-f0-9]{128}/,
+ stdout: ``,
  });
  await expect(runCli(cwd, [`use`, `[email protected]`], true)).resolves.toMatchObject({
  exitCode: 1,
@@ -1101,10 +1102,10 @@ describe(`handle integrity checks`, () => {
  const result = await runCli(cwd, [`[email protected]+sha1.deadbeef`, `--version`], true);
  expect(result).toMatchObject({
  exitCode: 1,
- stderr: ``,
+ stdout: ``,
  });
- const match = /Mismatch hashes. Expected deadbeef, got ([a-f0-9]{40})/.exec(result.stdout);
- if (match == null) throw new Error(`Invalid output`, {cause: result.stdout});
+ const match = /Mismatch hashes. Expected deadbeef, got ([a-f0-9]{40})/.exec(result.stderr);
+ if (match == null) throw new Error(`Invalid output`, {cause: result.stderr});
  await expect(runCli(cwd, [`[email protected]+sha1.${match[1]}`, `--version`], true)).resolves.toMatchObject({
  exitCode: 0,
  stdout: `yarn: Hello from custom registry\n`,