Merge pull request #20464 from storybookjs/find-closest-lockfile

CLI: Use closest lockfile to determine package manager

code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts

@@ -1,5 +1,6 @@
 import { sync as spawnSync } from 'cross-spawn';
 import { sync as findUpSync } from 'find-up';
+import path from 'path';
 import { JsPackageManagerFactory } from './JsPackageManagerFactory';
 import { NPMProxy } from './NPMProxy';
 import { PNPMProxy } from './PNPMProxy';
@@ -11,9 +12,12 @@ const spawnSyncMock = spawnSync as jest.Mock;
 
 jest.mock('find-up');
 const findUpSyncMock = findUpSync as unknown as jest.Mock;
-findUpSyncMock.mockReturnValue(undefined);
 
 describe('JsPackageManagerFactory', () => {
+  beforeEach(() => {
+    findUpSyncMock.mockReturnValue(undefined);
+  });
+
   describe('getPackageManager', () => {
     describe('return an NPM proxy', () => {
       it('when `force` option is `npm`', () => {
@@ -52,9 +56,7 @@ describe('JsPackageManagerFactory', () => {
         });
 
         // There is only a package-lock.json
-        findUpSyncMock.mockImplementation((file) =>
-          file === 'package-lock.json' ? 'found' : undefined
-        );
+        findUpSyncMock.mockImplementation(() => '/Users/johndoe/Documents/package-lock.json');
 
         expect(JsPackageManagerFactory.getPackageManager()).toBeInstanceOf(NPMProxy);
       });
@@ -97,15 +99,45 @@ describe('JsPackageManagerFactory', () => {
         });
 
         // There is only a pnpm-lock.yaml
-        findUpSyncMock.mockImplementation((file) => {
-          if (file === 'pnpm-lock.yaml') {
-            return 'found';
-          }
-          return undefined;
-        });
+        findUpSyncMock.mockImplementation(() => '/Users/johndoe/Documents/pnpm-lock.yaml');
 
         expect(JsPackageManagerFactory.getPackageManager()).toBeInstanceOf(PNPMProxy);
       });
+
+      it('when a pnpm-lock.yaml file is closer than a yarn.lock', () => {
+        // Allow find-up to work as normal, we'll set the cwd to our fixture package
+        findUpSyncMock.mockImplementation(jest.requireActual('find-up').sync);
+
+        spawnSyncMock.mockImplementation((command) => {
+          // Yarn is ok
+          if (command === 'yarn') {
+            return {
+              status: 0,
+              output: '1.22.4',
+            };
+          }
+          // NPM is ok
+          if (command === 'npm') {
+            return {
+              status: 0,
+              output: '6.5.12',
+            };
+          }
+          // PNPM is ok
+          if (command === 'pnpm') {
+            return {
+              status: 0,
+              output: '7.9.5',
+            };
+          }
+          // Unknown package manager is ko
+          return {
+            status: 1,
+          };
+        });
+        const fixture = path.join(__dirname, 'fixtures', 'pnpm-workspace', 'package');
+        expect(JsPackageManagerFactory.getPackageManager({}, fixture)).toBeInstanceOf(PNPMProxy);
+      });
     });
 
     describe('return a Yarn 1 proxy', () => {
@@ -178,12 +210,45 @@ describe('JsPackageManagerFactory', () => {
         });
 
         // There is a yarn.lock
-        findUpSyncMock.mockImplementation((file) =>
-          file === 'yarn.lock' ? '/Users/johndoe/Documents/yarn.lock' : undefined
-        );
+        findUpSyncMock.mockImplementation(() => '/Users/johndoe/Documents/yarn.lock');
 
         expect(JsPackageManagerFactory.getPackageManager()).toBeInstanceOf(Yarn1Proxy);
       });
+
+      it('when multiple lockfiles are in a project, prefers yarn', () => {
+        // Allow find-up to work as normal, we'll set the cwd to our fixture package
+        findUpSyncMock.mockImplementation(jest.requireActual('find-up').sync);
+
+        spawnSyncMock.mockImplementation((command) => {
+          // Yarn is ok
+          if (command === 'yarn') {
+            return {
+              status: 0,
+              output: '1.22.4',
+            };
+          }
+          // NPM is ok
+          if (command === 'npm') {
+            return {
+              status: 0,
+              output: '6.5.12',
+            };
+          }
+          // PNPM is ok
+          if (command === 'pnpm') {
+            return {
+              status: 0,
+              output: '7.9.5',
+            };
+          }
+          // Unknown package manager is ko
+          return {
+            status: 1,
+          };
+        });
+        const fixture = path.join(__dirname, 'fixtures', 'multiple-lockfiles');
+        expect(JsPackageManagerFactory.getPackageManager({}, fixture)).toBeInstanceOf(Yarn1Proxy);
+      });
     });
 
     describe('return a Yarn 2 proxy', () => {
@@ -253,9 +318,7 @@ describe('JsPackageManagerFactory', () => {
         });
 
         // There is a yarn.lock
-        findUpSyncMock.mockImplementation((file) =>
-          file === 'yarn.lock' ? '/Users/johndoe/Documents/yarn.lock' : undefined
-        );
+        findUpSyncMock.mockImplementation(() => '/Users/johndoe/Documents/yarn.lock');
 
         expect(JsPackageManagerFactory.getPackageManager()).toBeInstanceOf(Yarn2Proxy);
       });

