Skip to content
Merged
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
12 changes: 8 additions & 4 deletions packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
return this.specification.identifier === 'editor_extension_collection'
}

get hasDeploySteps(): boolean {
return (
this.specification.clientSteps?.some((group) => group.lifecycle === 'deploy' && group.steps.length > 0) ?? false
)
}

get features(): ExtensionFeature[] {
return this.specification.appModuleFeatures(this.configuration)
}
Expand Down Expand Up @@ -340,16 +346,14 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi

this.outputPath = this.getOutputPathForDirectory(bundleDirectory, extensionUuid)

const buildMode = this.specification.buildConfig.mode

if (this.isThemeExtension) {
await bundleThemeExtension(this, options)
} else if (buildMode !== 'none') {
} else if (this.hasDeploySteps) {
outputDebug(`Will copy pre-built file from ${defaultOutputPath} to ${this.outputPath}`)
if (await fileExists(defaultOutputPath)) {
await copyFile(defaultOutputPath, this.outputPath)

if (buildMode === 'function') {
if (this.isFunctionExtension) {
await bundleFunctionExtension(this.outputPath, this.outputPath)
Comment thread
isaacroldan marked this conversation as resolved.
}
}
Expand Down
10 changes: 0 additions & 10 deletions packages/app/src/cli/models/extensions/specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ export interface BuildAsset {
static?: boolean
}

type BuildConfig =
| {mode: 'ui' | 'theme' | 'function' | 'tax_calculation' | 'none' | 'hosted_app_home'}
| {mode: 'copy_files'; filePatterns: string[]; ignoredFilePatterns?: string[]}

/**
* Extension specification with all the needed properties and methods to load an extension.
*/
Expand All @@ -75,7 +71,6 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
registrationLimit: number
experience: ExtensionExperience
clientSteps?: ClientSteps
buildConfig: BuildConfig
dependency?: string
graphQLType?: string
getOutputRelativePath?: (extension: ExtensionInstance<TConfiguration>) => string
Expand Down Expand Up @@ -220,7 +215,6 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
clientSteps: spec.clientSteps,
buildConfig: spec.buildConfig ?? {mode: 'none'},
}
const merged = {...defaults, ...spec}

Expand Down Expand Up @@ -269,7 +263,6 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
identifier: string
schema: ZodSchemaType<TConfiguration>
clientSteps?: ClientSteps
buildConfig?: BuildConfig
appModuleFeatures?: (config?: TConfiguration) => ExtensionFeature[]
transformConfig: TransformationConfig | CustomTransformationConfig
uidStrategy?: UidStrategy
Expand All @@ -288,7 +281,6 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
experience: 'configuration',
uidStrategy: spec.uidStrategy ?? 'single',
clientSteps: spec.clientSteps,
buildConfig: spec.buildConfig ?? {mode: 'none'},
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
patchWithAppDevURLs: spec.patchWithAppDevURLs,
})
Expand All @@ -299,7 +291,6 @@ export function createContractBasedModuleSpecification<TConfiguration extends Ba
CreateExtensionSpecType<TConfiguration>,
| 'identifier'
| 'appModuleFeatures'
| 'buildConfig'
| 'uidStrategy'
| 'clientSteps'
| 'experience'
Expand All @@ -312,7 +303,6 @@ export function createContractBasedModuleSpecification<TConfiguration extends Ba
schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
appModuleFeatures: spec.appModuleFeatures,
experience: spec.experience,
buildConfig: spec.buildConfig ?? {mode: 'none'},
clientSteps: spec.clientSteps,
uidStrategy: spec.uidStrategy,
transformRemoteToLocal: spec.transformRemoteToLocal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ const adminSpecificationSpec = createExtensionSpecification<AdminConfigType>({
},
}
},
buildConfig: {
mode: 'copy_files',
filePatterns: [],
},
clientSteps: [
{
lifecycle: 'deploy',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {createContractBasedModuleSpecification} from '../specification.js'
import {joinPath} from '@shopify/cli-kit/node/path'

const SUBDIRECTORY_NAME = 'specifications'
const FILE_EXTENSIONS = ['json', 'toml', 'yaml', 'yml', 'svg']
Expand All @@ -8,10 +7,6 @@ const channelSpecificationSpec = createContractBasedModuleSpecification({
identifier: 'channel_config',
uidStrategy: 'single',
experience: 'extension',
buildConfig: {
mode: 'copy_files',
filePatterns: FILE_EXTENSIONS.map((ext) => joinPath(SUBDIRECTORY_NAME, '**', `*.${ext}`)),
},
clientSteps: [
{
lifecycle: 'deploy',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const checkoutPostPurchaseSpec = createExtensionSpecification({
partnersWebIdentifier: 'post_purchase',
schema: CheckoutPostPurchaseSchema,
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<CheckoutPostPurchaseConfigType>) =>
`dist/${extension.handle}.js`,
clientSteps: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const checkoutSpec = createExtensionSpecification({
dependency,
schema: CheckoutSchema,
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path', 'generates_source_maps'],
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<CheckoutConfigType>) => `dist/${extension.handle}.js`,
clientSteps: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const flowTemplateSpec = createExtensionSpecification({
identifier: 'flow_template',
schema: FlowTemplateExtensionSchema,
appModuleFeatures: (_) => ['ui_preview'],
buildConfig: {mode: 'copy_files', filePatterns: ['**/*.flow', '**/*.json', '**/*.toml']},
clientSteps: [
{
lifecycle: 'deploy',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ const functionSpec = createExtensionSpecification({
],
schema: FunctionExtensionSchema,
appModuleFeatures: (_) => ['function'],
buildConfig: {mode: 'function'},
getOutputRelativePath: (_extension: ExtensionInstance<FunctionConfigType>) => joinPath('dist', 'index.wasm'),
devSessionWatchConfig: (extension: ExtensionInstance<FunctionConfigType>) => {
const config = extension.configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const posUISpec = createExtensionSpecification({
dependency,
schema: PosUISchema,
appModuleFeatures: (_) => ['ui_preview', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<PosUIConfigType>) => `dist/${extension.handle}.js`,
clientSteps: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const productSubscriptionSpec = createExtensionSpecification({
graphQLType: 'subscription_management',
schema: BaseSchema,
appModuleFeatures: (_) => ['ui_preview', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<ProductSubscriptionConfigType>) => `dist/${extension.handle}.js`,
clientSteps: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const spec = createExtensionSpecification({
identifier: 'tax_calculation',
schema: TaxCalculationsSchema,
appModuleFeatures: (_) => [],
buildConfig: {mode: 'tax_calculation'},
getOutputRelativePath: (extension: ExtensionInstance<TaxCalculationsConfigType>) =>
joinPath('dist', `${extension.handle}.js`),
clientSteps: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const themeSpec = createExtensionSpecification({
schema: BaseSchema,
partnersWebIdentifier: 'theme_app_extension',
graphQLType: 'theme_app_extension',
buildConfig: {mode: 'theme'},
clientSteps: [
{
lifecycle: 'deploy',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ const uiExtensionSpec = createExtensionSpecification({
identifier: 'ui_extension',
dependency,
schema: UIExtensionSchema,
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<UIExtensionConfigType>) => `dist/${extension.handle}.js`,
clientSteps: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const webPixelSpec = createExtensionSpecification({
partnersWebIdentifier: 'web_pixel',
schema: WebPixelSchema,
appModuleFeatures: (_) => ['esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputRelativePath: (extension: ExtensionInstance<WebPixelConfigType>) => `dist/${extension.handle}.js`,
clientSteps: [
{
Expand Down
6 changes: 6 additions & 0 deletions packages/app/src/cli/services/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ beforeEach(() => {

// Mock filterOutImportedExtensions to return the extensions by default
vi.mocked(filterOutImportedExtensions).mockImplementation((_app, extensions) => extensions)

// Mirror real bundleAndBuildExtensions behavior: return the bundlePath when
// at least one extension has deploy steps, otherwise undefined.
vi.mocked(bundleAndBuildExtensions).mockImplementation(async (opts) =>
opts.app.allExtensions.some((ext) => ext.hasDeploySteps) ? opts.bundlePath : undefined,
)
})

describe('deploy', () => {
Expand Down
17 changes: 8 additions & 9 deletions packages/app/src/cli/services/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,20 +213,19 @@ export async function deploy(options: DeployOptions) {
let uploadExtensionsBundleResult!: UploadExtensionsBundleOutput

try {
const bundle = app.allExtensions.some((ext) => ext.specification.buildConfig.mode !== 'none')
let bundlePath: string | undefined

if (bundle) {
bundlePath = joinPath(options.app.directory, '.shopify', `deploy-bundle.${developerPlatformClient.bundleFormat}`)
await mkdir(dirname(bundlePath))
}
const candidateBundlePath = joinPath(
options.app.directory,
'.shopify',
`deploy-bundle.${developerPlatformClient.bundleFormat}`,
)
await mkdir(dirname(candidateBundlePath))

const appManifest = await app.manifest(identifiers)

await bundleAndBuildExtensions({
const bundlePath = await bundleAndBuildExtensions({
app,
appManifest,
bundlePath,
bundlePath: candidateBundlePath,
identifiers,
skipBuild: options.skipBuild,
isDevDashboardApp: developerPlatformClient.supportsAtomicDeployments,
Expand Down
42 changes: 41 additions & 1 deletion packages/app/src/cli/services/deploy/bundle.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {bundleAndBuildExtensions} from './bundle.js'
import {testApp, testFunctionExtension, testThemeExtensions, testUIExtension} from '../../models/app/app.test-data.js'
import {
testApp,
testAppConfigExtensions,
testFunctionExtension,
testThemeExtensions,
testUIExtension,
} from '../../models/app/app.test-data.js'
import {AppInterface, AppManifest, WebType} from '../../models/app/app.js'
import * as bundle from '../bundle.js'
import * as functionBuild from '../function/build.js'
Expand Down Expand Up @@ -477,4 +483,38 @@ describe('bundleAndBuildExtensions', () => {
await expect(file.fileExists(bundlePath)).resolves.toBeTruthy()
})
})

test('returns undefined and skips compression when no extension has deploy steps', async () => {
await file.inTemporaryDirectory(async (tmpDir: string) => {
// Given
const bundlePath = joinPath(tmpDir, '.shopify', 'deploy-bundle.zip')
const compressSpy = vi.spyOn(bundle, 'compressBundle').mockResolvedValue()

const configExtension = await testAppConfigExtensions()
const app = testApp({allExtensions: [configExtension], directory: tmpDir})

const identifiers = {
app: 'app-id',
extensions: {},
extensionIds: {},
extensionsNonUuidManaged: {[configExtension.localIdentifier]: configExtension.localIdentifier},
}
appManifest = await app.manifest(identifiers)

// When
const result = await bundleAndBuildExtensions({
app,
appManifest,
identifiers,
bundlePath,
skipBuild: false,
isDevDashboardApp: false,
})

// Then
expect(result).toBeUndefined()
expect(compressSpy).not.toHaveBeenCalled()
await expect(file.fileExists(bundlePath)).resolves.toBe(false)
})
})
})
17 changes: 14 additions & 3 deletions packages/app/src/cli/services/deploy/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,19 @@ import {Writable} from 'stream'
interface BundleOptions {
app: AppInterface
appManifest: AppManifest
bundlePath?: string
bundlePath: string
identifiers?: Identifiers
skipBuild: boolean
isDevDashboardApp: boolean
}

export async function bundleAndBuildExtensions(options: BundleOptions) {
/**
* Builds all extensions into a bundle directory and compresses it when at
* least one extension declares deploy steps. Returns the bundlePath in that
* case, or undefined when no extension has deploy steps and there's nothing
* to upload beyond the manifest.
*/
export async function bundleAndBuildExtensions(options: BundleOptions): Promise<string | undefined> {
Comment on lines +21 to +27

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says this returns undefined when “only the manifest was present (nothing to upload)”, but the decision is currently based on ext.hasDeploySteps, not on whether any files beyond manifest.json were actually written into the bundle directory. This can lead to returning a bundle path (and uploading) even when deploy steps copied 0 files. Consider basing this on bundle directory contents (excluding manifest.json) or tracking whether any step produced output.

Copilot uses AI. Check for mistakes.
const bundleDirectory = joinPath(options.app.directory, '.shopify', 'deploy-bundle')
await rmdir(bundleDirectory, {force: true})
await mkdir(bundleDirectory)
Expand Down Expand Up @@ -73,7 +79,12 @@ export async function bundleAndBuildExtensions(options: BundleOptions) {
showTimestamps: false,
})

if (options.bundlePath) {
const hasExtensionOutput = options.app.allExtensions.some((ext) => ext.hasDeploySteps)

if (hasExtensionOutput) {
await compressBundle(bundleDirectory, options.bundlePath)
return options.bundlePath
}

return undefined
Comment thread
isaacroldan marked this conversation as resolved.
}
Loading