diff --git a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts b/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts index f522bef92480..f4774268af6b 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts +++ b/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 `useNpm` option is used', () => { @@ -58,9 +62,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); }); @@ -103,15 +105,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', () => { @@ -184,12 +216,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', () => { @@ -259,9 +324,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); }); diff --git a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts b/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts index 8259c57caa6e..21d80e03bfe6 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts +++ b/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, useNpm }: { force?: PackageManagerName; useNpm?: boolean } = {}, @@ -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 }); } diff --git a/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/package.json b/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/package.json new file mode 100644 index 000000000000..c388f3265a2e --- /dev/null +++ b/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/package.json @@ -0,0 +1,3 @@ +{ + "name": "multiple-lockfiles" +} diff --git a/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/pnpm-lock.yaml b/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/pnpm-lock.yaml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/yarn.lock b/code/lib/cli/src/js-package-manager/fixtures/multiple-lockfiles/yarn.lock new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package.json b/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package.json new file mode 100644 index 000000000000..a07127694e1f --- /dev/null +++ b/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package.json @@ -0,0 +1,3 @@ +{ + "name": "pnpm-workspace" +} diff --git a/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package/package.json b/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package/package.json new file mode 100644 index 000000000000..0e16e0cebbdc --- /dev/null +++ b/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/package/package.json @@ -0,0 +1,3 @@ +{ + "name": "test-fixture" +} diff --git a/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/pnpm-lock.yaml b/code/lib/cli/src/js-package-manager/fixtures/pnpm-workspace/pnpm-lock.yaml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/code/lib/cli/src/js-package-manager/fixtures/yarn.lock b/code/lib/cli/src/js-package-manager/fixtures/yarn.lock new file mode 100644 index 000000000000..e69de29bb2d1