From 8a45a557a4444e710070fc1249619339a0c37ca0 Mon Sep 17 00:00:00 2001 From: Alfonso Noriega Date: Thu, 19 Feb 2026 18:33:15 +0100 Subject: [PATCH] Add a build manifest build step --- .../models/extensions/extension-instance.ts | 7 +- .../build/build-steps.integration.test.ts | 4 +- .../app/src/cli/services/build/build-steps.ts | 6 +- .../build/steps/build-manifest-step.test.ts | 609 ++++++++++++++++++ .../build/steps/build-manifest-step.ts | 271 ++++++++ .../build/steps/copy-files-step.test.ts | 168 ++++- .../services/build/steps/copy-files-step.ts | 113 +++- .../app/src/cli/services/build/steps/index.ts | 4 + .../app/src/cli/services/build/steps/utils.ts | 45 ++ 9 files changed, 1187 insertions(+), 40 deletions(-) create mode 100644 packages/app/src/cli/services/build/steps/build-manifest-step.test.ts create mode 100644 packages/app/src/cli/services/build/steps/build-manifest-step.ts create mode 100644 packages/app/src/cli/services/build/steps/utils.ts diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 1bfa2f13b4..3e35331ea8 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -351,19 +351,14 @@ export class ExtensionInstance - - /** Custom data that steps can write to (extensible) */ - [key: string]: unknown } /** * Result of a step execution */ -interface StepResult { +export interface StepResult { readonly stepId: string readonly displayName: string readonly success: boolean diff --git a/packages/app/src/cli/services/build/steps/build-manifest-step.test.ts b/packages/app/src/cli/services/build/steps/build-manifest-step.test.ts new file mode 100644 index 0000000000..6d4c11f717 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/build-manifest-step.test.ts @@ -0,0 +1,609 @@ +import {executeBuildManifestStep, ResolvedAsset, ResolvedAssets, PerItemManifest} from './build-manifest-step.js' +import {BuildStep, BuildContext} from '../build-steps.js' +import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' +import * as fs from '@shopify/cli-kit/node/fs' + +vi.mock('@shopify/cli-kit/node/fs') + +// Helpers to narrow the union return type +function asSingle(result: Awaited>) { + return result as {outputFile: string; assets: ResolvedAssets} +} +function asForEach(result: Awaited>) { + return result as {outputFile: string; manifests: PerItemManifest[]} +} + +describe('executeBuildManifestStep', () => { + let mockExtension: ExtensionInstance + let mockContext: BuildContext + let mockStdout: {write: ReturnType} + + beforeEach(() => { + mockStdout = {write: vi.fn()} + mockExtension = { + directory: '/test/extension', + outputPath: '/test/output/extension.js', + configuration: {handle: 'my-ext'}, + } as unknown as ExtensionInstance + + mockContext = { + extension: mockExtension, + options: { + stdout: mockStdout as any, + stderr: {write: vi.fn()} as any, + app: {} as any, + environment: 'production', + }, + stepResults: new Map(), + } + + vi.mocked(fs.mkdir).mockResolvedValue() + vi.mocked(fs.writeFile).mockResolvedValue() + }) + + // ── Filepath generation ────────────────────────────────────────────────── + + describe('filepath generation — single mode', () => { + test('generates {handle}-{assetKey}{extension} for each asset', async () => { + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + assets: { + main: {moduleKey: 'module'}, + should_render: {moduleKey: 'should_render.module'}, + }, + }, + } + + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx', should_render: {module: './src/conditions.tsx'}}, + } as unknown as ExtensionInstance, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.main as ResolvedAsset).filepath).toBe('my-ext-main.js') + expect((result.assets.should_render as ResolvedAsset).filepath).toBe('my-ext-should_render.js') + }) + + test('uses custom extension when specified', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', tools: './src/tools.json'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + assets: { + tools: {moduleKey: 'tools', static: true}, + }, + }, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.tools as ResolvedAsset).filepath).toBe('my-ext-tools.json') + }) + + test('throws when handle is missing from extension config', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + await expect(executeBuildManifestStep(step, mockContext)).rejects.toThrow("'handle' field") + }) + }) + + // ── Module resolution ──────────────────────────────────────────────────── + + describe('module resolution', () => { + test("resolves module from moduleKey: 'module' pointing to the extension's module field", async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.main as ResolvedAsset).module).toBe('./src/index.tsx') + }) + + test('resolves module from a custom moduleKey', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', entry: './src/entry.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'entry'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.main as ResolvedAsset).module).toBe('./src/entry.tsx') + }) + + test('resolves nested module key (dot notation)', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', should_render: {module: './src/conditions.tsx'}}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {should_render: {moduleKey: 'should_render.module'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.should_render as ResolvedAsset).module).toBe('./src/conditions.tsx') + }) + + test('includes static flag when set', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', tools: './src/tools.json'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {tools: {moduleKey: 'tools', static: true, extension: '.json'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect((result.assets.tools as ResolvedAsset).static).toBe(true) + }) + }) + + // ── Optional assets ────────────────────────────────────────────────────── + + describe('optional assets', () => { + test('silently skips optional asset when moduleKey cannot be resolved', async () => { + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + assets: { + main: {moduleKey: 'module'}, + should_render: {moduleKey: 'should_render.module', optional: true}, + }, + }, + } + + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect(result.assets.should_render).toBeUndefined() + expect(mockStdout.write).not.toHaveBeenCalledWith(expect.stringContaining('should_render')) + }) + + test('logs warning and skips non-optional asset when moduleKey cannot be resolved', async () => { + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'missing_key'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect(result.assets).toEqual({}) + expect(mockStdout.write) + .toHaveBeenCalledWith(expect.stringContaining("Could not resolve module for asset 'main'")) + }) + }) + + // ── forEach — per-target iteration ─────────────────────────────────────── + + describe('forEach — per-target iteration', () => { + test('generates {handle}-{target}-{assetKey}.js filepath for each item', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + extension_points: [ + {target: 'purchase.checkout.block.render', module: './src/checkout.tsx'}, + {target: 'admin.product-details.action.render', module: './src/admin.tsx'}, + ], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: {main: {moduleKey: 'module'}}, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + + expect(result.manifests).toHaveLength(2) + expect(result.manifests[0]).toEqual({ + target: 'purchase.checkout.block.render', + build_manifest: { + assets: {main: {filepath: 'my-ext-purchase.checkout.block.render-main.js', module: './src/checkout.tsx'}}, + }, + }) + expect(result.manifests[1]).toEqual({ + target: 'admin.product-details.action.render', + build_manifest: { + assets: {main: {filepath: 'my-ext-admin.product-details.action.render-main.js', module: './src/admin.tsx'}}, + }, + }) + }) + + test('includes all asset types with correct filepaths and static flag', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + extension_points: [ + { + target: 'purchase.checkout.block.render', + module: './src/checkout.tsx', + should_render: {module: './src/conditions.tsx'}, + tools: './src/tools.json', + }, + ], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: { + main: {moduleKey: 'module'}, + should_render: {moduleKey: 'should_render.module', optional: true}, + tools: {moduleKey: 'tools', static: true, optional: true}, + }, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + const target = 'purchase.checkout.block.render' + + expect(result.manifests[0]!.build_manifest.assets).toEqual({ + main: {filepath: `my-ext-${target}-main.js`, module: './src/checkout.tsx'}, + should_render: {filepath: `my-ext-${target}-should_render.js`, module: './src/conditions.tsx'}, + tools: {filepath: `my-ext-${target}-tools.json`, module: './src/tools.json', static: true}, + }) + }) + + test('skips optional asset when its moduleKey cannot be resolved in the item', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + extension_points: [{target: 'checkout.render', module: './src/index.tsx'}], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: { + main: {moduleKey: 'module'}, + should_render: {moduleKey: 'should_render.module', optional: true}, + }, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + + expect(result.manifests[0]!.build_manifest.assets).toEqual({ + main: {filepath: 'my-ext-checkout.render-main.js', module: './src/index.tsx'}, + }) + }) + + test('expands asset into array when item module key resolves to an inner string array', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + extension_points: [ + { + target: 'checkout.render', + module: './src/checkout.tsx', + should_render: [{module: './src/conditions-a.tsx'}, {module: './src/conditions-b.tsx'}], + }, + ], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: { + main: {moduleKey: 'module'}, + should_render: {moduleKey: 'should_render.module', optional: true}, + }, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + + expect(result.manifests[0]!.build_manifest.assets).toEqual({ + main: {filepath: 'my-ext-checkout.render-main.js', module: './src/checkout.tsx'}, + should_render: [ + {filepath: 'my-ext-checkout.render-should_render-0.js', module: './src/conditions-a.tsx'}, + {filepath: 'my-ext-checkout.render-should_render-1.js', module: './src/conditions-b.tsx'}, + ], + }) + }) + + test('logs count and returns empty array when forEach tomlKey is not an array', async () => { + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: {main: {moduleKey: 'module'}}, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + + expect(result.manifests).toEqual([]) + expect(mockStdout.write).toHaveBeenCalledWith( + expect.stringContaining("No array found for forEach tomlKey 'extension_points'"), + ) + }) + + test('resolves module from item before falling back to top-level config', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + module: './src/fallback.tsx', + extension_points: [ + {target: 'a', module: './src/a.tsx'}, + // no module on item → falls back to config + {target: 'b'}, + ], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: {main: {moduleKey: 'module'}}, + }, + } + + const result = asForEach(await executeBuildManifestStep(step, mockContext)) + + expect((result.manifests[0]!.build_manifest.assets.main as ResolvedAsset).module).toBe('./src/a.tsx') + expect((result.manifests[1]!.build_manifest.assets.main as ResolvedAsset).module).toBe('./src/fallback.tsx') + }) + + test('logs count in stdout on success', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + handle: 'my-ext', + extension_points: [{target: 'a', module: './a.tsx'}], + }, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: { + forEach: {tomlKey: 'extension_points', keyBy: 'target'}, + assets: {main: {moduleKey: 'module'}}, + }, + } + + await executeBuildManifestStep(step, mockContext) + + expect(mockStdout.write).toHaveBeenCalledWith('Build manifest written to build-manifest.json (1 entries)\n') + }) + }) + + // ── Output file ────────────────────────────────────────────────────────── + + describe('output file', () => { + test('uses custom outputFile when specified', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {outputFile: 'manifest.json', assets: {main: {moduleKey: 'module'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect(result.outputFile).toBe('/test/output/manifest.json') + expect(fs.writeFile).toHaveBeenCalledWith('/test/output/manifest.json', expect.any(String)) + }) + + test('uses parent dir of outputPath when outputPath has a file extension', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect(result.outputFile).toBe('/test/output/build-manifest.json') + }) + + test('uses outputPath directly when it has no file extension', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + outputPath: '/test/bundle-dir', + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + const result = asSingle(await executeBuildManifestStep(step, mockContext)) + + expect(result.outputFile).toBe('/test/bundle-dir/build-manifest.json') + }) + + test('logs manifest write to stdout', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + await executeBuildManifestStep(step, mockContext) + + expect(mockStdout.write).toHaveBeenCalledWith('Build manifest written to build-manifest.json\n') + }) + + test('writes the correct JSON content to disk', async () => { + mockContext = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {handle: 'my-ext', module: './src/index.tsx'}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'write-manifest', + displayName: 'Write Manifest', + type: 'build_manifest', + config: {assets: {main: {moduleKey: 'module'}}}, + } + + await executeBuildManifestStep(step, mockContext) + + expect(fs.writeFile).toHaveBeenCalledWith( + '/test/output/build-manifest.json', + JSON.stringify({assets: {main: {filepath: 'my-ext-main.js', module: './src/index.tsx'}}}, null, 2), + ) + }) + }) +}) diff --git a/packages/app/src/cli/services/build/steps/build-manifest-step.ts b/packages/app/src/cli/services/build/steps/build-manifest-step.ts new file mode 100644 index 0000000000..b8d4db3a58 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/build-manifest-step.ts @@ -0,0 +1,271 @@ +import {getNestedValue} from './utils.js' +import {writeFile, mkdir} from '@shopify/cli-kit/node/fs' +import {joinPath, dirname, extname} from '@shopify/cli-kit/node/path' +import {z} from 'zod' +import type {BuildStep, BuildContext} from '../build-steps.js' + +// ── forEach ─────────────────────────────────────────────────────────────── + +/** + * Iterates over a config array and produces one manifest per item. + * The output is an array of `{ [keyBy]: value, build_manifest: { assets } }` objects, + * mirroring the shape that UIExtensionSchema.transform attaches to each extension_point. + */ +const ForEachSchema = z.object({ + // config key pointing to an array + tomlKey: z.string(), + // field in each item used to identify the manifest (e.g. 'target') + keyBy: z.string(), +}) + +// ── Asset entry ─────────────────────────────────────────────────────────── + +/** + * Configuration for a single asset in the manifest. + * + * The asset filepath is always derived from the extension handle, the current + * forEach keyBy value (when iterating), and the asset map key: + * `{handle}-{keyByValue}-{assetKey}{extension}` (forEach mode) + * `{handle}-{assetKey}{extension}` (single mode) + * + * For inner-array module resolution the index is inserted before the extension: + * `{handle}-{keyByValue}-{assetKey}-{index}{extension}` + */ +const AssetEntrySchema = z.object({ + /** + * The extension config key whose value is the source module path. + * Resolved from the current forEach item first, falling back to the + * top-level extension config (dot-notation supported, e.g. `'should_render.module'`). + */ + moduleKey: z.string(), + + /** + * When true, the asset is a static file to be copied rather than a bundle entry. + * Static assets use the source module's own file extension in the generated filepath. + * Non-static (bundled) assets always use `.js`. + */ + static: z.boolean().optional(), + + /** + * When true, the asset is silently omitted from the manifest if its module + * tomlKey cannot be resolved. Non-optional assets log a warning and are skipped. + */ + optional: z.boolean().optional(), +}) + +// ── Top-level config ────────────────────────────────────────────────────── + +const BuildManifestConfigSchema = z.object({ + /** Output filename relative to the extension output directory. Defaults to `'build-manifest.json'`. */ + outputFile: z.string().default('build-manifest.json'), + + /** When set, iterates over the named config array and produces one manifest per item. */ + forEach: ForEachSchema.optional(), + + /** Map of asset identifier → asset configuration. */ + assets: z.record(z.string(), AssetEntrySchema), +}) + +// ── Types ───────────────────────────────────────────────────────────────── + +export interface ResolvedAsset { + filepath: string + module?: string + static?: boolean +} +export interface ResolvedAssets { + [key: string]: ResolvedAsset | ResolvedAsset[] +} +export interface PerItemManifest { + [key: string]: unknown + build_manifest: {assets: ResolvedAssets} +} + +export type BuildManifestStepOutput = + | {outputFile: string; assets: ResolvedAssets} + | {outputFile: string; manifests: PerItemManifest[]} + +// ── Filepath generation ─────────────────────────────────────────────────── + +/** + * Generates a deterministic filepath for a manifest asset. + * + * Format: + * - Single mode: `{handle}-{assetKey}{extension}` + * - forEach mode: `{handle}-{keyByValue}-{assetKey}{extension}` + * - Inner array: `{handle}-{keyByValue}-{assetKey}-{index}{extension}` + * + * `keyByValue` is the resolved value of the `forEach.keyBy` field on the current + * iteration item (e.g. the value of `target` when `keyBy: 'target'`). + */ +function generateFilepath( + handle: string, + keyByValue: string | undefined, + assetKey: string, + extension: string, + innerIndex?: number, +): string { + const base = keyByValue !== undefined ? `${handle}-${keyByValue}-${assetKey}` : `${handle}-${assetKey}` + return innerIndex !== undefined ? `${base}-${innerIndex}${extension}` : `${base}${extension}` +} + +// ── Module resolution ───────────────────────────────────────────────────── + +/** + * Resolves the module path for an asset entry. + * + * Lookup order (forEach context): + * 1. `item[tomlKey]` — current iteration item + * 2. `config[tomlKey]` — top-level extension config + * + * In single mode `item` is `null` and only the config is consulted. + * + * Returns: + * - `string` — a single resolved module path + * - `string[]` — the resolved key was an inner array; the asset will be + * expanded into one entry per element by `resolveAssets` + * - `undefined` — could not resolve + */ +function resolveModule( + entry: z.infer, + config: {[key: string]: unknown}, + item: {[key: string]: unknown} | null, +): string | string[] | undefined { + const key = entry.moduleKey + + if (item !== null) { + const value = getNestedValue(item, key) + if (typeof value === 'string') return value + if (Array.isArray(value) && value.length > 0 && value.every((val) => typeof val === 'string')) return value + } + + const value = getNestedValue(config, key) + return typeof value === 'string' ? value : undefined +} + +// ── Asset resolution ────────────────────────────────────────────────────── + +/** + * Derives the output file extension for an asset. + * + * - Static assets preserve the source module's own extension (e.g. `tools.json` → `.json`). + * - Bundled assets always output `.js` regardless of the source extension (`.tsx` → `.js`). + */ +function deriveExtension(modulePath: string, isStatic: boolean | undefined): string { + if (isStatic) return extname(modulePath) || '.js' + return '.js' +} + +function resolveAssets( + assetsDef: {[key: string]: z.infer}, + config: {[key: string]: unknown}, + item: {[key: string]: unknown} | null, + keyByValue: string | undefined, + stdout: NodeJS.WritableStream, +): ResolvedAssets { + const handle = config.handle + if (typeof handle !== 'string') { + throw new Error("Extension config must have a 'handle' field to generate asset filepaths") + } + + const resolved: ResolvedAssets = {} + + for (const [assetKey, entry] of Object.entries(assetsDef)) { + const mod = resolveModule(entry, config, item) + + if (mod === undefined) { + if (!entry.optional) stdout.write(`Could not resolve module for asset '${assetKey}', skipping\n`) + continue + } + + if (Array.isArray(mod)) { + // Inner array: expand into one entry per element, inserting the index before the extension. + // Each element uses its own source extension when static, otherwise '.js'. + resolved[assetKey] = mod.map((innerMod, innerIndex) => ({ + filepath: generateFilepath(handle, keyByValue, assetKey, deriveExtension(innerMod, entry.static), innerIndex), + module: innerMod, + ...(entry.static ? {static: entry.static} : {}), + })) + continue + } + + resolved[assetKey] = { + filepath: generateFilepath(handle, keyByValue, assetKey, deriveExtension(mod, entry.static)), + module: mod, + ...(entry.static ? {static: entry.static} : {}), + } + } + + return resolved +} + +// ── Executor ────────────────────────────────────────────────────────────── + +/** + * Executes a build_manifest step. + * + * **Single mode** (no `forEach`): writes one manifest JSON with a flat `assets` map. + * Each asset filepath is `{handle}-{assetKey}{extension}`. + * + * **forEach mode**: iterates the named config array and writes an array of + * `{ [keyBy]: value, build_manifest: { assets } }` objects — one per item. + * Each asset filepath is `{handle}-{keyByValue}-{assetKey}{extension}` where + * `keyByValue` is the resolved value of the `forEach.keyBy` field on the item. + * This mirrors the shape that `UIExtensionSchema.transform` produces on each + * `extension_point`, making it straightforward to feed the result back into the + * in-memory `extension.configuration.extension_points[].build_manifest`. + * + * When a module tomlKey resolves to an inner string array the asset is expanded: + * one entry per element, with the index inserted before the extension: + * `{handle}-{keyByValue}-{assetKey}-{index}{extension}`. + * + * Assets marked `optional: true` are silently omitted when their module key + * cannot be resolved. + */ +export async function executeBuildManifestStep( + step: BuildStep, + context: BuildContext, +): Promise { + const config = BuildManifestConfigSchema.parse(step.config) + const {extension, options} = context + const outputDir = extname(extension.outputPath) ? dirname(extension.outputPath) : extension.outputPath + const outputFilePath = joinPath(outputDir, config.outputFile) + const extensionConfig = extension.configuration as {[key: string]: unknown} + + let result: BuildManifestStepOutput + + if (config.forEach) { + const array = getNestedValue(extensionConfig, config.forEach.tomlKey) + + if (Array.isArray(array)) { + const keyBy = config.forEach.keyBy + const manifests: PerItemManifest[] = array.flatMap((raw, _index) => { + if (typeof raw !== 'object' || raw === null) return [] + const item = raw as {[key: string]: unknown} + const keyByValue = String(getNestedValue(item, keyBy) ?? '') + const assets = resolveAssets(config.assets, extensionConfig, item, keyByValue, options.stdout) + return [{[keyBy]: getNestedValue(item, keyBy), build_manifest: {assets}} as PerItemManifest] + }) + + await writeManifest(outputFilePath, manifests) + options.stdout.write(`Build manifest written to ${config.outputFile} (${manifests.length} entries)\n`) + result = {outputFile: outputFilePath, manifests} + } else { + options.stdout.write(`No array found for forEach tomlKey '${config.forEach.tomlKey}'\n`) + await writeManifest(outputFilePath, []) + result = {outputFile: outputFilePath, manifests: []} + } + } else { + const assets = resolveAssets(config.assets, extensionConfig, null, undefined, options.stdout) + await writeManifest(outputFilePath, {assets}) + options.stdout.write(`Build manifest written to ${config.outputFile}\n`) + result = {outputFile: outputFilePath, assets} + } + + return result +} + +async function writeManifest(outputFilePath: string, content: unknown): Promise { + await mkdir(dirname(outputFilePath)) + await writeFile(outputFilePath, JSON.stringify(content, null, 2)) +} diff --git a/packages/app/src/cli/services/build/steps/copy-files-step.test.ts b/packages/app/src/cli/services/build/steps/copy-files-step.test.ts index b1ee446c45..d977f1cbfc 100644 --- a/packages/app/src/cli/services/build/steps/copy-files-step.test.ts +++ b/packages/app/src/cli/services/build/steps/copy-files-step.test.ts @@ -1,8 +1,9 @@ import {executeCopyFilesStep} from './copy-files-step.js' -import {BuildStep, BuildContext} from '../build-steps.js' +import {BuildStep, BuildContext, StepResult} from '../build-steps.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import * as fs from '@shopify/cli-kit/node/fs' +import type {BuildManifestStepOutput} from './build-manifest-step.js' vi.mock('@shopify/cli-kit/node/fs') @@ -527,4 +528,169 @@ describe('executeCopyFilesStep', () => { await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow('Build step "Copy Build" requires a source') }) }) + + describe('manifest_result strategy', () => { + function makeManifestResult(output: BuildManifestStepOutput): StepResult { + return {stepId: 'build-manifest', displayName: 'Build Manifest', success: true, duration: 0, output} + } + + const step: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static Assets', + type: 'copy_files', + config: {strategy: 'manifest_result', definition: {}}, + } + + test('copies static assets from a single-mode manifest', async () => { + // Given + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const output: BuildManifestStepOutput = { + outputFile: '/test/output/build-manifest.json', + assets: { + main: {filepath: 'handle.js', module: 'src/index.ts'}, + tools: {filepath: 'handle-tools-tools.js', module: 'src/tools.js', static: true}, + }, + } + mockContext.stepResults.set('build-manifest', makeManifestResult(output)) + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then — only the static asset is copied + expect(fs.copyFile).toHaveBeenCalledTimes(1) + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/tools.js', '/test/output/handle-tools-tools.js') + expect(result.filesCopied).toBe(1) + expect(mockStdout.write).toHaveBeenCalledWith('Copied 1 static asset(s) from build manifest\n') + }) + + test('copies static assets across all targets in a forEach-mode manifest', async () => { + // Given + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const output: BuildManifestStepOutput = { + outputFile: '/test/output/build-manifest.json', + manifests: [ + { + target: 'purchase.checkout.block.render', + build_manifest: { + assets: { + main: {filepath: 'handle-0.js', module: 'src/checkout.ts'}, + tools: {filepath: 'handle-0-tools-tools.js', module: 'src/tools.js', static: true}, + }, + }, + }, + { + target: 'purchase.checkout.cart-line-item.render', + build_manifest: { + assets: { + main: {filepath: 'handle-1.js', module: 'src/cart.ts'}, + }, + }, + }, + ], + } + mockContext.stepResults.set('build-manifest', makeManifestResult(output)) + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then — one static asset across both targets + expect(fs.copyFile).toHaveBeenCalledTimes(1) + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/tools.js', '/test/output/handle-0-tools-tools.js') + expect(result.filesCopied).toBe(1) + }) + + test('skips assets without a module field', async () => { + // Given + const output: BuildManifestStepOutput = { + outputFile: '/test/output/build-manifest.json', + assets: { + // no module — already in place + icon: {filepath: 'icon.png', static: true}, + }, + } + mockContext.stepResults.set('build-manifest', makeManifestResult(output)) + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then — nothing copied, no error + expect(fs.copyFile).not.toHaveBeenCalled() + // counted but skipped in copy + expect(result.filesCopied).toBe(1) + }) + + test('returns zero and logs when there are no static assets', async () => { + // Given + const output: BuildManifestStepOutput = { + outputFile: '/test/output/build-manifest.json', + assets: { + main: {filepath: 'handle.js', module: 'src/index.ts'}, + }, + } + mockContext.stepResults.set('build-manifest', makeManifestResult(output)) + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(0) + expect(fs.copyFile).not.toHaveBeenCalled() + expect(mockStdout.write).toHaveBeenCalledWith('No static assets found in build manifest\n') + }) + + test('supports a custom stepId via definition config', async () => { + // Given + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const customStep: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static Assets', + type: 'copy_files', + config: {strategy: 'manifest_result', definition: {stepId: 'custom-manifest-step'}}, + } + const output: BuildManifestStepOutput = { + outputFile: '/test/output/build-manifest.json', + assets: { + tools: {filepath: 'tools.js', module: 'src/tools.js', static: true}, + }, + } + mockContext.stepResults.set('custom-manifest-step', makeManifestResult(output)) + + // When + const result = await executeCopyFilesStep(customStep, mockContext) + + // Then + expect(result.filesCopied).toBe(1) + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/tools.js', '/test/output/tools.js') + }) + + test('throws when the referenced step is not in stepResults', async () => { + // Given — stepResults is empty + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow( + "Step 'build-manifest' not found in step results", + ) + }) + + test('throws when the referenced step failed', async () => { + // Given + const failedResult: StepResult = { + stepId: 'build-manifest', + displayName: 'Build Manifest', + success: false, + duration: 0, + error: new Error('manifest generation failed'), + } + mockContext.stepResults.set('build-manifest', failedResult) + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow("Step 'build-manifest' didn't succeed") + }) + }) }) diff --git a/packages/app/src/cli/services/build/steps/copy-files-step.ts b/packages/app/src/cli/services/build/steps/copy-files-step.ts index a81abec4f2..4c9a274394 100644 --- a/packages/app/src/cli/services/build/steps/copy-files-step.ts +++ b/packages/app/src/cli/services/build/steps/copy-files-step.ts @@ -1,6 +1,8 @@ +import {getNestedValue} from './utils.js' import {joinPath, dirname, extname, relativePath, basename} from '@shopify/cli-kit/node/path' import {glob, copyFile, copyDirectoryContents, fileExists, mkdir} from '@shopify/cli-kit/node/fs' import {z} from 'zod' +import type {BuildManifestStepOutput, ResolvedAsset, ResolvedAssets} from './build-manifest-step.js' import type {BuildStep, BuildContext} from '../build-steps.js' /** @@ -37,6 +39,22 @@ const PatternDefinitionSchema = z.object({ preserveStructure: z.boolean().default(true), }) +/** + * manifest_result strategy definition. + * + * Reads a `BuildManifestStepOutput` from a previous step's result in the build context + * and copies all assets flagged `static: true` to the output directory. + * Each static asset's `module` field is the source path (relative to the extension + * directory) and its `filepath` field is the destination (relative to the output dir). + */ +const ManifestResultDefinitionSchema = z.object({ + /** + * The `id` of the step whose manifest result to consume. + * Defaults to `'build-manifest'`. + */ + stepId: z.string().default('build-manifest'), +}) + /** * Configuration schema for copy_files step. * Discriminated by strategy; definition shape is tied to the chosen strategy. @@ -50,6 +68,10 @@ const CopyFilesConfigSchema = z.discriminatedUnion('strategy', [ strategy: z.literal('pattern'), definition: PatternDefinitionSchema, }), + z.object({ + strategy: z.literal('manifest_result'), + definition: ManifestResultDefinitionSchema, + }), ]) /** @@ -105,6 +127,10 @@ export async function executeCopyFilesStep(step: BuildStep, context: BuildContex options, ) } + + case 'manifest_result': { + return copyStaticAssetsFromManifest(config.definition.stepId, outputDir, context) + } } } @@ -261,37 +287,70 @@ async function copyByPattern( } /** - * Resolves a dot-separated path from a config object. - * Handles TOML array-of-tables by plucking the next key across all elements. + * manifest_result strategy — copies all static assets from a previous step's manifest result. + * + * Reads the named step's output from `context.stepResults`, collects every asset + * with `static: true`, then copies each one from its `module` path (relative to the + * extension directory) to its `filepath` path (relative to the output directory). + * + * Assets without a `module` field are skipped — they are considered already in place. + * Both single-mode (`{assets}`) and forEach-mode (`{manifests}`) outputs are supported. */ -function getNestedValue(obj: {[key: string]: unknown}, path: string): unknown { - const parts = path.split('.') - let current: unknown = obj +async function copyStaticAssetsFromManifest( + stepId: string, + outputDir: string, + context: BuildContext, +): Promise<{filesCopied: number}> { + const stepResult = context.stepResults.get(stepId) - for (const part of parts) { - if (current === null || current === undefined) { - return undefined - } + if (!stepResult) { + throw new Error(`Step '${stepId}' not found in step results. Ensure the build_manifest step runs before this step.`) + } - if (Array.isArray(current)) { - const plucked = current - .map((item) => { - if (typeof item === 'object' && item !== null && part in (item as object)) { - return (item as {[key: string]: unknown})[part] - } - return undefined - }) - .filter((item): item is NonNullable => item !== undefined) - current = plucked.length > 0 ? plucked : undefined - continue - } + if (!stepResult.success) { + throw new Error(`Step '${stepId}' didn't succeed — can't copy static assets from its manifest.`) + } - if (typeof current === 'object' && part in current) { - current = (current as {[key: string]: unknown})[part] - } else { - return undefined - } + const output = stepResult.output as BuildManifestStepOutput + const staticAssets = collectStaticAssets(output) + + if (staticAssets.length === 0) { + context.options.stdout.write('No static assets found in build manifest\n') + return {filesCopied: 0} } - return current + await Promise.all( + staticAssets.map(async (asset) => { + // Assets without a module path are not file-copy candidates + if (!asset.module) return + const sourcePath = joinPath(context.extension.directory, asset.module) + const destPath = joinPath(outputDir, asset.filepath) + await mkdir(dirname(destPath)) + await copyFile(sourcePath, destPath) + }), + ) + + context.options.stdout.write(`Copied ${staticAssets.length} static asset(s) from build manifest\n`) + return {filesCopied: staticAssets.length} +} + +/** + * Collects all assets with `static: true` from a BuildManifestStepOutput. + * Flattens both single-mode and forEach-mode output shapes. + */ +function collectStaticAssets(output: BuildManifestStepOutput): ResolvedAsset[] { + const allAssets: ResolvedAssets = + 'assets' in output + ? output.assets + : output.manifests.reduce((acc, manifest) => ({...acc, ...manifest.build_manifest.assets}), {}) + + const result: ResolvedAsset[] = [] + for (const value of Object.values(allAssets)) { + if (Array.isArray(value)) { + result.push(...value.filter((asset) => asset.static === true)) + } else if (value.static === true) { + result.push(value) + } + } + return result } diff --git a/packages/app/src/cli/services/build/steps/index.ts b/packages/app/src/cli/services/build/steps/index.ts index 43b681f8a9..f8b787a91f 100644 --- a/packages/app/src/cli/services/build/steps/index.ts +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -1,4 +1,5 @@ import {executeCopyFilesStep} from './copy-files-step.js' +import {executeBuildManifestStep} from './build-manifest-step.js' import {executeBuildThemeStep} from './build-theme-step.js' import {executeBundleThemeStep} from './bundle-theme-step.js' import {executeBundleUIStep} from './bundle-ui-step.js' @@ -21,6 +22,9 @@ export async function executeStepByType(step: BuildStep, context: BuildContext): case 'copy_files': return executeCopyFilesStep(step, context) + case 'build_manifest': + return executeBuildManifestStep(step, context) + case 'build_theme': return executeBuildThemeStep(step, context) diff --git a/packages/app/src/cli/services/build/steps/utils.ts b/packages/app/src/cli/services/build/steps/utils.ts new file mode 100644 index 0000000000..c886b4ae80 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/utils.ts @@ -0,0 +1,45 @@ +/** + * Resolves a dot-separated path from a config object. + * + * When `arrayIndex` is provided, any array encountered mid-path is indexed + * into using that value rather than plucking the key across all elements. + * When omitted, the original plucking behaviour is preserved (used by + * copy_files and other callers that need all values from an array field). + */ +export function getNestedValue(obj: {[key: string]: unknown}, path: string, arrayIndex?: number): unknown { + const parts = path.split('.') + let current: unknown = obj + + for (const part of parts) { + if (current === null || current === undefined) { + return undefined + } + + if (Array.isArray(current)) { + if (arrayIndex !== undefined) { + const item = current[arrayIndex] + if (item == null || typeof item !== 'object') return undefined + current = (item as {[key: string]: unknown})[part] + } else { + const plucked = current + .map((item) => { + if (typeof item === 'object' && item !== null && part in (item as object)) { + return (item as {[key: string]: unknown})[part] + } + return undefined + }) + .filter((item): item is NonNullable => item !== undefined) + current = plucked.length > 0 ? plucked : undefined + } + continue + } + + if (typeof current === 'object' && part in current) { + current = (current as {[key: string]: unknown})[part] + } else { + return undefined + } + } + + return current +}