Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions packages/app/src/cli/models/project/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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

Expand All @@ -101,7 +101,7 @@ export class Project {
}

readonly directory: string
readonly packageManager: PackageManager
readonly packageManager: ProjectPackageManager | 'unknown'
readonly nodeDependencies: Record<string, string>
readonly usesWorkspaces: boolean
readonly appConfigFiles: TomlFile[]
Expand All @@ -119,7 +119,7 @@ export class Project {

private constructor(options: {
directory: string
packageManager: PackageManager
packageManager: ProjectPackageManager | 'unknown'
nodeDependencies: Record<string, string>
usesWorkspaces: boolean
appConfigFiles: TomlFile[]
Expand Down
47 changes: 47 additions & 0 deletions packages/cli-kit/src/public/node/node-package-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
addResolutionOrOverride,
writePackageJSON,
getPackageManager,
getPackageManagerForProjectRoot,
packageManagerBinaryCommandForDirectory,
installNPMDependenciesRecursively,
addNPMDependencies,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down
49 changes: 40 additions & 9 deletions packages/cli-kit/src/public/node/node-package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PackageManager, 'yarn' | 'npm' | 'pnpm' | 'bun'>
export type ProjectPackageManager = Extract<PackageManager, 'yarn' | 'npm' | 'pnpm' | 'bun'>

/**
* Returns an abort error that's thrown when the package manager can't be determined.
Expand Down Expand Up @@ -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<PackageManager> {
let directory: string | undefined
Expand All @@ -187,6 +199,25 @@ export async function getPackageManager(fromDirectory: string): Promise<PackageM
return (await inferProjectPackageManagerAtDirectory(directory)) ?? 'npm'
}

/**
* Resolves the package manager for a directory already known to be the project root.
*
* Use this when the caller already knows the root directory and wants to inspect that root
* directly rather than rediscovering it via `npm prefix`.
*
* @param rootDirectory - The known project root directory.
* @returns The package manager detected from root markers, defaulting to `npm` when no marker exists.
* @throws PackageJsonNotFoundError if the provided directory does not contain a package.json.
*/
export async function getPackageManagerForProjectRoot(rootDirectory: string): Promise<ProjectPackageManager> {
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,
Expand Down Expand Up @@ -764,7 +795,7 @@ export async function findUpAndReadPackageJson(fromDirectory: string): Promise<{
}

export async function addResolutionOrOverride(directory: string, dependencies: Record<string, string>): Promise<void> {
const packageManager = await getPackageManager(directory)
const packageManager = await getPackageManagerForProjectRoot(directory)
const packageJsonPath = joinPath(directory, 'package.json')
const packageJsonContent = await readAndParsePackageJson(packageJsonPath)

Expand Down
4 changes: 2 additions & 2 deletions packages/cli-kit/src/public/node/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down
Loading