From 25e1f67eef16e6537b929ccb3766070505066d05 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Mon, 13 Apr 2026 14:30:24 -0400 Subject: [PATCH] Add root-known package manager helper --- .../project/project-integration.test.ts | 15 ++++- .../app/src/cli/models/project/project.ts | 10 ++-- .../public/node/node-package-manager.test.ts | 55 +++++++++++++++++++ .../src/public/node/node-package-manager.ts | 43 ++++++++++++--- packages/cli-kit/src/public/node/upgrade.ts | 4 +- 5 files changed, 111 insertions(+), 16 deletions(-) diff --git a/packages/app/src/cli/models/project/project-integration.test.ts b/packages/app/src/cli/models/project/project-integration.test.ts index a72994a26da..ef15d47636e 100644 --- a/packages/app/src/cli/models/project/project-integration.test.ts +++ b/packages/app/src/cli/models/project/project-integration.test.ts @@ -6,7 +6,7 @@ import {loadLocalExtensionsSpecifications} from '../extensions/load-specificatio import {handleWatcherEvents} from '../../services/dev/app-events/app-event-watcher-handler.js' import {EventType} from '../../services/dev/app-events/app-event-watcher.js' import {describe, expect, test} from 'vitest' -import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, writeFile, mkdir, removeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {AbortController} from '@shopify/cli-kit/node/abort' @@ -199,6 +199,19 @@ describe('Project integration', () => { }) }) + test('Project uses unknown package manager metadata when the app root has no package.json', async () => { + await inTemporaryDirectory(async (dir) => { + await setupRealApp(dir) + await removeFile(joinPath(dir, 'package.json')) + + const project = await Project.load(dir) + + expect(project.packageManager).toBe('unknown') + expect(project.nodeDependencies).toStrictEqual({}) + expect(project.usesWorkspaces).toBe(false) + }) + }) + test('multi-config project discovers all configs', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index f34ac61bedb..3b86572186a 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -4,8 +4,8 @@ import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env' import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs' import { getDependencies, - getPackageManager, - PackageManager, + getPackageManagerForProjectRoot, + ProjectPackageManager, usesWorkspaces as detectUsesWorkspaces, } from '@shopify/cli-kit/node/node-package-manager' import {joinPath, basename} from '@shopify/cli-kit/node/path' @@ -76,7 +76,7 @@ export class Project { // Project metadata const packageJSONPath = joinPath(directory, 'package.json') const hasPackageJson = await fileExists(packageJSONPath) - const packageManager = hasPackageJson ? await getPackageManager(directory) : 'unknown' + const packageManager = hasPackageJson ? await getPackageManagerForProjectRoot(directory) : 'unknown' const nodeDependencies = hasPackageJson ? await getDependencies(packageJSONPath) : {} const usesWorkspaces = hasPackageJson ? await detectUsesWorkspaces(directory) : false @@ -101,7 +101,7 @@ export class Project { } readonly directory: string - readonly packageManager: PackageManager + readonly packageManager: ProjectPackageManager | 'unknown' readonly nodeDependencies: Record readonly usesWorkspaces: boolean readonly appConfigFiles: TomlFile[] @@ -119,7 +119,7 @@ export class Project { private constructor(options: { directory: string - packageManager: PackageManager + packageManager: ProjectPackageManager | 'unknown' nodeDependencies: Record usesWorkspaces: boolean appConfigFiles: TomlFile[] diff --git a/packages/cli-kit/src/public/node/node-package-manager.test.ts b/packages/cli-kit/src/public/node/node-package-manager.test.ts index df1d2ff0a46..c6294c5597b 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.test.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.test.ts @@ -12,6 +12,7 @@ import { addResolutionOrOverride, writePackageJSON, getPackageManager, + getPackageManagerForProjectRoot, packageManagerBinaryCommandForDirectory, installNPMDependenciesRecursively, addNPMDependencies, @@ -784,6 +785,23 @@ describe('addResolutionOrOverride', () => { }) }) + test('when package.json has no lockfile then npm overrides are used by default', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const reactType = {'@types/react': '17.0.30'} + const packageJsonPath = joinPath(tmpDir, 'package.json') + await writeFile(packageJsonPath, JSON.stringify({})) + + // When + await addResolutionOrOverride(tmpDir, reactType) + + // Then + const packageJsonContent = await readAndParsePackageJson(packageJsonPath) + expect(packageJsonContent.overrides).toEqual(reactType) + expect(packageJsonContent.resolutions).toBeUndefined() + }) + }) + test('when package.json with existing resolution type and yarn manager then dependency version is overwritten', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given @@ -951,6 +969,43 @@ describe('getPackageManager', () => { }) }) +describe('getPackageManagerForProjectRoot', () => { + test('detects the package manager from root markers without consulting the user agent', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'yarn.lock'), '') + vi.stubEnv('npm_config_user_agent', 'pnpm/9.0.0') + + try { + await expect(getPackageManagerForProjectRoot(tmpDir)).resolves.toBe('yarn') + } finally { + vi.unstubAllEnvs() + } + }) + }) + + test('defaults to npm when the project root has a package.json but no lockfile markers', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + vi.stubEnv('npm_config_user_agent', 'pnpm/9.0.0') + + try { + await expect(getPackageManagerForProjectRoot(tmpDir)).resolves.toBe('npm') + } finally { + vi.unstubAllEnvs() + } + }) + }) + + test('throws when the provided root does not contain a package.json', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await expect(getPackageManagerForProjectRoot(tmpDir)).rejects.toThrow( + new PackageJsonNotFoundError(normalizePath(tmpDir)), + ) + }) + }) +}) + describe('packageManagerBinaryCommandForDirectory', () => { test('uses npm exec with -- for npm', async () => { await inTemporaryDirectory(async (tmpDir) => { diff --git a/packages/cli-kit/src/public/node/node-package-manager.ts b/packages/cli-kit/src/public/node/node-package-manager.ts index 5e40ad92958..2206b432dce 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.ts @@ -57,7 +57,7 @@ export type DependencyType = 'dev' | 'prod' | 'peer' */ export const packageManager = ['yarn', 'npm', 'pnpm', 'bun', 'homebrew', 'unknown'] as const export type PackageManager = (typeof packageManager)[number] -type ProjectPackageManager = Extract +export type ProjectPackageManager = Extract /** * Returns an abort error that's thrown when the package manager can't be determined. @@ -145,6 +145,17 @@ function packageManagerBinaryCommand( } } +function getProjectPackageManagerAtDirectory(directory: string): ProjectPackageManager | undefined { + if (fileExistsSync(joinPath(directory, yarnLockfile))) return 'yarn' + if (fileExistsSync(joinPath(directory, pnpmLockfile)) || fileExistsSync(joinPath(directory, pnpmWorkspaceFile))) { + return 'pnpm' + } + if (hasBunLockfileSync(directory)) return 'bun' + if (fileExistsSync(joinPath(directory, npmLockfile))) return 'npm' + + return undefined +} + /** * Returns the dependency manager used in a directory. * Walks upward from `fromDirectory` so workspace packages (e.g. `extensions/my-fn/package.json`) @@ -158,12 +169,9 @@ export async function getPackageManager(fromDirectory: string): Promise { + const packageJsonPath = joinPath(rootDirectory, 'package.json') + if (!(await fileExists(packageJsonPath))) { + throw new PackageJsonNotFoundError(rootDirectory) + } + + return getProjectPackageManagerAtDirectory(rootDirectory) ?? 'npm' +} + /** * Builds the command and argv needed to execute a local binary using the package manager * detected from the provided directory or its ancestors. @@ -733,7 +760,7 @@ export async function findUpAndReadPackageJson(fromDirectory: string): Promise<{ } export async function addResolutionOrOverride(directory: string, dependencies: Record): Promise { - const packageManager = await getPackageManager(directory) + const packageManager = await getPackageManagerForProjectRoot(directory) const packageJsonPath = joinPath(directory, 'package.json') const packageJsonContent = await readAndParsePackageJson(packageJsonPath) diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index cd5031550f8..381a88e0d2f 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -8,7 +8,7 @@ import { DependencyType, usesWorkspaces, addNPMDependencies, - getPackageManager, + getPackageManagerForProjectRoot, } from './node-package-manager.js' import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from './output.js' import {cwd, moduleDirectory, sniffForPath} from './path.js' @@ -210,7 +210,7 @@ async function installJsonDependencies( if (packagesToUpdate.length > 0) { await addNPMDependencies(packagesToUpdate, { - packageManager: await getPackageManager(directory), + packageManager: await getPackageManagerForProjectRoot(directory), type: depsEnv, directory, stdout: process.stdout,