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 8516249743..df1d2ff0a4 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, @@ -880,6 +881,26 @@ describe('getPackageManager', () => { }) }) + test('finds if bun is being used from bun.lock', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'bun.lock'), '') + + const packageManager = await getPackageManager(tmpDir) + expect(packageManager).toEqual('bun') + }) + }) + + test('finds if bun is being used from bun.lockb', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, 'bun.lockb'), '') + + const packageManager = await getPackageManager(tmpDir) + expect(packageManager).toEqual('bun') + }) + }) + test('finds pnpm from a nested workspace package when the lockfile is only at the repo root', async () => { await inTemporaryDirectory(async (tmpDir) => { await writePackageJSON(tmpDir, {name: 'root'}) @@ -893,6 +914,19 @@ describe('getPackageManager', () => { }) }) + test('finds pnpm from a nested workspace package when pnpm-workspace.yaml is only at the repo root', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'root'}) + await writeFile(joinPath(tmpDir, 'pnpm-workspace.yaml'), '') + const nested = joinPath(tmpDir, 'extensions', 'cart-transformer') + await mkdir(nested) + await writePackageJSON(nested, {name: 'cart-transformer'}) + + const packageManager = await getPackageManager(nested) + expect(packageManager).toEqual('pnpm') + }) + }) + test('falls back to packageManagerFromUserAgent when no package.json is found', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given — no package.json in tmpDir, stub user agent to yarn @@ -917,6 +951,134 @@ describe('getPackageManager', () => { }) }) +describe('packageManagerBinaryCommandForDirectory', () => { + test('uses npm exec with -- for npm', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await writePackageJSON(tmpDir, {name: 'mock name'}) + await writeFile(joinPath(tmpDir, npmLockfile), '') + + 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 b35afb8b6d..5e40ad9295 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,40 @@ export function packageManagerFromUserAgent(env = process.env): PackageManager { return 'unknown' } +function hasBunLockfileSync(directory: string): boolean { + return fileExistsSync(joinPath(directory, bunLockfile)) || fileExistsSync(joinPath(directory, modernBunLockfile)) +} + +function normalizePackageManagerForProject(packageManager: PackageManager): ProjectPackageManager { + switch (packageManager) { + case 'yarn': + case 'npm': + case 'pnpm': + case 'bun': + return packageManager + case 'homebrew': + case 'unknown': + return 'npm' + } +} + +function packageManagerBinaryCommand( + packageManager: ProjectPackageManager, + binary: string, + ...binaryArgs: string[] +): {command: string; args: string[]} { + switch (packageManager) { + case 'npm': + return {command: 'npm', args: ['exec', '--', binary, ...binaryArgs]} + case 'pnpm': + return {command: 'pnpm', args: ['exec', binary, ...binaryArgs]} + case 'yarn': + return {command: 'yarn', args: ['run', binary, ...binaryArgs]} + case 'bun': + return {command: 'bun', args: ['x', binary, ...binaryArgs]} + } +} + /** * Returns the dependency manager used in a directory. * Walks upward from `fromDirectory` so workspace packages (e.g. `extensions/my-fn/package.json`) @@ -123,8 +159,10 @@ export async function getPackageManager(fromDirectory: string): Promise { + const packageManager = normalizePackageManagerForProject(await getPackageManager(fromDirectory)) + return packageManagerBinaryCommand(packageManager, binary, ...binaryArgs) +} + interface InstallNPMDependenciesRecursivelyOptions { /** * The dependency manager to use to install the dependencies.