Skip to content

Commit 0945d47

Browse files
alfonso-noriegaalfonso-noriega
committed
Abstract build steps to externalize the build configuration
1 parent 174ca57 commit 0945d47

22 files changed

+864
-65
lines changed

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ vi.mock('../../services/build/extension.js', async () => {
3232
return {
3333
...actual,
3434
buildUIExtension: vi.fn(),
35-
buildThemeExtension: vi.fn(),
3635
buildFunctionExtension: vi.fn(),
3736
}
3837
})
@@ -148,8 +147,16 @@ describe('build', async () => {
148147
// Given
149148
const extensionInstance = await testTaxCalculationExtension(tmpDir)
150149
const options: ExtensionBuildOptions = {
151-
stdout: new Writable(),
152-
stderr: new Writable(),
150+
stdout: new Writable({
151+
write(chunk, enc, cb) {
152+
cb()
153+
},
154+
}),
155+
stderr: new Writable({
156+
write(chunk, enc, cb) {
157+
cb()
158+
},
159+
}),
153160
app: testApp(),
154161
environment: 'production',
155162
}

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

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,8 @@ import {WebhooksSpecIdentifier} from './specifications/app_config_webhook.js'
1212
import {WebhookSubscriptionSpecIdentifier} from './specifications/app_config_webhook_subscription.js'
1313
import {EventsSpecIdentifier} from './specifications/app_config_events.js'
1414
import {HostedAppHomeSpecIdentifier} from './specifications/app_config_hosted_app_home.js'
15-
import {
16-
ExtensionBuildOptions,
17-
buildFunctionExtension,
18-
buildThemeExtension,
19-
buildUIExtension,
20-
bundleFunctionExtension,
21-
} from '../../services/build/extension.js'
22-
import {bundleThemeExtension, copyFilesForExtension} from '../../services/extensions/bundle.js'
15+
import {ExtensionBuildOptions, bundleFunctionExtension} from '../../services/build/extension.js'
16+
import {bundleThemeExtension} from '../../services/extensions/bundle.js'
2317
import {Identifiers} from '../app/identifiers.js'
2418
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
2519
import {AppConfigurationWithoutPath} from '../app/app.js'
@@ -29,11 +23,12 @@ import {constantize, slugify} from '@shopify/cli-kit/common/string'
2923
import {hashString, nonRandomUUID} from '@shopify/cli-kit/node/crypto'
3024
import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn'
3125
import {joinPath, basename, normalizePath, resolvePath} from '@shopify/cli-kit/node/path'
32-
import {fileExists, touchFile, moveFile, writeFile, glob, copyFile, globSync} from '@shopify/cli-kit/node/fs'
26+
import {fileExists, moveFile, glob, copyFile, globSync} from '@shopify/cli-kit/node/fs'
3327
import {getPathValue} from '@shopify/cli-kit/common/object'
3428
import {outputDebug} from '@shopify/cli-kit/node/output'
3529
import {extractJSImports, extractImportPathsRecursively} from '@shopify/cli-kit/node/import-extractor'
3630
import {uniq} from '@shopify/cli-kit/common/array'
31+
import { BuildContext } from '../../services/build/client-steps.js'
3732

3833
export const CONFIG_EXTENSION_IDS: string[] = [
3934
AppAccessSpecIdentifier,
@@ -140,7 +135,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
140135

141136
get outputFileName() {
142137
const mode = this.specification.buildConfig.mode
143-
if (mode === 'copy_files' || mode === 'theme') {
138+
if (mode === 'hosted_app_home' || mode === 'theme') {
144139
return ''
145140
} else if (mode === 'function') {
146141
return 'index.wasm'
@@ -350,34 +345,26 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
350345
}
351346

352347
async build(options: ExtensionBuildOptions): Promise<void> {
353-
const mode = this.specification.buildConfig.mode
348+
const {buildConfig, clientSteps} = this.specification
349+
350+
const context: BuildContext = {
351+
extension: this,
352+
options,
353+
stepResults: new Map(),
354+
signal: options.signal,
355+
}
354356

355-
switch (mode) {
356-
case 'theme':
357-
await buildThemeExtension(this, options)
358-
return bundleThemeExtension(this, options)
359-
case 'function':
360-
return buildFunctionExtension(this, options)
361-
case 'ui':
362-
await buildUIExtension(this, options)
363-
// Copy static assets after build completes
364-
return this.copyStaticAssets()
365-
case 'tax_calculation':
366-
await touchFile(this.outputPath)
367-
await writeFile(this.outputPath, '(()=>{})();')
368-
break
369-
case 'copy_files':
370-
return copyFilesForExtension(
371-
this,
372-
options,
373-
this.specification.buildConfig.filePatterns ?? [],
374-
this.specification.buildConfig.ignoredFilePatterns ?? [],
375-
)
376-
case 'hosted_app_home':
377-
await this.copyStaticAssets()
378-
break
379-
case 'none':
380-
break
357+
const steps = clientSteps.
358+
filter((lifecycle) => lifecycle.lifecycle === 'deploy').flatMap((lifecycle) => lifecycle.steps)
359+
360+
for (const step of steps) {
361+
// eslint-disable-next-line no-await-in-loop
362+
const result = await executeStep(step, context)
363+
context.stepResults.set(step.id, result)
364+
365+
if (!result.success && !step.continueOnError) {
366+
throw new Error(`Build step "${step.name}" failed: ${result.error?.message}`)
367+
}
381368
}
382369
}
383370

‎packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('hosted_app_home', () => {
9090

9191
describe('buildConfig', () => {
9292
test('should have hosted_app_home build mode', () => {
93-
expect(spec.buildConfig).toEqual({mode: 'hosted_app_home'})
93+
expect(spec.buildConfig).toEqual({mode: 'none'})
9494
})
9595
})
9696

‎packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export const HostedAppHomeSpecIdentifier = 'hosted_app_home'
1616

1717
const hostedAppHomeSpec = createConfigExtensionSpecification({
1818
identifier: HostedAppHomeSpecIdentifier,
19-
buildConfig: {mode: 'hosted_app_home'} as const,
19+
buildConfig: {mode: 'none'} as const,
2020
schema: HostedAppHomeSchema,
2121
transformConfig: HostedAppHomeTransformConfig,
2222
copyStaticAssets: async (config, directory, outputPath) => {

‎packages/app/src/cli/models/extensions/specifications/channel.test.ts‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
import spec from './channel.js'
2+
import {ExtensionInstance} from '../extension-instance.js'
3+
import {ExtensionBuildOptions} from '../../../services/build/extension.js'
4+
import {describe, expect, test} from 'vitest'
5+
import {inTemporaryDirectory, writeFile, fileExists, mkdir} from '@shopify/cli-kit/node/fs'
6+
import {joinPath} from '@shopify/cli-kit/node/path'
7+
import {Writable} from 'stream'
8+
9+
const SUBDIRECTORY = 'specifications'
10+
11+
describe('channel_config', () => {
12+
describe('buildConfig', () => {
13+
test('uses prepare_assets mode', () => {
14+
expect(spec.buildConfig.mode).toBe('prepare_assets')
15+
})
16+
17+
test('has a single copy-files step scoped to the specifications subdirectory', () => {
18+
if (spec.buildConfig.mode === 'none') throw new Error('Expected prepare_assets mode')
19+
20+
expect(spec.buildConfig.lifecycles[0]!.steps).toHaveLength(1)
21+
expect(spec.buildConfig.lifecycles[0]!.steps[0]).toMatchObject({
22+
id: 'copy-files',
23+
type: 'prepare_assets',
24+
config: {
25+
strategy: 'pattern',
26+
definition: {source: '.'},
27+
},
28+
})
29+
30+
const {patterns} = (spec.buildConfig.lifecycles[0]!.steps[0]!.config as {definition: {patterns: string[]}}).definition
31+
32+
expect(patterns).toEqual(
33+
expect.arrayContaining([
34+
`${SUBDIRECTORY}/**/*.json`,
35+
`${SUBDIRECTORY}/**/*.toml`,
36+
`${SUBDIRECTORY}/**/*.yaml`,
37+
`${SUBDIRECTORY}/**/*.yml`,
38+
`${SUBDIRECTORY}/**/*.svg`,
39+
]),
40+
)
41+
})
42+
43+
test('config is serializable to JSON', () => {
44+
if (spec.buildConfig.mode === 'none') throw new Error('Expected prepare_assets mode')
45+
46+
const serialized = JSON.stringify(spec.buildConfig)
47+
const deserialized = JSON.parse(serialized)
48+
49+
expect(deserialized.lifecycles[0].steps).toHaveLength(1)
50+
expect(deserialized.lifecycles[0].steps[0].config.strategy).toBe('pattern')
51+
})
52+
})
53+
54+
describe('build integration', () => {
55+
test('copies specification files to output, preserving subdirectory structure', async () => {
56+
await inTemporaryDirectory(async (tmpDir) => {
57+
// Given
58+
const extensionDir = joinPath(tmpDir, 'extension')
59+
const specsDir = joinPath(extensionDir, SUBDIRECTORY)
60+
const outputDir = joinPath(tmpDir, 'output')
61+
62+
await mkdir(specsDir)
63+
await mkdir(outputDir)
64+
65+
await writeFile(joinPath(specsDir, 'product.json'), '{}')
66+
await writeFile(joinPath(specsDir, 'order.toml'), '[spec]')
67+
await writeFile(joinPath(specsDir, 'logo.svg'), '<svg/>')
68+
// Root-level files should NOT be copied
69+
await writeFile(joinPath(extensionDir, 'README.md'), '# readme')
70+
await writeFile(joinPath(extensionDir, 'index.js'), 'ignored')
71+
72+
const extension = new ExtensionInstance({
73+
configuration: {name: 'my-channel', type: 'channel'},
74+
configurationPath: '',
75+
directory: extensionDir,
76+
specification: spec,
77+
})
78+
extension.outputPath = outputDir
79+
80+
const buildOptions: ExtensionBuildOptions = {
81+
stdout: new Writable({
82+
write(chunk, enc, cb) {
83+
cb()
84+
},
85+
}),
86+
stderr: new Writable({
87+
write(chunk, enc, cb) {
88+
cb()
89+
},
90+
}),
91+
app: {} as any,
92+
environment: 'production',
93+
}
94+
95+
// When
96+
await extension.build(buildOptions)
97+
98+
// Then — specification files copied with path preserved
99+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'product.json'))).resolves.toBe(true)
100+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'order.toml'))).resolves.toBe(true)
101+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'logo.svg'))).resolves.toBe(true)
102+
103+
// Root-level files not in specifications/ are not copied
104+
await expect(fileExists(joinPath(outputDir, 'README.md'))).resolves.toBe(false)
105+
await expect(fileExists(joinPath(outputDir, 'index.js'))).resolves.toBe(false)
106+
})
107+
})
108+
109+
test('does not copy files with non-matching extensions inside specifications/', async () => {
110+
await inTemporaryDirectory(async (tmpDir) => {
111+
// Given
112+
const extensionDir = joinPath(tmpDir, 'extension')
113+
const specsDir = joinPath(extensionDir, SUBDIRECTORY)
114+
const outputDir = joinPath(tmpDir, 'output')
115+
116+
await mkdir(specsDir)
117+
await mkdir(outputDir)
118+
119+
await writeFile(joinPath(specsDir, 'spec.json'), '{}')
120+
await writeFile(joinPath(specsDir, 'ignored.ts'), 'const x = 1')
121+
await writeFile(joinPath(specsDir, 'ignored.js'), 'const x = 1')
122+
123+
const extension = new ExtensionInstance({
124+
configuration: {name: 'my-channel', type: 'channel'},
125+
configurationPath: '',
126+
directory: extensionDir,
127+
specification: spec,
128+
})
129+
extension.outputPath = outputDir
130+
131+
const buildOptions: ExtensionBuildOptions = {
132+
stdout: new Writable({
133+
write(chunk, enc, cb) {
134+
cb()
135+
},
136+
}),
137+
stderr: new Writable({
138+
write(chunk, enc, cb) {
139+
cb()
140+
},
141+
}),
142+
app: {} as any,
143+
environment: 'production',
144+
}
145+
146+
// When
147+
await extension.build(buildOptions)
148+
149+
// Then
150+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'spec.json'))).resolves.toBe(true)
151+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'ignored.ts'))).resolves.toBe(false)
152+
await expect(fileExists(joinPath(outputDir, SUBDIRECTORY, 'ignored.js'))).resolves.toBe(false)
153+
})
154+
})
155+
})
156+
})

