Skip to content

Replace hosted_app_home build mode with copy_files mode #6868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alfonso-noriega wants to merge 1 commit into
base: 02-wire-build-config-into-extension-specs
Choose a base branch
from
+46 −46
Open

Replace hosted_app_home build mode with copy_files mode #6868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 30 additions & 34 deletions packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import spec from './app_config_hosted_app_home.js'
import {placeholderAppConfiguration} from '../../app/app.test-data.js'
import {copyDirectoryContents} from '@shopify/cli-kit/node/fs'
import {describe, expect, test, vi} from 'vitest'

vi.mock('@shopify/cli-kit/node/fs')
import {describe, expect, test} from 'vitest'

describe('hosted_app_home', () => {
describe('transform', () => {
Expand Down Expand Up @@ -54,43 +51,42 @@ describe('hosted_app_home', () => {
})
})

describe('copyStaticAssets', () => {
test('should copy static assets from source to output directory', async () => {
vi.mocked(copyDirectoryContents).mockResolvedValue(undefined)
const config = {static_root: 'public'}
const directory = '/app/root'
const outputPath = '/output/dist/bundle.js'

await spec.copyStaticAssets!(config, directory, outputPath)

expect(copyDirectoryContents).toHaveBeenCalledWith('/app/root/public', '/output/dist')
describe('buildConfig', () => {
test('should use copy_files mode', () => {
expect(spec.buildConfig.mode).toBe('copy_files')
})

test('should not copy assets when static_root is not provided', async () => {
const config = {}
const directory = '/app/root'
const outputPath = '/output/dist/bundle.js'

await spec.copyStaticAssets!(config, directory, outputPath)
test('should have copy-static-assets step with tomlKey entry', () => {
if (spec.buildConfig.mode === 'none') {
throw new Error('Expected build_steps mode')
}

expect(copyDirectoryContents).not.toHaveBeenCalled()
expect(spec.buildConfig.steps).toHaveLength(1)
expect(spec.buildConfig.steps[0]).toMatchObject({
id: 'copy-static-assets',
displayName: 'Copy Static Assets',
type: 'copy_files',
config: {
strategy: 'files',
definition: {files: [{tomlKey: 'static_root'}]},
},
})
})

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

await expect(spec.copyStaticAssets!(config, directory, outputPath)).rejects.toThrow(
'Failed to copy static assets from /app/root/public to /output/dist: Permission denied',
)
})
})
const serialized = JSON.stringify(spec.buildConfig)
expect(serialized).toBeDefined()

describe('buildConfig', () => {
test('should have hosted_app_home build mode', () => {
expect(spec.buildConfig).toEqual({mode: 'none'})
const deserialized = JSON.parse(serialized)
expect(deserialized.steps).toHaveLength(1)
expect(deserialized.steps[0].config).toEqual({
strategy: 'files',
definition: {files: [{tomlKey: 'static_root'}]},
})
})
})

Expand Down
28 changes: 16 additions & 12 deletions packages/app/src/cli/models/extensions/specifications/app_config_hosted_app_home.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import {BaseSchemaWithoutHandle} from '../schemas.js'
import {TransformationConfig, createConfigExtensionSpecification} from '../specification.js'
import {copyDirectoryContents} from '@shopify/cli-kit/node/fs'
import {dirname, joinPath} from '@shopify/cli-kit/node/path'
import {zod} from '@shopify/cli-kit/node/schema'

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

const hostedAppHomeSpec = createConfigExtensionSpecification({
identifier: HostedAppHomeSpecIdentifier,
buildConfig: {mode: 'none'} as const,
buildConfig: {
mode: 'copy_files',
steps: [
{
id: 'copy-static-assets',
displayName: 'Copy Static Assets',
type: 'copy_files',
config: {
strategy: 'files',
definition: {
files: [{tomlKey: 'static_root'}],
},
},
},
],
},
Copy link

@binks-code-reviewer binks-code-reviewer bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New copy_files mode is not part of BuildConfig union

This spec now sets buildConfig.mode: 'copy_files' with steps, but BuildConfig (in specification.ts) does not include 'copy_files' nor a steps field anywhere. This is a hard incompatibility: either TypeScript should fail, or if it’s being coerced, the build pipeline likely won’t know how to execute this mode and may ignore it or crash when it expects lifecycles. That can lead to static assets not being copied in dev/deploy, breaking Hosted App Home behavior for all users of that extension type.

schema: HostedAppHomeSchema,
transformConfig: HostedAppHomeTransformConfig,
copyStaticAssets: async (config, directory, outputPath) => {
if (!config.static_root) return
const sourceDir = joinPath(directory, config.static_root)
const outputDir = dirname(outputPath)

return copyDirectoryContents(sourceDir, outputDir).catch((error) => {
throw new Error(`Failed to copy static assets from ${sourceDir} to ${outputDir}: ${error.message}`)
})
},
})

export default hostedAppHomeSpec
Loading