Skip to content

Commit 68a6662

Browse files
committed
Require explicit package manager for project operations
1 parent 25e1f67 commit 68a6662

File tree

7 files changed

+82
-7
lines changed

7 files changed

+82
-7
lines changed

packages/app/src/cli/models/app/app.test-data.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ import {Project} from '../project/project.js'
8282
import {Session} from '@shopify/cli-kit/node/session'
8383
import {vi} from 'vitest'
8484
import {joinPath} from '@shopify/cli-kit/node/path'
85-
import {PackageManager} from '@shopify/cli-kit/node/node-package-manager'
85+
import {ProjectPackageManager} from '@shopify/cli-kit/node/node-package-manager'
86+
import {AbortError} from '@shopify/cli-kit/node/error'
8687

8788
export const DEFAULT_CONFIG = {
8889
application_url: 'https://myapp.com',
@@ -154,7 +155,7 @@ export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinked
154155

155156
interface TestProjectOptions {
156157
directory?: string
157-
packageManager?: PackageManager
158+
packageManager?: ProjectPackageManager | 'unknown'
158159
nodeDependencies?: Record<string, string>
159160
usesWorkspaces?: boolean
160161
}
@@ -164,9 +165,11 @@ interface TestProjectOptions {
164165
* Use this when a service needs a Project for packageManager, usesWorkspaces, or directory.
165166
*/
166167
export function testProject(options: TestProjectOptions = {}): Project {
168+
const packageManager = options.packageManager ?? 'yarn'
169+
167170
return {
168171
directory: options.directory ?? '/tmp/project',
169-
packageManager: options.packageManager ?? 'yarn',
172+
packageManager,
170173
nodeDependencies: options.nodeDependencies ?? {},
171174
usesWorkspaces: options.usesWorkspaces ?? false,
172175
appConfigFiles: [],
@@ -177,6 +180,13 @@ export function testProject(options: TestProjectOptions = {}): Project {
177180
appConfigByName: () => undefined,
178181
appConfigByClientId: () => undefined,
179182
defaultAppConfig: undefined,
183+
requirePackageManagerForOperations: () => {
184+
if (packageManager === 'unknown') {
185+
throw new AbortError('Could not determine the project package manager.')
186+
}
187+
188+
return packageManager
189+
},
180190
} as unknown as Project
181191
}
182192

packages/app/src/cli/models/project/project-integration.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,19 @@ describe('Project integration', () => {
212212
})
213213
})
214214

215+
test('Project.requirePackageManagerForOperations errors when the app root has no package.json', async () => {
216+
await inTemporaryDirectory(async (dir) => {
217+
await setupRealApp(dir)
218+
await removeFile(joinPath(dir, 'package.json'))
219+
220+
const project = await Project.load(dir)
221+
222+
expect(() => project.requirePackageManagerForOperations()).toThrow(
223+
/Could not determine the project package manager/,
224+
)
225+
})
226+
})
227+
215228
test('multi-config project discovers all configs', async () => {
216229
await inTemporaryDirectory(async (dir) => {
217230
await setupRealApp(dir)

packages/app/src/cli/models/project/project.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,20 @@ export class Project {
157157
get defaultAppConfig(): TomlFile | undefined {
158158
return this.appConfigByName(configurationFileNames.app)
159159
}
160+
161+
/**
162+
* Returns the project package manager for install/mutation operations.
163+
* Throws when the app root has no package.json and the package manager is therefore unknown.
164+
*/
165+
requirePackageManagerForOperations(): ProjectPackageManager {
166+
if (this.packageManager === 'unknown') {
167+
throw new AbortError(
168+
`Could not determine the project package manager for ${this.directory}. Add a package.json to the app root before running dependency operations.`,
169+
)
170+
}
171+
172+
return this.packageManager
173+
}
160174
}
161175

162176
// ── Filesystem discovery functions ──────────────────────────

packages/app/src/cli/services/dependencies.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,16 @@ describe('installAppDependencies', () => {
2828
deep: 3,
2929
})
3030
})
31+
32+
test('errors before install when the project package manager is unknown', async () => {
33+
const project = testProject({packageManager: 'unknown', directory: '/tmp/project'})
34+
35+
await installAppDependencies(project)
36+
37+
const tasks = vi.mocked(renderTasks).mock.calls[0]![0] as any
38+
const task = tasks[0]
39+
40+
await expect(task.task()).rejects.toThrow(/Could not determine the project package manager/)
41+
expect(installNPMDependenciesRecursively).not.toHaveBeenCalled()
42+
})
3143
})

packages/app/src/cli/services/dependencies.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export async function installAppDependencies(project: Project) {
1414
title: 'Installing dependencies',
1515
task: async () => {
1616
await installNPMDependenciesRecursively({
17-
packageManager: project.packageManager,
17+
packageManager: project.requirePackageManagerForOperations(),
1818
directory: project.directory,
1919
deep: 3,
2020
})

packages/app/src/cli/services/generate/extension.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as functionBuild from '../function/build.js'
1111
import {
1212
checkoutUITemplate,
1313
testDeveloperPlatformClient,
14+
testProject,
1415
testRemoteExtensionTemplates,
1516
} from '../../models/app/app.test-data.js'
1617
import {ExtensionTemplate} from '../../models/app/template.js'
@@ -91,6 +92,29 @@ describe('initialize a extension', async () => {
9192
})
9293
})
9394

95+
test('errors before installing extension dependencies when the project package manager is unknown', async () => {
96+
await withTemporaryApp(async (tmpDir) => {
97+
const app = (await loadApp({
98+
directory: tmpDir,
99+
specifications,
100+
userProvidedConfigName: undefined,
101+
})) as AppLinkedInterface
102+
103+
const result = generateExtensionTemplate({
104+
extensionTemplate: checkoutUITemplate,
105+
app,
106+
project: testProject({directory: tmpDir, packageManager: 'unknown'}),
107+
extensionChoices: {name: 'extension-name', flavor: 'vanilla-js'},
108+
developerPlatformClient: testDeveloperPlatformClient(),
109+
onGetTemplateRepository,
110+
})
111+
112+
await expect(result).rejects.toThrow(/Could not determine the project package manager/)
113+
expect(vi.mocked(installNodeModules)).not.toHaveBeenCalled()
114+
expect(vi.mocked(addNPMDependenciesIfNeeded)).not.toHaveBeenCalled()
115+
})
116+
})
117+
94118
test('successfully generates the extension when another extension exists', async () => {
95119
await withTemporaryApp(async (tmpDir) => {
96120
const name1 = 'my-ext-1'

packages/app/src/cli/services/generate/extension.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,14 +190,16 @@ async function functionExtensionInit({
190190
taskList.push({
191191
title: 'Installing additional dependencies',
192192
task: async () => {
193+
const packageManager = project.requirePackageManagerForOperations()
194+
193195
// We need to run install once to setup the workspace correctly
194196
if (project.usesWorkspaces) {
195-
await installNodeModules({packageManager: project.packageManager, directory: project.directory})
197+
await installNodeModules({packageManager, directory: project.directory})
196198
}
197199

198200
const requiredDependencies = getFunctionRuntimeDependencies(templateLanguage)
199201
await addNPMDependenciesIfNeeded(requiredDependencies, {
200-
packageManager: project.packageManager,
202+
packageManager,
201203
type: 'prod',
202204
directory: project.usesWorkspaces ? directory : project.directory,
203205
})
@@ -258,7 +260,7 @@ async function uiExtensionInit({
258260
{
259261
title: 'Installing dependencies',
260262
task: async () => {
261-
const packageManager = project.packageManager
263+
const packageManager = project.requirePackageManagerForOperations()
262264
if (project.usesWorkspaces) {
263265
// Only install dependencies if the extension is javascript
264266
if (getTemplateLanguage(extensionFlavor?.value) === 'javascript') {

0 commit comments

Comments
 (0)