‎packages/app/src/cli/models/extensions/specifications/channel.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,26 @@ const FILE_EXTENSIONS = ['json', 'toml', 'yaml', 'yml', 'svg']
77
const channelSpecificationSpec = createContractBasedModuleSpecification({
88
identifier: 'channel_config',
99
buildConfig: {
10-
mode: 'copy_files',
11-
filePatterns: FILE_EXTENSIONS.map((ext) => joinPath(SUBDIRECTORY_NAME, '**', `*.${ext}`)),
10+
mode: 'prepare_assets',
11+
lifecycles: [
12+
{
13+
lifecycle: 'deploy',
14+
steps: [
15+
{
16+
id: 'copy-files',
17+
displayName: 'Copy Files',
18+
type: 'prepare_assets',
19+
config: {
20+
strategy: 'pattern',
21+
definition: {
22+
source: '.',
23+
patterns: FILE_EXTENSIONS.map((ext) => joinPath(SUBDIRECTORY_NAME, '**', `*.${ext}`)),
24+
},
25+
},
26+
},
27+
],
28+
},
29+
],
1230
},
1331
appModuleFeatures: () => [],
1432
})

‎packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,18 @@ const checkoutPostPurchaseSpec = createExtensionSpecification({
1414
partnersWebIdentifier: 'post_purchase',
1515
schema: CheckoutPostPurchaseSchema,
1616
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path'],
17-
buildConfig: {mode: 'ui'},
17+
buildConfig: {
18+
mode: 'ui',
19+
lifecycles: [
20+
{
21+
lifecycle: 'deploy',
22+
steps: [
23+
{id: 'bundle-ui', displayName: 'Bundle UI Extension', type: 'bundle_ui', config: {}},
24+
{id: 'copy-static-assets', displayName: 'Copy Static Assets', type: 'copy_static_assets', config: {}},
25+
],
26+
},
27+
],
28+
},
1829
deployConfig: async (config, _) => {
1930
return {metafields: config.metafields ?? []}
2031
},

‎packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,18 @@ const checkoutSpec = createExtensionSpecification({
2121
dependency,
2222
schema: CheckoutSchema,
2323
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path', 'generates_source_maps'],
24-
buildConfig: {mode: 'ui'},
24+
buildConfig: {
25+
mode: 'ui',
26+
lifecycles: [
27+
{
28+
lifecycle: 'deploy',
29+
steps: [
30+
{id: 'bundle-ui', displayName: 'Bundle UI Extension', type: 'bundle_ui', config: {}},
31+
{id: 'copy-static-assets', displayName: 'Copy Static Assets', type: 'copy_static_assets', config: {}},
32+
],
33+
},
34+
],
35+
},
2536
deployConfig: async (config, directory) => {
2637
return {
2738
extension_points: config.extension_points,

0 commit comments

Comments
 (0)