feat: use range from devEngines when no packageManager is set

sources/Engine.ts

@@ -276,9 +276,21 @@ export class Engine {
         }
 
         case `NoSpec`: {
-          if (typeof locator.reference === `function`)
+          let rangeWasSet = false;
+          if (result.devEnginesValue) {
+            const {name, range} = result.devEnginesValue;
+            if (name !== fallbackDescriptor.name)
+              throw new UsageError(`This project is configured to use ${name} because ${result.target} has a "packageManager" field`);
+
+            if (range) {
+              fallbackDescriptor.range = range;
+              rangeWasSet = true;
+            }
+          }
+          if (!rangeWasSet && typeof locator.reference === `function`)
             fallbackDescriptor.range = await locator.reference();
 
+
           if (process.env.COREPACK_ENABLE_AUTO_PIN === `1`) {
             const resolved = await this.resolveDescriptor(fallbackDescriptor, {allowTags: true});
             if (resolved === null)

sources/commands/Base.ts

@@ -16,10 +16,11 @@ export abstract class BaseCommand extends Command<Context> {
           throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`);
 
         case `NoSpec`:
-          throw new UsageError(`The local project doesn't feature a 'packageManager' field nor a 'devEngines.packageManager' field - please specify the package manager to pack, or update the manifest to reference it`);
+          if (lookup.devEnginesValue?.range) return [lookup.devEnginesValue];
+          throw new UsageError(`The local project doesn't feature a 'packageManager' field ${lookup.devEnginesValue ? `` : `nor a 'devEngines.packageManager' field `}- please specify the package manager to pack, or update the manifest to reference it`);
 
         default: {
-          return [lookup.range ?? lookup.getSpec()];
+          return [lookup.devEnginesValue ?? lookup.getSpec()];
         }
       }
     } else {

sources/commands/deprecated/Prepare.ts

@@ -39,7 +39,11 @@ export class PrepareCommand extends Command<Context> {
           throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`);
 
         case `NoSpec`:
-          throw new UsageError(`The local project doesn't feature a 'packageManager' field - please specify the package manager to pack, or update the manifest to reference it`);
+          if (lookup.devEnginesValue?.range) {
+            specs.push(lookup.devEnginesValue);
+            break;
+          }
+          throw new UsageError(`The local project doesn't feature a 'packageManager' field ${lookup.devEnginesValue ? `` : `nor a 'devEngines.packageManager' field `}- please specify the package manager to pack, or update the manifest to reference it`);
 
         default: {
           specs.push(lookup.getSpec());

sources/specUtils.ts

@@ -101,7 +101,7 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
       return pm;
     }
 
-    debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`);
+    debugUtils.log(`devEngines.packageManager defines that ${name}${version ? `@${version}` : ``} should the local package manager`);
 
     if (pm) {
       if (!pm.startsWith?.(`${name}@`))
@@ -113,8 +113,9 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
       return pm;
     }
 
-
-    return `${name}@${version ?? `*`}`;
+    return {spec: `${name}@${version ?? `*`}`, name, version, toString() {
+      return this.spec;
+    }};
   }
 
   return pm;
@@ -123,14 +124,15 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
 export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) {
   const lookup = await loadSpecAndEnv(cwd);
 
-  const range = `range` in lookup && lookup.range;
+  const projectFound = lookup.type !== `NoProject`;
+  const range = projectFound && lookup.devEnginesValue;
   if (range) {
     if (info.locator.name !== range.name || !semverSatisfies(info.locator.reference, range.range)) {
       warnOrThrow(`The requested version of ${info.locator.name}@${info.locator.reference} does not match the devEngines specification (${range.name}@${range.range})`, range.onFail);
     }
   }
 
-  const content = lookup.type !== `NoProject`
+  const content = projectFound
     ? await fs.promises.readFile(lookup.target, `utf8`)
     : ``;
 
@@ -151,12 +153,12 @@ interface FoundSpecResult {
   type: `Found`;
   target: string;
   getSpec: (options?: {enforceExactVersion?: boolean}) => Descriptor;
-  range?: Descriptor & {onFail?: DevEngineDependency[`onFail`]};
+  devEnginesValue?: Descriptor & {onFail?: DevEngineDependency[`onFail`]};
   envFilePath?: string;
 }
 export type LoadSpecResult =
     | {type: `NoProject`, target: string, envFilePath?: string}
-    | {type: `NoSpec`, target: string, envFilePath?: string}
+    | {type: `NoSpec`, target: string, envFilePath?: string, devEnginesValue?: FoundSpecResult[`devEnginesValue`]}
     | FoundSpecResult;
 
 async function loadEnvFileIfExists(cwd: string): Promise<{env: LocalEnvFile, path: string} | void> {
@@ -238,18 +240,26 @@ export async function loadSpecAndEnv(initialCwd: string, {envOnly} = {envOnly: f
   if (typeof rawPmSpec === `undefined`)
     return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path};
 
-  debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`);
+  const devEnginesValue = selection.data.devEngines?.packageManager?.name && {
+    name: selection.data.devEngines.packageManager.name,
+    range: selection.data.devEngines.packageManager.version,
+    onFail: selection.data.devEngines.packageManager.onFail,
+  };
+
+  if (typeof rawPmSpec === `object` && !semverValid(rawPmSpec.version)) {
+    debugUtils.log(`${selection.manifestPath} devEngines does not specify a specific version`);
+    return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path, devEnginesValue};
+  }
+
+  const hasPackageManagerField = typeof rawPmSpec === `string`;
+  debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager${hasPackageManagerField ? ` using packageManager field` : ``}`);
 
   return {
     type: `Found`,
     target: selection.manifestPath,
     envFilePath: localEnv?.path,
-    range: selection.data.devEngines?.packageManager?.version && {
-      name: selection.data.devEngines.packageManager.name,
-      range: selection.data.devEngines.packageManager.version,
-      onFail: selection.data.devEngines.packageManager.onFail,
-    },
+    devEnginesValue: devEnginesValue?.range && devEnginesValue,
     // Lazy-loading it so we do not throw errors on commands that do not need valid spec.
-    getSpec: ({enforceExactVersion = true} = {}) => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
+    getSpec: ({enforceExactVersion = true} = {}) => parseSpec(`${rawPmSpec}`, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
   };
 }

tests/main.test.ts

@@ -272,23 +272,6 @@
 });
 
 describe(`should handle invalid devEngines values`, () => {
-  it(`throw on missing version`, async () => {
-    await xfs.mktempPromise(async cwd => {
-      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
-        devEngines: {
-          packageManager: {
-            name: `yarn`,
-          },
-        },
-      });
-
-      await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
-        exitCode: 1,
-        stderr: `Invalid package manager specification in package.json (yarn@*); expected a semver version\n`,
-        stdout: ``,
-      });
-    });
-  });
   it(`throw on invalid version`, async () => {
     await xfs.mktempPromise(async cwd => {
       await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
@@ -380,8 +363,8 @@
   });
 });
 
-describe(`should accept range in devEngines only if a specific version is provided`, () => {
-  it(`either in package.json#packageManager field`, async () => {
+describe(`should accept range in devEngines`, () => {
+  it(`should accept if package.json#packageManager field matches`, async () => {
     await xfs.mktempPromise(async cwd => {
       await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
         devEngines: {
@@ -390,18 +373,19 @@
             version: `6.x`,
           },
         },
+        packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
       });
       await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
-        exitCode: 1,
-        stderr: `Invalid package manager specification in package.json (pnpm@6.x); expected a semver version\n`,
-        stdout: ``,
+        exitCode: 0,
+        stderr: ``,
+        stdout: `6.6.2\n`,
       });
 
+      // No version should also work
       await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
         devEngines: {
           packageManager: {
             name: `pnpm`,
-            version: `6.x`,
           },
         },
         packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
@@ -411,20 +395,114 @@
         stderr: ``,
         stdout: `6.6.2\n`,
       });
