diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index addea2de09..9d006530ef 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 7e8caf4f6f..547401eb03 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 cbd5b629c9..f49bf2f6fd 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 d59172bee6..873d7e8813 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.