From 63d40f8dd0a48f3a2fc2f744a6d0728d588b0f61 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Thu, 9 Apr 2026 20:10:01 -0400 Subject: [PATCH] Require explicit package manager for project operations --- .../app/src/cli/models/app/app.test-data.ts | 16 ++++++++++--- .../project/project-integration.test.ts | 13 ++++++++++ .../app/src/cli/models/project/project.ts | 14 +++++++++++ .../app/src/cli/services/dependencies.test.ts | 12 ++++++++++ packages/app/src/cli/services/dependencies.ts | 2 +- .../cli/services/generate/extension.test.ts | 24 +++++++++++++++++++ .../src/cli/services/generate/extension.ts | 8 ++++--- 7 files changed, 82 insertions(+), 7 deletions(-) diff --git a/packages/app/src/cli/models/app/app.test-data.ts b/packages/app/src/cli/models/app/app.test-data.ts index 838f8b12d20..1615f2138f9 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -82,7 +82,8 @@ import {Project} from '../project/project.js' import {Session} from '@shopify/cli-kit/node/session' import {vi} from 'vitest' import {joinPath} from '@shopify/cli-kit/node/path' -import {PackageManager} from '@shopify/cli-kit/node/node-package-manager' +import {ProjectPackageManager} from '@shopify/cli-kit/node/node-package-manager' +import {AbortError} from '@shopify/cli-kit/node/error' export const DEFAULT_CONFIG = { application_url: 'https://myapp.com', @@ -154,7 +155,7 @@ export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinked interface TestProjectOptions { directory?: string - packageManager?: PackageManager + packageManager?: ProjectPackageManager | 'unknown' nodeDependencies?: Record usesWorkspaces?: boolean } @@ -164,9 +165,11 @@ interface TestProjectOptions { * Use this when a service needs a Project for packageManager, usesWorkspaces, or directory. */ export function testProject(options: TestProjectOptions = {}): Project { + const packageManager = options.packageManager ?? 'yarn' + return { directory: options.directory ?? '/tmp/project', - packageManager: options.packageManager ?? 'yarn', + packageManager, nodeDependencies: options.nodeDependencies ?? {}, usesWorkspaces: options.usesWorkspaces ?? false, appConfigFiles: [], @@ -177,6 +180,13 @@ export function testProject(options: TestProjectOptions = {}): Project { appConfigByName: () => undefined, appConfigByClientId: () => undefined, defaultAppConfig: undefined, + requirePackageManagerForOperations: () => { + if (packageManager === 'unknown') { + throw new AbortError('Could not determine the project package manager.') + } + + return packageManager + }, } as unknown as Project } diff --git a/packages/app/src/cli/models/project/project-integration.test.ts b/packages/app/src/cli/models/project/project-integration.test.ts index b503774b10f..6356779f57e 100644 --- a/packages/app/src/cli/models/project/project-integration.test.ts +++ b/packages/app/src/cli/models/project/project-integration.test.ts @@ -212,6 +212,19 @@ describe('Project integration', () => { }) }) + test('Project.requirePackageManagerForOperations errors when the app root has no package.json', async () => { + await inTemporaryDirectory(async (dir) => { + await setupRealApp(dir) + await removeFile(joinPath(dir, 'package.json')) + + const project = await Project.load(dir) + + expect(() => project.requirePackageManagerForOperations()).toThrow( + /Could not determine the project package manager/, + ) + }) + }) + test('multi-config project discovers all configs', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index 3b86572186a..a58f6c3fff6 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -157,6 +157,20 @@ export class Project { get defaultAppConfig(): TomlFile | undefined { return this.appConfigByName(configurationFileNames.app) } + + /** + * Returns the project package manager for install/mutation operations. + * Throws when the app root has no package.json and the package manager is therefore unknown. + */ + requirePackageManagerForOperations(): ProjectPackageManager { + if (this.packageManager === 'unknown') { + throw new AbortError( + `Could not determine the project package manager for ${this.directory}. Add a package.json to the app root before running dependency operations.`, + ) + } + + return this.packageManager + } } // ── Filesystem discovery functions ────────────────────────── diff --git a/packages/app/src/cli/services/dependencies.test.ts b/packages/app/src/cli/services/dependencies.test.ts index 363bbe98c25..1b893848e06 100644 --- a/packages/app/src/cli/services/dependencies.test.ts +++ b/packages/app/src/cli/services/dependencies.test.ts @@ -28,4 +28,16 @@ describe('installAppDependencies', () => { deep: 3, }) }) + + test('errors before install when the project package manager is unknown', async () => { + const project = testProject({packageManager: 'unknown', directory: '/tmp/project'}) + + await installAppDependencies(project) + + const tasks = vi.mocked(renderTasks).mock.calls[0]![0] as any + const task = tasks[0] + + await expect(task.task()).rejects.toThrow(/Could not determine the project package manager/) + expect(installNPMDependenciesRecursively).not.toHaveBeenCalled() + }) }) diff --git a/packages/app/src/cli/services/dependencies.ts b/packages/app/src/cli/services/dependencies.ts index 5ef3b16f0b4..c172db45aa8 100644 --- a/packages/app/src/cli/services/dependencies.ts +++ b/packages/app/src/cli/services/dependencies.ts @@ -14,7 +14,7 @@ export async function installAppDependencies(project: Project) { title: 'Installing dependencies', task: async () => { await installNPMDependenciesRecursively({ - packageManager: project.packageManager, + packageManager: project.requirePackageManagerForOperations(), directory: project.directory, deep: 3, }) diff --git a/packages/app/src/cli/services/generate/extension.test.ts b/packages/app/src/cli/services/generate/extension.test.ts index 8c97436e93d..5d739faeaa9 100644 --- a/packages/app/src/cli/services/generate/extension.test.ts +++ b/packages/app/src/cli/services/generate/extension.test.ts @@ -11,6 +11,7 @@ import * as functionBuild from '../function/build.js' import { checkoutUITemplate, testDeveloperPlatformClient, + testProject, testRemoteExtensionTemplates, } from '../../models/app/app.test-data.js' import {ExtensionTemplate} from '../../models/app/template.js' @@ -91,6 +92,29 @@ describe('initialize a extension', async () => { }) }) + test('errors before installing extension dependencies when the project package manager is unknown', async () => { + await withTemporaryApp(async (tmpDir) => { + const app = (await loadApp({ + directory: tmpDir, + specifications, + userProvidedConfigName: undefined, + })) as AppLinkedInterface + + const result = generateExtensionTemplate({ + extensionTemplate: checkoutUITemplate, + app, + project: testProject({directory: tmpDir, packageManager: 'unknown'}), + extensionChoices: {name: 'extension-name', flavor: 'vanilla-js'}, + developerPlatformClient: testDeveloperPlatformClient(), + onGetTemplateRepository, + }) + + await expect(result).rejects.toThrow(/Could not determine the project package manager/) + expect(vi.mocked(installNodeModules)).not.toHaveBeenCalled() + expect(vi.mocked(addNPMDependenciesIfNeeded)).not.toHaveBeenCalled() + }) + }) + test('successfully generates the extension when another extension exists', async () => { await withTemporaryApp(async (tmpDir) => { const name1 = 'my-ext-1' diff --git a/packages/app/src/cli/services/generate/extension.ts b/packages/app/src/cli/services/generate/extension.ts index ab50ff65835..eda842ff319 100644 --- a/packages/app/src/cli/services/generate/extension.ts +++ b/packages/app/src/cli/services/generate/extension.ts @@ -190,14 +190,16 @@ async function functionExtensionInit({ taskList.push({ title: 'Installing additional dependencies', task: async () => { + const packageManager = project.requirePackageManagerForOperations() + // We need to run install once to setup the workspace correctly if (project.usesWorkspaces) { - await installNodeModules({packageManager: project.packageManager, directory: project.directory}) + await installNodeModules({packageManager, directory: project.directory}) } const requiredDependencies = getFunctionRuntimeDependencies(templateLanguage) await addNPMDependenciesIfNeeded(requiredDependencies, { - packageManager: project.packageManager, + packageManager, type: 'prod', directory: project.usesWorkspaces ? directory : project.directory, }) @@ -258,7 +260,7 @@ async function uiExtensionInit({ { title: 'Installing dependencies', task: async () => { - const packageManager = project.packageManager + const packageManager = project.requirePackageManagerForOperations() if (project.usesWorkspaces) { // Only install dependencies if the extension is javascript if (getTemplateLanguage(extensionFlavor?.value) === 'javascript') {