WIP - prefactoring

vercel · Apr 16, 2024 · 618ddff · 618ddff
1 parent a1dfd90
commit 618ddff
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 163 deletions.
diff --git a/packages/build-utils/src/fs/run-user-scripts.ts b/packages/build-utils/src/fs/run-user-scripts.ts
@@ -546,7 +546,6 @@ export function getEnvForPackageManager({
     detectedLockfile,
     detectedPackageManager,
     path: newPath,
-    yarnNodeLinker,
   } = getPathForPackageManager({
     cliType,
     lockfileVersion,
@@ -558,33 +557,38 @@ export function getEnvForPackageManager({
     ...env,
   };
 
-  if (newPath) {
+  const alreadyInPath = (path: string) => {
+    const oldPath = env.PATH ?? '';
+    return oldPath.includes(path);
+  };
+
+  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;
 }
 
@@ -616,76 +620,76 @@ export function getPathForPackageManager({
    * Undefined if no $PATH are necessary.
    */
   path: string | undefined;
-  /**
-   * Set if yarn was identified as package manager and `YARN_NODE_LINKER`
-   * environment variable was not found on the input environment.
-   */
-  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 no_override = {
+    detectedLockfile: undefined,
+    detectedPackageManager: undefined,
+    path: undefined,
+  };
+
+  const shouldUseNpm7 = (
+    lockfileVersion: number | undefined,
+    nodeVersion: NodeVersion | undefined
+  ) => {
+    return (
+      lockfileVersion && lockfileVersion >= 2 && (nodeVersion?.major || 0) < 16
+    );
+  };
+
+  const shouldUsePnpm7 = (lockfileVersion: number | undefined) => {
+    return lockfileVersion && lockfileVersion === 5.4;
+  };
+
+  const shouldUsePnpm8 = (lockfileVersion: number | undefined) => {
+    return lockfileVersion && lockfileVersion >= 6.0;
+  };
+
   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) {
+  if (corepackEnabled) {
+    return no_override;
+  }
+  switch (cliType) {
+    case 'npm':
+      switch (true) {
+        case shouldUseNpm7(lockfileVersion, nodeVersion):
+          return {
+            path: '/node16/bin-npm7',
+            detectedLockfile: 'package-lock.json',
+            detectedPackageManager: 'npm 7+',
+          };
+        default:
+          return no_override;
+      }
+    case 'pnpm':
+      switch (true) {
+        case shouldUsePnpm7(lockfileVersion):
+          // pnpm 7
+          return {
+            path: '/pnpm7/node_modules/.bin',
+            detectedLockfile: 'pnpm-lock.yaml',
+            detectedPackageManager: 'pnpm 7',
+          };
+        case shouldUsePnpm8(lockfileVersion):
+          // pnpm 8
+          return {
+            path: '/pnpm8/node_modules/.bin',
+            detectedLockfile: 'pnpm-lock.yaml',
+            detectedPackageManager: 'pnpm 8',
+          };
+        default:
+          return no_override;
+      }
+    case 'bun':
       // 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 {
+        path: '/bun1',
+        detectedLockfile: 'bun.lockb',
+        detectedPackageManager: 'Bun',
+      };
+
+    case 'yarn':
+      return no_override;
   }
-  return {
-    detectedLockfile,
-    detectedPackageManager,
-    path: pathValue,
-    yarnNodeLinker,
-  };
 }
 
 export async function runCustomInstallCommand({

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
@@ -288,7 +288,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: undefined,
         detectedPackageManager: undefined,
         path: undefined,
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -306,7 +305,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: 'package-lock.json',
         detectedPackageManager: 'npm 7+',
         path: '/node16/bin-npm7',
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -324,27 +322,9 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: undefined,
         detectedPackageManager: undefined,
         path: undefined,
-        yarnNodeLinker: undefined,
-      },
-    },
-    {
-      name: 'should not prepend npm path again if already detected',
-      args: {
-        cliType: 'npm',
-        nodeVersion: { major: 14, range: '14.x', runtime: 'nodejs14.x' },
-        lockfileVersion: 2,
-        env: {
-          FOO: 'bar',
-          PATH: `/node16/bin-npm7${delimiter}foo`,
-        },
-      },
-      want: {
-        detectedLockfile: undefined,
-        detectedPackageManager: undefined,
-        path: undefined,
-        yarnNodeLinker: undefined,
       },
     },
+
     {
       name: 'should not set path if node is 16 and npm 7+ is detected',
       args: {
@@ -360,42 +340,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: undefined,
         detectedPackageManager: undefined,
         path: undefined,
-        yarnNodeLinker: undefined,
-      },
-    },
-    {
-      name: 'should set YARN_NODE_LINKER w/yarn if it is not already defined',
-      args: {
-        cliType: 'yarn',
-        nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
-        lockfileVersion: 2,
-        env: {
-          FOO: 'bar',
-        },
-      },
-      want: {
-        detectedLockfile: undefined,
-        detectedPackageManager: undefined,
-        path: undefined,
-        yarnNodeLinker: 'node-modules',
-      },
-    },
-    {
-      name: 'should not set YARN_NODE_LINKER if it already exists',
-      args: {
-        cliType: 'yarn',
-        nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
-        lockfileVersion: 2,
-        env: {
-          FOO: 'bar',
-          YARN_NODE_LINKER: 'exists',
-        },
-      },
-      want: {
-        detectedLockfile: undefined,
-        detectedPackageManager: undefined,
-        path: undefined,
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -413,7 +357,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: 'pnpm-lock.yaml',
         detectedPackageManager: 'pnpm 7',
         path: '/pnpm7/node_modules/.bin',
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -431,7 +374,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: 'bun.lockb',
         detectedPackageManager: 'Bun',
         path: '/bun1',
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -449,25 +391,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: undefined,
         detectedPackageManager: undefined,
         path: undefined,
-        yarnNodeLinker: undefined,
-      },
-    },
-    {
-      name: 'should not prepend pnpm path again if already detected',
-      args: {
-        cliType: 'pnpm',
-        nodeVersion: { major: 16, range: '16.x', runtime: 'nodejs16.x' },
-        lockfileVersion: 5.4,
-        env: {
-          FOO: 'bar',
-          PATH: `/pnpm7/node_modules/.bin${delimiter}foo`,
-        },
-      },
-      want: {
-        detectedLockfile: undefined,
-        detectedPackageManager: undefined,
-        path: undefined,
-        yarnNodeLinker: undefined,
       },
     },
     {
@@ -484,7 +407,6 @@ describe('Test `getPathForPackageManager()`', () => {
         detectedLockfile: undefined,
         detectedPackageManager: undefined,
         path: undefined,
-        yarnNodeLinker: undefined,
       },
     },
   ])('$name', ({ args, want }) => {