diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 932033f3154..aca98248beb 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -22,6 +22,7 @@ import { bundleFunctionExtension, } from '../../services/build/extension.js' import {bundleThemeExtension, copyFilesForExtension} from '../../services/extensions/bundle.js' +import {executeBuildSteps} from '../../services/build/build-steps.js' import {Identifiers} from '../app/identifiers.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {AppConfigurationWithoutPath} from '../app/app.js' @@ -370,9 +371,8 @@ export class ExtensionInstance { describe('transform', () => { @@ -54,43 +52,62 @@ describe('hosted_app_home', () => { }) }) - describe('copyStaticAssets', () => { - test('should copy static assets from source to output directory', async () => { - vi.mocked(copyDirectoryContents).mockResolvedValue(undefined) - const config = {static_root: 'public'} - const directory = '/app/root' - const outputPath = '/output/dist/bundle.js' - - await spec.copyStaticAssets!(config, directory, outputPath) - - expect(copyDirectoryContents).toHaveBeenCalledWith('/app/root/public', '/output/dist') + describe('buildConfig', () => { + test('should use build_steps mode', () => { + expect(spec.buildConfig.mode).toBe('build_steps') }) - test('should not copy assets when static_root is not provided', async () => { - const config = {} - const directory = '/app/root' - const outputPath = '/output/dist/bundle.js' + test('should have static stepsConfig (not a function)', () => { + if (spec.buildConfig.mode !== 'build_steps') { + throw new Error('Expected build_steps mode') + } + expect(typeof spec.buildConfig.stepsConfig).toBe('object') + expect(spec.buildConfig.stepsConfig).not.toBeNull() + }) - await spec.copyStaticAssets!(config, directory, outputPath) + test('should have copy-static-assets step with configPath reference', () => { + if (spec.buildConfig.mode !== 'build_steps') { + throw new Error('Expected build_steps mode') + } - expect(copyDirectoryContents).not.toHaveBeenCalled() + const stepsConfig = spec.buildConfig.stepsConfig + + // Verify the build steps config + expect(stepsConfig.steps).toHaveLength(1) + expect(stepsConfig.steps[0]).toMatchObject({ + id: 'copy-static-assets', + displayName: 'Copy Static Assets', + type: 'copy_files', + config: { + strategy: 'directory', + source: {configPath: 'static_root', optional: true}, // ← Uses configPath reference with optional flag + }, + }) + expect(stepsConfig.stopOnError).toBe(true) }) - test('should throw error when copy fails', async () => { - vi.mocked(copyDirectoryContents).mockRejectedValue(new Error('Permission denied')) - const config = {static_root: 'public'} - const directory = '/app/root' - const outputPath = '/output/dist/bundle.js' + test('should not have skip callback (must be serializable)', () => { + if (spec.buildConfig.mode !== 'build_steps') { + throw new Error('Expected build_steps mode') + } - await expect(spec.copyStaticAssets!(config, directory, outputPath)).rejects.toThrow( - 'Failed to copy static assets from /app/root/public to /output/dist: Permission denied', - ) + const step = spec.buildConfig.stepsConfig.steps[0] + // No skip function - config must be pure data (serializable) + expect(step?.skip).toBeUndefined() }) - }) - describe('buildConfig', () => { - test('should have hosted_app_home build mode', () => { - expect(spec.buildConfig).toEqual({mode: 'hosted_app_home'}) + test('config should be serializable to JSON', () => { + if (spec.buildConfig.mode !== 'build_steps') { + throw new Error('Expected build_steps mode') + } + + // Should be able to serialize and deserialize without errors + const serialized = JSON.stringify(spec.buildConfig.stepsConfig) + expect(serialized).toBeDefined() + + const deserialized = JSON.parse(serialized) + expect(deserialized.steps).toHaveLength(1) + expect(deserialized.steps[0].config.source).toEqual({configPath: 'static_root', optional: true}) }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts b/packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts index 6b71b710496..00bfe9c14d1 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts @@ -1,7 +1,6 @@ import {BaseSchemaWithoutHandle} from '../schemas.js' import {TransformationConfig, createConfigExtensionSpecification} from '../specification.js' -import {copyDirectoryContents} from '@shopify/cli-kit/node/fs' -import {dirname, joinPath} from '@shopify/cli-kit/node/path' +import {BuildStepsConfig} from '../../../services/build/build-steps.js' import {zod} from '@shopify/cli-kit/node/schema' const HostedAppHomeSchema = BaseSchemaWithoutHandle.extend({ @@ -14,20 +13,39 @@ const HostedAppHomeTransformConfig: TransformationConfig = { export const HostedAppHomeSpecIdentifier = 'hosted_app_home' +/** + * Static build steps configuration for hosted_app_home. + * Uses ConfigurableValue to reference the 'static_root' field from the TOML config. + * This configuration is pure data (no functions) and can be serialized to/from JSON. + * + * When static_root is not configured in TOML, the configPath will resolve to undefined, + * and the copy-files-step will handle it gracefully (skip with optional: true). + */ +const hostedAppHomeBuildSteps: BuildStepsConfig = { + steps: [ + { + id: 'copy-static-assets', + displayName: 'Copy Static Assets', + type: 'copy_files', + config: { + strategy: 'directory', + // Reference to TOML config field - resolves to undefined if not configured + // optional: true means skip silently if the field doesn't exist in TOML + source: {configPath: 'static_root', optional: true}, + }, + }, + ], + stopOnError: true, +} + const hostedAppHomeSpec = createConfigExtensionSpecification({ identifier: HostedAppHomeSpecIdentifier, - buildConfig: {mode: 'hosted_app_home'} as const, + buildConfig: { + mode: 'build_steps', + stepsConfig: hostedAppHomeBuildSteps, + }, schema: HostedAppHomeSchema, transformConfig: HostedAppHomeTransformConfig, - copyStaticAssets: async (config, directory, outputPath) => { - if (!config.static_root) return - const sourceDir = joinPath(directory, config.static_root) - const outputDir = dirname(outputPath) - - return copyDirectoryContents(sourceDir, outputDir).catch((error) => { - throw new Error(`Failed to copy static assets from ${sourceDir} to ${outputDir}: ${error.message}`) - }) - }, }) export default hostedAppHomeSpec diff --git a/packages/app/src/cli/services/build/build-steps.integration.test.ts b/packages/app/src/cli/services/build/build-steps.integration.test.ts new file mode 100644 index 00000000000..0159ff194bb --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.integration.test.ts @@ -0,0 +1,260 @@ +import {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {ExtensionBuildOptions} from './extension.js' +import {executeBuildSteps, BuildStepsConfig} from './build-steps.js' +import {describe, expect, test} from 'vitest' +import {inTemporaryDirectory, writeFile, readFile, mkdir, fileExists} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {Writable} from 'stream' + +describe('build_steps integration', () => { + test('executes copy_files step and copies files to output', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Setup: Create extension directory with assets + const extensionDir = joinPath(tmpDir, 'extension') + const assetsDir = joinPath(extensionDir, 'assets') + const outputDir = joinPath(tmpDir, 'output') + + await mkdir(extensionDir) + await mkdir(assetsDir) + await mkdir(outputDir) + + // Create test files + await writeFile(joinPath(assetsDir, 'logo.png'), 'fake-png-data') + await writeFile(joinPath(assetsDir, 'style.css'), 'body { color: red; }') + + // Create mock extension + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + } as ExtensionInstance + + // Create build steps config + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'copy-assets', + displayName: 'Copy Assets', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'assets', + patterns: ['**/*'], + }, + }, + ], + } + + const buildOptions: ExtensionBuildOptions = { + stdout: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + stderr: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + app: {} as any, + environment: 'production', + } + + // Execute: Call executeBuildSteps directly + await executeBuildSteps(mockExtension, stepsConfig, buildOptions) + + // Verify: Files were copied to output directory + const logoExists = await fileExists(joinPath(outputDir, 'logo.png')) + const styleExists = await fileExists(joinPath(outputDir, 'style.css')) + + expect(logoExists).toBe(true) + expect(styleExists).toBe(true) + + const logoContent = await readFile(joinPath(outputDir, 'logo.png')) + const styleContent = await readFile(joinPath(outputDir, 'style.css')) + + expect(logoContent).toBe('fake-png-data') + expect(styleContent).toBe('body { color: red; }') + }) + }) + + test('executes multiple steps in sequence', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Setup: Create extension with two asset directories + const extensionDir = joinPath(tmpDir, 'extension') + const imagesDir = joinPath(extensionDir, 'images') + const stylesDir = joinPath(extensionDir, 'styles') + const outputDir = joinPath(tmpDir, 'output') + + await mkdir(extensionDir) + await mkdir(imagesDir) + await mkdir(stylesDir) + await mkdir(outputDir) + + await writeFile(joinPath(imagesDir, 'logo.png'), 'logo-data') + await writeFile(joinPath(stylesDir, 'main.css'), 'css-data') + + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + } as ExtensionInstance + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'copy-images', + displayName: 'Copy Images', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'images', + patterns: ['**/*'], + destination: 'assets/images', + }, + }, + { + id: 'copy-styles', + displayName: 'Copy Styles', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'styles', + patterns: ['**/*'], + destination: 'assets/styles', + }, + }, + ], + } + + const buildOptions: ExtensionBuildOptions = { + stdout: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + stderr: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + app: {} as any, + environment: 'production', + } + + // Execute + await executeBuildSteps(mockExtension, stepsConfig, buildOptions) + + // Verify: Files from both steps were copied to correct destinations + const logoExists = await fileExists(joinPath(outputDir, 'assets/images/logo.png')) + const styleExists = await fileExists(joinPath(outputDir, 'assets/styles/main.css')) + + expect(logoExists).toBe(true) + expect(styleExists).toBe(true) + }) + }) + + test('handles optional source that does not exist', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDir = joinPath(tmpDir, 'extension') + const outputDir = joinPath(tmpDir, 'output') + + await mkdir(extensionDir) + await mkdir(outputDir) + + // Don't create the 'optional-assets' directory + + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + } as ExtensionInstance + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'copy-optional', + displayName: 'Copy Optional Assets', + type: 'copy_files', + config: { + strategy: 'directory', + source: 'optional-assets', + optional: true, + }, + }, + ], + } + + const buildOptions: ExtensionBuildOptions = { + stdout: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + stderr: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + app: {} as any, + environment: 'production', + } + + // Should not throw even though source doesn't exist + await expect(executeBuildSteps(mockExtension, stepsConfig, buildOptions)).resolves.not.toThrow() + }) + }) + + test('skips step when skip condition returns true', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDir = joinPath(tmpDir, 'extension') + const assetsDir = joinPath(extensionDir, 'assets') + const outputDir = joinPath(tmpDir, 'output') + + await mkdir(extensionDir) + await mkdir(assetsDir) + await mkdir(outputDir) + + await writeFile(joinPath(assetsDir, 'file.txt'), 'content') + + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + } as ExtensionInstance + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'skipped-step', + displayName: 'Skipped Step', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'assets', + patterns: ['**/*'], + }, + skip: () => true, + }, + ], + } + + const buildOptions: ExtensionBuildOptions = { + stdout: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + stderr: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + app: {} as any, + environment: 'production', + } + + await executeBuildSteps(mockExtension, stepsConfig, buildOptions) + + // Verify: File was not copied because step was skipped + const fileExistsResult = await fileExists(joinPath(outputDir, 'file.txt')) + expect(fileExistsResult).toBe(false) + }) + }) +}) diff --git a/packages/app/src/cli/services/build/build-steps.test.ts b/packages/app/src/cli/services/build/build-steps.test.ts new file mode 100644 index 00000000000..8359465d860 --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.test.ts @@ -0,0 +1,556 @@ +import { + executeBuildSteps, + BuildStep, + BuildContext, + BuildStepsConfig, + resolveConfigurableValue, +} from './build-steps.js' +import {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {beforeEach, describe, expect, test, vi} from 'vitest' +import * as stepsIndex from './steps/index.js' + +vi.mock('./steps/index.js') + +describe('executeBuildSteps', () => { + let mockExtension: ExtensionInstance + let mockStdout: {write: ReturnType} + let mockStderr: {write: ReturnType} + let mockOptions: any + + beforeEach(() => { + mockStdout = {write: vi.fn()} + mockStderr = {write: vi.fn()} + mockOptions = { + stdout: mockStdout, + stderr: mockStderr, + app: {} as any, + environment: 'production' as const, + } + mockExtension = { + directory: '/test/dir', + outputPath: '/test/output/index.js', + } as ExtensionInstance + + vi.clearAllMocks() + }) + + describe('sequential execution', () => { + test('executes steps in order and passes context', async () => { + // Given + const executionOrder: string[] = [] + + vi.mocked(stepsIndex.executeStepByType).mockImplementation(async (step: BuildStep) => { + executionOrder.push(step.id) + return {success: true} + }) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + parallel: false, + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(executionOrder).toEqual(['step1', 'step2', 'step3']) + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(3) + }) + + test('stops on first error when stopOnError is true', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType) + .mockResolvedValueOnce({success: true}) + .mockRejectedValueOnce(new Error('Step 2 failed')) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + stopOnError: true, + } + + // When/Then + await expect(executeBuildSteps(mockExtension, stepsConfig, mockOptions)).rejects.toThrow('Step 2 failed') + + // Only first two steps should be called + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(2) + }) + + test('continues on error when stopOnError is false', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType) + .mockResolvedValueOnce({success: true}) + .mockRejectedValueOnce(new Error('Step 2 failed')) + .mockResolvedValueOnce({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + continueOnError: false, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + continueOnError: true, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + stopOnError: true, + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(3) + expect(mockStderr.write).toHaveBeenCalledWith( + expect.stringContaining('Warning: Step "Step 2" failed but continuing'), + ) + }) + }) + + describe('skip conditions', () => { + test('skips step when skip function returns true', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + skip: () => true, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(2) + expect(mockStdout.write).toHaveBeenCalledWith('Skipping step: Step 2\n') + }) + + test('executes step when skip function returns false', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + skip: () => false, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(1) + expect(mockStdout.write).not.toHaveBeenCalledWith(expect.stringContaining('Skipping')) + }) + + test('passes context to skip function', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({success: true}) + + let capturedContext: BuildContext | undefined + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + skip: (context) => { + capturedContext = context + return false + }, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(capturedContext).toBeDefined() + expect(capturedContext?.extension).toBe(mockExtension) + expect(capturedContext?.options).toBe(mockOptions) + }) + }) + + describe('step results tracking', () => { + test('tracks results from completed steps', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType) + .mockResolvedValueOnce({filesCopied: 5}) + .mockResolvedValueOnce({filesCopied: 10}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + // We can't directly inspect the context, but we can verify the steps were executed + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(2) + expect(stepsIndex.executeStepByType).toHaveBeenNthCalledWith( + 1, + stepsConfig.steps[0], + expect.objectContaining({ + extension: mockExtension, + options: mockOptions, + }), + ) + }) + + test('tracks error results when continueOnError is true', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType) + .mockResolvedValueOnce({success: true}) + .mockRejectedValueOnce(new Error('Step 2 error')) + .mockResolvedValueOnce({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + continueOnError: true, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(3) + expect(mockStderr.write).toHaveBeenCalledWith(expect.stringContaining('Step 2 error')) + }) + }) + + describe('parallel execution', () => { + test('executes steps in parallel when parallel is true', async () => { + // Given + const startTimes: number[] = [] + const delays = [100, 50, 75] + + vi.mocked(stepsIndex.executeStepByType).mockImplementation(async (step: BuildStep) => { + const index = parseInt(step.id.replace('step', ''), 10) - 1 + startTimes.push(Date.now()) + await new Promise((resolve) => setTimeout(resolve, delays[index])) + return {success: true} + }) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + parallel: true, + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(stepsIndex.executeStepByType).toHaveBeenCalledTimes(3) + // All steps should start roughly at the same time (within 10ms) + const timeDiff1 = Math.abs(startTimes[1]! - startTimes[0]!) + const timeDiff2 = Math.abs(startTimes[2]! - startTimes[0]!) + expect(timeDiff1).toBeLessThan(10) + expect(timeDiff2).toBeLessThan(10) + }) + + test('reports failures when stopOnError is true in parallel mode', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType) + .mockResolvedValueOnce({success: true}) + .mockRejectedValueOnce(new Error('Step 2 failed')) + .mockResolvedValueOnce({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'step1', + displayName: 'Step 1', + type: 'copy_files', + config: {}, + }, + { + id: 'step2', + displayName: 'Step 2', + type: 'copy_files', + config: {}, + }, + { + id: 'step3', + displayName: 'Step 3', + type: 'copy_files', + config: {}, + }, + ], + parallel: true, + stopOnError: true, + } + + // When/Then + await expect(executeBuildSteps(mockExtension, stepsConfig, mockOptions)).rejects.toThrow( + '1 build step(s) failed', + ) + }) + }) + + describe('logging', () => { + test('logs step execution', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({success: true}) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'test-step', + displayName: 'Test Step', + type: 'copy_files', + config: {}, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(mockStdout.write).toHaveBeenCalledWith('Executing step: Test Step\n') + }) + + test('logs warnings for failed steps with continueOnError', async () => { + // Given + vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('Test error')) + + const stepsConfig: BuildStepsConfig = { + steps: [ + { + id: 'test-step', + displayName: 'Test Step', + type: 'copy_files', + config: {}, + continueOnError: true, + }, + ], + } + + // When + await executeBuildSteps(mockExtension, stepsConfig, mockOptions) + + // Then + expect(mockStderr.write).toHaveBeenCalledWith( + 'Warning: Step "Test Step" failed but continuing: Test error\n', + ) + }) + }) +}) + +describe('resolveConfigurableValue', () => { + let mockContext: BuildContext + + beforeEach(() => { + mockContext = { + extension: { + configuration: { + static_root: 'public', + nested: { + field: 'nested-value', + }, + }, + directory: '/test/dir', + }, + options: { + stdout: {write: vi.fn()}, + stderr: {write: vi.fn()}, + app: {} as any, + environment: 'production', + }, + stepResults: new Map(), + } as unknown as BuildContext + }) + + test('returns literal value as-is', () => { + const result = resolveConfigurableValue('literal-value', mockContext) + expect(result).toBe('literal-value') + }) + + test('returns literal number as-is', () => { + const result = resolveConfigurableValue(42, mockContext) + expect(result).toBe(42) + }) + + test('returns literal boolean as-is', () => { + const result = resolveConfigurableValue(true, mockContext) + expect(result).toBe(true) + }) + + test('returns literal array as-is', () => { + const arr = ['a', 'b', 'c'] + const result = resolveConfigurableValue(arr, mockContext) + expect(result).toBe(arr) + }) + + test('resolves configPath reference', () => { + const result = resolveConfigurableValue({configPath: 'static_root'}, mockContext) + expect(result).toBe('public') + }) + + test('resolves nested configPath reference', () => { + const result = resolveConfigurableValue({configPath: 'nested.field'}, mockContext) + expect(result).toBe('nested-value') + }) + + test('returns undefined for missing configPath', () => { + const result = resolveConfigurableValue({configPath: 'nonexistent'}, mockContext) + expect(result).toBeUndefined() + }) + + test('returns undefined for deeply nested missing configPath', () => { + const result = resolveConfigurableValue({configPath: 'nested.missing.field'}, mockContext) + expect(result).toBeUndefined() + }) + + test('resolves envVar reference', () => { + process.env.TEST_VAR = 'test-value' + const result = resolveConfigurableValue({envVar: 'TEST_VAR'}, mockContext) + expect(result).toBe('test-value') + delete process.env.TEST_VAR + }) + + test('returns undefined for missing envVar', () => { + const result = resolveConfigurableValue({envVar: 'NONEXISTENT_VAR'}, mockContext) + expect(result).toBeUndefined() + }) + + test('returns undefined when value is undefined', () => { + const result = resolveConfigurableValue(undefined, mockContext) + expect(result).toBeUndefined() + }) + + test('returns undefined when value is null', () => { + const result = resolveConfigurableValue(null as any, mockContext) + expect(result).toBeUndefined() + }) +}) diff --git a/packages/app/src/cli/services/build/build-steps.ts b/packages/app/src/cli/services/build/build-steps.ts new file mode 100644 index 00000000000..ac2663f84b7 --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.ts @@ -0,0 +1,352 @@ +import type {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import type {ExtensionBuildOptions} from './extension.js' +import {executeStepByType} from './steps/index.js' +import {AbortSignal} from '@shopify/cli-kit/node/abort' + +/** + * BuildStep represents a single build command configuration. + * Inspired by the existing Task pattern in: + * /packages/cli-kit/src/private/node/ui/components/Tasks.tsx + * + * Key differences from Task: + * - Not coupled to UI rendering + * - Pure configuration object (execution logic is separate) + * - Router pattern dispatches to type-specific executors + */ +export interface BuildStep { + /** Unique identifier for this step (e.g., 'copy_files', 'build') */ + readonly id: string + + /** Display name for logging */ + readonly displayName: string + + /** Optional description */ + readonly description?: string + + /** Step type (determines which executor handles it) */ + readonly type: 'copy_files' | 'esbuild' | 'validate' | 'transform' | 'custom' + + /** Step-specific configuration */ + readonly config: Record + + /** + * Optional function to determine if step should be skipped + * @param context - Build context + * @returns true if step should be skipped + */ + skip?(context: BuildContext): boolean + + /** + * Whether to continue on error (default: false) + */ + readonly continueOnError?: boolean +} + +/** + * BuildContext is passed through the pipeline (similar to Task). + * Each step can read from and write to the context. + * + * Key design: Immutable configuration, mutable context + */ +export interface BuildContext { + /** The extension being built */ + readonly extension: ExtensionInstance + + /** Build options (stdout, stderr, etc.) */ + readonly options: ExtensionBuildOptions + + /** Results from previous steps (for step dependencies) */ + readonly stepResults: Map + + /** Abort signal for cancellation */ + readonly signal?: AbortSignal + + /** Custom data that steps can write to (extensible) */ + [key: string]: unknown +} + +/** + * Result of a step execution + */ +export interface StepResult { + readonly stepId: string + readonly displayName: string + readonly success: boolean + readonly duration: number + readonly output?: unknown + readonly error?: Error +} + +/** + * Reference to a configuration value. + * Used to dynamically resolve values from the extension's configuration at build time. + */ +export interface ConfigReference { + /** Path to the config value (e.g., 'static_root' or 'nested.field') */ + configPath: string + /** Whether this reference is optional (if true, undefined values are acceptable) */ + optional?: boolean +} + +/** + * Reference to an environment variable. + * Used to dynamically resolve values from environment variables at build time. + */ +export interface EnvReference { + /** Name of the environment variable */ + envVar: string + /** Whether this reference is optional (if true, undefined values are acceptable) */ + optional?: boolean +} + +/** + * A value that can be either: + * - A literal value (T) + * - A reference to a config field ({configPath: string}) + * - A reference to an environment variable ({envVar: string}) + * + * This allows build step configurations to be static (serializable to JSON) + * while still supporting dynamic values resolved at build time. + * + * Example: + * ```typescript + * // Literal value + * source: 'public' + * + * // Reference to config + * source: {configPath: 'static_root'} + * + * // Reference to env var + * source: {envVar: 'BUILD_DIR'} + * ``` + */ +export type ConfigurableValue = T | ConfigReference | EnvReference + +/** + * Checks if a ConfigurableValue is a reference (ConfigReference or EnvReference). + * + * @param value - The value to check + * @returns true if the value is a reference object + */ +export function isReference(value: unknown): value is ConfigReference | EnvReference { + return ( + typeof value === 'object' && + value !== null && + ('configPath' in value || 'envVar' in value) + ) +} + +/** + * Checks if a reference is marked as optional. + * + * @param value - The reference to check + * @returns true if the reference has optional: true + */ +export function isOptionalReference(value: unknown): boolean { + if (!isReference(value)) { + return false + } + return value.optional === true +} + +/** + * Resolves a ConfigurableValue to its actual value. + * If the value is a reference (configPath or envVar), it will be resolved from the context. + * Otherwise, the literal value is returned as-is. + * + * @param value - The configurable value to resolve + * @param context - The build context containing extension config and options + * @returns The resolved value, or undefined if the reference cannot be resolved + */ +export function resolveConfigurableValue( + value: ConfigurableValue | undefined, + context: BuildContext, +): T | undefined { + if (!value) { + return undefined + } + + // Check if it's a config reference + if (typeof value === 'object' && value !== null && 'configPath' in value) { + const configRef = value as ConfigReference + return getNestedValue(context.extension.configuration, configRef.configPath) as T | undefined + } + + // Check if it's an env var reference + if (typeof value === 'object' && value !== null && 'envVar' in value) { + const envRef = value as EnvReference + return process.env[envRef.envVar] as T | undefined + } + + // It's a literal value + return value as T +} + +/** + * Helper function to get a nested value from an object using a dot-separated path. + * Example: getNestedValue({a: {b: {c: 'value'}}}, 'a.b.c') => 'value' + */ +function getNestedValue(obj: Record, path: string): unknown { + const parts = path.split('.') + let current: unknown = obj + + for (const part of parts) { + if (current === null || current === undefined) { + return undefined + } + if (typeof current === 'object' && part in current) { + current = (current as Record)[part] + } else { + return undefined + } + } + + return current +} + +/** + * BuildStepsConfig defines the pipeline configuration. + * Uses Builder Pattern principles for construction. + */ +export interface BuildStepsConfig { + /** Array of steps to execute in order */ + readonly steps: ReadonlyArray + + /** Whether to run steps in parallel (default: false) */ + readonly parallel?: boolean + + /** Whether to stop on first error (default: true) */ + readonly stopOnError?: boolean +} + +/** + * Executes a build steps pipeline for an extension. + * + * @param extension - The extension instance to build + * @param stepsConfig - Configuration defining the build steps + * @param options - Build options (stdout, stderr, etc.) + */ +export async function executeBuildSteps( + extension: ExtensionInstance, + stepsConfig: BuildStepsConfig, + options: ExtensionBuildOptions, +): Promise { + const context: BuildContext = { + extension, + options, + stepResults: new Map(), + signal: options.signal, + } + + const {steps, parallel = false, stopOnError = true} = stepsConfig + + if (parallel) { + await executeStepsInParallel(steps, context, stopOnError) + } else { + await executeStepsSequentially(steps, context, stopOnError) + } +} + +/** + * Executes steps sequentially, passing context through each step. + */ +async function executeStepsSequentially( + steps: ReadonlyArray, + context: BuildContext, + stopOnError: boolean, +): Promise { + for (const step of steps) { + const result = await executeStep(step, context) + context.stepResults.set(step.id, result) + + // Only stop on error if: + // 1. The step failed (result.success === false) + // 2. Global stopOnError is true + // 3. The step doesn't have continueOnError flag + if (!result.success && stopOnError && !step.continueOnError) { + throw new Error(`Build step "${step.displayName}" failed: ${result.error?.message}`) + } + } +} + +/** + * Executes steps in parallel (for independent steps). + */ +async function executeStepsInParallel( + steps: ReadonlyArray, + context: BuildContext, + stopOnError: boolean, +): Promise { + const results = await Promise.allSettled(steps.map((step) => executeStep(step, context))) + + // Store all results + results.forEach((result, index) => { + const step = steps[index]! + if (result.status === 'fulfilled') { + context.stepResults.set(step.id, result.value) + } else { + context.stepResults.set(step.id, { + stepId: step.id, + displayName: step.displayName, + success: false, + duration: 0, + error: result.reason as Error, + }) + } + }) + + const failures = results.filter((r) => r.status === 'rejected') + if (failures.length > 0 && stopOnError) { + throw new Error(`${failures.length} build step(s) failed`) + } +} + +/** + * Executes a single build step with error handling and skip logic. + */ +async function executeStep(step: BuildStep, context: BuildContext): Promise { + const startTime = Date.now() + + try { + // Check if step should be skipped + if (step.skip?.(context)) { + context.options.stdout.write(`Skipping step: ${step.displayName}\n`) + return { + stepId: step.id, + displayName: step.displayName, + success: true, + duration: Date.now() - startTime, + output: 'skipped', + } + } + + // Execute the step using type-specific executor + context.options.stdout.write(`Executing step: ${step.displayName}\n`) + const output = await executeStepByType(step, context) + + return { + stepId: step.id, + displayName: step.displayName, + success: true, + duration: Date.now() - startTime, + output, + } + } catch (error) { + const stepError = error as Error + + if (step.continueOnError) { + context.options.stderr.write( + `Warning: Step "${step.displayName}" failed but continuing: ${stepError.message}\n`, + ) + return { + stepId: step.id, + displayName: step.displayName, + success: false, + duration: Date.now() - startTime, + error: stepError, + } + } + + throw new Error(`Build step "${step.displayName}" failed: ${stepError.message}`) + } +} 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 new file mode 100644 index 00000000000..3a35e028f92 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/copy-files-step.test.ts @@ -0,0 +1,605 @@ +import {executeCopyFilesStep} from './copy-files-step.js' +import {BuildStep, BuildContext} from '../build-steps.js' +import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' +import {beforeEach, describe, expect, test, vi} from 'vitest' +import * as fs from '@shopify/cli-kit/node/fs' + +vi.mock('@shopify/cli-kit/node/fs') + +describe('executeCopyFilesStep', () => { + let mockExtension: ExtensionInstance + let mockContext: BuildContext + let mockStdout: any + let mockStderr: any + + beforeEach(() => { + mockStdout = {write: vi.fn()} + mockStderr = {write: vi.fn()} + mockExtension = { + directory: '/test/extension', + outputPath: '/test/output/extension.js', + } as ExtensionInstance + + mockContext = { + extension: mockExtension, + options: { + stdout: mockStdout, + stderr: mockStderr, + app: {} as any, + environment: 'production', + }, + stepResults: new Map(), + } + + vi.clearAllMocks() + }) + + describe('directory strategy', () => { + test('copies entire directory when source exists', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['file1.js', 'file2.js', 'dir/file3.js']) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-dir', + displayName: 'Copy Directory', + type: 'copy_files', + config: { + strategy: 'directory', + source: 'assets', + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.fileExists).toHaveBeenCalledWith('/test/extension/assets') + expect(fs.copyDirectoryContents).toHaveBeenCalledWith( + '/test/extension/assets', + '/test/output', + ) + expect(result.filesCopied).toBe(3) + expect(mockStdout.write).toHaveBeenCalledWith( + expect.stringContaining('Copied directory'), + ) + }) + + test('throws error when source does not exist and optional is false', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: BuildStep = { + id: 'copy-dir', + displayName: 'Copy Directory', + type: 'copy_files', + config: { + strategy: 'directory', + source: 'missing', + optional: false, + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow( + 'Source directory does not exist', + ) + }) + + test('throws error when source directory does not exist', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: BuildStep = { + id: 'copy-dir', + displayName: 'Copy Directory', + type: 'copy_files', + config: { + strategy: 'directory', + source: 'optional-assets', + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow( + 'Source directory does not exist: /test/extension/optional-assets', + ) + }) + + test('copies to custom destination when specified', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['file1.js']) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-dir', + displayName: 'Copy Directory', + type: 'copy_files', + config: { + strategy: 'directory', + source: 'assets', + destination: 'public', + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyDirectoryContents).toHaveBeenCalledWith( + '/test/extension/assets', + '/test/output/public', + ) + }) + }) + + describe('pattern strategy', () => { + test('copies files matching patterns', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([ + '/test/extension/assets/image1.png', + '/test/extension/assets/image2.jpg', + ]) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-images', + displayName: 'Copy Images', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'assets', + patterns: ['*.png', '*.jpg'], + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.glob).toHaveBeenCalledWith( + ['*.png', '*.jpg'], + expect.objectContaining({ + absolute: true, + cwd: '/test/extension/assets', + }), + ) + expect(fs.copyFile).toHaveBeenCalledTimes(2) + expect(result.filesCopied).toBe(2) + }) + + test('respects ignore patterns', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/extension/file1.js']) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-js', + displayName: 'Copy JS', + type: 'copy_files', + config: { + strategy: 'pattern', + patterns: ['**/*.js'], + ignore: ['**/*.test.js'], + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.glob).toHaveBeenCalledWith( + ['**/*.js'], + expect.objectContaining({ + ignore: ['**/*.test.js'], + }), + ) + }) + + test('uses default pattern when none specified', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/extension/file.txt']) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-all', + displayName: 'Copy All', + type: 'copy_files', + config: { + strategy: 'pattern', + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.glob).toHaveBeenCalledWith( + ['**/*'], + expect.objectContaining({ + cwd: '/test/extension', + }), + ) + }) + + test('preserves directory structure when preserveStructure is true', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([ + '/test/extension/src/components/Button.tsx', + ]) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-src', + displayName: 'Copy Source', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'src', + patterns: ['**/*.tsx'], + preserveStructure: true, + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).toHaveBeenCalledWith( + '/test/extension/src/components/Button.tsx', + '/test/output/components/Button.tsx', + ) + }) + + test('flattens files when preserveStructure is false', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([ + '/test/extension/src/components/Button.tsx', + ]) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-src', + displayName: 'Copy Source', + type: 'copy_files', + config: { + strategy: 'pattern', + source: 'src', + patterns: ['**/*.tsx'], + preserveStructure: false, + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).toHaveBeenCalledWith( + '/test/extension/src/components/Button.tsx', + '/test/output/Button.tsx', + ) + }) + + test('returns zero files when no matches and optional is false', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([]) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-none', + displayName: 'Copy None', + type: 'copy_files', + config: { + strategy: 'pattern', + patterns: ['*.nonexistent'], + optional: false, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(0) + expect(mockStdout.write).toHaveBeenCalledWith( + expect.stringContaining('No files matched patterns'), + ) + }) + + test('skips copy when same source and destination', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/output/file.js']) + vi.mocked(fs.mkdir).mockResolvedValue() + + mockExtension.directory = '/test/output' + + const step: BuildStep = { + id: 'copy-same', + displayName: 'Copy Same', + type: 'copy_files', + config: { + strategy: 'pattern', + patterns: ['*.js'], + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).not.toHaveBeenCalled() + }) + }) + + describe('files strategy', () => { + test('copies specific file list', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-files', + displayName: 'Copy Specific Files', + type: 'copy_files', + config: { + strategy: 'files', + files: [ + {source: 'config.json', destination: 'config.json'}, + {source: 'assets/logo.png', destination: 'images/logo.png'}, + ], + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).toHaveBeenCalledWith( + '/test/extension/config.json', + '/test/output/config.json', + ) + expect(fs.copyFile).toHaveBeenCalledWith( + '/test/extension/assets/logo.png', + '/test/output/images/logo.png', + ) + expect(result.filesCopied).toBe(2) + }) + + test('throws error when file does not exist and optional is false', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(false) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-missing', + displayName: 'Copy Missing File', + type: 'copy_files', + config: { + strategy: 'files', + files: [{source: 'missing.txt', destination: 'missing.txt'}], + optional: false, + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow( + 'Source file does not exist', + ) + }) + + test('throws error when file in list does not exist', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValueOnce(true).mockResolvedValueOnce(false) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-partial', + displayName: 'Copy Partial', + type: 'copy_files', + config: { + strategy: 'files', + files: [ + {source: 'exists.txt', destination: 'exists.txt'}, + {source: 'missing.txt', destination: 'missing.txt'}, + ], + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow( + 'Source file does not exist: /test/extension/missing.txt', + ) + }) + + test('returns zero when file list is empty', async () => { + // Given + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-empty', + displayName: 'Copy Empty', + type: 'copy_files', + config: { + strategy: 'files', + files: [], + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(0) + expect(mockStdout.write).toHaveBeenCalledWith('No files specified in file list\n') + }) + + test('skips copy when source and destination are the same', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.mkdir).mockResolvedValue() + + mockExtension.directory = '/test/output' + + const step: BuildStep = { + id: 'copy-same', + displayName: 'Copy Same', + type: 'copy_files', + config: { + strategy: 'files', + files: [{source: 'file.js', destination: 'file.js'}], + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).not.toHaveBeenCalled() + }) + }) + + describe('config validation', () => { + test('throws error for unknown strategy', async () => { + // Given + const step: BuildStep = { + id: 'invalid', + displayName: 'Invalid Strategy', + type: 'copy_files', + config: { + strategy: 'invalid_strategy', + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow() + }) + + test('uses default values for optional config', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([]) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'minimal', + displayName: 'Minimal Config', + type: 'copy_files', + config: {}, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(0) + // Should use default strategy 'pattern' + expect(fs.glob).toHaveBeenCalled() + }) + }) + + describe('undefined source handling (ConfigurableValue)', () => { + test('skips silently when source is undefined and optional is true', async () => { + // Given - source will resolve to undefined + const contextWithoutConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {}, // No static_root field + } as ExtensionInstance, + } + + const step: BuildStep = { + id: 'copy-optional', + displayName: 'Copy Optional Assets', + type: 'copy_files', + config: { + strategy: 'directory', + source: {configPath: 'static_root', optional: true}, // Will resolve to undefined, but that's okay + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithoutConfig) + + // Then + expect(result.filesCopied).toBe(0) + expect(mockStdout.write).toHaveBeenCalledWith( + expect.stringContaining('Skipping Copy Optional Assets: source is not configured'), + ) + // Should not attempt any file operations + expect(fs.fileExists).not.toHaveBeenCalled() + expect(fs.copyDirectoryContents).not.toHaveBeenCalled() + }) + + test('throws error when source is undefined and optional is false', async () => { + // Given - source will resolve to undefined + const contextWithoutConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {}, // No static_root field + } as ExtensionInstance, + } + + const step: BuildStep = { + id: 'copy-required', + displayName: 'Copy Required Assets', + type: 'copy_files', + config: { + strategy: 'directory', + source: {configPath: 'static_root'}, // Will resolve to undefined - should error + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, contextWithoutConfig)).rejects.toThrow( + 'Build step "Copy Required Assets" failed: source configuration is required but resolved to undefined', + ) + }) + + test('works normally when source resolves to a value', async () => { + // Given - source will resolve to 'public' + const contextWithConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {static_root: 'public'}, + } as unknown as ExtensionInstance, + } + + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['file1.js', 'file2.js']) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-configured', + displayName: 'Copy Configured Assets', + type: 'copy_files', + config: { + strategy: 'directory', + source: {configPath: 'static_root'}, // Will resolve to 'public' + optional: true, + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithConfig) + + // Then + expect(result.filesCopied).toBe(2) + expect(fs.fileExists).toHaveBeenCalledWith(expect.stringContaining('public')) + expect(fs.copyDirectoryContents).toHaveBeenCalled() + }) + }) +}) 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 new file mode 100644 index 00000000000..a8d8cf53923 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/copy-files-step.ts @@ -0,0 +1,274 @@ +import type {BuildStep, BuildContext, ConfigurableValue} from '../build-steps.js' +import {resolveConfigurableValue, isOptionalReference, isReference} from '../build-steps.js' +import {joinPath, dirname, relativePath, basename} from '@shopify/cli-kit/node/path' +import {glob, copyFile, copyDirectoryContents, fileExists, mkdir} from '@shopify/cli-kit/node/fs' +import {z} from 'zod' + +/** + * Zod schema for a configurable value (literal or reference). + * Accepts either a literal value of type T, or a reference object. + */ +const configurableValueSchema = (literalSchema: T) => + z.union([ + literalSchema, + z.object({configPath: z.string(), optional: z.boolean().optional()}), + z.object({envVar: z.string(), optional: z.boolean().optional()}), + ]) + +/** + * Configuration schema for copy_files step. + * Uses Zod for runtime validation of step configuration. + * Supports ConfigurableValue for dynamic resolution from extension config. + */ +const CopyFilesConfigSchema = z.object({ + /** Source directory or file pattern (relative to extension directory) */ + source: configurableValueSchema(z.string()).optional(), + + /** Destination directory (relative to outputPath) */ + destination: configurableValueSchema(z.string()).optional(), + + /** Glob patterns for files to include */ + patterns: configurableValueSchema(z.array(z.string())).optional(), + + /** Glob patterns for files to exclude */ + ignore: configurableValueSchema(z.array(z.string())).optional(), + + /** Copy strategy: 'directory' | 'files' | 'pattern' */ + strategy: z.enum(['directory', 'files', 'pattern']).default('pattern'), + + /** Preserve directory structure (default: true) */ + preserveStructure: z.boolean().default(true), + + /** File list (for 'files' strategy) */ + files: z + .array( + z.object({ + source: configurableValueSchema(z.string()), + destination: configurableValueSchema(z.string()), + }), + ) + .optional(), +}) + +type CopyFilesConfig = z.infer + +/** + * Resolved configuration after ConfigurableValues have been resolved to their actual values. + */ +interface ResolvedCopyFilesConfig { + strategy: 'directory' | 'files' | 'pattern' + source?: string + destination?: string + patterns?: string[] + ignore?: string[] + preserveStructure: boolean + files?: Array<{source: string; destination: string}> +} + +/** + * Executes a copy_files build step. + * Supports three strategies: directory, files, pattern. + * + * @param step - The copy_files step configuration + * @param context - The build context + * @returns Result with number of files copied + */ +export async function executeCopyFilesStep( + step: BuildStep, + context: BuildContext, +): Promise<{filesCopied: number}> { + // Validate and parse configuration + const config = CopyFilesConfigSchema.parse(step.config) + const {extension, options} = context + + // Resolve configurable values from extension config + const resolvedSource = resolveConfigurableValue(config.source, context) + const resolvedDestination = resolveConfigurableValue(config.destination, context) + const resolvedPatterns = resolveConfigurableValue(config.patterns, context) + const resolvedIgnore = resolveConfigurableValue(config.ignore, context) + + // Resolve file list if present + const resolvedFiles = config.files?.map((file) => ({ + source: resolveConfigurableValue(file.source, context) ?? '', + destination: resolveConfigurableValue(file.destination, context) ?? '', + })) + + // Create resolved config object + const resolvedConfig: ResolvedCopyFilesConfig = { + strategy: config.strategy, + source: resolvedSource, + destination: resolvedDestination, + patterns: resolvedPatterns, + ignore: resolvedIgnore, + preserveStructure: config.preserveStructure, + files: resolvedFiles, + } + + // Handle when source is a reference that resolves to undefined + // (e.g., {configPath: 'static_root', optional: true} but static_root is not in TOML) + // Note: If config.source is undefined (not specified), that's okay - we'll use extension directory + if ( + config.source !== undefined && + isReference(config.source) && + resolvedConfig.source === undefined && + (config.strategy === 'directory' || config.strategy === 'pattern') + ) { + // Check if the source is an optional reference + if (isOptionalReference(config.source)) { + // If optional, skip silently with a log message + options.stdout.write( + `Skipping ${step.displayName}: source is not configured (resolved to undefined)\n`, + ) + return {filesCopied: 0} + } + // If source is a required reference that resolved to undefined, throw a clear error + throw new Error( + `Build step "${step.displayName}" failed: source configuration is required but resolved to undefined. ` + + `Check that the referenced config field exists in your TOML file.`, + ) + } + + // Determine source and output directories + const sourceDir = resolvedConfig.source + ? joinPath(extension.directory, resolvedConfig.source) + : extension.directory + + const outputDir = resolvedConfig.destination + ? joinPath(dirname(extension.outputPath), resolvedConfig.destination) + : dirname(extension.outputPath) + + // Execute based on strategy + switch (resolvedConfig.strategy) { + case 'directory': + return copyDirectory(sourceDir, outputDir, resolvedConfig, options) + case 'files': + return copyFileList(resolvedFiles || [], sourceDir, outputDir, resolvedConfig, options) + case 'pattern': + return copyByPattern(sourceDir, outputDir, resolvedConfig, options) + default: + throw new Error(`Unknown copy strategy: ${resolvedConfig.strategy}`) + } +} + +/** + * Strategy: Copy entire directory contents + */ +async function copyDirectory( + sourceDir: string, + outputDir: string, + config: ResolvedCopyFilesConfig, + options: {stdout: NodeJS.WritableStream; stderr: NodeJS.WritableStream}, +): Promise<{filesCopied: number}> { + const exists = await fileExists(sourceDir) + + if (!exists) { + throw new Error(`Source directory does not exist: ${sourceDir}`) + } + + // Ensure output directory exists + await mkdir(outputDir) + + // Copy entire directory + await copyDirectoryContents(sourceDir, outputDir) + options.stdout.write(`Copied directory ${sourceDir} to ${outputDir}\n`) + + // Count files copied + const files = await glob(['**/*'], {cwd: outputDir, absolute: false}) + return {filesCopied: files.length} +} + +/** + * Strategy: Copy files matching glob patterns + */ +async function copyByPattern( + sourceDir: string, + outputDir: string, + config: ResolvedCopyFilesConfig, + options: {stdout: NodeJS.WritableStream; stderr: NodeJS.WritableStream}, +): Promise<{filesCopied: number}> { + const patterns = config.patterns || ['**/*'] + const ignore = config.ignore || [] + + // Find matching files + const files = await glob(patterns, { + absolute: true, + cwd: sourceDir, + ignore, + }) + + if (files.length === 0) { + options.stdout.write(`Warning: No files matched patterns in ${sourceDir}\n`) + return {filesCopied: 0} + } + + // Ensure output directory exists + await mkdir(outputDir) + + // Copy each file + await Promise.all( + files.map(async (filepath) => { + const relPath = config.preserveStructure ? relativePath(sourceDir, filepath) : basename(filepath) + + const destPath = joinPath(outputDir, relPath) + + // Skip if source and destination are the same + if (filepath === destPath) return + + // Ensure destination directory exists + await mkdir(dirname(destPath)) + + // Copy file + await copyFile(filepath, destPath) + }), + ) + + options.stdout.write(`Copied ${files.length} file(s) from ${sourceDir} to ${outputDir}\n`) + return {filesCopied: files.length} +} + +/** + * Strategy: Copy specific file list + */ +async function copyFileList( + files: Array<{source: string; destination: string}>, + sourceDir: string, + outputDir: string, + config: ResolvedCopyFilesConfig, + options: {stdout: NodeJS.WritableStream; stderr: NodeJS.WritableStream}, +): Promise<{filesCopied: number}> { + if (files.length === 0) { + options.stdout.write('No files specified in file list\n') + return {filesCopied: 0} + } + + // Ensure output directory exists + await mkdir(outputDir) + + let copiedCount = 0 + + await Promise.all( + files.map(async ({source, destination}) => { + const sourcePath = joinPath(sourceDir, source) + const destPath = joinPath(outputDir, destination) + + // Skip if source and destination are the same + if (sourcePath === destPath) return + + // Check if source exists + const exists = await fileExists(sourcePath) + if (!exists) { + throw new Error(`Source file does not exist: ${sourcePath}`) + } + + // Ensure destination directory exists + await mkdir(dirname(destPath)) + + // Copy file + await copyFile(sourcePath, destPath) + copiedCount++ + }), + ) + + options.stdout.write(`Copied ${copiedCount} file(s) to ${outputDir}\n`) + return {filesCopied: copiedCount} +} diff --git a/packages/app/src/cli/services/build/steps/index.ts b/packages/app/src/cli/services/build/steps/index.ts new file mode 100644 index 00000000000..f2ca239df68 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -0,0 +1,31 @@ +import type {BuildStep, BuildContext} from '../build-steps.js' +import {executeCopyFilesStep} from './copy-files-step.js' + +/** + * Routes step execution to the appropriate handler based on step type. + * This implements the Command Pattern router, dispatching to type-specific executors. + * + * @param step - The build step configuration + * @param context - The build context + * @returns The output from the step execution + * @throws Error if the step type is not implemented or unknown + */ +export async function executeStepByType(step: BuildStep, context: BuildContext): Promise { + switch (step.type) { + case 'copy_files': + return executeCopyFilesStep(step, context) + + // Future step types (not implemented yet): + case 'esbuild': + case 'validate': + case 'transform': + case 'custom': + throw new Error( + `Build step type "${step.type}" is not yet implemented. Only "copy_files" is currently supported.`, + ) + + default: + // This should never happen due to TypeScript type checking, but provides a runtime safety net + throw new Error(`Unknown build step type: ${(step as BuildStep).type}`) + } +}