From 7cf64a7759f1be691c38b3d3f488e238a6e008b6 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Thu, 9 Apr 2026 16:38:18 -0400 Subject: [PATCH] Use function directory package manager for GraphQL typegen Detect the package manager from the function directory when running the default JavaScript GraphQL type generation path, instead of hardcoding npm.\n\nKeep the broader getPackageManager() behavior unchanged by adding a narrow cli-kit helper that resolves the correct package-manager-specific binary invocation for a project directory. --- .../src/cli/services/function/build.test.ts | 42 +++++ .../app/src/cli/services/function/build.ts | 10 +- .../public/node/node-package-manager.test.ts | 143 ++++++++++++++++++ .../src/public/node/node-package-manager.ts | 98 ++++++++++-- 4 files changed, 281 insertions(+), 12 deletions(-) diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index addea2de099..9d006530eff 100644 --- a/packages/app/src/cli/services/function/build.test.ts +++ b/packages/app/src/cli/services/function/build.test.ts @@ -22,12 +22,20 @@ import { import {testApp, testFunctionExtension} from '../../models/app/app.test-data.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import {exec} from '@shopify/cli-kit/node/system' +import {packageManagerBinaryCommandForDirectory} from '@shopify/cli-kit/node/node-package-manager' import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {inTemporaryDirectory, mkdir, readFileSync, writeFile, removeFile} from '@shopify/cli-kit/node/fs' import {build as esBuild} from 'esbuild' vi.mock('@shopify/cli-kit/node/fs') vi.mock('@shopify/cli-kit/node/system') +vi.mock('@shopify/cli-kit/node/node-package-manager', async () => { + const actual: any = await vi.importActual('@shopify/cli-kit/node/node-package-manager') + return { + ...actual, + packageManagerBinaryCommandForDirectory: vi.fn(), + } +}) vi.mock('./binaries.js', async (importOriginal) => { const actual: any = await importOriginal() @@ -76,6 +84,10 @@ beforeEach(async () => { stderr = {write: vi.fn()} stdout = {write: vi.fn()} signal = vi.fn() + vi.mocked(packageManagerBinaryCommandForDirectory).mockResolvedValue({ + command: 'npm', + args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], + }) }) describe('buildGraphqlTypes', () => { @@ -88,6 +100,13 @@ describe('buildGraphqlTypes', () => { // Then await expect(got).resolves.toBeUndefined() + expect(packageManagerBinaryCommandForDirectory).toHaveBeenCalledTimes(1) + expect(packageManagerBinaryCommandForDirectory).toHaveBeenCalledWith( + ourFunction.directory, + 'graphql-code-generator', + '--config', + 'package.json', + ) expect(exec).toHaveBeenCalledWith('npm', ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], { cwd: ourFunction.directory, stderr, @@ -95,6 +114,26 @@ describe('buildGraphqlTypes', () => { }) }) + test('generate types executes the command returned by the shared helper', {timeout: 20000}, async () => { + // Given + const ourFunction = await testFunctionExtension({entryPath: 'src/index.js'}) + vi.mocked(packageManagerBinaryCommandForDirectory).mockResolvedValue({ + command: 'pnpm', + args: ['exec', 'graphql-code-generator', '--config', 'package.json'], + }) + + // When + const got = buildGraphqlTypes(ourFunction, {stdout, stderr, signal, app}) + + // Then + await expect(got).resolves.toBeUndefined() + expect(exec).toHaveBeenCalledWith('pnpm', ['exec', 'graphql-code-generator', '--config', 'package.json'], { + cwd: ourFunction.directory, + stderr, + signal, + }) + }) + test('errors if function is not a JS function and no typegen_command', async () => { // Given const ourFunction = await testFunctionExtension() @@ -105,6 +144,7 @@ describe('buildGraphqlTypes', () => { // Then await expect(got).rejects.toThrow(/No typegen_command specified/) + expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled() }) test('runs custom typegen_command when provided', async () => { @@ -129,6 +169,7 @@ describe('buildGraphqlTypes', () => { // Then await expect(got).resolves.toBeUndefined() + expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled() expect(exec).toHaveBeenCalledWith('npx', ['shopify-function-codegen', '--schema', 'schema.graphql'], { cwd: ourFunction.directory, stdout, @@ -159,6 +200,7 @@ describe('buildGraphqlTypes', () => { // Then await expect(got).resolves.toBeUndefined() + expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled() expect(exec).toHaveBeenCalledWith('custom-typegen', ['--output', 'types.ts'], { cwd: ourFunction.directory, stdout, diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index 7e8caf4f6fa..547401eb03a 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -24,6 +24,7 @@ import {renderTasks} from '@shopify/cli-kit/node/ui' import {pickBy} from '@shopify/cli-kit/common/object' import {runWithTimer} from '@shopify/cli-kit/node/metadata' import {AbortError} from '@shopify/cli-kit/node/error' +import {packageManagerBinaryCommandForDirectory} from '@shopify/cli-kit/node/node-package-manager' import {Writable} from 'stream' export const PREFERRED_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '2' @@ -143,8 +144,15 @@ export async function buildGraphqlTypes( ) } + const command = await packageManagerBinaryCommandForDirectory( + fun.directory, + 'graphql-code-generator', + '--config', + 'package.json', + ) + return runWithTimer('cmd_all_timing_network_ms')(async () => { - return exec('npm', ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], { + return exec(command.command, command.args, { cwd: fun.directory, stderr: options.stderr, signal: options.signal, 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 cbd5b629c96..f49bf2f6fd9 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, + packageManagerBinaryCommandForDirectory, installNPMDependenciesRecursively, addNPMDependencies, DependencyVersion, @@ -892,6 +893,21 @@ describe('getPackageManager', () => { }) }) + test('finds if bun is being used from bun.lock', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'bun.lock'), '') + mockedCaptureOutput.mockReturnValueOnce(Promise.resolve(tmpDir)) + + // When + const packageManager = await getPackageManager(tmpDir) + + // Then + expect(packageManager).toEqual('bun') + }) + }) + test('falls back to packageManagerFromUserAgent when npm prefix fails', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given @@ -927,6 +943,133 @@ describe('getPackageManager', () => { }) }) +describe('packageManagerBinaryCommandForDirectory', () => { + test('uses npm exec with -- for npm', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + + await expect( + packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'), + ).resolves.toEqual({ + command: 'npm', + args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], + }) + }) + }) + + test('uses exec without -- for pnpm when detected from an ancestor workspace marker', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'app-root'}) + await writeFile(joinPath(tmpDir, 'pnpm-workspace.yaml'), '') + const extensionDirectory = joinPath(tmpDir, 'extensions', 'my-function') + await mkdir(extensionDirectory) + await writePackageJSON(extensionDirectory, {name: 'my-function'}) + + await expect( + packageManagerBinaryCommandForDirectory( + extensionDirectory, + 'graphql-code-generator', + '--config', + 'package.json', + ), + ).resolves.toEqual({ + command: 'pnpm', + args: ['exec', 'graphql-code-generator', '--config', 'package.json'], + }) + }) + }) + + test('uses yarn run when detected from yarn.lock', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'yarn.lock'), '') + + await expect( + packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'), + ).resolves.toEqual({ + command: 'yarn', + args: ['run', 'graphql-code-generator', '--config', 'package.json'], + }) + }) + }) + + test('uses bun x for bun when detected from bun.lock', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'bun.lock'), '') + + await expect( + packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'), + ).resolves.toEqual({ + command: 'bun', + args: ['x', 'graphql-code-generator', '--config', 'package.json'], + }) + }) + }) + + test('uses bun x for bun when detected from bun.lockb', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'bun.lockb'), '') + + await expect( + packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'), + ).resolves.toEqual({ + command: 'bun', + args: ['x', 'graphql-code-generator', '--config', 'package.json'], + }) + }) + }) + + test('falls back to yarn run when the user agent is yarn', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDirectory = joinPath(tmpDir, 'subdir') + await mkdir(extensionDirectory) + vi.stubEnv('npm_config_user_agent', 'yarn/1.22.0') + + try { + await expect( + packageManagerBinaryCommandForDirectory( + extensionDirectory, + 'graphql-code-generator', + '--config', + 'package.json', + ), + ).resolves.toEqual({ + command: 'yarn', + args: ['run', 'graphql-code-generator', '--config', 'package.json'], + }) + } finally { + vi.unstubAllEnvs() + } + }) + }) + + test('falls back to npm when no package manager markers or user agent are available', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDirectory = joinPath(tmpDir, 'subdir') + await mkdir(extensionDirectory) + vi.stubEnv('npm_config_user_agent', '') + + try { + await expect( + packageManagerBinaryCommandForDirectory( + extensionDirectory, + 'graphql-code-generator', + '--config', + 'package.json', + ), + ).resolves.toEqual({ + command: 'npm', + args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], + }) + } finally { + vi.unstubAllEnvs() + } + }) + }) +}) + describe('addNPMDependencies', () => { test('when using npm with multiple dependencies they should be installed one by one, adding --save-exact if needed', 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 d59172bee66..873d7e88135 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.ts @@ -27,6 +27,7 @@ export const pnpmLockfile = 'pnpm-lock.yaml' /** The name of the bun lock file */ export const bunLockfile = 'bun.lockb' +const modernBunLockfile = 'bun.lock' /** The name of the pnpm workspace file */ export const pnpmWorkspaceFile = 'pnpm-workspace.yaml' @@ -56,6 +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 /** * Returns an abort error that's thrown when the package manager can't be determined. @@ -109,6 +111,58 @@ export function packageManagerFromUserAgent(env = process.env): PackageManager { return 'unknown' } +async function hasBunLockfile(directory: string): Promise { + const bunLockPath = joinPath(directory, bunLockfile) + if (await fileExists(bunLockPath)) return true + + const modernBunLockPath = joinPath(directory, modernBunLockfile) + return fileExists(modernBunLockPath) +} + +async function inferProjectPackageManagerAtDirectory(directory: string): Promise { + const yarnLockPath = joinPath(directory, yarnLockfile) + const pnpmLockPath = joinPath(directory, pnpmLockfile) + const pnpmWorkspacePath = joinPath(directory, pnpmWorkspaceFile) + const npmLockPath = joinPath(directory, npmLockfile) + + if (await fileExists(yarnLockPath)) return 'yarn' + if ((await fileExists(pnpmLockPath)) || (await fileExists(pnpmWorkspacePath))) return 'pnpm' + if (await hasBunLockfile(directory)) return 'bun' + if (await fileExists(npmLockPath)) return 'npm' + + return undefined +} + +async function getProjectPackageManagerFromDirectory( + fromDirectory: string, + foundPackageJson = false, +): Promise { + const detectedPackageManager = await inferProjectPackageManagerAtDirectory(fromDirectory) + if (detectedPackageManager) return detectedPackageManager + + const packageJsonPath = joinPath(fromDirectory, 'package.json') + const hasPackageJson = foundPackageJson || (await fileExists(packageJsonPath)) + + const parentDirectory = dirname(fromDirectory) + if (parentDirectory === fromDirectory) return hasPackageJson ? 'npm' : undefined + + return getProjectPackageManagerFromDirectory(parentDirectory, hasPackageJson) +} + +function projectPackageManagerFromUserAgentOrNpm(env = process.env): ProjectPackageManager { + const detectedPackageManager = packageManagerFromUserAgent(env) + switch (detectedPackageManager) { + case 'yarn': + case 'npm': + case 'pnpm': + case 'bun': + return detectedPackageManager + case 'homebrew': + case 'unknown': + return 'npm' + } +} + /** * Returns the dependency manager used in a directory. * @param fromDirectory - The starting directory @@ -129,20 +183,42 @@ export async function getPackageManager(fromDirectory: string): Promise { + outputDebug(outputContent`Obtaining the dependency manager in directory ${outputToken.path(fromDirectory)}...`) + const packageManager = + (await getProjectPackageManagerFromDirectory(fromDirectory)) ?? projectPackageManagerFromUserAgentOrNpm() + return packageManagerBinaryCommand(packageManager, binary, ...binaryArgs) +} + interface InstallNPMDependenciesRecursivelyOptions { /** * The dependency manager to use to install the dependencies.