Skip to content

Commit 9964055

Browse files
Abstract build steps to externalize the build configuration
1 parent ae7ed38 commit 9964055

14 files changed

+1333
-8
lines changed

packages/app/src/cli/models/extensions/extension-instance.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,8 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
370370
return copyFilesForExtension(
371371
this,
372372
options,
373-
this.specification.buildConfig.filePatterns,
374-
this.specification.buildConfig.ignoredFilePatterns,
373+
this.specification.buildConfig.filePatterns ?? [],
374+
this.specification.buildConfig.ignoredFilePatterns ?? [],
375375
)
376376
case 'hosted_app_home':
377377
await this.copyStaticAssets()

packages/app/src/cli/models/extensions/specification.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {ZodSchemaType, BaseConfigType, BaseSchema} from './schemas.js'
22
import {ExtensionInstance} from './extension-instance.js'
33
import {blocks} from '../../constants.js'
4+
import {ClientSteps} from '../../services/build/client-steps.js'
45

56
import {Flag} from '../../utilities/developer-platform-client.js'
67
import {AppConfigurationWithoutPath} from '../app/app.js'
@@ -53,9 +54,14 @@ export interface BuildAsset {
5354
static?: boolean
5455
}
5556

56-
type BuildConfig =
57-
| {mode: 'ui' | 'theme' | 'function' | 'tax_calculation' | 'none' | 'hosted_app_home'}
58-
| {mode: 'copy_files'; filePatterns: string[]; ignoredFilePatterns?: string[]}
57+
type BuildMode = 'copy_files' | 'theme' | 'function' | 'ui' | 'tax_calculation' | 'hosted_app_home' | 'none'
58+
59+
interface BuildConfig {
60+
mode: BuildMode
61+
filePatterns?: string[]
62+
ignoredFilePatterns?: string[]
63+
}
64+
5965
/**
6066
* Extension specification with all the needed properties and methods to load an extension.
6167
*/
@@ -69,6 +75,7 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
6975
surface: string
7076
registrationLimit: number
7177
experience: ExtensionExperience
78+
clientSteps: ClientSteps
7279
buildConfig: BuildConfig
7380
dependency?: string
7481
graphQLType?: string
@@ -203,6 +210,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
203210
experience: spec.experience ?? 'extension',
204211
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
205212
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
213+
clientSteps: spec.clientSteps ?? [],
206214
buildConfig: spec.buildConfig ?? {mode: 'none'},
207215
}
208216
const merged = {...defaults, ...spec}
@@ -245,6 +253,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
245253
export function createConfigExtensionSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(spec: {
246254
identifier: string
247255
schema: ZodSchemaType<TConfiguration>
256+
clientSteps?: ClientSteps
248257
buildConfig?: BuildConfig
249258
appModuleFeatures?: (config?: TConfiguration) => ExtensionFeature[]
250259
transformConfig: TransformationConfig | CustomTransformationConfig
@@ -264,21 +273,23 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
264273
transformRemoteToLocal: resolveReverseAppConfigTransform(spec.schema, spec.transformConfig),
265274
experience: 'configuration',
266275
uidStrategy: spec.uidStrategy ?? 'single',
267-
buildConfig: spec.buildConfig ?? {mode: 'none'},
276+
clientSteps: spec.clientSteps ?? [],
277+
buildConfig: spec.buildConfig,
268278
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
269279
patchWithAppDevURLs: spec.patchWithAppDevURLs,
270280
copyStaticAssets: spec.copyStaticAssets,
271281
})
272282
}
273283

