diff --git a/.changeset/soft-bananas-care.md b/.changeset/soft-bananas-care.md new file mode 100644 index 000000000..78df72014 --- /dev/null +++ b/.changeset/soft-bananas-care.md @@ -0,0 +1,5 @@ +--- +"@vercel/build-utils": minor +--- + +fix corepack detection for package manager version determination diff --git a/packages/build-utils/src/fs/run-user-scripts.ts b/packages/build-utils/src/fs/run-user-scripts.ts index fa7b00b7f..2844fcc45 100644 --- a/packages/build-utils/src/fs/run-user-scripts.ts +++ b/packages/build-utils/src/fs/run-user-scripts.ts @@ -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), }), diff --git a/packages/build-utils/test/unit.get-env-for-package-manager.test.ts b/packages/build-utils/test/unit.get-env-for-package-manager.test.ts index f065310e0..38b91cbcb 100644 --- a/packages/build-utils/test/unit.get-env-for-package-manager.test.ts +++ b/packages/build-utils/test/unit.get-env-for-package-manager.test.ts @@ -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[0]; + args: Parameters[0]; want: unknown; }>([ { diff --git a/packages/hydrogen/src/build.ts b/packages/hydrogen/src/build.ts index 725f31d35..7fc97ce90 100644 --- a/packages/hydrogen/src/build.ts +++ b/packages/hydrogen/src/build.ts @@ -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 || {}, }); diff --git a/packages/next/src/index.ts b/packages/next/src/index.ts index d674fe46d..caba9dc91 100644 --- a/packages/next/src/index.ts +++ b/packages/next/src/index.ts @@ -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 || {}, }); diff --git a/packages/redwood/src/index.ts b/packages/redwood/src/index.ts index cd41f4b9e..ea80eff8b 100644 --- a/packages/redwood/src/index.ts +++ b/packages/redwood/src/index.ts @@ -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 || {}, }); diff --git a/packages/remix/src/build-legacy.ts b/packages/remix/src/build-legacy.ts index 7a7475b16..06c82f1ce 100644 --- a/packages/remix/src/build-legacy.ts +++ b/packages/remix/src/build-legacy.ts @@ -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, }); diff --git a/packages/remix/src/build-vite.ts b/packages/remix/src/build-vite.ts index 0e145874f..bdf62e26a 100644 --- a/packages/remix/src/build-vite.ts +++ b/packages/remix/src/build-vite.ts @@ -99,6 +99,7 @@ export const build: BuildV2 = async ({ spawnOpts.env = getEnvForPackageManager({ cliType, lockfileVersion, + packageJsonPackageManager: packageJson?.packageManager, nodeVersion, env: spawnOpts.env, }); diff --git a/packages/static-build/src/index.ts b/packages/static-build/src/index.ts index fa31e4160..84eae04ae 100644 --- a/packages/static-build/src/index.ts +++ b/packages/static-build/src/index.ts @@ -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 || {}, });