Skip to content

Commit f264d10

Browse files
authored
Merge pull request #7227 from Shopify/04-09-notify_extension_dev_server_of_app_assets_updates
notify extension dev server of app assets updates
2 parents 24b6476 + b93e0fe commit f264d10

17 files changed

Lines changed: 344 additions & 36 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
285285
return this.specification.devSessionWatchConfig(this)
286286
}
287287

288-
return this.specification.experience === 'configuration' ? {paths: []} : undefined
288+
return this.isAppConfigExtension ? {paths: []} : undefined
289289
}
290290

291291
async watchConfigurationPaths() {

packages/app/src/cli/models/extensions/specifications/admin.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ const AdminSchema = zod.object({
77
admin: zod
88
.object({
99
static_root: zod.string().optional(),
10+
allowed_domains: zod.array(zod.string()).optional(),
1011
})
1112
.optional(),
1213
})
1314

14-
type AdminConfigType = zod.infer<typeof AdminSchema> & BaseConfigType
15+
export type AdminConfigType = zod.infer<typeof AdminSchema> & BaseConfigType
1516

1617
const adminSpecificationSpec = createExtensionSpecification<AdminConfigType>({
1718
identifier: 'admin',
@@ -33,6 +34,8 @@ const adminSpecificationSpec = createExtensionSpecification<AdminConfigType>({
3334
admin: {
3435
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3536
static_root: (remoteContent as any).admin.static_root,
37+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
38+
allowed_domains: (remoteContent as any).admin.allowed_domains,
3639
},
3740
}
3841
},

packages/app/src/cli/services/build/steps/include-assets-step.test.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,7 @@ describe('executeIncludeAssetsStep', () => {
10441044
)
10451045
})
10461046

1047-
test('throws when manifest.json already exists in the output directory', async () => {
1047+
test('overwrites manifest.json when it already exists in the output directory', async () => {
10481048
// Given — a prior inclusion already copied a manifest.json to the output dir
10491049
const contextWithConfig = {
10501050
...mockContext,
@@ -1056,8 +1056,7 @@ describe('executeIncludeAssetsStep', () => {
10561056
} as unknown as ExtensionInstance,
10571057
}
10581058

1059-
// Source files exist; output manifest.json already exists (simulating conflict);
1060-
// candidate output paths for tools.json are free so copyConfigKeyEntry succeeds.
1059+
// Source files exist; output manifest.json already exists
10611060
vi.mocked(fs.fileExists).mockImplementation(async (path) => {
10621061
const pathStr = String(path)
10631062
return pathStr === '/test/output/manifest.json' || pathStr.startsWith('/test/extension/')
@@ -1081,10 +1080,9 @@ describe('executeIncludeAssetsStep', () => {
10811080
},
10821081
}
10831082

1084-
// When / Then — throws rather than silently overwriting
1085-
await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow(
1086-
`Can't write manifest.json: a file already exists at '/test/output/manifest.json'`,
1087-
)
1083+
// When / Then — overwrites existing manifest.json
1084+
await expect(executeIncludeAssetsStep(step, contextWithConfig)).resolves.not.toThrow()
1085+
expect(fs.writeFile).toHaveBeenCalledWith('/test/output/manifest.json', expect.any(String))
10881086
})
10891087

10901088
test('writes an empty manifest when anchor resolves to a non-array value', async () => {

packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {getNestedValue, tokenizePath} from './copy-config-key-entry.js'
22
import {joinPath} from '@shopify/cli-kit/node/path'
3-
import {fileExists, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
3+
import {mkdir, writeFile} from '@shopify/cli-kit/node/fs'
44
import {outputDebug} from '@shopify/cli-kit/node/output'
55
import type {BuildContext} from '../../client-steps.js'
66

@@ -20,7 +20,7 @@ interface ConfigKeyManifestEntry {
2020
* 3. Build root-level entries.
2121
* 4. Build grouped entries (anchor/groupBy logic) with path strings resolved
2222
* via `resolveManifestPaths` using the copy-tracked `pathMap`.
23-
* 5. Write `outputDir/manifest.json`; throw if the file already exists.
23+
* 5. Write `outputDir/manifest.json`, overwriting any existing file.
2424
*
2525
* @param pathMap - Map from raw config path values to their output-relative
2626
* paths, as recorded during the copy phase by `copyConfigKeyEntry`.
@@ -113,12 +113,6 @@ export async function generateManifestFile(
113113
}
114114

115115
const manifestPath = joinPath(outputDir, 'manifest.json')
116-
if (await fileExists(manifestPath)) {
117-
throw new Error(
118-
`Can't write manifest.json: a file already exists at '${manifestPath}'. ` +
119-
`Remove or rename the conflicting inclusion to avoid overwriting the generated manifest.`,
120-
)
121-
}
122116
await mkdir(outputDir)
123117
await writeFile(manifestPath, JSON.stringify(manifest, null, 2))
124118
outputDebug(`Generated manifest.json in ${outputDir}\n`, options.stdout)

packages/app/src/cli/services/dev/app-events/file-watcher.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ export class FileWatcher {
150150
private getAllWatchedFiles(): string[] {
151151
this.extensionWatchedFiles.clear()
152152

153-
const extensionResults = this.app.nonConfigExtensions.map((extension) => ({
153+
const extensionResults = this.app.realExtensions.map((extension) => ({
154154
extension,
155155
watchedFiles: extension.watchedFiles(),
156156
}))

packages/app/src/cli/services/dev/extension.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,14 @@ describe('devUIExtensions()', () => {
3333

3434
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
3535
// @ts-ignore
36-
vi.spyOn(store, 'ExtensionsPayloadStore').mockImplementation(() => ({mock: 'payload-store'}))
36+
vi.spyOn(store, 'ExtensionsPayloadStore').mockImplementation(
37+
() =>
38+
({
39+
mock: 'payload-store',
40+
updateAdminConfigFromExtensionEvents: vi.fn(),
41+
getAppAssets: vi.fn(),
42+
}) as unknown as store.ExtensionsPayloadStore,
43+
)
3744
vi.spyOn(server, 'setupHTTPServer').mockReturnValue({
3845
mock: 'http-server',
3946
close: serverCloseSpy,
@@ -67,8 +74,9 @@ describe('devUIExtensions()', () => {
6774
// THEN
6875
expect(server.setupHTTPServer).toHaveBeenCalledWith({
6976
devOptions: {...options, websocketURL: 'wss://mock.url/extensions'},
70-
payloadStore: {mock: 'payload-store'},
77+
payloadStore: expect.objectContaining({mock: 'payload-store'}),
7178
getExtensions: expect.any(Function),
79+
getAppAssets: expect.any(Function),
7280
})
7381
})
7482

@@ -94,7 +102,7 @@ describe('devUIExtensions()', () => {
94102
expect(websocket.setupWebsocketConnection).toHaveBeenCalledWith({
95103
...options,
96104
httpServer: expect.objectContaining({mock: 'http-server'}),
97-
payloadStore: {mock: 'payload-store'},
105+
payloadStore: expect.objectContaining({mock: 'payload-store'}),
98106
websocketURL: 'wss://mock.url/extensions',
99107
})
100108
})

packages/app/src/cli/services/dev/extension.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ export interface ExtensionDevOptions {
3535
buildDirectory?: string
3636

3737
/**
38-
* The extension to be built.
38+
* All real extensions in the app, including non-previewable ones (e.g., admin config).
39+
* Previewable extensions are filtered internally for the UI payload.
3940
*/
4041
extensions: ExtensionInstance[]
4142

@@ -126,14 +127,20 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
126127
const bundlePath = payloadOptions.appWatcher.buildOutputPath
127128
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadOptions, bundlePath)
128129
const payloadStore = new ExtensionsPayloadStore(payloadStoreRawPayload, payloadOptions)
129-
let extensions = payloadOptions.extensions
130+
let extensions = payloadOptions.extensions.filter((ext) => ext.isPreviewable)
130131

131132
const getExtensions = () => {
132133
return extensions
133134
}
134135

135136
outputDebug(`Setting up the UI extensions HTTP server...`, payloadOptions.stdout)
136-
const httpServer = setupHTTPServer({devOptions: payloadOptions, payloadStore, getExtensions})
137+
const getAppAssets = () => payloadStore.getAppAssets()
138+
const httpServer = setupHTTPServer({
139+
devOptions: payloadOptions,
140+
payloadStore,
141+
getExtensions,
142+
getAppAssets,
143+
})
137144

138145
outputDebug(`Setting up the UI extensions Websocket server...`, payloadOptions.stdout)
139146
const websocketConnection = setupWebsocketConnection({...payloadOptions, httpServer, payloadStore})
@@ -144,6 +151,11 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
144151
extensions = app.allExtensions.filter((ext) => ext.isPreviewable)
145152
}
146153

154+
// Exception for AdminConfig: it's a extension that needs its config extracted at the app level
155+
// for the payloed. No other exceptions should be added, if this pattern is needed again we should
156+
// generalize it.
157+
payloadStore.updateAdminConfigFromExtensionEvents(extensionEvents)
158+
147159
for (const event of extensionEvents) {
148160
if (!event.extension.isPreviewable) continue
149161
const status = event.buildResult?.status === 'ok' ? 'success' : 'error'

packages/app/src/cli/services/dev/extension/payload/models.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ interface ExtensionsPayloadInterface {
88
url: string
99
mobileUrl: string
1010
title: string
11+
allowedDomains?: string[]
12+
assets?: {
13+
[key: string]: {
14+
url: string
15+
lastUpdated: number
16+
}
17+
}
1118
}
1219
appId?: string
1320
store: string

0 commit comments

Comments
 (0)