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 5d7eb1fb90e..a7c285dd3d5 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,7 @@ 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' export const DEFAULT_CONFIG = { application_url: 'https://myapp.com', @@ -154,7 +154,7 @@ export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinked interface TestProjectOptions { directory?: string - packageManager?: PackageManager + packageManager?: ProjectPackageManager | 'unknown' nodeDependencies?: Record usesWorkspaces?: boolean } 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 ef15d47636e..328df103ca4 100644 --- a/packages/app/src/cli/models/project/project-integration.test.ts +++ b/packages/app/src/cli/models/project/project-integration.test.ts @@ -1,5 +1,6 @@ import {Project} from './project.js' import {resolveDotEnv, resolveHiddenConfig, extensionFilesForConfig, webFilesForConfig} from './config-selection.js' +import {requireProjectPackageManagerForOperations} from '../../utilities/project-package-manager.js' import {loadApp, reloadApp} from '../app/loader.js' import {AppLinkedInterface} from '../app/app.js' import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js' @@ -212,6 +213,19 @@ describe('Project integration', () => { }) }) + test('requireProjectPackageManagerForOperations 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(() => requireProjectPackageManagerForOperations(project)).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/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..0d62365fdee 100644 --- a/packages/app/src/cli/services/dependencies.ts +++ b/packages/app/src/cli/services/dependencies.ts @@ -1,4 +1,5 @@ import {Project} from '../models/project/project.js' +import {requireProjectPackageManagerForOperations} from '../utilities/project-package-manager.js' import {installNPMDependenciesRecursively} from '@shopify/cli-kit/node/node-package-manager' import {renderTasks} from '@shopify/cli-kit/node/ui' @@ -14,7 +15,7 @@ export async function installAppDependencies(project: Project) { title: 'Installing dependencies', task: async () => { await installNPMDependenciesRecursively({ - packageManager: project.packageManager, + packageManager: requireProjectPackageManagerForOperations(project), 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..71800423fb3 100644 --- a/packages/app/src/cli/services/generate/extension.ts +++ b/packages/app/src/cli/services/generate/extension.ts @@ -5,6 +5,7 @@ import {buildGraphqlTypes, PREFERRED_FUNCTION_NPM_PACKAGE_MAJOR_VERSION} from '. import {GenerateExtensionContentOutput} from '../../prompts/generate/extension.js' import {ExtensionFlavor, ExtensionTemplate} from '../../models/app/template.js' import {ensureDownloadedExtensionFlavorExists, ensureExtensionDirectoryExists} from '../extensions/common.js' +import {requireProjectPackageManagerForOperations} from '../../utilities/project-package-manager.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {reloadApp} from '../../models/app/loader.js' import { @@ -190,14 +191,16 @@ async function functionExtensionInit({ taskList.push({ title: 'Installing additional dependencies', task: async () => { + const packageManager = requireProjectPackageManagerForOperations(project) + // 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 +261,7 @@ async function uiExtensionInit({ { title: 'Installing dependencies', task: async () => { - const packageManager = project.packageManager + const packageManager = requireProjectPackageManagerForOperations(project) if (project.usesWorkspaces) { // Only install dependencies if the extension is javascript if (getTemplateLanguage(extensionFlavor?.value) === 'javascript') { diff --git a/packages/app/src/cli/utilities/project-package-manager.test.ts b/packages/app/src/cli/utilities/project-package-manager.test.ts new file mode 100644 index 00000000000..2ecca3409d3 --- /dev/null +++ b/packages/app/src/cli/utilities/project-package-manager.test.ts @@ -0,0 +1,22 @@ +import {requireProjectPackageManagerForOperations} from './project-package-manager.js' +import {describe, expect, test} from 'vitest' + +describe('requireProjectPackageManagerForOperations', () => { + test.each(['npm', 'pnpm', 'yarn', 'bun'])('returns %s for supported project package managers', (packageManager) => { + const result = requireProjectPackageManagerForOperations({ + packageManager, + directory: '/tmp/project', + } as any) + + expect(result).toBe(packageManager) + }) + + test('throws when the project package manager is unknown', () => { + expect(() => + requireProjectPackageManagerForOperations({ + packageManager: 'unknown', + directory: '/tmp/project', + } as any), + ).toThrow(/Could not determine the project package manager for \/tmp\/project/) + }) +}) diff --git a/packages/app/src/cli/utilities/project-package-manager.ts b/packages/app/src/cli/utilities/project-package-manager.ts new file mode 100644 index 00000000000..1070ecae122 --- /dev/null +++ b/packages/app/src/cli/utilities/project-package-manager.ts @@ -0,0 +1,18 @@ +import {Project} from '../models/project/project.js' +import {ProjectPackageManager} from '@shopify/cli-kit/node/node-package-manager' +import {AbortError} from '@shopify/cli-kit/node/error' + +/** + * Narrows project package-manager metadata for install/mutation operations. + */ +export function requireProjectPackageManagerForOperations( + project: Pick, +): ProjectPackageManager { + if (project.packageManager === 'unknown') { + throw new AbortError( + `Could not determine the project package manager for ${project.directory}. Add a package.json to the app root before running dependency operations.`, + ) + } + + return project.packageManager +}