Skip to content

Commit 445a4c4

Browse files
Abstract build steps to externalize the build configuration
1 parent ff3f4e0 commit 445a4c4

12 files changed

+1324
-0
lines changed
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
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({
12+
write(chunk, encoding, callback) {
13+
callback()
14+
},
15+
}),
16+
stderr: new Writable({
17+
write(chunk, encoding, callback) {
18+
callback()
19+
},
20+
}),
21+
app: {} as any,
22+
environment: 'production',
23+
}
24+
}
25+
26+
describe('build_steps integration', () => {
27+
test('executes copy_files step and copies files to output', async () => {
28+
await inTemporaryDirectory(async (tmpDir) => {
29+
// Setup: Create extension directory with assets
30+
const extensionDir = joinPath(tmpDir, 'extension')
31+
const assetsDir = joinPath(extensionDir, 'assets')
32+
const outputDir = joinPath(tmpDir, 'output')
33+
34+
await mkdir(extensionDir)
35+
await mkdir(assetsDir)
36+
await mkdir(outputDir)
37+
38+
// Create test files
39+
await writeFile(joinPath(assetsDir, 'logo.png'), 'fake-png-data')
40+
await writeFile(joinPath(assetsDir, 'style.css'), 'body { color: red; }')
41+
42+
const mockExtension = {
43+
directory: extensionDir,
44+
outputPath: joinPath(outputDir, 'extension.js'),
45+
} as ExtensionInstance
46+
47+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
48+
49+
await executeStep(
50+
{
51+
id: 'copy-assets',
52+
displayName: 'Copy Assets',
53+
type: 'copy_files',
54+
config: {
55+
strategy: 'pattern',
56+
definition: {source: 'assets', patterns: ['**/*']},
57+
},
58+
},
59+
context,
60+
)
61+
62+
// Verify: Files were copied to output directory
63+
const logoExists = await fileExists(joinPath(outputDir, 'logo.png'))
64+
const styleExists = await fileExists(joinPath(outputDir, 'style.css'))
65+
66+
expect(logoExists).toBe(true)
67+
expect(styleExists).toBe(true)
68+
69+
const logoContent = await readFile(joinPath(outputDir, 'logo.png'))
70+
const styleContent = await readFile(joinPath(outputDir, 'style.css'))
71+
72+
expect(logoContent).toBe('fake-png-data')
73+
expect(styleContent).toBe('body { color: red; }')
74+
})
75+
})
76+
77+
test('executes multiple steps in sequence', async () => {
78+
await inTemporaryDirectory(async (tmpDir) => {
79+
// Setup: Create extension with two asset directories
80+
const extensionDir = joinPath(tmpDir, 'extension')
81+
const imagesDir = joinPath(extensionDir, 'images')
82+
const stylesDir = joinPath(extensionDir, 'styles')
83+
const outputDir = joinPath(tmpDir, 'output')
84+
85+
await mkdir(extensionDir)
86+
await mkdir(imagesDir)
87+
await mkdir(stylesDir)
88+
await mkdir(outputDir)
89+
90+
await writeFile(joinPath(imagesDir, 'logo.png'), 'logo-data')
91+
await writeFile(joinPath(stylesDir, 'main.css'), 'css-data')
92+
93+
const mockExtension = {
94+
directory: extensionDir,
95+
outputPath: joinPath(outputDir, 'extension.js'),
96+
} as ExtensionInstance
97+
98+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
99+
100+
await executeStep(
101+
{
102+
id: 'copy-images',
103+
displayName: 'Copy Images',
104+
type: 'copy_files',
105+
config: {
106+
strategy: 'pattern',
107+
definition: {source: 'images', patterns: ['**/*'], destination: 'assets/images'},
108+
},
109+
},
110+
context,
111+
)
112+
await executeStep(
113+
{
114+
id: 'copy-styles',
115+
displayName: 'Copy Styles',
116+
type: 'copy_files',
117+
config: {
118+
strategy: 'pattern',
119+
definition: {source: 'styles', patterns: ['**/*'], destination: 'assets/styles'},
120+
},
121+
},
122+
context,
123+
)
124+
125+
// Verify: Files from both steps were copied to correct destinations
126+
const logoExists = await fileExists(joinPath(outputDir, 'assets/images/logo.png'))
127+
const styleExists = await fileExists(joinPath(outputDir, 'assets/styles/main.css'))
128+
129+
expect(logoExists).toBe(true)
130+
expect(styleExists).toBe(true)
131+
})
132+
})
133+
134+
test('silently skips tomlKeys step when TOML key is absent from extension config', async () => {
135+
await inTemporaryDirectory(async (tmpDir) => {
136+
const extensionDir = joinPath(tmpDir, 'extension')
137+
const outputDir = joinPath(tmpDir, 'output')
138+
139+
await mkdir(extensionDir)
140+
await mkdir(outputDir)
141+
142+
// Extension has no configuration — static_root key is absent
143+
const mockExtension = {
144+
directory: extensionDir,
145+
outputPath: joinPath(outputDir, 'extension.js'),
146+
configuration: {},
147+
} as unknown as ExtensionInstance
148+
149+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
150+
151+
// Should not throw — absent tomlKeys are silently skipped
152+
await expect(
153+
executeStep(
154+
{
155+
id: 'copy-static',
156+
displayName: 'Copy Static Assets',
157+
type: 'copy_files',
158+
config: {strategy: 'files', definition: {files: [{tomlKey: 'static_root'}]}},
159+
},
160+
context,
161+
),
162+
).resolves.not.toThrow()
163+
})
164+
})
165+
})
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: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
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+
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+
* Executes a single build step with error handling and skip logic.
82+
*/
83+
export async function executeStep(step: BuildStep, context: BuildContext): Promise<StepResult> {
84+
const startTime = Date.now()
85+
86+
try {
87+
// Execute the step using type-specific executor
88+
context.options.stdout.write(`Executing step: ${step.displayName}\n`)
89+
const output = await executeStepByType(step, context)
90+
91+
return {
92+
stepId: step.id,
93+
displayName: step.displayName,
94+
success: true,
95+
duration: Date.now() - startTime,
96+
output,
97+
}
98+
} catch (error) {
99+
const stepError = error as Error
100+
101+
if (step.continueOnError) {
102+
context.options.stderr.write(`Warning: Step "${step.displayName}" failed but continuing: ${stepError.message}\n`)
103+
return {
104+
stepId: step.id,
105+
displayName: step.displayName,
106+
success: false,
107+
duration: Date.now() - startTime,
108+
error: stepError,
109+
}
110+
}
111+
112+
throw new Error(`Build step "${step.displayName}" failed: ${stepError.message}`)
113+
}
114+
}
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)