Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions packages/app/src/cli/models/app/app.test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -154,7 +155,7 @@ export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinked

interface TestProjectOptions {
directory?: string
packageManager?: PackageManager
packageManager?: ProjectPackageManager | 'unknown'
nodeDependencies?: Record<string, string>
usesWorkspaces?: boolean
}
Expand All @@ -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: [],
Expand All @@ -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
}

Expand Down
13 changes: 13 additions & 0 deletions packages/app/src/cli/models/project/project-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions packages/app/src/cli/models/project/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──────────────────────────
Expand Down
12 changes: 12 additions & 0 deletions packages/app/src/cli/services/dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down
24 changes: 24 additions & 0 deletions packages/app/src/cli/services/generate/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
8 changes: 5 additions & 3 deletions packages/app/src/cli/services/generate/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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') {
Expand Down
Loading