Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pnpm version detection logic #11445

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1ccdded
[build-utils] Update pnpm version detection logic
erikareads Apr 17, 2024
1f22ccd
[build-utils] update tested version of pnpm in test
erikareads Apr 17, 2024
13425b5
[build-utils] Avoid false positives in alreadyInPath
erikareads Apr 18, 2024
165278b
add changeset
erikareads Apr 18, 2024
1e45587
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
erikareads Apr 18, 2024
208c987
[build-utils] Avoid breaking return signature of getPathForPackageMan…
erikareads Apr 22, 2024
f73059c
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
erikareads Apr 23, 2024
068e96f
[build-utils] Support pnpm9
erikareads Apr 25, 2024
485623a
[build-utils] Remove commented code
erikareads Apr 25, 2024
cc31d23
[build-utils] Add new tests to cover pnpm 9 case
erikareads Apr 25, 2024
8a618f2
Update cold-boxes-tan.md
erikareads Apr 25, 2024
de426f3
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
erikareads Apr 25, 2024
3180f43
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
EndangeredMassa Apr 26, 2024
0801d12
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
EndangeredMassa May 10, 2024
4884e00
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
kodiakhq[bot] May 10, 2024
8248da2
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
EndangeredMassa May 10, 2024
89e22e2
Merge branch 'main' into erikarowland/zero-1812-detect-pnpm-lockfile-…
EndangeredMassa May 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/cold-boxes-tan.md
@@ -0,0 +1,6 @@
---
'@vercel/build-utils': minor
---

