fix corepack detection for package manager version determination (#11596)

The previous logic was checking for the env var `ENABLE_EXPERIMENTAL_COREPACK` to determine if corepack was being used by a project. However, this value only means that the build system should consider corepack, not that it's actively being used.

We need to check that flag AND the existence of a `packageManager` property in the project's `package.json`.
This commit is contained in:
Sean Massa
2024-05-15 14:25:01 -05:00
committed by GitHub
parent 41c44d6594
commit ccd7eb1fb7
9 changed files with 95 additions and 23 deletions

View File

@@ -0,0 +1,5 @@
---
"@vercel/build-utils": minor
---
fix corepack detection for package manager version determination

View File

@@ -426,9 +426,8 @@ export async function runNpmInstall(
try {
await runNpmInstallSema.acquire();
const { cliType, packageJsonPath, lockfileVersion } = await scanParentDirs(
destPath
);
const { cliType, packageJsonPath, packageJson, lockfileVersion } =
await scanParentDirs(destPath, true);
if (!packageJsonPath) {
debug(
@@ -463,6 +462,7 @@ export async function runNpmInstall(
opts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env,
});
@@ -547,14 +547,19 @@ export async function runNpmInstall(
export function getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager,
nodeVersion,
env,
}: {
cliType: CliType;
lockfileVersion: number | undefined;
packageJsonPackageManager?: string | undefined;
nodeVersion: NodeVersion | undefined;
env: { [x: string]: string | undefined };
}) {
const corepackFlagged = env.ENABLE_EXPERIMENTAL_COREPACK === '1';
const corepackEnabled = corepackFlagged && Boolean(packageJsonPackageManager);
const {
detectedLockfile,
detectedPackageManager,
@@ -562,14 +567,19 @@ export function getEnvForPackageManager({
} = getPathOverrideForPackageManager({
cliType,
lockfileVersion,
corepackEnabled,
nodeVersion,
env,
});
const corepackEnabled = env.ENABLE_EXPERIMENTAL_COREPACK === '1';
debug(
`Detected ${detectedPackageManager} given lockfileVersion "${lockfileVersion}", package manager cli "${cliType}", and corepack enabled? ${corepackEnabled}: ${newPath}`
);
if (corepackEnabled) {
debug(
`Detected corepack use for "${packageJsonPackageManager}". Not overriding package manager version.`
);
} else {
debug(
`Detected ${detectedPackageManager}. Added "${newPath}" to path. Based on assumed package manager "${cliType}", lockfile "${detectedLockfile}", and lockfileVersion "${lockfileVersion}"`
);
}
const newEnv: { [x: string]: string | undefined } = {
...env,
@@ -655,13 +665,13 @@ function shouldUseNpm7(
export function getPathOverrideForPackageManager({
cliType,
lockfileVersion,
corepackEnabled,
nodeVersion,
env,
}: {
cliType: CliType;
lockfileVersion: number | undefined;
corepackEnabled: boolean;
nodeVersion: NodeVersion | undefined;
env: { [x: string]: string | undefined };
}): {
/**
* Which lockfile was detected.
@@ -683,8 +693,6 @@ export function getPathOverrideForPackageManager({
path: undefined,
};
const corepackEnabled = env.ENABLE_EXPERIMENTAL_COREPACK === '1';
switch (cliType) {
case 'npm':
switch (true) {
@@ -748,6 +756,7 @@ export function getPathOverrideForPackageManager({
/**
* Helper to get the binary paths that link to the used package manager.
* Note: Make sure it doesn't contain any `console.log` calls.
* @deprecated use `getEnvForPackageManager` instead
*/
export function getPathForPackageManager({
cliType,
@@ -779,11 +788,16 @@ export function getPathForPackageManager({
*/
yarnNodeLinker: string | undefined;
} {
// This is not the correct check for whether or not corepack is being used. For that, you'd have to check
// the package.json's `packageManager` property. However, this deprecated function is keeping it's old,
// broken behavior.
const corepackEnabled = env.ENABLE_EXPERIMENTAL_COREPACK === '1';
const overrides = getPathOverrideForPackageManager({
cliType,
lockfileVersion,
corepackEnabled,
nodeVersion,
env,
});
const alreadyInPath = (newPath: string) => {
@@ -818,10 +832,14 @@ export async function runCustomInstallCommand({
spawnOpts?: SpawnOptions;
}) {
console.log(`Running "install" command: \`${installCommand}\`...`);
const { cliType, lockfileVersion } = await scanParentDirs(destPath);
const { cliType, lockfileVersion, packageJson } = await scanParentDirs(
destPath,
true
);
const env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts?.env || {},
});
@@ -859,6 +877,7 @@ export async function runPackageJsonScript(
env: getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion: undefined,
env: cloneEnv(process.env, spawnOpts?.env),
}),

View File

@@ -35,6 +35,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 1,
env: {
FOO: 'bar',
@@ -51,6 +52,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -69,6 +71,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: 'pnpm@latest',
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -87,6 +90,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -105,6 +109,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -123,6 +128,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'yarn',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -140,6 +146,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'yarn',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -158,6 +165,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 5.4,
env: {
FOO: 'bar',
@@ -177,6 +185,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 6.0,
env: {
FOO: 'bar',
@@ -196,6 +205,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 9.0,
env: {
FOO: 'bar',
@@ -215,6 +225,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'bun',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 0,
env: {
FOO: 'bar',
@@ -234,6 +245,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: 'npm@latest',
lockfileVersion: 5.4,
env: {
FOO: 'bar',
@@ -252,6 +264,7 @@ describe('Test `getEnvForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 5.4,
env: {
FOO: 'bar',
@@ -270,6 +283,7 @@ describe('Test `getEnvForPackageManager()`', () => {
getEnvForPackageManager({
cliType: args.cliType,
lockfileVersion: args.lockfileVersion,
packageJsonPackageManager: args.packageJsonPackageManager,
nodeVersion: args.nodeVersion,
env: args.env,
})
@@ -302,6 +316,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 1,
env: {
FOO: 'bar',
@@ -318,6 +333,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -335,6 +351,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
packageJsonPackageManager: 'pnpm@latest',
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -353,6 +370,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'npm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 2,
env: {
FOO: 'bar',
@@ -370,6 +388,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 5.3, // detects as pnpm@6, which is the default
env: {
FOO: 'bar',
@@ -387,6 +406,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 5.4,
env: {
FOO: 'bar',
@@ -404,6 +424,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 6.1,
env: {
FOO: 'bar',
@@ -421,6 +442,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 7.0,
env: {
FOO: 'bar',
@@ -438,6 +460,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'bun',
nodeVersion: { major: 18, range: '18.x', runtime: 'nodejs18.x' },
packageJsonPackageManager: undefined,
lockfileVersion: 0,
env: {
FOO: 'bar',
@@ -455,6 +478,7 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
args: {
cliType: 'pnpm',
nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
packageJsonPackageManager: 'npm@latest',
lockfileVersion: 5.4,
env: {
FOO: 'bar',
@@ -472,16 +496,18 @@ describe('Test `getPathOverrideForPackageManager()`', () => {
getPathOverrideForPackageManager({
cliType: args.cliType,
lockfileVersion: args.lockfileVersion,
// naive assumption that enabling corepack as a feature means it's used, but this is fine for tests
corepackEnabled: Boolean(args.env.ENABLE_EXPERIMENTAL_COREPACK),
nodeVersion: args.nodeVersion,
env: args.env,
})
).toStrictEqual(want);
});
});
describe('Test `getPathForPackageManager()`', () => {
test.each<{
name: string;
args: Parameters<typeof getEnvForPackageManager>[0];
args: Parameters<typeof getPathForPackageManager>[0];
want: unknown;
}>([
{

View File

@@ -51,11 +51,15 @@ export const build: BuildV2 = async ({
);
const spawnOpts = getSpawnOptions(meta, nodeVersion);
const { cliType, lockfileVersion } = await scanParentDirs(entrypointDir);
const { cliType, lockfileVersion, packageJson } = await scanParentDirs(
entrypointDir,
true
);
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env || {},
});

View File

@@ -257,10 +257,15 @@ export const build: BuildV2 = async ({
const nextVersionRange = await getNextVersionRange(entryPath);
const nodeVersion = await getNodeVersion(entryPath, undefined, config, meta);
const spawnOpts = getSpawnOptions(meta, nodeVersion);
const { cliType, lockfileVersion } = await scanParentDirs(entryPath);
const { cliType, lockfileVersion, packageJson } = await scanParentDirs(
entryPath,
true
);
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env || {},
});

View File

@@ -78,13 +78,15 @@ export const build: BuildV2 = async ({
if (!spawnOpts.env) {
spawnOpts.env = {};
}
const { cliType, lockfileVersion } = await scanParentDirs(
entrypointFsDirname
const { cliType, lockfileVersion, packageJson } = await scanParentDirs(
entrypointFsDirname,
true
);
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env || {},
});

View File

@@ -104,8 +104,13 @@ export const build: BuildV2 = async ({
meta
);
const { cliType, packageJsonPath, lockfileVersion, lockfilePath } =
await scanParentDirs(entrypointFsDirname);
const {
cliType,
packageJsonPath,
packageJson,
lockfileVersion,
lockfilePath,
} = await scanParentDirs(entrypointFsDirname, true);
if (!packageJsonPath) {
throw new Error('Failed to locate `package.json` file in your project');
@@ -125,6 +130,7 @@ export const build: BuildV2 = async ({
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env,
});

View File

@@ -99,6 +99,7 @@ export const build: BuildV2 = async ({
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env,
});

View File

@@ -482,11 +482,15 @@ export const build: BuildV2 = async ({
spawnOpts.env.CI = 'false';
}
const { cliType, lockfileVersion } = await scanParentDirs(entrypointDir);
const { cliType, lockfileVersion, packageJson } = await scanParentDirs(
entrypointDir,
true
);
spawnOpts.env = getEnvForPackageManager({
cliType,
lockfileVersion,
packageJsonPackageManager: packageJson?.packageManager,
nodeVersion,
env: spawnOpts.env || {},
});