From 7713ab49d1b54a0f114100c632305d4f65b616ce Mon Sep 17 00:00:00 2001 From: andromia3 Date: Tue, 14 Apr 2026 17:48:08 +0100 Subject: [PATCH] fix(cli-kit): respect package manager when installNodeModules adds specific packages (#7042) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `installNodeModules()` always ran ` install `, but `yarn`, `pnpm`, and `bun` all require the `add` subcommand to install a specific package with a version specifier — their `install` subcommand installs from the lockfile. So a call like installNodeModules({packageManager: 'yarn', args: ['@shopify/hydrogen@2025.7.1']}) produced `yarn install @shopify/hydrogen@2025.7.1`, which yarn rejects. The same applied to pnpm. Hydrogen PR #3462 worked around this by bypassing `installNodeModules()` with a direct `exec()` call that mapped the subcommand per package manager; this upstream fix lets that workaround be removed. Rather than silently change the meaning of `args` (which breaks `init/template/npm.ts`, which uses it for flags like `['--network-concurrency', '1']`), added a new optional `packages?: string[]` field to `InstallNodeModulesOptions`. When `packages` is provided: - `npm` keeps using `install` (no change, and npm supports both). - `yarn`, `pnpm`, and `bun` use `add`, matching the existing internal helpers `argumentsToAddDependenciesWith{Yarn,PNPM,Bun}` further down the same file. When `packages` is omitted the function behaves identically to the old implementation, so every existing call site is unaffected. Added six new tests in `node-package-manager.test.ts` covering: - npm with packages (`install`) - yarn with packages (`add`) - pnpm with packages (`add`) - bun with packages (`add`) - packages + args ordering (packages first, then flags) - yarn with `args` only (no packages) — unchanged `install` behaviour Fixes #7042. --- .../fix-install-node-modules-subcommand.md | 5 + .../public/node/node-package-manager.test.ts | 103 ++++++++++++++++++ .../src/public/node/node-package-manager.ts | 24 +++- 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-install-node-modules-subcommand.md diff --git a/.changeset/fix-install-node-modules-subcommand.md b/.changeset/fix-install-node-modules-subcommand.md new file mode 100644 index 0000000000..6d2dd2675f --- /dev/null +++ b/.changeset/fix-install-node-modules-subcommand.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Fix `installNodeModules()` using the wrong subcommand for yarn, pnpm, and bun when adding specific packages. The function now accepts an optional `packages: string[]` parameter; when provided, yarn, pnpm, and bun use their `add` subcommand (required for adding new packages) while npm continues to use `install`. Existing call sites that pass only `args` are unaffected. 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 df1d2ff0a4..8a02b9b69a 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 @@ -130,6 +130,109 @@ describe('install', () => { cwd: directory, }) }) + + test('uses `install` when packages are provided for npm', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'npm', + packages: ['@shopify/hydrogen@2025.7.1'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('npm', ['install', '@shopify/hydrogen@2025.7.1'], { + cwd: directory, + }) + }) + + test('uses `add` when packages are provided for yarn', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'yarn', + packages: ['@shopify/hydrogen@2025.7.1'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('yarn', ['add', '@shopify/hydrogen@2025.7.1'], { + cwd: directory, + }) + }) + + test('uses `add` when packages are provided for pnpm', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'pnpm', + packages: ['@shopify/hydrogen@2025.7.1'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('pnpm', ['add', '@shopify/hydrogen@2025.7.1'], { + cwd: directory, + }) + }) + + test('uses `add` when packages are provided for bun', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'bun', + packages: ['@shopify/hydrogen@2025.7.1'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('bun', ['add', '@shopify/hydrogen@2025.7.1'], { + cwd: directory, + }) + }) + + test('appends `args` after `packages` so flags follow the package specifiers', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'yarn', + packages: ['@shopify/hydrogen@2025.7.1'], + args: ['--exact'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('yarn', ['add', '@shopify/hydrogen@2025.7.1', '--exact'], { + cwd: directory, + }) + }) + + test('uses `install` (unchanged) when no packages are provided, even for yarn', async () => { + // Given + const directory = '/path/to/project' + + // When + await installNodeModules({ + directory, + packageManager: 'yarn', + args: ['--network-concurrency', '1'], + }) + + // Then + expect(mockedExec).toHaveBeenCalledWith('yarn', ['install', '--network-concurrency', '1'], { + cwd: directory, + }) + }) }) describe('getPackageName', () => { 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 5e40ad9295..b27c151046 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.ts @@ -242,7 +242,20 @@ export async function installNPMDependenciesRecursively( interface InstallNodeModulesOptions { directory: string + /** + * Additional flags passed to the package manager after the subcommand and + * any `packages`. Use this for things like `--network-concurrency 1`. + */ args?: string[] + /** + * Specific package specifiers (e.g. `['@shopify/hydrogen@2025.7.1']`) to + * add to the project. When provided, `yarn`, `pnpm`, and `bun` use their + * `add` subcommand instead of `install`, since those package managers' + * `install` subcommand installs from the lockfile rather than adding new + * packages. When omitted, the function runs ` install` to install + * dependencies from the lockfile (the existing behaviour). + */ + packages?: string[] packageManager: PackageManager stdout?: Writable stderr?: Writable @@ -257,7 +270,16 @@ export async function installNodeModules(options: InstallNodeModulesOptions): Pr stderr: options.stderr, signal: options.signal, } - let args = ['install'] + const hasPackages = Boolean(options.packages?.length) + const usesAddSubcommand = + hasPackages && + (options.packageManager === 'yarn' || + options.packageManager === 'pnpm' || + options.packageManager === 'bun') + let args = [usesAddSubcommand ? 'add' : 'install'] + if (options.packages) { + args = args.concat(options.packages) + } if (options.args) { args = args.concat(options.args) }