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 d9eca152934..b503774b10f 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' @@ -188,6 +188,8 @@ describe('Project integration', () => { test('Project loads correct metadata from filesystem', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) + // Make package-manager detection depend on root markers instead of ambient user-agent fallback. + await writeFile(joinPath(dir, 'package-lock.json'), '') const project = await Project.load(dir) @@ -197,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 f49bf2f6fd9..b0e891e11db 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, @@ -783,6 +784,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 @@ -943,6 +961,35 @@ describe('getPackageManager', () => { }) }) +describe('getPackageManagerForProjectRoot', () => { + test('detects the package manager from root markers without calling npm prefix', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'yarn.lock'), '') + + await expect(getPackageManagerForProjectRoot(tmpDir)).resolves.toBe('yarn') + expect(mockedCaptureOutput).not.toHaveBeenCalled() + }) + }) + + 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'}) + + await expect(getPackageManagerForProjectRoot(tmpDir)).resolves.toBe('npm') + expect(mockedCaptureOutput).not.toHaveBeenCalled() + }) + }) + + 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 873d7e88135..3b2c3df019a 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. @@ -149,24 +149,36 @@ async function getProjectPackageManagerFromDirectory( return getProjectPackageManagerFromDirectory(parentDirectory, hasPackageJson) } -function projectPackageManagerFromUserAgentOrNpm(env = process.env): ProjectPackageManager { - const detectedPackageManager = packageManagerFromUserAgent(env) - switch (detectedPackageManager) { +/** + * Narrows a broad package-manager value to the set that can safely drive project-local + * install, mutation, and binary-execution operations. + */ +function normalizePackageManagerForProject(packageManager: PackageManager): ProjectPackageManager { + switch (packageManager) { case 'yarn': case 'npm': case 'pnpm': case 'bun': - return detectedPackageManager + return packageManager case 'homebrew': case 'unknown': return 'npm' } } +function projectPackageManagerFromUserAgentOrNpm(env = process.env): ProjectPackageManager { + return normalizePackageManagerForProject(packageManagerFromUserAgent(env)) +} + /** - * Returns the dependency manager used in a directory. - * @param fromDirectory - The starting directory - * @returns The dependency manager + * Returns the dependency manager governing the project or workspace root discovered from a + * starting directory. + * + * Use this when the caller does not already know the project root and wants the historical + * root-finding behavior based on `npm prefix`. + * + * @param fromDirectory - The starting directory. + * @returns The detected package manager, or a user-agent fallback if the root can't be resolved. */ export async function getPackageManager(fromDirectory: string): Promise { let directory: string | undefined @@ -187,6 +199,25 @@ export async function getPackageManager(fromDirectory: string): Promise { + const packageJsonPath = joinPath(rootDirectory, 'package.json') + if (!(await fileExists(packageJsonPath))) { + throw new PackageJsonNotFoundError(rootDirectory) + } + + return (await inferProjectPackageManagerAtDirectory(rootDirectory)) ?? 'npm' +} + function packageManagerBinaryCommand( packageManager: ProjectPackageManager, binary: string, @@ -764,7 +795,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 79541ecd4c3..543098c4cf5 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' @@ -198,7 +198,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,