Skip to content

Commit feab6d2

Browse files
Abstract build steps to externalize the build configuration
1 parent f9f6e10 commit feab6d2

12 files changed

Lines changed: 1233 additions & 0 deletions
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import {ExtensionBuildOptions} from './extension.js'
2+
import {executeStep, BuildContext} from './build-steps.js'
3+
import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
4+
import {describe, expect, test} from 'vitest'
5+
import {inTemporaryDirectory, writeFile, readFile, mkdir, fileExists} from '@shopify/cli-kit/node/fs'
6+
import {joinPath} from '@shopify/cli-kit/node/path'
7+
import {Writable} from 'stream'
8+
9+
function buildOptions(): ExtensionBuildOptions {
10+
return {
11+
stdout: new Writable({write(chunk, encoding, callback) { callback() }}),
12+
stderr: new Writable({write(chunk, encoding, callback) { callback() }}),
13+
app: {} as any,
14+
environment: 'production',
15+
}
16+
}
17+
18+
describe('build_steps integration', () => {
19+
test('executes copy_files step and copies files to output', async () => {
20+
await inTemporaryDirectory(async (tmpDir) => {
21+
// Setup: Create extension directory with assets
22+
const extensionDir = joinPath(tmpDir, 'extension')
23+
const assetsDir = joinPath(extensionDir, 'assets')
24+
const outputDir = joinPath(tmpDir, 'output')
25+
26+
await mkdir(extensionDir)
27+
await mkdir(assetsDir)
28+
await mkdir(outputDir)
29+
30+
// Create test files
31+
await writeFile(joinPath(assetsDir, 'logo.png'), 'fake-png-data')
32+
await writeFile(joinPath(assetsDir, 'style.css'), 'body { color: red; }')
33+
34+
const mockExtension = {
35+
directory: extensionDir,
36+
outputPath: joinPath(outputDir, 'extension.js'),
37+
} as ExtensionInstance
38+
39+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
40+
41+
await executeStep(
42+
{
43+
id: 'copy-assets',
44+
displayName: 'Copy Assets',
45+
type: 'copy_files',
46+
config: {
47+
strategy: 'pattern',
48+
definition: {source: 'assets', patterns: ['**/*']},
49+
},
50+
},
51+
context,
52+
)
53+
54+
// Verify: Files were copied to output directory
55+
const logoExists = await fileExists(joinPath(outputDir, 'logo.png'))
56+
const styleExists = await fileExists(joinPath(outputDir, 'style.css'))
57+
58+
expect(logoExists).toBe(true)
59+
expect(styleExists).toBe(true)
60+
61+
const logoContent = await readFile(joinPath(outputDir, 'logo.png'))
62+
const styleContent = await readFile(joinPath(outputDir, 'style.css'))
63+
64+
expect(logoContent).toBe('fake-png-data')
65+
expect(styleContent).toBe('body { color: red; }')
66+
})
67+
})
68+
69+
test('executes multiple steps in sequence', async () => {
70+
await inTemporaryDirectory(async (tmpDir) => {
71+
// Setup: Create extension with two asset directories
72+
const extensionDir = joinPath(tmpDir, 'extension')
73+
const imagesDir = joinPath(extensionDir, 'images')
74+
const stylesDir = joinPath(extensionDir, 'styles')
75+
const outputDir = joinPath(tmpDir, 'output')
76+
77+
await mkdir(extensionDir)
78+
await mkdir(imagesDir)
79+
await mkdir(stylesDir)
80+
await mkdir(outputDir)
81+
82+
await writeFile(joinPath(imagesDir, 'logo.png'), 'logo-data')
83+
await writeFile(joinPath(stylesDir, 'main.css'), 'css-data')
84+
85+
const mockExtension = {
86+
directory: extensionDir,
87+
outputPath: joinPath(outputDir, 'extension.js'),
88+
} as ExtensionInstance
89+
90+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
91+
92+
await executeStep(
93+
{
94+
id: 'copy-images',
95+
displayName: 'Copy Images',
96+
type: 'copy_files',
97+
config: {strategy: 'pattern', definition: {source: 'images', patterns: ['**/*'], destination: 'assets/images'}},
98+
},
99+
context,
100+
)
101+
await executeStep(
102+
{
103+
id: 'copy-styles',
104+
displayName: 'Copy Styles',
105+
type: 'copy_files',
106+
config: {strategy: 'pattern', definition: {source: 'styles', patterns: ['**/*'], destination: 'assets/styles'}},
107+
},
108+
context,
109+
)
110+
111+
// Verify: Files from both steps were copied to correct destinations
112+
const logoExists = await fileExists(joinPath(outputDir, 'assets/images/logo.png'))
113+
const styleExists = await fileExists(joinPath(outputDir, 'assets/styles/main.css'))
114+
115+
expect(logoExists).toBe(true)
116+
expect(styleExists).toBe(true)
117+
})
118+
})
119+
120+
test('silently skips tomlKeys step when TOML key is absent from extension config', async () => {
121+
await inTemporaryDirectory(async (tmpDir) => {
122+
const extensionDir = joinPath(tmpDir, 'extension')
123+
const outputDir = joinPath(tmpDir, 'output')
124+
125+
await mkdir(extensionDir)
126+
await mkdir(outputDir)
127+
128+
// Extension has no configuration — static_root key is absent
129+
const mockExtension = {
130+
directory: extensionDir,
131+
outputPath: joinPath(outputDir, 'extension.js'),
132+
configuration: {},
133+
} as unknown as ExtensionInstance
134+
135+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
136+
137+
// Should not throw — absent tomlKeys are silently skipped
138+
await expect(
139+
executeStep(
140+
{
141+
id: 'copy-static',
142+
displayName: 'Copy Static Assets',
143+
type: 'copy_files',
144+
config: {strategy: 'files', definition: {files: [{tomlKey: 'static_root'}]}},
145+
},
146+
context,
147+
),
148+
).resolves.not.toThrow()
149+
})
150+
})
151+
})
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import {executeStep, BuildStep, BuildContext} from './build-steps.js'
2+
import * as stepsIndex from './steps/index.js'
3+
import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
4+
import {beforeEach, describe, expect, test, vi} from 'vitest'
5+
6+
vi.mock('./steps/index.js')
7+
8+
describe('executeStep', () => {
9+
let mockContext: BuildContext
10+
11+
beforeEach(() => {
12+
mockContext = {
13+
extension: {
14+
directory: '/test/dir',
15+
outputPath: '/test/output/index.js',
16+
} as ExtensionInstance,
17+
options: {
18+
stdout: {write: vi.fn()} as any,
19+
stderr: {write: vi.fn()} as any,
20+
app: {} as any,
21+
environment: 'production' as const,
22+
},
23+
stepResults: new Map(),
24+
}
25+
})
26+
27+
const step: BuildStep = {
28+
id: 'test-step',
29+
displayName: 'Test Step',
30+
type: 'copy_files',
31+
config: {},
32+
}
33+
34+
describe('success', () => {
35+
test('returns a successful StepResult with output', async () => {
36+
vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({filesCopied: 3})
37+
38+
const result = await executeStep(step, mockContext)
39+
40+
expect(result.stepId).toBe('test-step')
41+
expect(result.displayName).toBe('Test Step')
42+
expect(result.success).toBe(true)
43+
expect(result.output).toEqual({filesCopied: 3})
44+
expect(result.duration).toBeGreaterThanOrEqual(0)
45+
})
46+
47+
test('logs step execution to stdout', async () => {
48+
vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({})
49+
50+
await executeStep(step, mockContext)
51+
52+
expect(mockContext.options.stdout.write).toHaveBeenCalledWith('Executing step: Test Step\n')
53+
})
54+
})
55+
56+
describe('failure', () => {
57+
test('throws a wrapped error when the step fails', async () => {
58+
vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong'))
59+
60+
await expect(executeStep(step, mockContext)).rejects.toThrow(
61+
'Build step "Test Step" failed: something went wrong',
62+
)
63+
})
64+
65+
test('returns a failure result and logs a warning when continueOnError is true', async () => {
66+
vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong'))
67+
68+
const result = await executeStep({...step, continueOnError: true}, mockContext)
69+
70+
expect(result.success).toBe(false)
71+
expect(result.error?.message).toBe('something went wrong')
72+
expect(mockContext.options.stderr.write).toHaveBeenCalledWith(
73+
'Warning: Step "Test Step" failed but continuing: something went wrong\n',
74+
)
75+
})
76+
})
77+
})
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import {executeStepByType} from './steps/index.js'
2+
import type {ExtensionInstance} from '../../models/extensions/extension-instance.js'
3+
import type {ExtensionBuildOptions} from './extension.js'
4+
5+
/**
6+
* BuildStep represents a single build command configuration.
7+
* Inspired by the existing Task<TContext> pattern in:
8+
* /packages/cli-kit/src/private/node/ui/components/Tasks.tsx
9+
*
10+
* Key differences from Task<TContext>:
11+
* - Not coupled to UI rendering
12+
* - Pure configuration object (execution logic is separate)
13+
* - Router pattern dispatches to type-specific executors
14+
*/
15+
export interface BuildStep {
16+
/** Unique identifier for this step (e.g., 'copy_files', 'build') */
17+
readonly id: string
18+
19+
/** Display name for logging */
20+
readonly displayName: string
21+
22+
/** Optional description */
23+
readonly description?: string
24+
25+
/** Step type (determines which executor handles it) */
26+
readonly type:
27+
| 'copy_files'
28+
| 'build_theme'
29+
| 'bundle_theme'
30+
| 'bundle_ui'
31+
| 'copy_static_assets'
32+
| 'build_function'
33+
| 'create_tax_stub'
34+
| 'esbuild'
35+
| 'validate'
36+
| 'transform'
37+
| 'custom'
38+
39+
/** Step-specific configuration */
40+
readonly config: {[key: string]: unknown}
41+
42+
/**
43+
* Whether to continue on error (default: false)
44+
*/
45+
readonly continueOnError?: boolean
46+
}
47+
48+
/**
49+
* BuildContext is passed through the pipeline (similar to Task<TContext>).
50+
* Each step can read from and write to the context.
51+
*
52+
* Key design: Immutable configuration, mutable context
53+
*/
54+
export interface BuildContext {
55+
/** The extension being built */
56+
readonly extension: ExtensionInstance
57+
58+
/** Build options (stdout, stderr, etc.) */
59+
readonly options: ExtensionBuildOptions
60+
61+
/** Results from previous steps (for step dependencies) */
62+
readonly stepResults: Map<string, StepResult>
63+
64+
/** Custom data that steps can write to (extensible) */
65+
[key: string]: unknown
66+
}
67+
68+
/**
69+
* Result of a step execution
70+
*/
71+
export interface StepResult {
72+
readonly stepId: string
73+
readonly displayName: string
74+
readonly success: boolean
75+
readonly duration: number
76+
readonly output?: unknown
77+
readonly error?: Error
78+
}
79+
80+
/**
81+
* BuildStepsConfig defines the pipeline configuration.
82+
*/
83+
export interface BuildStepsConfig {
84+
/** Array of steps to execute in order */
85+
readonly steps: ReadonlyArray<BuildStep>
86+
}
87+
88+
/**
89+
* Executes a single build step with error handling and skip logic.
90+
*/
91+
export async function executeStep(step: BuildStep, context: BuildContext): Promise<StepResult> {
92+
const startTime = Date.now()
93+
94+
try {
95+
// Execute the step using type-specific executor
96+
context.options.stdout.write(`Executing step: ${step.displayName}\n`)
97+
const output = await executeStepByType(step, context)
98+
99+
return {
100+
stepId: step.id,
101+
displayName: step.displayName,
102+
success: true,
103+
duration: Date.now() - startTime,
104+
output,
105+
}
106+
} catch (error) {
107+
const stepError = error as Error
108+
109+
if (step.continueOnError) {
110+
context.options.stderr.write(`Warning: Step "${step.displayName}" failed but continuing: ${stepError.message}\n`)
111+
return {
112+
stepId: step.id,
113+
displayName: step.displayName,
114+
success: false,
115+
duration: Date.now() - startTime,
116+
error: stepError,
117+
}
118+
}
119+
120+
throw new Error(`Build step "${step.displayName}" failed: ${stepError.message}`)
121+
}
122+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import {buildFunctionExtension} from '../extension.js'
2+
import type {BuildStep, BuildContext} from '../build-steps.js'
3+
4+
/**
5+
* Executes a build_function build step.
6+
*
7+
* Compiles the function extension (JavaScript or other language) to WASM,
8+
* applying wasm-opt and trampoline as configured.
9+
*/
10+
export async function executeBuildFunctionStep(_step: BuildStep, context: BuildContext): Promise<void> {
11+
return buildFunctionExtension(context.extension, context.options)
12+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import {runThemeCheck} from '../theme-check.js'
2+
import type {BuildStep, BuildContext} from '../build-steps.js'
3+
4+
/**
5+
* Executes a build_theme build step.
6+
*
7+
* Runs theme check on the extension directory and writes any offenses to stdout.
8+
*/
9+
export async function executeBuildThemeStep(_step: BuildStep, context: BuildContext): Promise<void> {
10+
const {extension, options} = context
11+
options.stdout.write(`Running theme check on your Theme app extension...`)
12+
const offenses = await runThemeCheck(extension.directory)
13+
if (offenses) options.stdout.write(offenses)
14+
}

0 commit comments

Comments
 (0)