From 177752f2fe5e5b3be002ef2f57ba3a5eda26b821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 9 Apr 2026 12:10:35 +0200 Subject: [PATCH 1/2] notify extension dev server of app assets updates --- packages/app/src/cli/models/app/app.ts | 10 ++++++++ .../models/extensions/extension-instance.ts | 2 +- .../cli/models/extensions/specification.ts | 13 +++++++++++ .../models/extensions/specifications/admin.ts | 5 ++++ .../build/steps/include-assets-step.test.ts | 13 ++++++----- .../steps/include-assets/generate-manifest.ts | 10 ++------ .../services/dev/app-events/file-watcher.ts | 2 +- .../app/src/cli/services/dev/extension.ts | 22 +++++++++++++++++- .../services/dev/extension/payload/models.ts | 6 +++++ .../services/dev/extension/payload/store.ts | 23 ++++++++++++++++++- .../src/cli/services/dev/extension/server.ts | 5 ++++ .../dev/extension/server/middlewares.ts | 14 +++++++++++ .../dev/processes/dev-session/dev-session.ts | 4 ++++ .../dev/processes/previewable-extension.ts | 3 +++ .../dev/processes/setup-dev-processes.ts | 3 ++- .../ui-extensions-server-kit/src/types.ts | 6 +++++ 16 files changed, 122 insertions(+), 19 deletions(-) diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index 5c6a5eab81c..c557afbc62b 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -229,6 +229,7 @@ export interface AppInterface< realExtensions: ExtensionInstance[] nonConfigExtensions: ExtensionInstance[] draftableExtensions: ExtensionInstance[] + appAssetsConfigs: Record | undefined errors: AppErrors hiddenConfig: AppHiddenConfig includeConfigOnDeploy: boolean | undefined @@ -334,6 +335,15 @@ export class App< ) } + get appAssetsConfigs(): Record | undefined { + if (!this.realExtensions.some((ext) => ext.specification.appAssetsConfig)) return undefined + return this.realExtensions.reduce>((acc, ext) => { + const config = ext.specification.appAssetsConfig?.(ext.configuration) + if (config) acc[config.assetsKey] = joinPath(this.directory, config.assetsDir) + return acc + }, {}) + } + setDevApplicationURLs(devApplicationURLs: ApplicationURLs) { this.patchAppConfiguration(devApplicationURLs) this.realExtensions.forEach((ext) => ext.patchWithAppDevURLs(devApplicationURLs)) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 6da9a76ebf7..1a495d3b022 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -285,7 +285,7 @@ export class ExtensionInstance) => DevSessionWatchConfig | undefined + + /** + * App assets configuration for this extension. + * Return undefined if this extension doesn't serve app assets. + */ + appAssetsConfig?: (config: TConfiguration) => AppAssetsConfig | undefined +} + +export interface AppAssetsConfig { + /** The config key that points to the assets directory (e.g. 'admin.static_root') */ + assetsKey: string + /** The assets directory relative to the extension directory */ + assetsDir: string } export interface DevSessionWatchConfig { diff --git a/packages/app/src/cli/models/extensions/specifications/admin.ts b/packages/app/src/cli/models/extensions/specifications/admin.ts index e8b132e233b..e166eb5d35e 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin.ts @@ -62,6 +62,11 @@ const adminSpecificationSpec = createExtensionSpecification({ }, ], appModuleFeatures: () => [], + appAssetsConfig: (config) => { + const dir = config.admin?.static_root + if (!dir) return undefined + return {assetsKey: 'staticRoot', assetsDir: dir} + }, }) export default adminSpecificationSpec diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts index 6429801a725..6daf807c33a 100644 --- a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts @@ -1044,7 +1044,7 @@ describe('executeIncludeAssetsStep', () => { ) }) - test('throws when manifest.json already exists in the output directory', async () => { + test('overwrites manifest.json when it already exists in the output directory', async () => { // Given — a prior inclusion already copied a manifest.json to the output dir const contextWithConfig = { ...mockContext, @@ -1056,8 +1056,7 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - // Source files exist; output manifest.json already exists (simulating conflict); - // candidate output paths for tools.json are free so copyConfigKeyEntry succeeds. + // Source files exist; output manifest.json already exists vi.mocked(fs.fileExists).mockImplementation(async (path) => { const pathStr = String(path) return pathStr === '/test/output/manifest.json' || pathStr.startsWith('/test/extension/') @@ -1081,9 +1080,11 @@ describe('executeIncludeAssetsStep', () => { }, } - // When / Then — throws rather than silently overwriting - await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow( - `Can't write manifest.json: a file already exists at '/test/output/manifest.json'`, + // When / Then — overwrites existing manifest.json + await expect(executeIncludeAssetsStep(step, contextWithConfig)).resolves.not.toThrow() + expect(fs.writeFile).toHaveBeenCalledWith( + '/test/output/manifest.json', + expect.any(String), ) }) diff --git a/packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts b/packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts index c39cc8b9529..20cdc405ead 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts @@ -1,6 +1,6 @@ import {getNestedValue, tokenizePath} from './copy-config-key-entry.js' import {joinPath} from '@shopify/cli-kit/node/path' -import {fileExists, mkdir, writeFile} from '@shopify/cli-kit/node/fs' +import {mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' import type {BuildContext} from '../../client-steps.js' @@ -20,7 +20,7 @@ interface ConfigKeyManifestEntry { * 3. Build root-level entries. * 4. Build grouped entries (anchor/groupBy logic) with path strings resolved * via `resolveManifestPaths` using the copy-tracked `pathMap`. - * 5. Write `outputDir/manifest.json`; throw if the file already exists. + * 5. Write `outputDir/manifest.json`, overwriting any existing file. * * @param pathMap - Map from raw config path values to their output-relative * paths, as recorded during the copy phase by `copyConfigKeyEntry`. @@ -113,12 +113,6 @@ export async function generateManifestFile( } const manifestPath = joinPath(outputDir, 'manifest.json') - if (await fileExists(manifestPath)) { - throw new Error( - `Can't write manifest.json: a file already exists at '${manifestPath}'. ` + - `Remove or rename the conflicting inclusion to avoid overwriting the generated manifest.`, - ) - } await mkdir(outputDir) await writeFile(manifestPath, JSON.stringify(manifest, null, 2)) outputDebug(`Generated manifest.json in ${outputDir}\n`, options.stdout) diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.ts index ca55158bae2..6b022e82cf2 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.ts @@ -150,7 +150,7 @@ export class FileWatcher { private getAllWatchedFiles(): string[] { this.extensionWatchedFiles.clear() - const extensionResults = this.app.nonConfigExtensions.map((extension) => ({ + const extensionResults = this.app.realExtensions.map((extension) => ({ extension, watchedFiles: extension.watchedFiles(), })) diff --git a/packages/app/src/cli/services/dev/extension.ts b/packages/app/src/cli/services/dev/extension.ts index 301f1313c4d..b1e1929f732 100644 --- a/packages/app/src/cli/services/dev/extension.ts +++ b/packages/app/src/cli/services/dev/extension.ts @@ -12,6 +12,7 @@ import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {AbortSignal} from '@shopify/cli-kit/node/abort' import {outputDebug} from '@shopify/cli-kit/node/output' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' +import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' import {Writable} from 'stream' export interface ExtensionDevOptions { @@ -112,6 +113,11 @@ export interface ExtensionDevOptions { * The app watcher that emits events when the app is updated */ appWatcher: AppEventWatcher + + /** + * Map of asset key to absolute directory path for app-level assets (e.g., admin static_root) + */ + appAssets?: Record } export async function devUIExtensions(options: ExtensionDevOptions): Promise { @@ -133,7 +139,13 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise payloadOptions.appAssets + const httpServer = setupHTTPServer({ + devOptions: payloadOptions, + payloadStore, + getExtensions, + getAppAssets, + }) outputDebug(`Setting up the UI extensions Websocket server...`, payloadOptions.stdout) const websocketConnection = setupWebsocketConnection({...payloadOptions, httpServer, payloadStore}) @@ -144,6 +156,14 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise ext.isPreviewable) } + // Handle App Assets updates. + const appAssetsConfigs = extensionEvents.map((event) => + event.extension.specification.appAssetsConfig?.(event.extension.configuration), + ) + getArrayRejectingUndefined(appAssetsConfigs).forEach((config) => { + payloadStore.updateAppAssetTimestamp(config.assetsKey) + }) + for (const event of extensionEvents) { if (!event.extension.isPreviewable) continue const status = event.buildResult?.status === 'ok' ? 'success' : 'error' diff --git a/packages/app/src/cli/services/dev/extension/payload/models.ts b/packages/app/src/cli/services/dev/extension/payload/models.ts index 796f93108bd..9495c31aab6 100644 --- a/packages/app/src/cli/services/dev/extension/payload/models.ts +++ b/packages/app/src/cli/services/dev/extension/payload/models.ts @@ -8,6 +8,12 @@ interface ExtensionsPayloadInterface { url: string mobileUrl: string title: string + assets?: { + [key: string]: { + url: string + lastUpdated: number + } + } } appId?: string store: string diff --git a/packages/app/src/cli/services/dev/extension/payload/store.ts b/packages/app/src/cli/services/dev/extension/payload/store.ts index ae4919485c1..ef2676178ea 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.ts @@ -9,6 +9,7 @@ import {EventEmitter} from 'events' export interface ExtensionsPayloadStoreOptions extends ExtensionDevOptions { websocketURL: string + appAssets?: Record } export enum ExtensionsPayloadStoreEvent { @@ -19,7 +20,7 @@ export async function getExtensionsPayloadStoreRawPayload( options: Omit, bundlePath: string, ): Promise { - return { + const payload: ExtensionsEndpointPayload = { app: { title: options.appName, apiKey: options.apiKey, @@ -40,6 +41,18 @@ export async function getExtensionsPayloadStoreRawPayload( store: options.storeFqdn, extensions: await Promise.all(options.extensions.map((ext) => getUIExtensionPayload(ext, bundlePath, options))), } + + if (options.appAssets) { + const assets: Record = {} + for (const assetKey of Object.keys(options.appAssets)) { + assets[assetKey] = { + url: new URL(`/extensions/assets/${assetKey}/`, options.url).toString(), + lastUpdated: Date.now(), + } + } + payload.app.assets = assets + } + return payload } export class ExtensionsPayloadStore extends EventEmitter { @@ -170,6 +183,14 @@ export class ExtensionsPayloadStore extends EventEmitter { this.emitUpdate([extension.devUUID]) } + updateAppAssetTimestamp(assetKey: string) { + const asset = this.rawPayload.app.assets?.[assetKey] + if (asset) { + asset.lastUpdated = Date.now() + this.emitUpdate([]) + } + } + private emitUpdate(extensionIds: string[]) { this.emit(ExtensionsPayloadStoreEvent.Update, extensionIds) } diff --git a/packages/app/src/cli/services/dev/extension/server.ts b/packages/app/src/cli/services/dev/extension/server.ts index 456c8364c61..b7a5bd7adec 100644 --- a/packages/app/src/cli/services/dev/extension/server.ts +++ b/packages/app/src/cli/services/dev/extension/server.ts @@ -2,6 +2,7 @@ import { corsMiddleware, devConsoleAssetsMiddleware, devConsoleIndexMiddleware, + getAppAssetsMiddleware, getExtensionAssetMiddleware, getExtensionPayloadMiddleware, getExtensionPointMiddleware, @@ -19,6 +20,7 @@ interface SetupHTTPServerOptions { devOptions: ExtensionsPayloadStoreOptions payloadStore: ExtensionsPayloadStore getExtensions: () => ExtensionInstance[] + getAppAssets?: () => Record | undefined } export function setupHTTPServer(options: SetupHTTPServerOptions) { @@ -28,6 +30,9 @@ export function setupHTTPServer(options: SetupHTTPServerOptions) { httpApp.use(getLogMiddleware(options)) httpApp.use(corsMiddleware) httpApp.use(noCacheMiddleware) + if (options.getAppAssets) { + httpRouter.use('/extensions/assets/:assetKey/**:filePath', getAppAssetsMiddleware(options.getAppAssets)) + } httpRouter.use('/extensions/dev-console', devConsoleIndexMiddleware) httpRouter.use('/extensions/dev-console/assets/**:assetPath', devConsoleAssetsMiddleware) httpRouter.use('/extensions/:extensionId', getExtensionPayloadMiddleware(options)) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.ts index ed17a0f474d..98b034d63fe 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -134,6 +134,20 @@ export const devConsoleAssetsMiddleware = defineEventHandler(async (event) => { }) }) +export function getAppAssetsMiddleware(getAppAssets: () => Record | undefined) { + return defineEventHandler(async (event) => { + const {assetKey = '', filePath = ''} = getRouterParams(event) + const appAssets = getAppAssets() + const directory = appAssets?.[assetKey] + if (!directory) { + return sendError(event, {statusCode: 404, statusMessage: `No app assets configured for key: ${assetKey}`}) + } + return fileServerMiddleware(event, { + filePath: joinPath(directory, filePath), + }) + }) +} + export function getLogMiddleware({devOptions}: GetExtensionsMiddlewareOptions) { return defineEventHandler((event) => { outputDebug(`UI extensions server received a ${event.method} request to URL ${event.path}`, devOptions.stdout) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index 1e9602ce520..f5914193acd 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -351,6 +351,10 @@ export class DevSession { .filter((event) => event.type !== 'deleted') .map((event) => event.extension.uid) + // PENDING: Clean up. This is a temporary workaround because `admin` is not compatible with inheritedUids in Core. + // It needs to be included in the manifest always. + updatedUids.push('admin') + const nonUpdatedUids = appEvent.app.allExtensions .filter((ext) => !updatedUids.includes(ext.uid)) .map((ext) => ext.uid) diff --git a/packages/app/src/cli/services/dev/processes/previewable-extension.ts b/packages/app/src/cli/services/dev/processes/previewable-extension.ts index 387d97ebeed..935fbc948d4 100644 --- a/packages/app/src/cli/services/dev/processes/previewable-extension.ts +++ b/packages/app/src/cli/services/dev/processes/previewable-extension.ts @@ -24,6 +24,7 @@ interface PreviewableExtensionOptions { grantedScopes: string[] previewableExtensions: ExtensionInstance[] appWatcher: AppEventWatcher + appAssetsConfigs: Record | undefined } export interface PreviewableExtensionProcess extends BaseProcess { @@ -47,6 +48,7 @@ export const launchPreviewableExtensionProcess: DevProcessFunction { await devUIExtensions({ @@ -68,6 +70,7 @@ export const launchPreviewableExtensionProcess: DevProcessFunction Date: Fri, 10 Apr 2026 13:26:49 +0200 Subject: [PATCH 2/2] Fix CI: update test expectations, lint formatting, unexport AppAssetsConfig Co-authored-by: Claude Code --- packages/app/src/cli/models/extensions/specification.ts | 2 +- .../cli/services/build/steps/include-assets-step.test.ts | 5 +---- packages/app/src/cli/services/dev/extension.test.ts | 1 + .../cli/services/dev/processes/dev-session/dev-session.ts | 8 +++++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index d5a89f377bf..7c1843774d3 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -151,7 +151,7 @@ export interface ExtensionSpecification AppAssetsConfig | undefined } -export interface AppAssetsConfig { +interface AppAssetsConfig { /** The config key that points to the assets directory (e.g. 'admin.static_root') */ assetsKey: string /** The assets directory relative to the extension directory */ diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts index 6daf807c33a..ac4e89dfaba 100644 --- a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts @@ -1082,10 +1082,7 @@ describe('executeIncludeAssetsStep', () => { // When / Then — overwrites existing manifest.json await expect(executeIncludeAssetsStep(step, contextWithConfig)).resolves.not.toThrow() - expect(fs.writeFile).toHaveBeenCalledWith( - '/test/output/manifest.json', - expect.any(String), - ) + expect(fs.writeFile).toHaveBeenCalledWith('/test/output/manifest.json', expect.any(String)) }) test('writes an empty manifest when anchor resolves to a non-array value', async () => { diff --git a/packages/app/src/cli/services/dev/extension.test.ts b/packages/app/src/cli/services/dev/extension.test.ts index a6b696159bf..b4a984be782 100644 --- a/packages/app/src/cli/services/dev/extension.test.ts +++ b/packages/app/src/cli/services/dev/extension.test.ts @@ -69,6 +69,7 @@ describe('devUIExtensions()', () => { devOptions: {...options, websocketURL: 'wss://mock.url/extensions'}, payloadStore: {mock: 'payload-store'}, getExtensions: expect.any(Function), + getAppAssets: expect.any(Function), }) }) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index f5914193acd..c30c47f94ec 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -351,9 +351,11 @@ export class DevSession { .filter((event) => event.type !== 'deleted') .map((event) => event.extension.uid) - // PENDING: Clean up. This is a temporary workaround because `admin` is not compatible with inheritedUids in Core. - // It needs to be included in the manifest always. - updatedUids.push('admin') + // WORKAROUND. This is a temporary fix because `admin` is not compatible with inheritedUids in Core. + // It needs to be included in the manifest always if present in the app. + if (appEvent.app.allExtensions.some((ext) => ext.type === 'admin')) { + updatedUids.push('admin') + } const nonUpdatedUids = appEvent.app.allExtensions .filter((ext) => !updatedUids.includes(ext.uid))