code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts

@@ -1,17 +1,20 @@
+import path from 'node:path';
 import { sync as spawnSync } from 'cross-spawn';
 import { sync as findUpSync } from 'find-up';
 
 import { NPMProxy } from './NPMProxy';
-
 import { PNPMProxy } from './PNPMProxy';
 
 import type { JsPackageManager } from './JsPackageManager';
 import { type PackageManagerName } from './JsPackageManager';
 
 import { Yarn2Proxy } from './Yarn2Proxy';
-
 import { Yarn1Proxy } from './Yarn1Proxy';
 
+const NPM_LOCKFILE = 'package-lock.json';
+const PNPM_LOCKFILE = 'pnpm-lock.yaml';
+const YARN_LOCKFILE = 'yarn.lock';
+
 export class JsPackageManagerFactory {
   public static getPackageManager(
     { force }: { force?: PackageManagerName } = {},
@@ -31,17 +34,20 @@ export class JsPackageManagerFactory {
     }
 
     const yarnVersion = getYarnVersion(cwd);
-    const hasYarnLockFile = Boolean(findUpSync('yarn.lock', { cwd }));
-    const hasPNPMLockFile = Boolean(findUpSync('pnpm-lock.yaml', { cwd }));
+
+    const closestLockfilePath = findUpSync([YARN_LOCKFILE, PNPM_LOCKFILE, NPM_LOCKFILE], {
+      cwd,
+    });
+    const closestLockfile = closestLockfilePath && path.basename(closestLockfilePath);
 
     const hasNPMCommand = hasNPM(cwd);
     const hasPNPMCommand = hasPNPM(cwd);
 
-    if (yarnVersion && (hasYarnLockFile || (!hasNPMCommand && !hasPNPMCommand))) {
+    if (yarnVersion && (closestLockfile === YARN_LOCKFILE || (!hasNPMCommand && !hasPNPMCommand))) {
       return yarnVersion === 1 ? new Yarn1Proxy({ cwd }) : new Yarn2Proxy({ cwd });
     }
 
-    if (hasPNPMCommand && hasPNPMLockFile) {
+    if (hasPNPMCommand && closestLockfile === PNPM_LOCKFILE) {
       return new PNPMProxy({ cwd });
     }
 

code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "multiple-lockfiles"
+}

code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "pnpm-workspace"
+}

code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "test-fixture"
+}