From b903a96f96c41409afa125d6e1e3d394a16455f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 28 Apr 2026 11:23:19 +0200 Subject: [PATCH 1/3] Split bundle_ui manifest emission into a separate 'bundle' lifecycle phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `bundle_ui` step previously carried a `generatesAssetsManifest: boolean` config that fired for every command (`build`, `dev`, `deploy`) because the framework hardcoded a single `'deploy'` lifecycle. The flag is the smell: bundle_ui was doing two jobs and conditionally turning the second one off. This change splits the manifest emission into its own step (`generate_ui_assets_manifest`) and replaces the single `'deploy'` lifecycle with two additive phases: - `'build'` — local build (run by `shopify app build`). - `'bundle'` — extra steps run after `'build'` when bundling for upload (driven by `dev` and `deploy`). Specs only declare a `'bundle'` group when they need to do more than the local build. `extension.build()` composes them: `'build'` runs only the build steps; `'bundle'` runs `[...buildSteps, ...bundleSteps]` in declaration order. No spec needs to duplicate steps across both groups. For `ui_extension`: `'build'` runs `bundle_ui`; `'bundle'` adds `generate_ui_assets_manifest` and `include_assets`. Net result: `shopify app build` no longer emits `manifest.json` for `ui_extension`; `dev` and `deploy` are unchanged. The other 13 specs simply rename `lifecycle: 'deploy'` → `lifecycle: 'build'` — same behavior as before across all three commands. `include_assets` keeps its own `generatesAssetsManifest` flag (out of scope for this PR; same cleanup pattern can be applied later). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../extensions/extension-instance.test.ts | 24 +++- .../models/extensions/extension-instance.ts | 16 ++- .../specification.integration.test.ts | 2 +- .../models/extensions/specifications/admin.ts | 2 +- .../specifications/admin_link.test.ts | 2 +- .../extensions/specifications/admin_link.ts | 2 +- .../extensions/specifications/channel.ts | 2 +- .../specifications/checkout_post_purchase.ts | 2 +- .../specifications/checkout_ui_extension.ts | 2 +- .../specifications/flow_template.ts | 2 +- .../extensions/specifications/function.ts | 2 +- .../order_attribution_config.ts | 2 +- .../specifications/pos_ui_extension.ts | 2 +- .../specifications/product_subscription.ts | 2 +- .../specifications/tax_calculation.ts | 2 +- .../models/extensions/specifications/theme.ts | 2 +- .../extensions/specifications/ui_extension.ts | 15 ++- .../specifications/web_pixel_extension.ts | 2 +- packages/app/src/cli/services/build.ts | 2 +- .../src/cli/services/build/client-steps.ts | 32 +++++- .../build/steps/bundle-ui-step.test.ts | 2 +- .../services/build/steps/bundle-ui-step.ts | 48 +------- .../generate-ui-assets-manifest-step.test.ts | 107 ++++++++++++++++++ .../steps/generate-ui-assets-manifest-step.ts | 57 ++++++++++ .../app/src/cli/services/build/steps/index.ts | 4 + 25 files changed, 262 insertions(+), 75 deletions(-) create mode 100644 packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.test.ts create mode 100644 packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.ts diff --git a/packages/app/src/cli/models/extensions/extension-instance.test.ts b/packages/app/src/cli/models/extensions/extension-instance.test.ts index 3fea6b4a055..79fab7ac4aa 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -164,7 +164,29 @@ describe('build', async () => { const outputFilePath = joinPath(tmpDir, `dist/${extensionInstance.outputFileName}`) // When - await extensionInstance.build(options) + await extensionInstance.build(options, 'build') + + // Then + const outputFileContent = await readFile(outputFilePath) + expect(outputFileContent).toEqual('(()=>{})();') + }) + }) + + test("'bundle' lifecycle still runs the build steps even when no 'bundle' group is declared", async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given — tax_calculation only declares a 'build' lifecycle group. + const extensionInstance = await testTaxCalculationExtension(tmpDir) + const options: ExtensionBuildOptions = { + stdout: new Writable({write: (_chunk, _enc, cb) => cb()}), + stderr: new Writable({write: (_chunk, _enc, cb) => cb()}), + app: testApp(), + environment: 'production', + } + + const outputFilePath = joinPath(tmpDir, `dist/${extensionInstance.outputFileName}`) + + // When — request the 'bundle' lifecycle. Composition still runs build steps even without a bundle group. + await extensionInstance.build(options, 'bundle') // Then const outputFileContent = await readFile(outputFilePath) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index e266cb54c0c..7d70c85ed0a 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -8,7 +8,7 @@ import {Identifiers} from '../app/identifiers.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {AppConfiguration} from '../app/app.js' import {ApplicationURLs} from '../../services/dev/urls.js' -import {executeStep, BuildContext} from '../../services/build/client-steps.js' +import {executeStep, BuildContext, LifecyclePhase} from '../../services/build/client-steps.js' import {ok} from '@shopify/cli-kit/node/result' import {constantize, slugify} from '@shopify/cli-kit/common/string' import {hashString, nonRandomUUID} from '@shopify/cli-kit/node/crypto' @@ -114,7 +114,9 @@ export class ExtensionInstance group.lifecycle === 'deploy' && group.steps.length > 0) ?? false + this.specification.clientSteps?.some( + (group) => (group.lifecycle === 'build' || group.lifecycle === 'bundle') && group.steps.length > 0, + ) ?? false ) } @@ -315,7 +317,7 @@ export class ExtensionInstance { + async build(options: ExtensionBuildOptions, lifecycle: LifecyclePhase): Promise { const {clientSteps = []} = this.specification const context: BuildContext = { @@ -324,7 +326,11 @@ export class ExtensionInstance lifecycle.lifecycle === 'deploy')?.steps ?? [] + // Phases compose additively: 'bundle' runs build steps first, then bundle steps. + // Steps within each group preserve declaration order. + const buildSteps = clientSteps.find((group) => group.lifecycle === 'build')?.steps ?? [] + const bundleSteps = clientSteps.find((group) => group.lifecycle === 'bundle')?.steps ?? [] + const steps = lifecycle === 'build' ? buildSteps : [...buildSteps, ...bundleSteps] for (const step of steps) { // eslint-disable-next-line no-await-in-loop @@ -335,7 +341,7 @@ export class ExtensionInstance { const testClientSteps: ClientSteps = [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'bundle-ui', diff --git a/packages/app/src/cli/models/extensions/specifications/admin.ts b/packages/app/src/cli/models/extensions/specifications/admin.ts index 7aad744d238..be8eea3a8d5 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin.ts @@ -43,7 +43,7 @@ const adminSpecificationSpec = createExtensionSpecification({ }, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'hosted_app_copy_files', diff --git a/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts b/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts index 0030fb7b266..bde8fb079b4 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts @@ -52,7 +52,7 @@ describe('admin_link', async () => { const extension = await getTestAdminLink(tmpDir) const clientSteps = extension.specification.clientSteps! expect(clientSteps).toHaveLength(1) - expect(clientSteps[0]!.lifecycle).toBe('deploy') + expect(clientSteps[0]!.lifecycle).toBe('build') const steps = clientSteps[0]!.steps expect(steps).toHaveLength(1) diff --git a/packages/app/src/cli/models/extensions/specifications/admin_link.ts b/packages/app/src/cli/models/extensions/specifications/admin_link.ts index a375e12f556..2e1dff4b0e9 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin_link.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin_link.ts @@ -6,7 +6,7 @@ const adminLinkSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'include-admin-link-assets', diff --git a/packages/app/src/cli/models/extensions/specifications/channel.ts b/packages/app/src/cli/models/extensions/specifications/channel.ts index 3973fa91d26..071bb8dcaa8 100644 --- a/packages/app/src/cli/models/extensions/specifications/channel.ts +++ b/packages/app/src/cli/models/extensions/specifications/channel.ts @@ -9,7 +9,7 @@ const channelSpecificationSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'copy-files', diff --git a/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts b/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts index fad34726ed6..d8d3faaa475 100644 --- a/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts +++ b/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts @@ -20,7 +20,7 @@ const checkoutPostPurchaseSpec = createExtensionSpecification({ `dist/${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts index 239f164c9b2..992975b163d 100644 --- a/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts @@ -26,7 +26,7 @@ const checkoutSpec = createExtensionSpecification({ getOutputRelativePath: (extension: ExtensionInstance) => `dist/${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/flow_template.ts b/packages/app/src/cli/models/extensions/specifications/flow_template.ts index 805f953c98d..da870807174 100644 --- a/packages/app/src/cli/models/extensions/specifications/flow_template.ts +++ b/packages/app/src/cli/models/extensions/specifications/flow_template.ts @@ -51,7 +51,7 @@ const flowTemplateSpec = createExtensionSpecification({ appModuleFeatures: (_) => ['ui_preview'], clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'copy-files', diff --git a/packages/app/src/cli/models/extensions/specifications/function.ts b/packages/app/src/cli/models/extensions/specifications/function.ts index 2d73e819d01..7c96f15c7f2 100644 --- a/packages/app/src/cli/models/extensions/specifications/function.ts +++ b/packages/app/src/cli/models/extensions/specifications/function.ts @@ -103,7 +103,7 @@ const functionSpec = createExtensionSpecification({ }, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'build-function', name: 'Build Function', type: 'build_function', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts b/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts index d2af493e274..4ef76aca1fa 100644 --- a/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts +++ b/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts @@ -9,7 +9,7 @@ const orderAttributionConfigSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'copy-files', diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts index 3835d873ec7..668ed6cd85a 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts @@ -18,7 +18,7 @@ const posUISpec = createExtensionSpecification({ getOutputRelativePath: (extension: ExtensionInstance) => `dist/${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/product_subscription.ts b/packages/app/src/cli/models/extensions/specifications/product_subscription.ts index 77c95fde10f..14edab2f64f 100644 --- a/packages/app/src/cli/models/extensions/specifications/product_subscription.ts +++ b/packages/app/src/cli/models/extensions/specifications/product_subscription.ts @@ -19,7 +19,7 @@ const productSubscriptionSpec = createExtensionSpecification({ getOutputRelativePath: (extension: ExtensionInstance) => `dist/${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/tax_calculation.ts b/packages/app/src/cli/models/extensions/specifications/tax_calculation.ts index 1cc2caa3517..3b75d35b9b5 100644 --- a/packages/app/src/cli/models/extensions/specifications/tax_calculation.ts +++ b/packages/app/src/cli/models/extensions/specifications/tax_calculation.ts @@ -35,7 +35,7 @@ const spec = createExtensionSpecification({ joinPath('dist', `${extension.handle}.js`), clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'create-tax-stub', name: 'Create Tax Stub', type: 'create_tax_stub', config: {}}], }, ], diff --git a/packages/app/src/cli/models/extensions/specifications/theme.ts b/packages/app/src/cli/models/extensions/specifications/theme.ts index 8e6d27d27e4..2412d4a6eab 100644 --- a/packages/app/src/cli/models/extensions/specifications/theme.ts +++ b/packages/app/src/cli/models/extensions/specifications/theme.ts @@ -14,7 +14,7 @@ const themeSpec = createExtensionSpecification({ graphQLType: 'theme_app_extension', clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ {id: 'build-theme', name: 'Build Theme Extension', type: 'build_theme', config: {}}, {id: 'bundle-theme', name: 'Bundle Theme Extension', type: 'bundle_theme', config: {}}, diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index e2a33fda99c..f7feeb66e45 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -87,13 +87,24 @@ const uiExtensionSpec = createExtensionSpecification({ getOutputRelativePath: (extension: ExtensionInstance) => `${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [ { id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', - config: {generatesAssetsManifest: true, bundleFolder: 'dist/'}, + config: {bundleFolder: 'dist/'}, + }, + ], + }, + { + lifecycle: 'bundle', + steps: [ + { + id: 'generate-ui-assets-manifest', + name: 'Generate UI Assets Manifest', + type: 'generate_ui_assets_manifest', + config: {bundleFolder: 'dist/'}, }, { id: 'include-ui-extension-assets', diff --git a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts b/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts index 500db617485..53b5ea9309d 100644 --- a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts @@ -36,7 +36,7 @@ const webPixelSpec = createExtensionSpecification({ getOutputRelativePath: (extension: ExtensionInstance) => `dist/${extension.handle}.js`, clientSteps: [ { - lifecycle: 'deploy', + lifecycle: 'build', steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], diff --git a/packages/app/src/cli/services/build.ts b/packages/app/src/cli/services/build.ts index e1b0e597f7a..67f93dce514 100644 --- a/packages/app/src/cli/services/build.ts +++ b/packages/app/src/cli/services/build.ts @@ -42,7 +42,7 @@ async function build(options: BuildOptions) { return { prefix: ext.localIdentifier, action: async (stdout: Writable, stderr: Writable, signal: AbortSignal) => { - await ext.build({stdout, stderr, signal, app: options.app, environment: 'production'}) + await ext.build({stdout, stderr, signal, app: options.app, environment: 'production'}, 'build') }, } }), diff --git a/packages/app/src/cli/services/build/client-steps.ts b/packages/app/src/cli/services/build/client-steps.ts index a5092f0baec..a446637f58b 100644 --- a/packages/app/src/cli/services/build/client-steps.ts +++ b/packages/app/src/cli/services/build/client-steps.ts @@ -25,7 +25,14 @@ interface IncludeAssetsStep extends BaseStep { export interface BundleUIStep extends BaseStep { readonly type: 'bundle_ui' readonly config?: { - readonly generatesAssetsManifest?: boolean + readonly bundleFolder?: string + } +} + +/** Step with typed config specific to generate_ui_assets_manifest. */ +export interface GenerateUIAssetsManifestStep extends BaseStep { + readonly type: 'generate_ui_assets_manifest' + readonly config?: { readonly bundleFolder?: string } } @@ -43,14 +50,31 @@ interface NoConfigStep extends BaseStep { * This is a discriminated union on `type`: each step type carries its own * typed `config`, so TypeScript catches config typos at compile time. */ -export type LifecycleStep = IncludeAssetsStep | BundleUIStep | NoConfigStep +export type LifecycleStep = IncludeAssetsStep | BundleUIStep | GenerateUIAssetsManifestStep | NoConfigStep + +/** + * Lifecycle phases supported by the client-step pipeline. + * + * Phases compose additively. A spec lists only the steps that are *unique* to a + * phase; the runtime combines them per command: + * + * - `'build'` — local build (e.g. `shopify app build`). Steps required to + * produce the on-disk artifact. + * - `'bundle'` — extra steps needed when bundling for upload (driven by `dev` + * and `deploy`). Run *after* the build steps; never on their own. + * + * The `build` command runs `'build'` steps. The `dev`/`deploy` pipeline runs + * `'build'` then `'bundle'`. A spec only declares a `'bundle'` group when the + * bundle phase needs to do something the local build doesn't. + */ +export type LifecyclePhase = 'build' | 'bundle' /** * A group of steps scoped to a specific lifecycle phase. - * Allows executing only the steps relevant to a given lifecycle (e.g. 'deploy'). + * Allows executing only the steps relevant to a given lifecycle. */ interface ClientLifecycleGroup { - readonly lifecycle: 'deploy' + readonly lifecycle: LifecyclePhase readonly steps: ReadonlyArray } diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts index 5f73580949d..da95f3d2951 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts +++ b/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts @@ -32,7 +32,7 @@ describe('executeBundleUIStep', () => { id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', - config: {generatesAssetsManifest: false}, + config: {}, } test('copies when local and bundle output directories differ', async () => { diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts index dd65b01ce7c..312f227c2a0 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts +++ b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts @@ -1,25 +1,16 @@ -import {createOrUpdateManifestFile} from './include-assets/generate-manifest.js' import {buildUIExtension} from '../extension.js' -import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' import {copyFile} from '@shopify/cli-kit/node/fs' import {dirname, joinPath, resolvePath} from '@shopify/cli-kit/node/path' import type {BundleUIStep, BuildContext} from '../client-steps.js' -interface ExtensionPointWithBuildManifest { - target: string - build_manifest: BuildManifest -} - /** * Executes a bundle_ui build step. * * Bundles the UI extension using esbuild into the extension's local directory - * and copies the output to the bundle. When `generatesAssetsManifest` is true, - * writes built asset entries (from build_manifest) into manifest.json so - * downstream steps can merge on top. + * and copies the output to the bundle. Manifest emission lives in the separate + * `generate_ui_assets_manifest` step so it only runs in lifecycles that need it. */ export async function executeBundleUIStep(step: BundleUIStep, context: BuildContext): Promise { - const config = context.extension.configuration context.options.buildDirectory = step.config?.bundleFolder ?? undefined const localOutputPath = await buildUIExtension(context.extension, context.options) const localOutputDir = dirname(localOutputPath) @@ -29,39 +20,4 @@ export async function executeBundleUIStep(step: BundleUIStep, context: BuildCont if (resolvePath(localOutputDir) !== resolvePath(bundleOutputDir)) { await copyFile(localOutputDir, bundleOutputDir) } - - if (!step.config?.generatesAssetsManifest) return - - if (!Array.isArray(config.extension_points)) return - - const pointsWithManifest = config.extension_points.filter( - (ep): ep is ExtensionPointWithBuildManifest => typeof ep === 'object' && ep.build_manifest, - ) - - const entries = extractBuiltAssetEntries(pointsWithManifest, step.config?.bundleFolder) - if (Object.keys(entries).length > 0) { - await createOrUpdateManifestFile(context, entries) - } -} - -/** - * Extracts built asset filepaths from `build_manifest` on each extension point, - * grouped by target. Returns a map of target → `{assetName: filepath}`. - */ -function extractBuiltAssetEntries( - extensionPoints: {target: string; build_manifest: BuildManifest}[], - bundleFolder?: string, -): { - [target: string]: {[assetName: string]: string} -} { - const entries: {[target: string]: {[assetName: string]: string}} = {} - for (const {target, build_manifest: buildManifest} of extensionPoints) { - if (!buildManifest?.assets) continue - const assets: {[name: string]: string} = {} - for (const [name, asset] of Object.entries(buildManifest.assets)) { - if (asset?.filepath) assets[name] = bundleFolder ? joinPath(bundleFolder, asset.filepath) : asset.filepath - } - if (Object.keys(assets).length > 0) entries[target] = assets - } - return entries } diff --git a/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.test.ts b/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.test.ts new file mode 100644 index 00000000000..e0b47f09f03 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.test.ts @@ -0,0 +1,107 @@ +import {executeGenerateUIAssetsManifestStep} from './generate-ui-assets-manifest-step.js' +import * as generateManifest from './include-assets/generate-manifest.js' +import {GenerateUIAssetsManifestStep, BuildContext} from '../client-steps.js' +import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' + +vi.mock('./include-assets/generate-manifest.js') + +const step: GenerateUIAssetsManifestStep = { + id: 'generate-ui-assets-manifest', + name: 'Generate UI Assets Manifest', + type: 'generate_ui_assets_manifest', + config: {bundleFolder: 'dist/'}, +} + +describe('executeGenerateUIAssetsManifestStep', () => { + let mockContext: BuildContext + + beforeEach(() => { + vi.mocked(generateManifest.createOrUpdateManifestFile).mockResolvedValue() + mockContext = { + extension: { + directory: '/test/extension', + outputPath: '/test/extension/dist/handle.js', + configuration: {}, + } as ExtensionInstance, + options: { + stdout: {write: vi.fn()} as any, + stderr: {write: vi.fn()} as any, + app: {} as any, + environment: 'production', + }, + stepResults: new Map(), + } + }) + + test('writes manifest entries derived from build_manifest assets, prefixed with bundleFolder', async () => { + // Given + mockContext.extension.configuration = { + extension_points: [ + { + target: 'admin.product-details.block.render', + build_manifest: { + assets: { + main: {filepath: 'main.js'}, + styles: {filepath: 'main.css'}, + }, + }, + }, + ], + } as any + + // When + await executeGenerateUIAssetsManifestStep(step, mockContext) + + // Then + expect(generateManifest.createOrUpdateManifestFile).toHaveBeenCalledWith(mockContext, { + 'admin.product-details.block.render': { + main: 'dist/main.js', + styles: 'dist/main.css', + }, + }) + }) + + test('omits the bundleFolder prefix when not configured', async () => { + // Given + const stepWithoutFolder: GenerateUIAssetsManifestStep = { + id: 'generate-ui-assets-manifest', + name: 'Generate UI Assets Manifest', + type: 'generate_ui_assets_manifest', + } + mockContext.extension.configuration = { + extension_points: [ + { + target: 'admin.x', + build_manifest: {assets: {main: {filepath: 'main.js'}}}, + }, + ], + } as any + + // When + await executeGenerateUIAssetsManifestStep(stepWithoutFolder, mockContext) + + // Then + expect(generateManifest.createOrUpdateManifestFile).toHaveBeenCalledWith(mockContext, { + 'admin.x': {main: 'main.js'}, + }) + }) + + test('does nothing when extension has no extension_points', async () => { + mockContext.extension.configuration = {} as any + + await executeGenerateUIAssetsManifestStep(step, mockContext) + + expect(generateManifest.createOrUpdateManifestFile).not.toHaveBeenCalled() + }) + + test('does nothing when no extension_point has a build_manifest with assets', async () => { + mockContext.extension.configuration = { + extension_points: [{target: 'admin.x'}], + } as any + + await executeGenerateUIAssetsManifestStep(step, mockContext) + + expect(generateManifest.createOrUpdateManifestFile).not.toHaveBeenCalled() + }) +}) diff --git a/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.ts b/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.ts new file mode 100644 index 00000000000..427a889e9b0 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/generate-ui-assets-manifest-step.ts @@ -0,0 +1,57 @@ +import {createOrUpdateManifestFile} from './include-assets/generate-manifest.js' +import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import type {GenerateUIAssetsManifestStep, BuildContext} from '../client-steps.js' + +interface ExtensionPointWithBuildManifest { + target: string + build_manifest: BuildManifest +} + +/** + * Executes a generate_ui_assets_manifest build step. + * + * Reads `extension_points[].build_manifest.assets` from the extension + * configuration and writes the resulting target → asset map into manifest.json + * in the bundle output directory. Intended to run after a `bundle_ui` step in + * lifecycles that need a deployable manifest (deploy, dev) and to be omitted + * from the build-only lifecycle. + */ +export async function executeGenerateUIAssetsManifestStep( + step: GenerateUIAssetsManifestStep, + context: BuildContext, +): Promise { + const config = context.extension.configuration + if (!Array.isArray(config.extension_points)) return + + const pointsWithManifest = config.extension_points.filter( + (ep): ep is ExtensionPointWithBuildManifest => typeof ep === 'object' && ep.build_manifest, + ) + + const entries = extractBuiltAssetEntries(pointsWithManifest, step.config?.bundleFolder) + if (Object.keys(entries).length > 0) { + await createOrUpdateManifestFile(context, entries) + } +} + +/** + * Extracts built asset filepaths from `build_manifest` on each extension point, + * grouped by target. Returns a map of target → `{assetName: filepath}`. + */ +function extractBuiltAssetEntries( + extensionPoints: {target: string; build_manifest: BuildManifest}[], + bundleFolder?: string, +): { + [target: string]: {[assetName: string]: string} +} { + const entries: {[target: string]: {[assetName: string]: string}} = {} + for (const {target, build_manifest: buildManifest} of extensionPoints) { + if (!buildManifest?.assets) continue + const assets: {[name: string]: string} = {} + for (const [name, asset] of Object.entries(buildManifest.assets)) { + if (asset?.filepath) assets[name] = bundleFolder ? joinPath(bundleFolder, asset.filepath) : asset.filepath + } + if (Object.keys(assets).length > 0) entries[target] = assets + } + return entries +} diff --git a/packages/app/src/cli/services/build/steps/index.ts b/packages/app/src/cli/services/build/steps/index.ts index bf0a52f4f31..25241a2e7a1 100644 --- a/packages/app/src/cli/services/build/steps/index.ts +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -2,6 +2,7 @@ import {executeIncludeAssetsStep} from './include-assets-step.js' import {executeBuildThemeStep} from './build-theme-step.js' import {executeBundleThemeStep} from './bundle-theme-step.js' import {executeBundleUIStep} from './bundle-ui-step.js' +import {executeGenerateUIAssetsManifestStep} from './generate-ui-assets-manifest-step.js' import {executeBuildFunctionStep} from './build-function-step.js' import {executeCreateTaxStubStep} from './create-tax-stub-step.js' import type {LifecycleStep, BuildContext} from '../client-steps.js' @@ -29,6 +30,9 @@ export async function executeStepByType(step: LifecycleStep, context: BuildConte case 'bundle_ui': return executeBundleUIStep(step, context) + case 'generate_ui_assets_manifest': + return executeGenerateUIAssetsManifestStep(step, context) + case 'build_function': return executeBuildFunctionStep(step, context) From 881473dac1d9e3e34e50c057c11f7ce063abe58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 28 Apr 2026 11:30:15 +0200 Subject: [PATCH 2/3] Move include_assets-only specs to the 'bundle' lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit include_assets is conceptually a bundle-time step: it copies extension assets into the bundle output and (for some specs) emits manifest.json. It has no role in producing local build artifacts. For the 5 specs whose only client step is include_assets — admin, admin_link, channel, flow_template, order_attribution_config — move the group from 'build' to 'bundle'. After this, `shopify app build` is a no-op for these extension types; they run during dev/deploy as before. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/app/src/cli/models/extensions/specifications/admin.ts | 2 +- .../src/cli/models/extensions/specifications/admin_link.test.ts | 2 +- .../app/src/cli/models/extensions/specifications/admin_link.ts | 2 +- .../app/src/cli/models/extensions/specifications/channel.ts | 2 +- .../src/cli/models/extensions/specifications/flow_template.ts | 2 +- .../extensions/specifications/order_attribution_config.ts | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/admin.ts b/packages/app/src/cli/models/extensions/specifications/admin.ts index be8eea3a8d5..df8ae6042e8 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin.ts @@ -43,7 +43,7 @@ const adminSpecificationSpec = createExtensionSpecification({ }, clientSteps: [ { - lifecycle: 'build', + lifecycle: 'bundle', steps: [ { id: 'hosted_app_copy_files', diff --git a/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts b/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts index bde8fb079b4..b41872d7b5f 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin_link.test.ts @@ -52,7 +52,7 @@ describe('admin_link', async () => { const extension = await getTestAdminLink(tmpDir) const clientSteps = extension.specification.clientSteps! expect(clientSteps).toHaveLength(1) - expect(clientSteps[0]!.lifecycle).toBe('build') + expect(clientSteps[0]!.lifecycle).toBe('bundle') const steps = clientSteps[0]!.steps expect(steps).toHaveLength(1) diff --git a/packages/app/src/cli/models/extensions/specifications/admin_link.ts b/packages/app/src/cli/models/extensions/specifications/admin_link.ts index 2e1dff4b0e9..34fed0219ae 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin_link.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin_link.ts @@ -6,7 +6,7 @@ const adminLinkSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'build', + lifecycle: 'bundle', steps: [ { id: 'include-admin-link-assets', diff --git a/packages/app/src/cli/models/extensions/specifications/channel.ts b/packages/app/src/cli/models/extensions/specifications/channel.ts index 071bb8dcaa8..c38f24d478d 100644 --- a/packages/app/src/cli/models/extensions/specifications/channel.ts +++ b/packages/app/src/cli/models/extensions/specifications/channel.ts @@ -9,7 +9,7 @@ const channelSpecificationSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'build', + lifecycle: 'bundle', steps: [ { id: 'copy-files', diff --git a/packages/app/src/cli/models/extensions/specifications/flow_template.ts b/packages/app/src/cli/models/extensions/specifications/flow_template.ts index da870807174..7c19bf27f16 100644 --- a/packages/app/src/cli/models/extensions/specifications/flow_template.ts +++ b/packages/app/src/cli/models/extensions/specifications/flow_template.ts @@ -51,7 +51,7 @@ const flowTemplateSpec = createExtensionSpecification({ appModuleFeatures: (_) => ['ui_preview'], clientSteps: [ { - lifecycle: 'build', + lifecycle: 'bundle', steps: [ { id: 'copy-files', diff --git a/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts b/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts index 4ef76aca1fa..2141fa197e4 100644 --- a/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts +++ b/packages/app/src/cli/models/extensions/specifications/order_attribution_config.ts @@ -9,7 +9,7 @@ const orderAttributionConfigSpec = createContractBasedModuleSpecification({ experience: 'extension', clientSteps: [ { - lifecycle: 'build', + lifecycle: 'bundle', steps: [ { id: 'copy-files', From a8f8574a69fb50fb13bf14952af415793f6a2604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 28 Apr 2026 17:49:06 +0200 Subject: [PATCH 3/3] Replace build(options, lifecycle) with separate build() and bundle() methods The lifecycle parameter was a flag argument signaling "this method does two things." Splitting into two named methods makes each one do one thing, and the composition (build then bundle) becomes plain code at the one call site that needs it: buildForBundle. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../extensions/extension-instance.test.ts | 13 ++++--- .../models/extensions/extension-instance.ts | 36 ++++++++++--------- packages/app/src/cli/services/build.ts | 2 +- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/app/src/cli/models/extensions/extension-instance.test.ts b/packages/app/src/cli/models/extensions/extension-instance.test.ts index 79fab7ac4aa..9cebe167321 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -164,7 +164,7 @@ describe('build', async () => { const outputFilePath = joinPath(tmpDir, `dist/${extensionInstance.outputFileName}`) // When - await extensionInstance.build(options, 'build') + await extensionInstance.build(options) // Then const outputFileContent = await readFile(outputFilePath) @@ -172,7 +172,7 @@ describe('build', async () => { }) }) - test("'bundle' lifecycle still runs the build steps even when no 'bundle' group is declared", async () => { + test('bundle() is a no-op when the spec declares no bundle lifecycle group', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given — tax_calculation only declares a 'build' lifecycle group. const extensionInstance = await testTaxCalculationExtension(tmpDir) @@ -185,12 +185,11 @@ describe('build', async () => { const outputFilePath = joinPath(tmpDir, `dist/${extensionInstance.outputFileName}`) - // When — request the 'bundle' lifecycle. Composition still runs build steps even without a bundle group. - await extensionInstance.build(options, 'bundle') + // When — bundle() runs only the bundle steps; with none declared this is a no-op. + await extensionInstance.bundle(options) - // Then - const outputFileContent = await readFile(outputFilePath) - expect(outputFileContent).toEqual('(()=>{})();') + // Then — no output file is produced (build() was not called). + await expect(readFile(outputFilePath)).rejects.toThrow() }) }) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 7d70c85ed0a..af6cc3e113f 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -317,8 +317,26 @@ export class ExtensionInstance { - const {clientSteps = []} = this.specification + async build(options: ExtensionBuildOptions): Promise { + await this.runLifecyclePhase('build', options) + } + + async bundle(options: ExtensionBuildOptions): Promise { + await this.runLifecyclePhase('bundle', options) + } + + async buildForBundle(options: ExtensionBuildOptions, bundleDirectory: string, outputId?: string) { + this.outputPath = this.getOutputPathForDirectory(bundleDirectory, outputId) + await this.build(options) + await this.bundle(options) + + const bundleInputPath = joinPath(bundleDirectory, this.getOutputFolderId(outputId)) + await this.keepBuiltSourcemapsLocally(bundleInputPath) + } + + private async runLifecyclePhase(phase: LifecyclePhase, options: ExtensionBuildOptions): Promise { + const steps = this.specification.clientSteps?.find((group) => group.lifecycle === phase)?.steps ?? [] + if (steps.length === 0) return const context: BuildContext = { extension: this, @@ -326,12 +344,6 @@ export class ExtensionInstance group.lifecycle === 'build')?.steps ?? [] - const bundleSteps = clientSteps.find((group) => group.lifecycle === 'bundle')?.steps ?? [] - const steps = lifecycle === 'build' ? buildSteps : [...buildSteps, ...bundleSteps] - for (const step of steps) { // eslint-disable-next-line no-await-in-loop const result = await executeStep(step, context) @@ -339,14 +351,6 @@ export class ExtensionInstance { - await ext.build({stdout, stderr, signal, app: options.app, environment: 'production'}, 'build') + await ext.build({stdout, stderr, signal, app: options.app, environment: 'production'}) }, } }),