Skip to content

Commit 9bdcacc

Browse files
dmeranddmerand
committed
Use function directory package manager for GraphQL typegen
1 parent 9dfe97c commit 9bdcacc

File tree

4 files changed

+266
-3
lines changed

4 files changed

+266
-3
lines changed

‎packages/app/src/cli/services/function/build.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,20 @@ import {
2222
import {testApp, testFunctionExtension} from '../../models/app/app.test-data.js'
2323
import {beforeEach, describe, expect, test, vi} from 'vitest'
2424
import {exec} from '@shopify/cli-kit/node/system'
25+
import {packageManagerBinaryCommandForDirectory} from '@shopify/cli-kit/node/node-package-manager'
2526
import {dirname, joinPath} from '@shopify/cli-kit/node/path'
2627
import {inTemporaryDirectory, mkdir, readFileSync, writeFile, removeFile} from '@shopify/cli-kit/node/fs'
2728
import {build as esBuild} from 'esbuild'
2829

2930
vi.mock('@shopify/cli-kit/node/fs')
3031
vi.mock('@shopify/cli-kit/node/system')
32+
vi.mock('@shopify/cli-kit/node/node-package-manager', async () => {
33+
const actual: any = await vi.importActual('@shopify/cli-kit/node/node-package-manager')
34+
return {
35+
...actual,
36+
packageManagerBinaryCommandForDirectory: vi.fn(),
37+
}
38+
})
3139

3240
vi.mock('./binaries.js', async (importOriginal) => {
3341
const actual: any = await importOriginal()
@@ -76,6 +84,10 @@ beforeEach(async () => {
7684
stderr = {write: vi.fn()}
7785
stdout = {write: vi.fn()}
7886
signal = vi.fn()
87+
vi.mocked(packageManagerBinaryCommandForDirectory).mockResolvedValue({
88+
command: 'npm',
89+
args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'],
90+
})
7991
})
8092

8193
describe('buildGraphqlTypes', () => {
@@ -88,13 +100,40 @@ describe('buildGraphqlTypes', () => {
88100

89101
// Then
90102
await expect(got).resolves.toBeUndefined()
103+
expect(packageManagerBinaryCommandForDirectory).toHaveBeenCalledTimes(1)
104+
expect(packageManagerBinaryCommandForDirectory).toHaveBeenCalledWith(
105+
ourFunction.directory,
106+
'graphql-code-generator',
107+
'--config',
108+
'package.json',
109+
)
91110
expect(exec).toHaveBeenCalledWith('npm', ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], {
92111
cwd: ourFunction.directory,
93112
stderr,
94113
signal,
95114
})
96115
})
97116

117+
test('generate types executes the command returned by the shared helper', {timeout: 20000}, async () => {
118+
// Given
119+
const ourFunction = await testFunctionExtension({entryPath: 'src/index.js'})
120+
vi.mocked(packageManagerBinaryCommandForDirectory).mockResolvedValue({
121+
command: 'pnpm',
122+
args: ['exec', 'graphql-code-generator', '--config', 'package.json'],
123+
})
124+
125+
// When
126+
const got = buildGraphqlTypes(ourFunction, {stdout, stderr, signal, app})
127+
128+
// Then
129+
await expect(got).resolves.toBeUndefined()
130+
expect(exec).toHaveBeenCalledWith('pnpm', ['exec', 'graphql-code-generator', '--config', 'package.json'], {
131+
cwd: ourFunction.directory,
132+
stderr,
133+
signal,
134+
})
135+
})
136+
98137
test('errors if function is not a JS function and no typegen_command', async () => {
99138
// Given
100139
const ourFunction = await testFunctionExtension()
@@ -105,6 +144,7 @@ describe('buildGraphqlTypes', () => {
105144

106145
// Then
107146
await expect(got).rejects.toThrow(/No typegen_command specified/)
147+
expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled()
108148
})
109149

110150
test('runs custom typegen_command when provided', async () => {
@@ -129,6 +169,7 @@ describe('buildGraphqlTypes', () => {
129169

130170
// Then
131171
await expect(got).resolves.toBeUndefined()
172+
expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled()
132173
expect(exec).toHaveBeenCalledWith('npx', ['shopify-function-codegen', '--schema', 'schema.graphql'], {
133174
cwd: ourFunction.directory,
134175
stdout,
@@ -159,6 +200,7 @@ describe('buildGraphqlTypes', () => {
159200

160201
// Then
161202
await expect(got).resolves.toBeUndefined()
203+
expect(packageManagerBinaryCommandForDirectory).not.toHaveBeenCalled()
162204
expect(exec).toHaveBeenCalledWith('custom-typegen', ['--output', 'types.ts'], {
163205
cwd: ourFunction.directory,
164206
stdout,

‎packages/app/src/cli/services/function/build.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {renderTasks} from '@shopify/cli-kit/node/ui'
2424
import {pickBy} from '@shopify/cli-kit/common/object'
2525
import {runWithTimer} from '@shopify/cli-kit/node/metadata'
2626
import {AbortError} from '@shopify/cli-kit/node/error'
27+
import {packageManagerBinaryCommandForDirectory} from '@shopify/cli-kit/node/node-package-manager'
2728
import {Writable} from 'stream'
2829

2930
export const PREFERRED_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '2'
@@ -143,8 +144,15 @@ export async function buildGraphqlTypes(
143144
)
144145
}
145146

147+
const command = await packageManagerBinaryCommandForDirectory(
148+
fun.directory,
149+
'graphql-code-generator',
150+
'--config',
151+
'package.json',
152+
)
153+
146154
return runWithTimer('cmd_all_timing_network_ms')(async () => {
147-
return exec('npm', ['exec', '--', 'graphql-code-generator', '--config', 'package.json'], {
155+
return exec(command.command, command.args, {
148156
cwd: fun.directory,
149157
stderr: options.stderr,
150158
signal: options.signal,

‎packages/cli-kit/src/public/node/node-package-manager.test.ts‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
addResolutionOrOverride,
1313
writePackageJSON,
1414
getPackageManager,
15+
packageManagerBinaryCommandForDirectory,
1516
installNPMDependenciesRecursively,
1617
addNPMDependencies,
1718
DependencyVersion,
@@ -880,6 +881,26 @@ describe('getPackageManager', () => {
880881
})
881882
})
882883

884+
test('finds if bun is being used from bun.lock', async () => {
885+
await inTemporaryDirectory(async (tmpDir) => {
886+
await writePackageJSON(tmpDir, {name: 'mock name'})
887+
await writeFile(joinPath(tmpDir, 'bun.lock'), '')
888+
889+
const packageManager = await getPackageManager(tmpDir)
890+
expect(packageManager).toEqual('bun')
891+
})
892+
})
893+
894+
test('finds if bun is being used from bun.lockb', async () => {
895+
await inTemporaryDirectory(async (tmpDir) => {
896+
await writePackageJSON(tmpDir, {name: 'mock name'})
897+
await writeFile(joinPath(tmpDir, 'bun.lockb'), '')
898+
899+
const packageManager = await getPackageManager(tmpDir)
900+
expect(packageManager).toEqual('bun')
901+
})
902+
})
903+
883904
test('finds pnpm from a nested workspace package when the lockfile is only at the repo root', async () => {
884905
await inTemporaryDirectory(async (tmpDir) => {
885906
await writePackageJSON(tmpDir, {name: 'root'})
@@ -893,6 +914,19 @@ describe('getPackageManager', () => {
893914
})
894915
})
895916

917+
test('finds pnpm from a nested workspace package when pnpm-workspace.yaml is only at the repo root', async () => {
918+
await inTemporaryDirectory(async (tmpDir) => {
919+
await writePackageJSON(tmpDir, {name: 'root'})
920+
await writeFile(joinPath(tmpDir, 'pnpm-workspace.yaml'), '')
921+
const nested = joinPath(tmpDir, 'extensions', 'cart-transformer')
922+
await mkdir(nested)
923+
await writePackageJSON(nested, {name: 'cart-transformer'})
924+
925+
const packageManager = await getPackageManager(nested)
926+
expect(packageManager).toEqual('pnpm')
927+
})
928+
})
929+
896930
test('falls back to packageManagerFromUserAgent when no package.json is found', async () => {
897931
await inTemporaryDirectory(async (tmpDir) => {
898932
// Given — no package.json in tmpDir, stub user agent to yarn
@@ -917,6 +951,134 @@ describe('getPackageManager', () => {
917951
})
918952
})
919953

954+
describe('packageManagerBinaryCommandForDirectory', () => {
955+
test('uses npm exec with -- for npm', async () => {
956+
await inTemporaryDirectory(async (tmpDir) => {
957+
await writePackageJSON(tmpDir, {name: 'mock name'})
958+
await writeFile(joinPath(tmpDir, npmLockfile), '')
959+
960+
await expect(
961+
packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'),
962+
).resolves.toEqual({
963+
command: 'npm',
964+
args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'],
965+
})
966+
})
967+
})
968+
969+
test('uses exec without -- for pnpm when detected from an ancestor workspace marker', async () => {
970+
await inTemporaryDirectory(async (tmpDir) => {
971+
await writePackageJSON(tmpDir, {name: 'app-root'})
972+
await writeFile(joinPath(tmpDir, 'pnpm-workspace.yaml'), '')
973+
const extensionDirectory = joinPath(tmpDir, 'extensions', 'my-function')
974+
await mkdir(extensionDirectory)
975+
await writePackageJSON(extensionDirectory, {name: 'my-function'})
976+
977+
await expect(
978+
packageManagerBinaryCommandForDirectory(
979+
extensionDirectory,
980+
'graphql-code-generator',
981+
'--config',
982+
'package.json',
983+
),
984+
).resolves.toEqual({
985+
command: 'pnpm',
986+
args: ['exec', 'graphql-code-generator', '--config', 'package.json'],
987+
})
988+
})
989+
})
990+
991+
test('uses yarn run when detected from yarn.lock', async () => {
992+
await inTemporaryDirectory(async (tmpDir) => {
993+
await writePackageJSON(tmpDir, {name: 'mock name'})
994+
await writeFile(joinPath(tmpDir, 'yarn.lock'), '')
995+
996+
await expect(
997+
packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'),
998+
).resolves.toEqual({
999+
command: 'yarn',
1000+
args: ['run', 'graphql-code-generator', '--config', 'package.json'],
1001+
})
1002+
})
1003+
})
1004+
1005+
test('uses bun x for bun when detected from bun.lock', async () => {
1006+
await inTemporaryDirectory(async (tmpDir) => {
1007+
await writePackageJSON(tmpDir, {name: 'mock name'})
1008+
await writeFile(joinPath(tmpDir, 'bun.lock'), '')
1009+
1010+
await expect(
1011+
packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'),
1012+
).resolves.toEqual({
1013+
command: 'bun',
1014+
args: ['x', 'graphql-code-generator', '--config', 'package.json'],
1015+
})
1016+
})
1017+
})
1018+
1019+
test('uses bun x for bun when detected from bun.lockb', async () => {
1020+
await inTemporaryDirectory(async (tmpDir) => {
1021+
await writePackageJSON(tmpDir, {name: 'mock name'})
1022+
await writeFile(joinPath(tmpDir, 'bun.lockb'), '')
1023+
1024+
await expect(
1025+
packageManagerBinaryCommandForDirectory(tmpDir, 'graphql-code-generator', '--config', 'package.json'),
1026+
).resolves.toEqual({
1027+
command: 'bun',
1028+
args: ['x', 'graphql-code-generator', '--config', 'package.json'],
1029+
})
1030+
})
1031+
})
1032+
1033+
test('falls back to yarn run when the user agent is yarn', async () => {
1034+
await inTemporaryDirectory(async (tmpDir) => {
1035+
const extensionDirectory = joinPath(tmpDir, 'subdir')
1036+
await mkdir(extensionDirectory)
1037+
vi.stubEnv('npm_config_user_agent', 'yarn/1.22.0')
1038+
1039+
try {
1040+
await expect(
1041+
packageManagerBinaryCommandForDirectory(
1042+
extensionDirectory,
1043+
'graphql-code-generator',
1044+
'--config',
1045+
'package.json',
1046+
),
1047+
).resolves.toEqual({
1048+
command: 'yarn',
1049+
args: ['run', 'graphql-code-generator', '--config', 'package.json'],
1050+
})
1051+
} finally {
1052+
vi.unstubAllEnvs()
1053+
}
1054+
})
1055+
})
1056+
1057+
test('falls back to npm when no package manager markers or user agent are available', async () => {
1058+
await inTemporaryDirectory(async (tmpDir) => {
1059+
const extensionDirectory = joinPath(tmpDir, 'subdir')
1060+
await mkdir(extensionDirectory)
1061+
vi.stubEnv('npm_config_user_agent', '')
1062+
1063+
try {
1064+
await expect(
1065+
packageManagerBinaryCommandForDirectory(
1066+
extensionDirectory,
1067+
'graphql-code-generator',
1068+
'--config',
1069+
'package.json',
1070+
),
1071+
).resolves.toEqual({
1072+
command: 'npm',
1073+
args: ['exec', '--', 'graphql-code-generator', '--config', 'package.json'],
1074+
})
1075+
} finally {
1076+
vi.unstubAllEnvs()
1077+
}
1078+
})
1079+
})
1080+
})
1081+
9201082
describe('addNPMDependencies', () => {
9211083
test('when using npm with multiple dependencies they should be installed one by one, adding --save-exact if needed', async () => {
9221084
await inTemporaryDirectory(async (tmpDir) => {

0 commit comments

Comments
 (0)