Skip to content

Commit f042eff

Browse files
alfonso-noriegaalfonso-noriega
committed
Abstract build steps to externalize the build configuration
1 parent 61f4d35 commit f042eff

File tree

3 files changed

+53
-52
lines changed

3 files changed

+53
-52
lines changed

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ export interface BuildAsset {
5454
static?: boolean
5555
}
5656

57-
type BuildConfig =
58-
| {mode: 'none'}
59-
| {mode: 'ui' | 'theme' | 'function' | 'tax_calculation' | 'copy_files'; steps: ReadonlyArray<BuildStep>}
57+
interface BuildConfig {
58+
mode: 'ui' | 'theme' | 'function' | 'tax_calculation' | 'copy_files' | 'none'
59+
steps: ReadonlyArray<BuildStep>
60+
}
6061
/**
6162
* Extension specification with all the needed properties and methods to load an extension.
6263
*/
@@ -204,7 +205,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
204205
experience: spec.experience ?? 'extension',
205206
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
206207
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
207-
buildConfig: spec.buildConfig ?? {mode: 'none'},
208+
buildConfig: spec.buildConfig ?? {mode: 'none', steps: []},
208209
}
209210
const merged = {...defaults, ...spec}
210211

@@ -265,7 +266,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
265266
transformRemoteToLocal: resolveReverseAppConfigTransform(spec.schema, spec.transformConfig),
266267
experience: 'configuration',
267268
uidStrategy: spec.uidStrategy ?? 'single',
268-
buildConfig: spec.buildConfig ?? {mode: 'none'},
269+
buildConfig: spec.buildConfig ?? {mode: 'none', steps: []},
269270
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
270271
patchWithAppDevURLs: spec.patchWithAppDevURLs,
271272
copyStaticAssets: spec.copyStaticAssets,
@@ -279,7 +280,7 @@ export function createContractBasedModuleSpecification<TConfiguration extends Ba
279280
identifier: spec.identifier,
280281
schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
281282
appModuleFeatures: spec.appModuleFeatures,
282-
buildConfig: spec.buildConfig ?? {mode: 'none'},
283+
buildConfig: spec.buildConfig ?? {mode: 'none', steps: []},
283284
deployConfig: async (config, directory) => {
284285
let parsedConfig = configWithoutFirstClassFields(config)
285286
if (spec.appModuleFeatures().includes('localization')) {

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

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import spec from './app_config_hosted_app_home.js'
22
import {placeholderAppConfiguration} from '../../app/app.test-data.js'
3-
import {copyDirectoryContents} from '@shopify/cli-kit/node/fs'
4-
import {describe, expect, test, vi} from 'vitest'
5-
6-
vi.mock('@shopify/cli-kit/node/fs')
3+
import {describe, expect, test} from 'vitest'
74

85
describe('hosted_app_home', () => {
96
describe('transform', () => {
@@ -54,43 +51,42 @@ describe('hosted_app_home', () => {
5451
})
5552
})
5653

57-
describe('copyStaticAssets', () => {
58-
test('should copy static assets from source to output directory', async () => {
59-
vi.mocked(copyDirectoryContents).mockResolvedValue(undefined)
60-
const config = {static_root: 'public'}
61-
const directory = '/app/root'
62-
const outputPath = '/output/dist/bundle.js'
63-
64-
await spec.copyStaticAssets!(config, directory, outputPath)
65-
66-
expect(copyDirectoryContents).toHaveBeenCalledWith('/app/root/public', '/output/dist')
54+
describe('buildConfig', () => {
55+
test('should use copy_files mode', () => {
56+
expect(spec.buildConfig.mode).toBe('copy_files')
6757
})
6858

69-
test('should not copy assets when static_root is not provided', async () => {
70-
const config = {}
71-
const directory = '/app/root'
72-
const outputPath = '/output/dist/bundle.js'
73-
74-
await spec.copyStaticAssets!(config, directory, outputPath)
59+
test('should have copy-static-assets step with tomlKey entry', () => {
60+
if (spec.buildConfig.mode === 'none') {
61+
throw new Error('Expected build_steps mode')
62+
}
7563

76-
expect(copyDirectoryContents).not.toHaveBeenCalled()
64+
expect(spec.buildConfig.steps).toHaveLength(1)
65+
expect(spec.buildConfig.steps[0]).toMatchObject({
66+
id: 'copy-static-assets',
67+
displayName: 'Copy Static Assets',
68+
type: 'copy_files',
69+
config: {
70+
strategy: 'files',
71+
definition: {files: [{tomlKey: 'static_root'}]},
72+
},
73+
})
7774
})
7875

79-
test('should throw error when copy fails', async () => {
80-
vi.mocked(copyDirectoryContents).mockRejectedValue(new Error('Permission denied'))
81-
const config = {static_root: 'public'}
82-
const directory = '/app/root'
83-
const outputPath = '/output/dist/bundle.js'
76+
test('config should be serializable to JSON', () => {
77+
if (spec.buildConfig.mode === 'none') {
78+
throw new Error('Expected build_steps mode')
79+
}
8480

85-
await expect(spec.copyStaticAssets!(config, directory, outputPath)).rejects.toThrow(
86-
'Failed to copy static assets from /app/root/public to /output/dist: Permission denied',
87-
)
88-
})
89-
})
81+
const serialized = JSON.stringify(spec.buildConfig)
82+
expect(serialized).toBeDefined()
9083

91-
describe('buildConfig', () => {
92-
test('should have hosted_app_home build mode', () => {
93-
expect(spec.buildConfig).toEqual({mode: 'none'})
84+
const deserialized = JSON.parse(serialized)
85+
expect(deserialized.steps).toHaveLength(1)
86+
expect(deserialized.steps[0].config).toEqual({
87+
strategy: 'files',
88+
definition: {files: [{tomlKey: 'static_root'}]},
89+
})
9490
})
9591
})
9692

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import {BaseSchemaWithoutHandle} from '../schemas.js'
22
import {TransformationConfig, createConfigExtensionSpecification} from '../specification.js'
3-
import {copyDirectoryContents} from '@shopify/cli-kit/node/fs'
4-
import {dirname, joinPath} from '@shopify/cli-kit/node/path'
53
import {zod} from '@shopify/cli-kit/node/schema'
64

75
const HostedAppHomeSchema = BaseSchemaWithoutHandle.extend({
@@ -16,18 +14,24 @@ export const HostedAppHomeSpecIdentifier = 'hosted_app_home'
1614

1715
const hostedAppHomeSpec = createConfigExtensionSpecification({
1816
identifier: HostedAppHomeSpecIdentifier,
19-
buildConfig: {mode: 'none'} as const,
17+
buildConfig: {
18+
mode: 'copy_files',
19+
steps: [
20+
{
21+
id: 'copy-static-assets',
22+
displayName: 'Copy Static Assets',
23+
type: 'copy_files',
24+
config: {
25+
strategy: 'files',
26+
definition: {
27+
files: [{tomlKey: 'static_root'}],
28+
},
29+
},
30+
},
31+
],
32+
},
2033
schema: HostedAppHomeSchema,
2134
transformConfig: HostedAppHomeTransformConfig,
22-
copyStaticAssets: async (config, directory, outputPath) => {
23-
if (!config.static_root) return
24-
const sourceDir = joinPath(directory, config.static_root)
25-
const outputDir = dirname(outputPath)
26-
27-
return copyDirectoryContents(sourceDir, outputDir).catch((error) => {
28-
throw new Error(`Failed to copy static assets from ${sourceDir} to ${outputDir}: ${error.message}`)
29-
})
30-
},
3135
})
3236

3337
export default hostedAppHomeSpec

0 commit comments

Comments
 (0)