+    });
+  });
 
-      // No version should also work
-      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
+  it(`should accept without a package.json#packageManager field`, async () => {
+    process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`;
+    process.env.TEST_INTEGRITY = `valid`;
+
+    await xfs.mktempPromise(async cwd => {
+    // When no user version is specified, range versions in devEngines should still cause error
+      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
         devEngines: {
           packageManager: {
             name: `pnpm`,
+            version: `^1.0.0`,
           },
         },
-        packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
       });
+
+      // Should fail if trying to use a different package manager than the one defined in devEngines
+      await expect(runCli(cwd, [`yarn`, `install`], true)).resolves.toMatchObject({
+        exitCode: 1,
+        stderr: expect.stringMatching(/This project is configured to use pnpm because .+\/package\.json has a "packageManager" field/),
+        stdout: ``,
+      });
+
+      // Without user-specified version, should resolve to the range in devEngines
+      await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
+        exitCode: 0,
+        stderr: ``,
+        stdout: `pnpm: Hello from custom registry\n`,
+      });
+    });
+  });
+
+  it(`should pin a specific if COREPACK_ENABLE_AUTO_PIN is set`, async () => {
+    process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`;
+    process.env.TEST_INTEGRITY = `valid`;
+    process.env.COREPACK_ENABLE_AUTO_PIN = `1`;
+
+    await xfs.mktempPromise(async cwd => {
+      const devEngines = {
+        packageManager: {
+          name: `pnpm`,
+          version: `^1.0.0`,
+        },
+      };
+
+      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
+        devEngines,
+      });
+
+      // Without user-specified version, should still fail due to range version in devEngines
+      await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
+        exitCode: 0,
+        stderr: expect.stringContaining(`local project doesn't define a 'packageManager' field`),
+        stdout: `pnpm: Hello from custom registry\n`,
+      });
+
+      await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json` as Filename))).resolves.toMatchObject({
+        packageManager: expect.stringMatching(/^pnpm@1\.9998\.9999\+sha512\.[0-9a-z]{128}$/),
+        devEngines,
+      });
+    });
+  });
+});
+
+describe(`devEngines.packageManager without a version`, () => {
+  it(`should still enforce the package manager name`, async () => {
+    await xfs.mktempPromise(async cwd => {
+      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
+        devEngines: {
+          packageManager: {
+            name: `yarn`,
+          },
+        },
+      });
+
+      process.env.FORCE_COLOR = `0`;
+
       await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
+        stdout: ``,
+        stderr: expect.stringContaining(`This project is configured to use yarn`),
+        exitCode: 1,
+      });
+
+      // The matching package manager runs, using the default version as no range is given.
+      await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
+        stdout: `${config.definitions.yarn.default.split(`+`, 1)[0]}\n`,
+        stderr: ``,
         exitCode: 0,
+      });
+    });
+  });
+
+  it(`should not claim the devEngines.packageManager field is missing`, async () => {
+    await xfs.mktempPromise(async cwd => {
+      await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
+        devEngines: {
+          packageManager: {
+            name: `yarn`,
+          },
+        },
+      });
+
+      await expect(runCli(cwd, [`pack`])).resolves.toMatchObject({
+        exitCode: 1,
+        stdout: expect.stringContaining(`The local project doesn't feature a 'packageManager' field - please specify the package manager to pack, or update the manifest to reference it`),
         stderr: ``,
-        stdout: `6.6.2\n`,
       });
     });
   });
@@ -1824,24 +1902,3 @@
     });
   }
 });
-
-it(`should still validate devEngines.packageManager.version format when no user version specified`, async () => {
-  await xfs.mktempPromise(async cwd => {
-    // When no user version is specified, range versions in devEngines should still cause error
-    await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
-      devEngines: {
-        packageManager: {
-          name: `npm`,
-          version: `^6.14.2`,
-        },
-      },
-    });
-
-    // Without user-specified version, should still fail due to range version in devEngines
-    await expect(runCli(cwd, [`npm`, `--version`])).resolves.toMatchObject({
-      exitCode: 1,
-      stderr: expect.stringContaining(`Invalid package manager specification in package.json (npm@^6.14.2); expected a semver version`),
-      stdout: ``,
-    });
-  });
-});