Update pnpm version detection logic
Add support for pnpm 9
260 changes: 178 additions & 82 deletions packages/build-utils/src/fs/run-user-scripts.ts
Expand Up @@ -546,8 +546,7 @@ export function getEnvForPackageManager({
detectedLockfile,
detectedPackageManager,
path: newPath,
yarnNodeLinker,
} = getPathForPackageManager({
} = getPathOverrideForPackageManager({
cliType,
lockfileVersion,
nodeVersion,
Expand All @@ -558,36 +557,172 @@ export function getEnvForPackageManager({
...env,
};

if (newPath) {
const alreadyInPath = (newPath: string) => {
const oldPath = env.PATH ?? '';
return oldPath.split(path.delimiter).includes(newPath);
};

if (newPath && !alreadyInPath(newPath)) {
// Ensure that the binaries of the detected package manager are at the
// beginning of the `$PATH`.
const oldPath = env.PATH + '';
newEnv.PATH = `${newPath}${path.delimiter}${oldPath}`;
}

if (yarnNodeLinker) {
newEnv.YARN_NODE_LINKER = yarnNodeLinker;
}

if (detectedLockfile && detectedPackageManager) {
// For pnpm we also show the version of the lockfile we found
const versionString =
cliType === 'pnpm' ? `version ${lockfileVersion} ` : '';
if (detectedLockfile && detectedPackageManager) {
// For pnpm we also show the version of the lockfile we found
const versionString =
cliType === 'pnpm' ? `version ${lockfileVersion} ` : '';

console.log(
`Detected \`${detectedLockfile}\` ${versionString}generated by ${detectedPackageManager}`
);

if (cliType === 'bun') {
console.warn(
'Warning: Bun is used as a package manager at build time only, not at runtime with Functions'
console.log(
`Detected \`${detectedLockfile}\` ${versionString}generated by ${detectedPackageManager}`
);

if (cliType === 'bun') {
console.warn(
'Warning: Bun is used as a package manager at build time only, not at runtime with Functions'
);
}
}
}

if (cliType === 'yarn' && !env.YARN_NODE_LINKER) {
newEnv.YARN_NODE_LINKER = 'node-modules';
}

return newEnv;
}

type DetectedPnpmVersion =
| 'not found'
| 'pnpm 7'
| 'pnpm 8'
| 'pnpm 9'
| 'corepack_enabled';

function detectPnpmVersion(
lockfileVersion: number | undefined,
corepackEnabled: boolean
): DetectedPnpmVersion {
switch (true) {
case corepackEnabled:
return 'corepack_enabled';
case lockfileVersion === undefined:
return 'not found';
case lockfileVersion === 5.3 || lockfileVersion === 5.4:
return 'pnpm 7';
case lockfileVersion === 6.0 || lockfileVersion === 6.1:
return 'pnpm 8';
case lockfileVersion === 7.0 || lockfileVersion === 9.0:
return 'pnpm 9';
default:
return 'not found';
}
}

function shouldUseNpm7(
lockfileVersion: number | undefined,
nodeVersion: NodeVersion | undefined
): boolean {
if (lockfileVersion === undefined) return false;
return lockfileVersion >= 2 && (nodeVersion?.major || 0) < 16;
}

/**
* Helper to get the binary paths that link to the used package manager.
* Note: Make sure it doesn't contain any `console.log` calls.
*/
export function getPathOverrideForPackageManager({
cliType,
lockfileVersion,
nodeVersion,
env,
}: {
cliType: CliType;
lockfileVersion: number | undefined;
nodeVersion: NodeVersion | undefined;
env: { [x: string]: string | undefined };
}): {
/**
* Which lockfile was detected.
*/
detectedLockfile: string | undefined;
/**
* Detected package manager that generated the found lockfile.
*/
detectedPackageManager: string | undefined;
/**
* Value of $PATH that includes the binaries for the detected package manager.
* Undefined if no $PATH are necessary.
*/
path: string | undefined;
} {
const no_override = {
detectedLockfile: undefined,
detectedPackageManager: undefined,
path: undefined,
};

const corepackEnabled = env.ENABLE_EXPERIMENTAL_COREPACK === '1';

switch (cliType) {
case 'npm':
switch (true) {
case corepackEnabled:
return no_override;
case shouldUseNpm7(lockfileVersion, nodeVersion):
return {
path: '/node16/bin-npm7',
detectedLockfile: 'package-lock.json',
detectedPackageManager: 'npm 7+',
};
default:
return no_override;
}
case 'pnpm':
switch (detectPnpmVersion(lockfileVersion, corepackEnabled)) {
case 'corepack_enabled':
return no_override;
case 'pnpm 7':
// pnpm 7
return {
path: '/pnpm7/node_modules/.bin',
detectedLockfile: 'pnpm-lock.yaml',
detectedPackageManager: 'pnpm 7',
};
case 'pnpm 8':
// pnpm 8
return {
path: '/pnpm8/node_modules/.bin',
detectedLockfile: 'pnpm-lock.yaml',
detectedPackageManager: 'pnpm 8',
};
case 'pnpm 9':
// pnpm 9
return {
path: '/pnpm9/node_modules/.bin',
detectedLockfile: 'pnpm-lock.yaml',
detectedPackageManager: 'pnpm 9',
};
default:
return no_override;
}
case 'bun':
switch (true) {
case corepackEnabled:
return no_override;
default:
// Bun 1
return {
path: '/bun1',
detectedLockfile: 'bun.lockb',
detectedPackageManager: 'Bun',
};
}
case 'yarn':
return no_override;
}
}

/**
* Helper to get the binary paths that link to the used package manager.
* Note: Make sure it doesn't contain any `console.log` calls.
Expand Down Expand Up @@ -622,70 +757,31 @@ export function getPathForPackageManager({
*/
yarnNodeLinker: string | undefined;
} {
let detectedLockfile: string | undefined;
let detectedPackageManager: string | undefined;
let pathValue: string | undefined;
let yarnNodeLinker: string | undefined;
const oldPath = env.PATH + '';
const npm7 = '/node16/bin-npm7';
const pnpm7 = '/pnpm7/node_modules/.bin';
const pnpm8 = '/pnpm8/node_modules/.bin';
const bun1 = '/bun1';
const corepackEnabled = env.ENABLE_EXPERIMENTAL_COREPACK === '1';
if (cliType === 'npm') {
if (
typeof lockfileVersion === 'number' &&
lockfileVersion >= 2 &&
(nodeVersion?.major || 0) < 16 &&
!oldPath.includes(npm7) &&
!corepackEnabled
) {
// npm 7
pathValue = npm7;
detectedLockfile = 'package-lock.json';
detectedPackageManager = 'npm 7+';
}
} else if (cliType === 'pnpm') {
if (
typeof lockfileVersion === 'number' &&
lockfileVersion === 5.4 &&
!oldPath.includes(pnpm7) &&
!corepackEnabled
) {
// pnpm 7
pathValue = pnpm7;
detectedLockfile = 'pnpm-lock.yaml';
detectedPackageManager = 'pnpm 7';
} else if (
typeof lockfileVersion === 'number' &&
lockfileVersion >= 6 &&
!oldPath.includes(pnpm8) &&
!corepackEnabled
) {
// pnpm 8
pathValue = pnpm8;
detectedLockfile = 'pnpm-lock.yaml';
detectedPackageManager = 'pnpm 8';
}
} else if (cliType === 'bun') {
if (!oldPath.includes(bun1) && !corepackEnabled) {
// Bun 1
pathValue = bun1;
detectedLockfile = 'bun.lockb';
detectedPackageManager = 'Bun';
}
} else {
// Yarn v2 PnP mode may be activated, so force "node-modules" linker style
if (!env.YARN_NODE_LINKER) {
yarnNodeLinker = 'node-modules';
}
}
return {
detectedLockfile,
detectedPackageManager,
path: pathValue,
yarnNodeLinker,
const overrides = getPathOverrideForPackageManager({
cliType,
lockfileVersion,
nodeVersion,
env,
});

const alreadyInPath = (newPath: string) => {
const oldPath = env.PATH ?? '';
return oldPath.split(path.delimiter).includes(newPath);
};

switch (true) {
case cliType === 'yarn' && !env.YARN_NODE_LINKER:
return { ...overrides, yarnNodeLinker: 'node-modules' };
case alreadyInPath(overrides.path ?? ''):
return {
detectedLockfile: undefined,
detectedPackageManager: undefined,
path: undefined,
yarnNodeLinker: undefined,
};
default:
return { ...overrides, yarnNodeLinker: undefined };
}
Comment on lines +772 to +784
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): ‏ifs over switches?

The switch-true pattern is harder to understand than the equivalent if-return or if-else, in my opinion. I suggest changing this, but it's up to you.

}

export async function runCustomInstallCommand({
Expand Down
2 changes: 1 addition & 1 deletion packages/build-utils/test/fixtures/22-pnpm/vercel.json
Expand Up @@ -4,7 +4,7 @@
"probes": [
{
"path": "/",
"mustContain": "pnpm version: 6",
"mustContain": "pnpm version: 7",
"logMustContain": "pnpm run build"
}
]
Expand Down