-
Notifications
You must be signed in to change notification settings - Fork 244
Add root-known package manager helper #7241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
|
@@ -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<PackageM | |
| let current = fromDirectory | ||
| outputDebug(outputContent`Looking for a lockfile in ${outputToken.path(current)}...`) | ||
| while (true) { | ||
| if (fileExistsSync(joinPath(current, yarnLockfile))) return 'yarn' | ||
| if (fileExistsSync(joinPath(current, pnpmLockfile)) || fileExistsSync(joinPath(current, pnpmWorkspaceFile))) { | ||
| return 'pnpm' | ||
| } | ||
| if (hasBunLockfileSync(current)) return 'bun' | ||
| if (fileExistsSync(joinPath(current, npmLockfile))) return 'npm' | ||
| const detectedPackageManager = getProjectPackageManagerAtDirectory(current) | ||
| if (detectedPackageManager) return detectedPackageManager | ||
|
|
||
| const parent = dirname(current) | ||
| if (parent === current) break | ||
| current = parent | ||
|
|
@@ -175,6 +183,25 @@ export async function getPackageManager(fromDirectory: string): Promise<PackageM | |
| return '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 walking upward from a child directory. | ||
| * | ||
| * @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> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you work with the CLI, I think you typically run it from the project root or from an extension. But both of them already have a package.json, so why using this instead of getPackageManager? Isn't it the same? |
||
| 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<string, string>): Promise<void> { | ||
| const packageManager = await getPackageManager(directory) | ||
| const packageManager = await getPackageManagerForProjectRoot(directory) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you know that this directory is the root or not? If it must be, I'd make it explicit with the param name or documentation. |
||
| const packageJsonPath = joinPath(directory, 'package.json') | ||
| const packageJsonContent = await readAndParsePackageJson(packageJsonPath) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove unknown and either make the operation sync or force the caller to narrow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I looked at that. I kept
'unknown'here becauseProjectis still just modeling what’s on disk, and “app root with no package.json” is a real state.The sync narrowing is in the next PR which adds
requirePackageManagerForOperations(), so install/mutation callers have to narrow before using it.Happy to fold that in here instead if you’d prefer, but I split it to keep metadata vs operation-owned use separate.