From 34005f5bc788390b88afeb02424478def94924f3 Mon Sep 17 00:00:00 2001 From: Alfonso Noriega Date: Thu, 19 Feb 2026 13:39:05 +0100 Subject: [PATCH] Abstract build steps to externalize the build configuration --- .../build/build-steps.integration.test.ts | 165 ++++++ .../cli/services/build/build-steps.test.ts | 77 +++ .../app/src/cli/services/build/build-steps.ts | 114 +++++ .../build/steps/build-function-step.ts | 12 + .../services/build/steps/build-theme-step.ts | 14 + .../services/build/steps/bundle-theme-step.ts | 27 + .../services/build/steps/bundle-ui-step.ts | 11 + .../build/steps/copy-files-step.test.ts | 470 ++++++++++++++++++ .../services/build/steps/copy-files-step.ts | 272 ++++++++++ .../build/steps/copy-static-assets-step.ts | 11 + .../build/steps/create-tax-stub-step.ts | 14 + .../app/src/cli/services/build/steps/index.ts | 52 ++ 12 files changed, 1239 insertions(+) create mode 100644 packages/app/src/cli/services/build/build-steps.integration.test.ts create mode 100644 packages/app/src/cli/services/build/build-steps.test.ts create mode 100644 packages/app/src/cli/services/build/build-steps.ts create mode 100644 packages/app/src/cli/services/build/steps/build-function-step.ts create mode 100644 packages/app/src/cli/services/build/steps/build-theme-step.ts create mode 100644 packages/app/src/cli/services/build/steps/bundle-theme-step.ts create mode 100644 packages/app/src/cli/services/build/steps/bundle-ui-step.ts create mode 100644 packages/app/src/cli/services/build/steps/copy-files-step.test.ts create mode 100644 packages/app/src/cli/services/build/steps/copy-files-step.ts create mode 100644 packages/app/src/cli/services/build/steps/copy-static-assets-step.ts create mode 100644 packages/app/src/cli/services/build/steps/create-tax-stub-step.ts create mode 100644 packages/app/src/cli/services/build/steps/index.ts 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..be51068f51d --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.integration.test.ts @@ -0,0 +1,165 @@ +import {ExtensionBuildOptions} from './extension.js' +import {executeStep, BuildContext} from './build-steps.js' +import {ExtensionInstance} from '../../models/extensions/extension-instance.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' + +function buildOptions(): ExtensionBuildOptions { + return { + stdout: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + stderr: new Writable({ + write(chunk, encoding, callback) { + callback() + }, + }), + app: {} as any, + environment: 'production', + } +} + +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; }') + + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + } as ExtensionInstance + + const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()} + + await executeStep( + { + id: 'copy-assets', + displayName: 'Copy Assets', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: {source: 'assets', patterns: ['**/*']}, + }, + }, + context, + ) + + // 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 context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()} + + await executeStep( + { + id: 'copy-images', + displayName: 'Copy Images', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: {source: 'images', patterns: ['**/*'], destination: 'assets/images'}, + }, + }, + context, + ) + await executeStep( + { + id: 'copy-styles', + displayName: 'Copy Styles', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: {source: 'styles', patterns: ['**/*'], destination: 'assets/styles'}, + }, + }, + context, + ) + + // 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('silently skips tomlKeys step when TOML key is absent from extension config', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDir = joinPath(tmpDir, 'extension') + const outputDir = joinPath(tmpDir, 'output') + + await mkdir(extensionDir) + await mkdir(outputDir) + + // Extension has no configuration — static_root key is absent + const mockExtension = { + directory: extensionDir, + outputPath: joinPath(outputDir, 'extension.js'), + configuration: {}, + } as unknown as ExtensionInstance + + const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()} + + // Should not throw — absent tomlKeys are silently skipped + await expect( + executeStep( + { + id: 'copy-static', + displayName: 'Copy Static Assets', + type: 'copy_files', + config: {strategy: 'files', definition: {files: [{tomlKey: 'static_root'}]}}, + }, + context, + ), + ).resolves.not.toThrow() + }) + }) +}) 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..0e3998dbf22 --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.test.ts @@ -0,0 +1,77 @@ +import {executeStep, BuildStep, BuildContext} from './build-steps.js' +import * as stepsIndex from './steps/index.js' +import {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {beforeEach, describe, expect, test, vi} from 'vitest' + +vi.mock('./steps/index.js') + +describe('executeStep', () => { + let mockContext: BuildContext + + beforeEach(() => { + mockContext = { + extension: { + directory: '/test/dir', + outputPath: '/test/output/index.js', + } as ExtensionInstance, + options: { + stdout: {write: vi.fn()} as any, + stderr: {write: vi.fn()} as any, + app: {} as any, + environment: 'production' as const, + }, + stepResults: new Map(), + } + }) + + const step: BuildStep = { + id: 'test-step', + displayName: 'Test Step', + type: 'copy_files', + config: {}, + } + + describe('success', () => { + test('returns a successful StepResult with output', async () => { + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({filesCopied: 3}) + + const result = await executeStep(step, mockContext) + + expect(result.stepId).toBe('test-step') + expect(result.displayName).toBe('Test Step') + expect(result.success).toBe(true) + expect(result.output).toEqual({filesCopied: 3}) + expect(result.duration).toBeGreaterThanOrEqual(0) + }) + + test('logs step execution to stdout', async () => { + vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({}) + + await executeStep(step, mockContext) + + expect(mockContext.options.stdout.write).toHaveBeenCalledWith('Executing step: Test Step\n') + }) + }) + + describe('failure', () => { + test('throws a wrapped error when the step fails', async () => { + vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong')) + + await expect(executeStep(step, mockContext)).rejects.toThrow( + 'Build step "Test Step" failed: something went wrong', + ) + }) + + test('returns a failure result and logs a warning when continueOnError is true', async () => { + vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong')) + + const result = await executeStep({...step, continueOnError: true}, mockContext) + + expect(result.success).toBe(false) + expect(result.error?.message).toBe('something went wrong') + expect(mockContext.options.stderr.write).toHaveBeenCalledWith( + 'Warning: Step "Test Step" failed but continuing: something went wrong\n', + ) + }) + }) +}) 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..0c39562b31d --- /dev/null +++ b/packages/app/src/cli/services/build/build-steps.ts @@ -0,0 +1,114 @@ +import {executeStepByType} from './steps/index.js' +import type {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import type {ExtensionBuildOptions} from './extension.js' + +/** + * 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' + | 'build_theme' + | 'bundle_theme' + | 'bundle_ui' + | 'copy_static_assets' + | 'build_function' + | 'create_tax_stub' + | 'esbuild' + | 'validate' + | 'transform' + | 'custom' + + /** Step-specific configuration */ + readonly config: {[key: string]: unknown} + + /** + * 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 + + /** Custom data that steps can write to (extensible) */ + [key: string]: unknown +} + +/** + * Result of a step execution + */ +interface StepResult { + readonly stepId: string + readonly displayName: string + readonly success: boolean + readonly duration: number + readonly output?: unknown + readonly error?: Error +} + +/** + * Executes a single build step with error handling and skip logic. + */ +export async function executeStep(step: BuildStep, context: BuildContext): Promise { + const startTime = Date.now() + + try { + // 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/build-function-step.ts b/packages/app/src/cli/services/build/steps/build-function-step.ts new file mode 100644 index 00000000000..d76af40da9f --- /dev/null +++ b/packages/app/src/cli/services/build/steps/build-function-step.ts @@ -0,0 +1,12 @@ +import {buildFunctionExtension} from '../extension.js' +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a build_function build step. + * + * Compiles the function extension (JavaScript or other language) to WASM, + * applying wasm-opt and trampoline as configured. + */ +export async function executeBuildFunctionStep(_step: BuildStep, context: BuildContext): Promise { + return buildFunctionExtension(context.extension, context.options) +} diff --git a/packages/app/src/cli/services/build/steps/build-theme-step.ts b/packages/app/src/cli/services/build/steps/build-theme-step.ts new file mode 100644 index 00000000000..cd16d229376 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/build-theme-step.ts @@ -0,0 +1,14 @@ +import {runThemeCheck} from '../theme-check.js' +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a build_theme build step. + * + * Runs theme check on the extension directory and writes any offenses to stdout. + */ +export async function executeBuildThemeStep(_step: BuildStep, context: BuildContext): Promise { + const {extension, options} = context + options.stdout.write(`Running theme check on your Theme app extension...`) + const offenses = await runThemeCheck(extension.directory) + if (offenses) options.stdout.write(offenses) +} diff --git a/packages/app/src/cli/services/build/steps/bundle-theme-step.ts b/packages/app/src/cli/services/build/steps/bundle-theme-step.ts new file mode 100644 index 00000000000..fa6e1b32db5 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/bundle-theme-step.ts @@ -0,0 +1,27 @@ +import {themeExtensionFiles} from '../../../utilities/extensions/theme.js' +import {copyFile} from '@shopify/cli-kit/node/fs' +import {relativePath, joinPath} from '@shopify/cli-kit/node/path' +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a bundle_theme build step. + * + * Copies theme extension files to the output directory, preserving relative paths. + * Respects the extension's .shopifyignore file and the standard ignore patterns. + */ +export async function executeBundleThemeStep(_step: BuildStep, context: BuildContext): Promise<{filesCopied: number}> { + const {extension, options} = context + options.stdout.write(`Bundling theme extension ${extension.localIdentifier}...`) + const files = await themeExtensionFiles(extension) + + await Promise.all( + files.map(async (filepath) => { + const relativePathName = relativePath(extension.directory, filepath) + const outputFile = joinPath(extension.outputPath, relativePathName) + if (filepath === outputFile) return + await copyFile(filepath, outputFile) + }), + ) + + return {filesCopied: files.length} +} diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts new file mode 100644 index 00000000000..9b157722219 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts @@ -0,0 +1,11 @@ +import {buildUIExtension} from '../extension.js' +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a bundle_ui build step. + * + * Bundles the UI extension using esbuild, writing output to extension.outputPath. + */ +export async function executeBundleUIStep(_step: BuildStep, context: BuildContext): Promise { + return buildUIExtension(context.extension, context.options) +} 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..02e07191f96 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/copy-files-step.test.ts @@ -0,0 +1,470 @@ +import {executeCopyFilesStep} from './copy-files-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') + +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(), + } + }) + + describe('files strategy — explicit file list', () => { + test('copies directory contents to output root when no destination', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['index.html', 'assets/logo.png']) + + const step: BuildStep = { + id: 'copy-dist', + displayName: 'Copy Dist', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{source: 'dist'}], + }, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/dist', '/test/output') + expect(result.filesCopied).toBe(2) + expect(mockStdout.write).toHaveBeenCalledWith(expect.stringContaining('Copied contents of dist to output root')) + }) + + test('throws when source directory does not exist', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: BuildStep = { + id: 'copy-dist', + displayName: 'Copy Dist', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{source: 'dist'}], + }, + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow('Source does not exist') + }) + + test('copies file to explicit destination path', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-icon', + displayName: 'Copy Icon', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{source: 'src/icon.png', destination: 'assets/icon.png'}], + }, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/icon.png', '/test/output/assets/icon.png') + expect(result.filesCopied).toBe(1) + expect(mockStdout.write).toHaveBeenCalledWith('Copied src/icon.png to assets/icon.png\n') + }) + + test('throws when source file does not exist (with destination)', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: BuildStep = { + id: 'copy-icon', + displayName: 'Copy Icon', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{source: 'src/missing.png', destination: 'assets/missing.png'}], + }, + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow('Source does not exist') + }) + + test('handles mixed entries: directory-to-root and explicit file', async () => { + // Given + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['index.html']) + + const step: BuildStep = { + id: 'copy-mixed', + displayName: 'Copy Mixed', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{source: 'dist'}, {source: 'src/icon.png', destination: 'assets/icon.png'}], + }, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/dist', '/test/output') + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/icon.png', '/test/output/assets/icon.png') + expect(result.filesCopied).toBe(2) + }) + }) + + describe('files strategy — tomlKey entries', () => { + test('copies directory contents for resolved tomlKey', async () => { + // Given + 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(['index.html', 'logo.png']) + + const step: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static', + type: 'copy_files', + config: { + strategy: 'files', + definition: {files: [{tomlKey: 'static_root'}]}, + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithConfig) + + // Then + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/public', '/test/output') + expect(result.filesCopied).toBe(2) + }) + + test('skips silently when tomlKey is absent from config', async () => { + // Given — configuration has no static_root + const contextWithoutConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {}, + } as unknown as ExtensionInstance, + } + + const step: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static', + type: 'copy_files', + config: { + strategy: 'files', + definition: {files: [{tomlKey: 'static_root'}]}, + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithoutConfig) + + // Then — no error, no copies + expect(result.filesCopied).toBe(0) + expect(fs.copyDirectoryContents).not.toHaveBeenCalled() + expect(mockStdout.write).toHaveBeenCalledWith(expect.stringContaining("No value for tomlKey 'static_root'")) + }) + + test('skips path that does not exist on disk but logs a warning', async () => { + // Given + const contextWithConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {static_root: 'nonexistent'}, + } as unknown as ExtensionInstance, + } + + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static', + type: 'copy_files', + config: { + strategy: 'files', + definition: {files: [{tomlKey: 'static_root'}]}, + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithConfig) + + // Then — no error, logged warning + expect(result.filesCopied).toBe(0) + expect(mockStdout.write).toHaveBeenCalledWith( + expect.stringContaining("Warning: path 'nonexistent' does not exist"), + ) + }) + + test('resolves TOML array field and copies each path', async () => { + // Given — static_root is an array + const contextWithArrayConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: {static_root: ['public', 'assets']}, + } as unknown as ExtensionInstance, + } + + vi.mocked(fs.fileExists).mockResolvedValue(true) + vi.mocked(fs.copyDirectoryContents).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['file.html']) + + const step: BuildStep = { + id: 'copy-static', + displayName: 'Copy Static', + type: 'copy_files', + config: { + strategy: 'files', + definition: {files: [{tomlKey: 'static_root'}]}, + }, + } + + // When + await executeCopyFilesStep(step, contextWithArrayConfig) + + // Then — both paths copied + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/public', '/test/output') + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/assets', '/test/output') + }) + + test('handles mixed source and tomlKey entries in a single step', async () => { + // Given + 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.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + vi.mocked(fs.glob).mockResolvedValue(['index.html']) + + const step: BuildStep = { + id: 'copy-mixed', + displayName: 'Copy Mixed', + type: 'copy_files', + config: { + strategy: 'files', + definition: { + files: [{tomlKey: 'static_root'}, {source: 'src/icon.png', destination: 'assets/icon.png'}], + }, + }, + } + + // When + const result = await executeCopyFilesStep(step, contextWithConfig) + + // Then + expect(fs.copyDirectoryContents).toHaveBeenCalledWith('/test/extension/public', '/test/output') + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/icon.png', '/test/output/assets/icon.png') + expect(result.filesCopied).toBe(2) + }) + }) + + describe('pattern strategy', () => { + test('copies files matching patterns', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/extension/public/logo.png', '/test/extension/public/style.css']) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-public', + displayName: 'Copy Public', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: { + source: 'public', + patterns: ['**/*'], + }, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(2) + expect(fs.copyFile).toHaveBeenCalledTimes(2) + }) + + test('respects ignore patterns', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/extension/public/style.css']) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-public', + displayName: 'Copy Public', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: { + source: 'public', + ignore: ['**/*.png'], + }, + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.glob).toHaveBeenCalledWith(expect.any(Array), expect.objectContaining({ignore: ['**/*.png']})) + }) + + test('copies to destination subdirectory when specified', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue(['/test/extension/public/logo.png']) + vi.mocked(fs.copyFile).mockResolvedValue() + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-public', + displayName: 'Copy Public', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: { + source: 'public', + destination: 'static', + }, + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then + expect(fs.glob).toHaveBeenCalledWith(expect.any(Array), expect.objectContaining({cwd: '/test/extension/public'})) + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/public/logo.png', '/test/output/static/logo.png') + }) + + 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', + definition: { + source: 'src', + preserveStructure: false, + }, + }, + } + + // When + await executeCopyFilesStep(step, mockContext) + + // Then — filename only, no subdirectory + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/src/components/Button.tsx', '/test/output/Button.tsx') + }) + + test('returns zero and warns when no files match', async () => { + // Given + vi.mocked(fs.glob).mockResolvedValue([]) + vi.mocked(fs.mkdir).mockResolvedValue() + + const step: BuildStep = { + id: 'copy-public', + displayName: 'Copy Public', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: {source: 'public'}, + }, + } + + // When + const result = await executeCopyFilesStep(step, mockContext) + + // Then + expect(result.filesCopied).toBe(0) + expect(mockStdout.write).toHaveBeenCalledWith(expect.stringContaining('No files matched patterns')) + }) + + test('throws when source is missing', async () => { + // Given — no source provided + const step: BuildStep = { + id: 'copy-build', + displayName: 'Copy Build', + type: 'copy_files', + config: { + strategy: 'pattern', + definition: {}, + }, + } + + // When/Then + await expect(executeCopyFilesStep(step, mockContext)).rejects.toThrow('Build step "Copy Build" requires a source') + }) + }) +}) 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..735e6be029a --- /dev/null +++ b/packages/app/src/cli/services/build/steps/copy-files-step.ts @@ -0,0 +1,272 @@ +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 {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Files strategy definition. + * + * Each entry in `files` is one of: + * - `{source}` only: copy the directory's contents into the output root. + * - `{source, destination}`: copy the file to an explicit destination path. + * - `{tomlKey}`: resolve a path from the extension's TOML config and copy its + * directory contents into the output root. Silently skipped when the key is absent. + */ +const FilesDefinitionSchema = z.object({ + files: z.array( + z.union([z.object({source: z.string(), destination: z.string().optional()}), z.object({tomlKey: z.string()})]), + ), +}) + +/** + * Pattern strategy definition. + * + * Selects files from a single source directory using glob patterns. + */ +const PatternDefinitionSchema = z.object({ + source: z.string().optional(), + patterns: z.array(z.string()).optional(), + ignore: z.array(z.string()).optional(), + destination: z.string().optional(), + preserveStructure: z.boolean().default(true), +}) + +/** + * Configuration schema for copy_files step. + * Discriminated by strategy; definition shape is tied to the chosen strategy. + */ +const CopyFilesConfigSchema = z.discriminatedUnion('strategy', [ + z.object({ + strategy: z.literal('files'), + definition: FilesDefinitionSchema, + }), + z.object({ + strategy: z.literal('pattern'), + definition: PatternDefinitionSchema, + }), +]) + +/** + * Executes a copy_files build step. + * + * Supports two strategies: + * + * 1. **'files' strategy**: each entry in `definition.files` is either: + * - `{source}` — copy directory contents into the output root. + * - `{source, destination}` — copy a file to an explicit destination path. + * - `{tomlKey}` — resolve a path from the extension's TOML config and copy + * its directory contents into the output root; silently skipped if absent. + * + * 2. **'pattern' strategy**: glob-based file selection from a single source directory. + */ +export async function executeCopyFilesStep(step: BuildStep, context: BuildContext): Promise<{filesCopied: number}> { + const config = CopyFilesConfigSchema.parse(step.config) + const {extension, options} = context + // When outputPath is a file (e.g. index.js, index.wasm), the output directory is its + // parent. When outputPath has no extension, it IS the output directory (copy_files mode + // extensions where outputPath points to a bundle directory, not a single file). + const outputDir = extname(extension.outputPath) ? dirname(extension.outputPath) : extension.outputPath + + switch (config.strategy) { + case 'files': { + return copyFilesList(config.definition.files, extension.directory, outputDir, context, options) + } + case 'pattern': { + const {definition} = config + + if (!definition.source) { + throw new Error(`Build step "${step.displayName}" requires a source`) + } + + const sourceDir = joinPath(extension.directory, definition.source) + const resolvedPatterns = definition.patterns ?? ['**/*'] + const resolvedIgnore = definition.ignore ?? [] + const destinationDir = definition.destination ? joinPath(outputDir, definition.destination) : outputDir + + return copyByPattern( + sourceDir, + destinationDir, + resolvedPatterns, + resolvedIgnore, + definition.preserveStructure, + options, + ) + } + } +} + +/** + * Files strategy — processes a mixed list of `source` and `tomlKey` entries. + */ +async function copyFilesList( + files: ({source: string; destination?: string} | {tomlKey: string})[], + baseDir: string, + outputDir: string, + context: BuildContext, + options: {stdout: NodeJS.WritableStream}, +): Promise<{filesCopied: number}> { + const counts = await Promise.all( + files.map(async (entry) => { + if ('tomlKey' in entry) { + return copyTomlKeyEntry(entry.tomlKey, baseDir, outputDir, context, options) + } + return copySourceEntry(entry.source, entry.destination, baseDir, outputDir, options) + }), + ) + return {filesCopied: counts.reduce((sum, count) => sum + count, 0)} +} + +/** + * Handles a `{source}` or `{source, destination}` files entry. + * + * - No `destination`: copy directory contents into the output root. + * - With `destination`: copy the file to the explicit destination path. + */ +async function copySourceEntry( + source: string, + destination: string | undefined, + baseDir: string, + outputDir: string, + options: {stdout: NodeJS.WritableStream}, +): Promise { + const sourcePath = joinPath(baseDir, source) + const exists = await fileExists(sourcePath) + if (!exists) { + throw new Error(`Source does not exist: ${sourcePath}`) + } + + if (destination !== undefined) { + const destPath = joinPath(outputDir, destination) + await mkdir(dirname(destPath)) + await copyFile(sourcePath, destPath) + options.stdout.write(`Copied ${source} to ${destination}\n`) + return 1 + } + + await copyDirectoryContents(sourcePath, outputDir) + const copied = await glob(['**/*'], {cwd: outputDir, absolute: false}) + options.stdout.write(`Copied contents of ${source} to output root\n`) + return copied.length +} + +/** + * Handles a `{tomlKey}` files entry. + * + * Resolves the key from the extension's TOML config. String values and string + * arrays are each used as source paths. Unresolved keys and missing paths are + * skipped silently with a log message. + */ +async function copyTomlKeyEntry( + key: string, + baseDir: string, + outputDir: string, + context: BuildContext, + options: {stdout: NodeJS.WritableStream}, +): Promise { + const value = getNestedValue(context.extension.configuration, key) + let paths: string[] + if (typeof value === 'string') { + paths = [value] + } else if (Array.isArray(value)) { + paths = value.filter((item): item is string => typeof item === 'string') + } else { + paths = [] + } + + if (paths.length === 0) { + options.stdout.write(`No value for tomlKey '${key}', skipping\n`) + return 0 + } + + const counts = await Promise.all( + paths.map(async (sourcePath) => { + const fullPath = joinPath(baseDir, sourcePath) + const exists = await fileExists(fullPath) + if (!exists) { + options.stdout.write(`Warning: path '${sourcePath}' does not exist, skipping\n`) + return 0 + } + await copyDirectoryContents(fullPath, outputDir) + const copied = await glob(['**/*'], {cwd: outputDir, absolute: false}) + options.stdout.write(`Copied contents of '${sourcePath}' to output root\n`) + return copied.length + }), + ) + return counts.reduce((sum, count) => sum + count, 0) +} + +/** + * Pattern strategy: glob-based file selection. + */ +async function copyByPattern( + sourceDir: string, + outputDir: string, + patterns: string[], + ignore: string[], + preserveStructure: boolean, + options: {stdout: NodeJS.WritableStream}, +): Promise<{filesCopied: number}> { + 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} + } + + await mkdir(outputDir) + + await Promise.all( + files.map(async (filepath) => { + const relPath = preserveStructure ? relativePath(sourceDir, filepath) : basename(filepath) + const destPath = joinPath(outputDir, relPath) + + if (filepath === destPath) return + + await mkdir(dirname(destPath)) + await copyFile(filepath, destPath) + }), + ) + + options.stdout.write(`Copied ${files.length} file(s) from ${sourceDir} to ${outputDir}\n`) + return {filesCopied: files.length} +} + +/** + * Resolves a dot-separated path from a config object. + * Handles TOML array-of-tables by plucking the next key across all elements. + */ +function getNestedValue(obj: {[key: string]: unknown}, path: string): 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)) { + 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 +} diff --git a/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts b/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts new file mode 100644 index 00000000000..02221ef441e --- /dev/null +++ b/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts @@ -0,0 +1,11 @@ +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a copy_static_assets build step. + * + * Copies static assets defined in the extension's build_manifest to the output directory. + * This is a no-op for extensions that do not define static assets. + */ +export async function executeCopyStaticAssetsStep(_step: BuildStep, context: BuildContext): Promise { + return context.extension.copyStaticAssets() +} diff --git a/packages/app/src/cli/services/build/steps/create-tax-stub-step.ts b/packages/app/src/cli/services/build/steps/create-tax-stub-step.ts new file mode 100644 index 00000000000..50bcc09bc12 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/create-tax-stub-step.ts @@ -0,0 +1,14 @@ +import {touchFile, writeFile} from '@shopify/cli-kit/node/fs' +import type {BuildStep, BuildContext} from '../build-steps.js' + +/** + * Executes a create_tax_stub build step. + * + * Creates a minimal JavaScript stub file at the extension's output path, + * satisfying the tax calculation extension bundle format. + */ +export async function executeCreateTaxStubStep(_step: BuildStep, context: BuildContext): Promise { + const {extension} = context + await touchFile(extension.outputPath) + await writeFile(extension.outputPath, '(()=>{})();') +} 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..43b681f8a9a --- /dev/null +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -0,0 +1,52 @@ +import {executeCopyFilesStep} from './copy-files-step.js' +import {executeBuildThemeStep} from './build-theme-step.js' +import {executeBundleThemeStep} from './bundle-theme-step.js' +import {executeBundleUIStep} from './bundle-ui-step.js' +import {executeCopyStaticAssetsStep} from './copy-static-assets-step.js' +import {executeBuildFunctionStep} from './build-function-step.js' +import {executeCreateTaxStubStep} from './create-tax-stub-step.js' +import type {BuildStep, BuildContext} from '../build-steps.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) + + case 'build_theme': + return executeBuildThemeStep(step, context) + + case 'bundle_theme': + return executeBundleThemeStep(step, context) + + case 'bundle_ui': + return executeBundleUIStep(step, context) + + case 'copy_static_assets': + return executeCopyStaticAssetsStep(step, context) + + case 'build_function': + return executeBuildFunctionStep(step, context) + + case 'create_tax_stub': + return executeCreateTaxStubStep(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.`) + + default: + throw new Error(`Unknown build step type: ${(step as {type: string}).type}`) + } +}