274284
export function createContractBasedModuleSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
275-
spec: Pick<CreateExtensionSpecType<TConfiguration>, 'identifier' | 'appModuleFeatures' | 'buildConfig'>,
285+
spec: Pick<CreateExtensionSpecType<TConfiguration>, 'identifier' | 'appModuleFeatures' | 'clientSteps' | 'buildConfig'>,
276286
) {
277287
return createExtensionSpecification({
278288
identifier: spec.identifier,
279289
schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
280290
appModuleFeatures: spec.appModuleFeatures,
281-
buildConfig: spec.buildConfig ?? {mode: 'none'},
291+
clientSteps: spec.clientSteps ?? [],
292+
buildConfig: spec.buildConfig,
282293
deployConfig: async (config, directory) => {
283294
let parsedConfig = configWithoutFirstClassFields(config)
284295
if (spec.appModuleFeatures().includes('localization')) {
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import {ExtensionBuildOptions} from './extension.js'
2+
import {executeStep, BuildContext} from './client-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('client_steps integration', () => {
27+
test('executes include_assets 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+
name: 'Copy Assets',
53+
type: 'include_assets',
54+
config: {
55+
inclusions: [{type: 'pattern', baseDir: 'assets', include: ['**/*']}],
56+
},
57+
},
58+
context,
59+
)
60+
61+
// Verify: Files were copied to output directory
62+
const logoExists = await fileExists(joinPath(outputDir, 'logo.png'))
63+
const styleExists = await fileExists(joinPath(outputDir, 'style.css'))
64+
65+
expect(logoExists).toBe(true)
66+
expect(styleExists).toBe(true)
67+
68+
const logoContent = await readFile(joinPath(outputDir, 'logo.png'))
69+
const styleContent = await readFile(joinPath(outputDir, 'style.css'))
70+
71+
expect(logoContent).toBe('fake-png-data')
72+
expect(styleContent).toBe('body { color: red; }')
73+
})
74+
})
75+
76+
test('executes multiple steps in sequence', async () => {
77+
await inTemporaryDirectory(async (tmpDir) => {
78+
// Setup: Create extension with two asset directories
79+
const extensionDir = joinPath(tmpDir, 'extension')
80+
const imagesDir = joinPath(extensionDir, 'images')
81+
const stylesDir = joinPath(extensionDir, 'styles')
82+
const outputDir = joinPath(tmpDir, 'output')
83+
84+
await mkdir(extensionDir)
85+
await mkdir(imagesDir)
86+
await mkdir(stylesDir)
87+
await mkdir(outputDir)
88+
89+
await writeFile(joinPath(imagesDir, 'logo.png'), 'logo-data')
90+
await writeFile(joinPath(stylesDir, 'main.css'), 'css-data')
91+
92+
const mockExtension = {
93+
directory: extensionDir,
94+
outputPath: joinPath(outputDir, 'extension.js'),
95+
} as ExtensionInstance
96+
97+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
98+
99+
await executeStep(
100+
{
101+
id: 'copy-images',
102+
name: 'Copy Images',
103+
type: 'include_assets',
104+
config: {
105+
inclusions: [{type: 'pattern', baseDir: 'images', include: ['**/*'], destination: 'assets/images'}],
106+
},
107+
},
108+
context,
109+
)
110+
await executeStep(
111+
{
112+
id: 'copy-styles',
113+
name: 'Copy Styles',
114+
type: 'include_assets',
115+
config: {
116+
inclusions: [{type: 'pattern', baseDir: 'styles', include: ['**/*'], destination: 'assets/styles'}],
117+
},
118+
},
119+
context,
120+
)
121+
122+
// Verify: Files from both steps were copied to correct destinations
123+
const logoExists = await fileExists(joinPath(outputDir, 'assets/images/logo.png'))
124+
const styleExists = await fileExists(joinPath(outputDir, 'assets/styles/main.css'))
125+
126+
expect(logoExists).toBe(true)
127+
expect(styleExists).toBe(true)
128+
})
129+
})
130+
131+
test('silently skips configKey step when config key is absent from extension config', async () => {
132+
await inTemporaryDirectory(async (tmpDir) => {
133+
const extensionDir = joinPath(tmpDir, 'extension')
134+
const outputDir = joinPath(tmpDir, 'output')
135+
136+
await mkdir(extensionDir)
137+
await mkdir(outputDir)
138+
139+
// Extension has no configuration — static_root key is absent
140+
const mockExtension = {
141+
directory: extensionDir,
142+
outputPath: joinPath(outputDir, 'extension.js'),
143+
configuration: {},
144+
} as unknown as ExtensionInstance
145+
146+
const context: BuildContext = {extension: mockExtension, options: buildOptions(), stepResults: new Map()}
147+
148+
// Should not throw — absent configKey values are silently skipped
149+
await expect(
150+
executeStep(
151+
{
152+
id: 'copy-static-assets',
153+
name: 'Copy Static Assets',
154+
type: 'include_assets',
155+
config: {inclusions: [{type: 'configKey', configKey: 'static_root'}]},
156+
},
157+
context,
158+
),
159+
).resolves.not.toThrow()
160+
})
161+
})
162+
})
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import {executeStep, ClientStep, BuildContext} from './client-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: ClientStep = {
28+
id: 'test-step',
29+
name: 'Test Step',
30+
type: 'include_assets',
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.id).toBe('test-step')
41+
expect(result.success).toBe(true)
42+
expect(result.output).toEqual({filesCopied: 3})
43+
expect(result.duration).toBeGreaterThanOrEqual(0)
44+
})
45+
46+
test('logs step execution to stdout', async () => {
47+
vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({})
48+
49+
await executeStep(step, mockContext)
50+
51+
expect(mockContext.options.stdout.write).toHaveBeenCalledWith('Executing step: Test Step\n')
52+
})
53+
})
54+
55+
describe('failure', () => {
56+
test('throws a wrapped error when the step fails', async () => {
57+
vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong'))
58+
59+
await expect(executeStep(step, mockContext)).rejects.toThrow(
60+
'Build step "Test Step" failed: something went wrong',
61+
)
62+
})
63+
64+
test('returns a failure result and logs a warning when continueOnError is true', async () => {
65+
vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong'))
66+
67+
const result = await executeStep({...step, continueOnError: true}, mockContext)
68+
69+
expect(result.success).toBe(false)
70+
expect(result.error?.message).toBe('something went wrong')
71+
expect(mockContext.options.stderr.write).toHaveBeenCalledWith(
72+
'Warning: Step "Test Step" failed but continuing: something went wrong\n',
73+
)
74+
})
75+
})
76+
})

0 commit comments

Comments
 (0)