-
Notifications
You must be signed in to change notification settings - Fork 11
feat(monorepo-tools): add pnpm mode support #621
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import assert from 'assert'; | ||
| import * as packageManager from './package-manager'; | ||
|
|
||
| describe('package-manager', function () { | ||
| let originalEnv: string | undefined; | ||
|
|
||
| beforeEach(function () { | ||
| originalEnv = process.env.MONOREPO_TOOLS_USE_PNPM; | ||
| }); | ||
|
|
||
| afterEach(function () { | ||
| if (originalEnv === undefined) { | ||
| delete process.env.MONOREPO_TOOLS_USE_PNPM; | ||
| } else { | ||
| process.env.MONOREPO_TOOLS_USE_PNPM = originalEnv; | ||
| } | ||
| }); | ||
|
|
||
| describe('getPackageManager', function () { | ||
| it('returns npm by default', function () { | ||
| delete process.env.MONOREPO_TOOLS_USE_PNPM; | ||
| assert.strictEqual(packageManager.getPackageManager(), 'npm'); | ||
| }); | ||
|
|
||
| it('returns npm when MONOREPO_TOOLS_USE_PNPM is false', function () { | ||
| process.env.MONOREPO_TOOLS_USE_PNPM = 'false'; | ||
| assert.strictEqual(packageManager.getPackageManager(), 'npm'); | ||
| }); | ||
|
|
||
| it('returns npm when MONOREPO_TOOLS_USE_PNPM is empty string', function () { | ||
| process.env.MONOREPO_TOOLS_USE_PNPM = ''; | ||
| assert.strictEqual(packageManager.getPackageManager(), 'npm'); | ||
| }); | ||
|
|
||
| it('returns pnpm when MONOREPO_TOOLS_USE_PNPM is true', function () { | ||
| process.env.MONOREPO_TOOLS_USE_PNPM = 'true'; | ||
| assert.strictEqual(packageManager.getPackageManager(), 'pnpm'); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { promisify } from 'util'; | ||
| import { execFile as execFileCallback, execFileSync } from 'child_process'; | ||
|
|
||
| const execFile = promisify(execFileCallback); | ||
|
|
||
| /** | ||
| * Get the package manager to use based on environment variable. | ||
| * Defaults to 'npm' if MONOREPO_TOOLS_USE_PNPM is not set or set to anything other than 'true'. | ||
| * | ||
| * Set MONOREPO_TOOLS_USE_PNPM=true to use pnpm instead of npm. | ||
| */ | ||
| export function getPackageManager(): 'npm' | 'pnpm' { | ||
| return process.env.MONOREPO_TOOLS_USE_PNPM === 'true' ? 'pnpm' : 'npm'; | ||
| } | ||
|
|
||
| /** | ||
| * Install dependencies using the configured package manager. | ||
| * Equivalent to: npm install / pnpm install | ||
| * | ||
| * @param options.cwd Working directory (defaults to process.cwd()) | ||
| * @param options.packageLockOnly If true, only updates lock file without installing (defaults to false) | ||
| */ | ||
| export async function installDependencies( | ||
| options: { cwd?: string; packageLockOnly?: boolean } = {}, | ||
| ): Promise<{ stdout: string; stderr: string }> { | ||
| const { cwd, packageLockOnly = false } = options; | ||
| const packageManager = getPackageManager(); | ||
| const args = ['install']; | ||
|
|
||
| if (packageLockOnly) { | ||
| // npm uses --package-lock-only, pnpm uses --lockfile-only | ||
| args.push( | ||
| packageManager === 'pnpm' ? '--lockfile-only' : '--package-lock-only', | ||
| ); | ||
| } | ||
|
|
||
| return await execFile(packageManager, args, { cwd }); | ||
| } | ||
|
|
||
| /** | ||
| * Get the version of the configured package manager. | ||
| * Returns the version string (e.g., "9.0.0" for npm or "8.15.0" for pnpm). | ||
| */ | ||
| export async function getPackageManagerVersion(): Promise<string> { | ||
| const packageManager = getPackageManager(); | ||
| const { stdout } = await execFile(packageManager, ['-v']); | ||
| return stdout.trim(); | ||
| } | ||
|
|
||
| /** | ||
| * Run a package manager command for specific workspaces. | ||
| * For npm: uses --workspace flags | ||
| * For pnpm: uses --filter flags | ||
| * | ||
| * @param options.workspaces Array of workspace names to target | ||
| * @param options.args Package manager command arguments (e.g., ['run', 'test']) | ||
| * @param options.stdio stdio configuration for child process (defaults to 'inherit') | ||
| */ | ||
| export function runForWorkspaces(options: { | ||
| workspaces: string[]; | ||
| args: string[]; | ||
| stdio?: 'inherit' | 'pipe' | 'ignore'; | ||
| }): void { | ||
| const { workspaces, args, stdio = 'inherit' } = options; | ||
| const packageManager = getPackageManager(); | ||
|
|
||
| const workspaceArgs = | ||
| packageManager === 'pnpm' | ||
| ? workspaces.map((name) => `--filter=${name}`) | ||
| : workspaces.map((name) => `--workspace=${name}`); | ||
|
|
||
| execFileSync(packageManager, [...workspaceArgs, ...args], { stdio }); | ||
| } | ||
|
|
||
| /** | ||
| * Execute a package with the configured package executor (npx or pnpm dlx). | ||
| * | ||
| * @param options.packageName The package/command to execute | ||
| * @param options.args Arguments to pass to the package | ||
| * @param options.cwd Working directory (defaults to process.cwd()) | ||
| */ | ||
| export async function executePackage(options: { | ||
| packageName: string; | ||
| args: string[]; | ||
| cwd?: string; | ||
| }): Promise<{ stdout: string; stderr: string }> { | ||
| const { packageName, args, cwd } = options; | ||
| const packageManager = getPackageManager(); | ||
|
|
||
| if (packageManager === 'pnpm') { | ||
| return await execFile('pnpm', ['dlx', packageName, ...args], { cwd }); | ||
| } | ||
| return await execFile('npx', [packageName, ...args], { cwd }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need a pnpm-lock.yaml to align behavior locally and on CI? And if we do add pnpm-lock do we have a way of keeping it in sync with package-lock.json?
Uh oh!
There was an error while loading. Please reload this page.
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.
if a repo uses pnpm, then we will only have pnpm-lock, we should not have package-lock.
One can also set pnpm as default package manager in package.json so then even if someone runs
npm iit'll redirect topnpmThere 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.
These tools could try to look for pnpm-lock to determine what mode to run in maybe but pnpm lock might be at a higher level folder etc so env just seems more flexible
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.
Understood but this PR is for adding optional use of pnpm, so there is no pnpm-lock file, as a result is anyone who uses pnpm going to suffer from a broken experience at least locally when they go to install and they get a very different set of transient dependencies than are used on CI? (as a comparable example, I often accidentally work in compass on the wrong npm version and even that deviation causes a change in the result of
npm iand tests fail)Main concern here, sure we exec pnpm, but is it useful? (will it be useful? because it may work today but when it breaks what do we do about it to fix it, what will be the urgency to fix it, etc.)