[build-utils] Add getNodeBinPaths() function (#10150)

The `getNodeBinPath()` function is problematic because it assumes that commands are installed in the `node_modules` directory alongside the detected lockfile. This works fine the majority of the time, but ends up not being the case when using a monorepo that uses a package manager in "linked" mode (i.e. pnpm by default).

Consider the following:

```
.
├── pnpm-lock.yaml
├── node_modules
├── blog
│   ├── node_modules
│   │   ├── hexo -> .pnpm/hexo@3.9.0/node_modules/hexo
```

In this setup, adding the root-level `node_modules/.bin` would not make the `hexo` command be visible in the `$PATH`.

To solve this issue, the new `getNodeBinPaths()` function returns an array of all directories up to the specified `root`, which can then be placed into the `$PATH`. It's also more efficient (synchronous) since it does not need to scan for a lockfile anymore (the `root` needs to be specified explicitly).

The new function is being used in `@vercel/next` and `@vercel/static-build`.

The `traverseUpDirectories()` function from CLI was moved to `build-utils` to implement this function. Consequently, that makes the implementations of `walkParentDirs()` and `walkParentDirsMulti()` simpler, since it's using this generator now.

.changeset/perfect-wasps-wait.md

@@ -0,0 +1,6 @@
+---
+'@vercel/static-build': patch
+'@vercel/next': patch
+---
+
+Use `getNodeBinPaths()` function to improve monorepo support

.changeset/tough-singers-live.md

@@ -0,0 +1,5 @@
+---
+'@vercel/build-utils': minor
+---
+
+Add `getNodeBinPaths()` and `traverseUpDirectories()` functions

packages/build-utils/src/fs/run-user-scripts.ts

@@ -44,23 +44,33 @@ export interface ScanParentDirsResult {
   lockfileVersion?: number;
 }
 
-export interface WalkParentDirsProps {
+export interface TraverseUpDirectoriesProps {
   /**
-   * The highest directory, typically the workPath root of the project.
-   * If this directory is reached and it doesn't contain the file, null is returned.
-   */
-  base: string;
-  /**
-   * The directory to start searching, typically the same directory of the entrypoint.
-   * If this directory doesn't contain the file, the parent is checked, etc.
+   * The directory to start iterating from, typically the same directory of the entrypoint.
    */
   start: string;
+  /**
+   * The highest directory, typically the workPath root of the project.
+   */
+  base?: string;
+}
+
+export interface WalkParentDirsProps
+  extends Required<TraverseUpDirectoriesProps> {
   /**
    * The name of the file to search for, typically `package.json` or `Gemfile`.
    */
   filename: string;
 }
 
+export interface WalkParentDirsMultiProps
+  extends Required<TraverseUpDirectoriesProps> {
+  /**
+   * The name of the file to search for, typically `package.json` or `Gemfile`.
+   */
+  filenames: string[];
+}
+
 export interface SpawnOptionsExtended extends SpawnOptions {
   /**
    * Pretty formatted command that is being spawned for logging purposes.
@@ -131,6 +141,24 @@ export async function execCommand(command: string, options: SpawnOptions = {}) {
   return true;
 }
 
+export function* traverseUpDirectories({
+  start,
+  base,
+}: TraverseUpDirectoriesProps) {
+  let current: string | undefined = path.normalize(start);
+  const normalizedRoot = base ? path.normalize(base) : undefined;
+  while (current) {
+    yield current;
+    if (current === normalizedRoot) break;
+    // Go up one directory
+    const next = path.join(current, '..');
+    current = next === current ? undefined : next;
+  }
+}
+
+/**
+ * @deprecated Use `getNodeBinPaths()` instead.
+ */
 export async function getNodeBinPath({
   cwd,
 }: {
@@ -141,6 +169,15 @@ export async function getNodeBinPath({
   return path.join(dir, 'node_modules', '.bin');
 }
 
+export function getNodeBinPaths({
+  start,
+  base,
+}: TraverseUpDirectoriesProps): string[] {
+  return Array.from(traverseUpDirectories({ start, base })).map(dir =>
+    path.join(dir, 'node_modules/.bin')
+  );
+}
+
 async function chmodPlusX(fsPath: string) {
   const s = await fs.stat(fsPath);
   const newMode = s.mode | 64 | 8 | 1; // eslint-disable-line no-bitwise
@@ -297,22 +334,14 @@ export async function walkParentDirs({
 }: WalkParentDirsProps): Promise<string | null> {
   assert(path.isAbsolute(base), 'Expected "base" to be absolute path');
   assert(path.isAbsolute(start), 'Expected "start" to be absolute path');
-  let parent = '';
 
-  for (let current = start; base.length <= current.length; current = parent) {
-    const fullPath = path.join(current, filename);
+  for (const dir of traverseUpDirectories({ start, base })) {
+    const fullPath = path.join(dir, filename);
 
     // eslint-disable-next-line no-await-in-loop
     if (await fs.pathExists(fullPath)) {
       return fullPath;
     }
-
-    parent = path.dirname(current);
-
-    if (parent === current) {
-      // Reached root directory of the filesystem
-      break;
-    }
   }
 
   return null;
@@ -322,14 +351,9 @@ async function walkParentDirsMulti({
   base,
   start,
   filenames,
-}: {
-  base: string;
-  start: string;
-  filenames: string[];
-}): Promise<(string | undefined)[]> {
-  let parent = '';
-  for (let current = start; base.length <= current.length; current = parent) {
-    const fullPaths = filenames.map(f => path.join(current, f));
+}: WalkParentDirsMultiProps): Promise<(string | undefined)[]> {
+  for (const dir of traverseUpDirectories({ start, base })) {
+    const fullPaths = filenames.map(f => path.join(dir, f));
     const existResults = await Promise.all(
       fullPaths.map(f => fs.pathExists(f))
     );
@@ -338,13 +362,6 @@ async function walkParentDirsMulti({
     if (foundOneOrMore) {
       return fullPaths.map((f, i) => (existResults[i] ? f : undefined));
     }
-
-    parent = path.dirname(current);
-
-    if (parent === current) {
-      // Reached root directory of the filesystem
-      break;
-    }
   }
 
   return [];

packages/build-utils/src/index.ts

@@ -30,7 +30,9 @@ import {
   getNodeVersion,
   getSpawnOptions,
   getNodeBinPath,
+  getNodeBinPaths,
   scanParentDirs,
+  traverseUpDirectories,
 } from './fs/run-user-scripts';
 import {
   getLatestNodeVersion,
@@ -68,6 +70,7 @@ export {
   spawnCommand,
   walkParentDirs,
   getNodeBinPath,
+  getNodeBinPaths,
   runNpmInstall,
   runBundleInstall,
   runPipInstall,
@@ -89,6 +92,7 @@ export {
   getIgnoreFilter,
   cloneEnv,
   hardLinkDir,
+  traverseUpDirectories,
   validateNpmrc,
 };
 

packages/build-utils/test/unit.get-node-bin-paths.test.ts

@@ -0,0 +1,17 @@
+import { join } from 'path';
+import { getNodeBinPaths } from '../src/fs/run-user-scripts';
+
+describe('getNodeBinPaths()', () => {
+  const cwd = process.cwd();
+
+  it('should return array of `node_modules/.bin` paths', () => {
+    const start = join(cwd, 'foo/bar/baz');
+    const paths = getNodeBinPaths({ start, base: cwd });
+    expect(paths).toEqual([
+      join(cwd, 'foo/bar/baz/node_modules/.bin'),
+      join(cwd, 'foo/bar/node_modules/.bin'),
+      join(cwd, 'foo/node_modules/.bin'),
+      join(cwd, 'node_modules/.bin'),
+    ]);
+  });
+});

packages/build-utils/test/unit.traverse-up-directories.test.ts

@@ -0,0 +1,50 @@
+import { traverseUpDirectories } from '../src/fs/run-user-scripts';
+
+const isWindows = process.platform === 'win32';
+
+describe('traverseUpDirectories()', () => {
+  test.each(
+    isWindows
+      ? [
+          {
+            start: 'C:\\foo\\bar\\baz',
+            expected: ['C:\\foo\\bar\\baz', 'C:\\foo\\bar', 'C:\\foo', 'C:\\'],
+          },
+          {
+            start: 'C:\\foo\\..\\bar\\.\\baz',
+            expected: ['C:\\bar\\baz', 'C:\\bar', 'C:\\'],
+          },
+          {
+            start: 'C:\\foo\\bar\\baz\\another',
+            base: 'C:\\foo\\bar',
+            expected: [
+              'C:\\foo\\bar\\baz\\another',
+              'C:\\foo\\bar\\baz',
+              'C:\\foo\\bar',
+            ],
+          },
+        ]
+      : [
+          {
+            start: '/foo/bar/baz',
+            expected: ['/foo/bar/baz', '/foo/bar', '/foo', '/'],
+          },
+          {
+            start: '/foo/../bar/./baz',
+            expected: ['/bar/baz', '/bar', '/'],
+          },
+          {
+            start: '/foo/bar/baz/another',
+            base: '/foo/bar',
+            expected: ['/foo/bar/baz/another', '/foo/bar/baz', '/foo/bar'],
+          },
+        ]
+  )(
+    'should traverse start="$start", base="$base"',
+    ({ start, base, expected }) => {
+      expect(Array.from(traverseUpDirectories({ start, base }))).toEqual(
+        expected
+      );
+    }
+  );
+});

packages/cli/src/util/link/repo.ts

@@ -2,7 +2,7 @@ import chalk from 'chalk';
 import pluralize from 'pluralize';
 import { homedir } from 'os';
 import { join, normalize } from 'path';
-import { normalizePath } from '@vercel/build-utils';
+import { normalizePath, traverseUpDirectories } from '@vercel/build-utils';
 import { lstat, readJSON, outputJSON } from 'fs-extra';
 import confirm from '../input/confirm';
 import toHumanPath from '../humanize-path';
@@ -229,7 +229,7 @@ export async function findRepoRoot(
   const REPO_JSON_PATH = join(VERCEL_DIR, VERCEL_DIR_REPO);
   const GIT_CONFIG_PATH = normalize('.git/config');
 
-  for (const current of traverseUpDirectories(start)) {
+  for (const current of traverseUpDirectories({ start })) {
     if (current === home) {
       // Sometimes the $HOME directory is set up as a Git repo
       // (for dotfiles, etc.). In this case it's safe to say that
@@ -264,16 +264,6 @@ export async function findRepoRoot(
   debug('Aborting search for repo root');
 }
 
-export function* traverseUpDirectories(start: string) {
-  let current: string | undefined = normalize(start);
-  while (current) {
-    yield current;
-    // Go up one directory
-    const next = join(current, '..');
-    current = next === current ? undefined : next;
-  }
-}
-
 function sortByDirectory(a: RepoProjectConfig, b: RepoProjectConfig): number {
   const aParts = a.directory.split('/');
   const bParts = b.directory.split('/');

packages/cli/test/unit/util/link/repo.test.ts

@@ -4,12 +4,9 @@ import {
   findProjectsFromPath,
   findRepoRoot,
   RepoProjectConfig,
-  traverseUpDirectories,
 } from '../../../../src/util/link/repo';
 import { client } from '../../../mocks/client';
 
-const isWindows = process.platform === 'win32';
-
 // Root of `vercel/vercel` repo
 const vercelRepoRoot = join(__dirname, '../../../../../..');
 
@@ -30,34 +27,6 @@ describe('findRepoRoot()', () => {
   });
 });
 
-describe('traverseUpDirectories()', () => {
-  test.each(
-    isWindows
-      ? [
-          {
-            input: 'C:\\foo\\bar\\baz',
-            expected: ['C:\\foo\\bar\\baz', 'C:\\foo\\bar', 'C:\\foo', 'C:\\'],
-          },
-          {
-            input: 'C:\\foo\\..\\bar\\.\\baz',
-            expected: ['C:\\bar\\baz', 'C:\\bar', 'C:\\'],
-          },
-        ]
-      : [
-          {
-            input: '/foo/bar/baz',
-            expected: ['/foo/bar/baz', '/foo/bar', '/foo', '/'],
-          },
-          {
-            input: '/foo/../bar/./baz',
-            expected: ['/bar/baz', '/bar', '/'],
-          },
-        ]
-  )('should traverse "$input"', ({ input, expected }) => {
-    expect(Array.from(traverseUpDirectories(input))).toEqual(expected);
-  });
-});
-
 describe('findProjectsFromPath()', () => {
   const projects: RepoProjectConfig[] = [
     { id: 'root', name: 'r', directory: '.' },

packages/next/src/index.ts

@@ -18,7 +18,7 @@ import {
   runPackageJsonScript,
   execCommand,
   getEnvForPackageManager,
-  getNodeBinPath,
+  getNodeBinPaths,
   scanParentDirs,
   BuildV2,
   PrepareCache,
@@ -431,7 +431,11 @@ export const build: BuildV2 = async ({
 
   if (buildCommand) {
     // Add `node_modules/.bin` to PATH
-    const nodeBinPath = await getNodeBinPath({ cwd: entryPath });
+    const nodeBinPaths = getNodeBinPaths({
+      start: entryPath,
+      base: repoRootPath,
+    });
+    const nodeBinPath = nodeBinPaths.join(path.delimiter);
     env.PATH = `${nodeBinPath}${path.delimiter}${env.PATH}`;
 
     // Yarn v2 PnP mode may be activated, so force "node-modules" linker style

packages/static-build/src/index.ts

@@ -21,7 +21,7 @@ import {
   runNpmInstall,
   getEnvForPackageManager,
   getPrefixedEnvVars,
-  getNodeBinPath,
+  getNodeBinPaths,
   runBundleInstall,
   runPipInstall,
   runPackageJsonScript,
@@ -305,6 +305,7 @@ export const build: BuildV2 = async ({
   files,
   entrypoint,
   workPath,
+  repoRootPath,
   config,
   meta = {},
 }) => {
@@ -519,10 +520,15 @@ export const build: BuildV2 = async ({
     const pathList = [];
 
     if (isNpmInstall || (pkg && (buildCommand || devCommand))) {
-      const nodeBinPath = await getNodeBinPath({ cwd: entrypointDir });
-      pathList.push(nodeBinPath); // Add `./node_modules/.bin`
+      const nodeBinPaths = getNodeBinPaths({
+        start: entrypointDir,
+        base: repoRootPath,
+      });
+      pathList.push(...nodeBinPaths); // Add `./node_modules/.bin`
       debug(
-        `Added "${nodeBinPath}" to PATH env because a package.json file was found`
+        `Added "${nodeBinPaths.join(
+          path.delimiter
+        )}" to PATH env because a package.json file was found`
       );
     }