From 4f2ef046aa4521ad8c4ad1efd3e1fec3e8202f2f Mon Sep 17 00:00:00 2001 From: Trish Ta Date: Tue, 31 Mar 2026 11:17:07 -0400 Subject: [PATCH 1/3] Support intents for ui extensions --- .../app/src/cli/models/extensions/schemas.ts | 11 + .../cli/models/extensions/specification.ts | 1 + .../extensions/specifications/ui_extension.ts | 29 +- .../services/dev/extension/payload.test.ts | 445 +++++------------- .../src/cli/services/dev/extension/payload.ts | 108 ++++- .../services/dev/extension/payload/models.ts | 11 +- 6 files changed, 248 insertions(+), 357 deletions(-) diff --git a/packages/app/src/cli/models/extensions/schemas.ts b/packages/app/src/cli/models/extensions/schemas.ts index cab04621ec8..98943c09187 100644 --- a/packages/app/src/cli/models/extensions/schemas.ts +++ b/packages/app/src/cli/models/extensions/schemas.ts @@ -51,6 +51,17 @@ const NewExtensionPointSchema = zod.object({ should_render: ShouldRenderSchema.optional(), tools: zod.string().optional(), instructions: zod.string().optional(), + intents: zod + .array( + zod.object({ + type: zod.string(), + action: zod.string(), + schema: zod.string(), + name: zod.string().optional(), + description: zod.string().optional(), + }), + ) + .optional(), metafields: zod.array(MetafieldSchema).optional(), default_placement: zod.string().optional(), urls: zod diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index 94907693f10..1581072beec 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -42,6 +42,7 @@ export enum AssetIdentifier { Main = 'main', Tools = 'tools', Instructions = 'instructions', + Intents = 'intents', } export interface Asset { diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 5d5a7550f41..7030fc95201 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -109,7 +109,34 @@ const uiExtensionSpec = createExtensionSpecification({ lifecycle: 'deploy', steps: [ {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, + { + id: 'include-ui-extension-assets', + name: 'Include UI Extension Assets', + type: 'include_assets', + config: { + generatesAssetsManifest: true, + inclusions: [ + { + type: 'configKey', + anchor: 'targeting[]', + groupBy: 'target', + key: 'targeting[].tools', + }, + { + type: 'configKey', + anchor: 'targeting[]', + groupBy: 'target', + key: 'targeting[].instructions', + }, + { + type: 'configKey', + anchor: 'targeting[]', + groupBy: 'target', + key: 'targeting[].intents[].schema', + }, + ], + }, + }, ], }, ], diff --git a/packages/app/src/cli/services/dev/extension/payload.test.ts b/packages/app/src/cli/services/dev/extension/payload.test.ts index 5fd9f088599..566d0cd20cb 100644 --- a/packages/app/src/cli/services/dev/extension/payload.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload.test.ts @@ -4,8 +4,8 @@ import {ExtensionsPayloadStoreOptions} from './payload/store.js' import {testUIExtension} from '../../../models/app/app.test-data.js' import * as appModel from '../../../models/app/app.js' import {describe, expect, test, vi, beforeEach} from 'vitest' -import {inTemporaryDirectory, touchFile, writeFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' +import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' +import {dirname, joinPath} from '@shopify/cli-kit/node/path' describe('getUIExtensionPayload', () => { beforeEach(() => { @@ -37,9 +37,29 @@ describe('getUIExtensionPayload', () => { } } + async function setupBuildOutput( + extension: any, + bundlePath: string, + manifest: Record, + sourceFiles: Record, + ) { + const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath) + const buildDir = dirname(extensionOutputPath) + await mkdir(buildDir) + await touchFile(extensionOutputPath) + await writeFile(joinPath(buildDir, 'manifest.json'), JSON.stringify(manifest)) + + for (const [filepath, content] of Object.entries(sourceFiles)) { + const fullPath = joinPath(extension.directory, filepath) + // eslint-disable-next-line no-await-in-loop + await mkdir(dirname(fullPath)) + // eslint-disable-next-line no-await-in-loop + await writeFile(fullPath, content) + } + } + test('returns the right payload', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const outputPath = joinPath(tmpDir, 'test-ui-extension.js') await touchFile(outputPath) @@ -67,13 +87,11 @@ describe('getUIExtensionPayload', () => { devUUID: 'devUUID', }) - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { ...createMockOptions(tmpDir, [uiExtension]), currentDevelopmentPayload: {hidden: true, status: 'success'}, }) - // Then expect(got).toMatchObject({ assets: { main: { @@ -96,19 +114,14 @@ describe('getUIExtensionPayload', () => { development: { hidden: true, localizationStatus: '', - resource: { - url: 'https://my-domain.com/cart', - }, - root: { - url: 'http://tunnel-url.com/extensions/devUUID', - }, + resource: {url: 'https://my-domain.com/cart'}, + root: {url: 'http://tunnel-url.com/extensions/devUUID'}, status: 'success', }, extensionPoints: ['CUSTOM_EXTENSION_POINT'], externalType: 'checkout_ui_extension_external', localization: null, metafields: null, - // as surfaces come from remote specs, we dont' have real values here surface: 'test-surface', title: 'test-ui-extension', type: 'checkout_ui_extension', @@ -119,165 +132,49 @@ describe('getUIExtensionPayload', () => { }) }) - test('returns the right payload for UI Extensions with build_manifest', async () => { + test('maps tools and instructions from manifest.json to asset payloads', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given - const outputPath = joinPath(tmpDir, 'test-ui-extension.js') - await touchFile(outputPath) - - const buildManifest = { - assets: { - main: {identifier: 'main', module: './src/ExtensionPointA.js', filepath: '/test-ui-extension.js'}, - should_render: { - identifier: 'should_render', - module: './src/ShouldRender.js', - filepath: '/test-ui-extension-conditions.js', - }, - }, - } - const uiExtension = await testUIExtension({ - outputPath, directory: tmpDir, configuration: { name: 'test-ui-extension', type: 'ui_extension', - metafields: [], - capabilities: { - network_access: true, - api_access: true, - block_progress: false, - collect_buyer_consent: { - sms_marketing: false, - customer_privacy: false, - }, - iframe: { - sources: ['https://my-iframe.com'], - }, - }, extension_points: [ { target: 'CUSTOM_EXTENSION_POINT', - build_manifest: buildManifest, + module: './src/ExtensionPointA.js', + tools: './tools.json', + instructions: './instructions.md', }, ], }, devUUID: 'devUUID', }) - // When - const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...createMockOptions(tmpDir, [uiExtension]), - currentDevelopmentPayload: {hidden: true, status: 'success'}, - }) + await setupBuildOutput( + uiExtension, + tmpDir, + {CUSTOM_EXTENSION_POINT: {tools: 'tools.json', instructions: 'instructions.md'}}, + {'tools.json': '{"tools": []}', 'instructions.md': '# Instructions'}, + ) - // Then - expect(got).toMatchObject({ - assets: { - main: { - lastUpdated: expect.any(Number), - name: 'main', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js', - }, - }, - capabilities: { - blockProgress: false, - networkAccess: true, - apiAccess: true, - collectBuyerConsent: { - smsMarketing: false, - }, - iframe: { - sources: ['https://my-iframe.com'], - }, - }, - development: { - hidden: true, - localizationStatus: '', - resource: { - url: '', - }, - root: { - url: 'http://tunnel-url.com/extensions/devUUID', - }, - status: 'success', - }, - extensionPoints: [ - { - target: 'CUSTOM_EXTENSION_POINT', - build_manifest: buildManifest, - assets: { - main: { - lastUpdated: expect.any(Number), - name: 'main', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js', - }, - should_render: { - lastUpdated: expect.any(Number), - name: 'should_render', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension-conditions.js', - }, - }, - }, - ], - externalType: 'ui_extension_external', - localization: null, - metafields: null, - // as surfaces come from remote specs, we dont' have real values here - surface: 'test-surface', - title: 'test-ui-extension', - type: 'ui_extension', - uuid: 'devUUID', - version: '1.2.3', - approvalScopes: ['scope-a'], - }) - }) - }) - - test('returns the right payload for UI Extensions with tools in build_manifest', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const outputPath = joinPath(tmpDir, 'test-ui-extension.js') - await touchFile(outputPath) - await writeFile(joinPath(tmpDir, 'tools.json'), '{"tools": []}') - - const buildManifest = { - assets: { - main: {module: './src/ExtensionPointA.js', filepath: '/test-ui-extension.js'}, - tools: {module: './tools.json', filepath: '/test-ui-extension-tools.json', static: true}, - }, - } - - const uiExtension = await testUIExtension({ - outputPath, - directory: tmpDir, - configuration: { - name: 'test-ui-extension', - type: 'ui_extension', - extension_points: [{target: 'CUSTOM_EXTENSION_POINT', build_manifest: buildManifest}], - }, - devUUID: 'devUUID', - }) - - // When - const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { + const got = await getUIExtensionPayload(uiExtension, tmpDir, { ...createMockOptions(tmpDir, [uiExtension]), currentDevelopmentPayload: {hidden: true, status: 'success'}, }) - // Then expect(got.extensionPoints).toMatchObject([ { target: 'CUSTOM_EXTENSION_POINT', assets: { - main: { - name: 'main', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js', - lastUpdated: expect.any(Number), - }, tools: { name: 'tools', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension-tools.json', + url: 'http://tunnel-url.com/extensions/devUUID/assets/tools.json', + lastUpdated: expect.any(Number), + }, + instructions: { + name: 'instructions', + url: 'http://tunnel-url.com/extensions/devUUID/assets/instructions.md', lastUpdated: expect.any(Number), }, }, @@ -286,53 +183,62 @@ describe('getUIExtensionPayload', () => { }) }) - test('returns the right payload for UI Extensions with instructions in build_manifest', async () => { + test('maps intents from manifest.json to asset payloads', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given - const outputPath = joinPath(tmpDir, 'test-ui-extension.js') - await touchFile(outputPath) - await writeFile(joinPath(tmpDir, 'instructions.md'), '# Instructions') - - const buildManifest = { - assets: { - main: {module: './src/ExtensionPointA.js', filepath: '/test-ui-extension.js'}, - instructions: {module: './instructions.md', filepath: '/test-ui-extension-instructions.md', static: true}, - }, - } - const uiExtension = await testUIExtension({ - outputPath, directory: tmpDir, configuration: { name: 'test-ui-extension', type: 'ui_extension', - extension_points: [{target: 'CUSTOM_EXTENSION_POINT', build_manifest: buildManifest}], + extension_points: [ + { + target: 'CUSTOM_EXTENSION_POINT', + module: './src/ExtensionPointA.js', + intents: [ + {type: 'application/email', action: 'create', schema: './intents/create-schema.json'}, + {type: 'application/email', action: 'update', schema: './intents/update-schema.json'}, + ], + }, + ], }, devUUID: 'devUUID', }) - // When - const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { + await setupBuildOutput( + uiExtension, + tmpDir, + {CUSTOM_EXTENSION_POINT: {intents: [{schema: 'create-schema.json'}, {schema: 'update-schema.json'}]}}, + {'intents/create-schema.json': '{"type": "object"}', 'intents/update-schema.json': '{"type": "object"}'}, + ) + + const got = await getUIExtensionPayload(uiExtension, tmpDir, { ...createMockOptions(tmpDir, [uiExtension]), currentDevelopmentPayload: {hidden: true, status: 'success'}, }) - // Then expect(got.extensionPoints).toMatchObject([ { target: 'CUSTOM_EXTENSION_POINT', - assets: { - main: { - name: 'main', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js', - lastUpdated: expect.any(Number), + intents: [ + { + type: 'application/email', + action: 'create', + schema: { + name: 'schema', + url: 'http://tunnel-url.com/extensions/devUUID/assets/intents/create-schema.json', + lastUpdated: expect.any(Number), + }, }, - instructions: { - name: 'instructions', - url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension-instructions.md', - lastUpdated: expect.any(Number), + { + type: 'application/email', + action: 'update', + schema: { + name: 'schema', + url: 'http://tunnel-url.com/extensions/devUUID/assets/intents/update-schema.json', + lastUpdated: expect.any(Number), + }, }, - }, + ], }, ]) }) @@ -340,7 +246,6 @@ describe('getUIExtensionPayload', () => { test('returns the right payload for post-purchase extensions', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const outputPath = joinPath(tmpDir, 'test-post-purchase-extension.js') await touchFile(outputPath) @@ -363,22 +268,16 @@ describe('getUIExtensionPayload', () => { sources: ['https://my-iframe.com'], }, }, - extension_points: [ - { - target: 'CUSTOM_EXTENSION_POINT', - }, - ], + extension_points: [{target: 'CUSTOM_EXTENSION_POINT'}], }, devUUID: 'devUUID', }) - // When const got = await getUIExtensionPayload(postPurchaseExtension, 'mock-bundle-path', { ...createMockOptions(tmpDir, [postPurchaseExtension]), currentDevelopmentPayload: {hidden: true, status: 'success'}, }) - // Then expect(got).toMatchObject({ assets: { main: { @@ -387,84 +286,42 @@ describe('getUIExtensionPayload', () => { url: 'http://tunnel-url.com/extensions/devUUID/assets/test-post-purchase-extension.js', }, }, - capabilities: { - blockProgress: false, - networkAccess: true, - apiAccess: true, - collectBuyerConsent: { - smsMarketing: false, - }, - iframe: { - sources: ['https://my-iframe.com'], - }, - }, development: { hidden: true, - localizationStatus: '', - resource: { - url: 'https://my-domain.com/cart', - }, - root: { - url: 'http://tunnel-url.com/extensions/devUUID', - }, status: 'success', }, - extensionPoints: [ - { - target: 'purchase.post.render', - }, - ], - externalType: 'checkout_post_purchase_external', - localization: null, - metafields: null, - // as surfaces come from remote specs, we dont' have real values here - surface: 'test-surface', - title: 'test-post-purchase-extension', + extensionPoints: [{target: 'purchase.post.render'}], type: 'checkout_post_purchase', uuid: 'devUUID', - version: '1.2.3', - approvalScopes: ['scope-a'], }) }) }) test('default values', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const uiExtension = await testUIExtension({directory: tmpDir}) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - const development: Partial = {} - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, - currentDevelopmentPayload: development, + ...({} as ExtensionsPayloadStoreOptions), + currentDevelopmentPayload: {}, }) - // Then expect(got).toMatchObject({ - development: { - hidden: false, - }, + development: {hidden: false}, capabilities: { blockProgress: false, networkAccess: false, apiAccess: false, - collectBuyerConsent: { - smsMarketing: false, - }, - iframe: { - sources: [], - }, + collectBuyerConsent: {smsMarketing: false}, + iframe: {sources: []}, }, }) }) }) describe('supportedFeatures', () => { - test('returns supportedFeatures with runsOffline true when runs_offline is enabled', async () => { + test('returns runsOffline true when runs_offline is enabled', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const uiExtension = await testUIExtension({ directory: tmpDir, configuration: { @@ -472,30 +329,22 @@ describe('getUIExtensionPayload', () => { type: 'ui_extension', metafields: [], capabilities: {}, - supported_features: { - runs_offline: true, - }, + supported_features: {runs_offline: true}, extension_points: [], }, }) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, + ...({} as ExtensionsPayloadStoreOptions), currentDevelopmentPayload: {}, }) - // Then - expect(got.supportedFeatures).toStrictEqual({ - runsOffline: true, - }) + expect(got.supportedFeatures).toStrictEqual({runsOffline: true}) }) }) - test('returns supportedFeatures with runsOffline false when runs_offline is disabled', async () => { + test('returns runsOffline false when runs_offline is disabled', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const uiExtension = await testUIExtension({ directory: tmpDir, configuration: { @@ -503,30 +352,22 @@ describe('getUIExtensionPayload', () => { type: 'ui_extension', metafields: [], capabilities: {}, - supported_features: { - runs_offline: false, - }, + supported_features: {runs_offline: false}, extension_points: [], }, }) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, + ...({} as ExtensionsPayloadStoreOptions), currentDevelopmentPayload: {}, }) - // Then - expect(got.supportedFeatures).toStrictEqual({ - runsOffline: false, - }) + expect(got.supportedFeatures).toStrictEqual({runsOffline: false}) }) }) - test('returns supportedFeatures with runsOffline false when supported_features is not configured', async () => { + test('returns runsOffline false when supported_features is not configured', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given const uiExtension = await testUIExtension({ directory: tmpDir, configuration: { @@ -537,25 +378,19 @@ describe('getUIExtensionPayload', () => { extension_points: [], }, }) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, + ...({} as ExtensionsPayloadStoreOptions), currentDevelopmentPayload: {}, }) - // Then - expect(got.supportedFeatures).toStrictEqual({ - runsOffline: false, - }) + expect(got.supportedFeatures).toStrictEqual({runsOffline: false}) }) }) }) test('adds root.url, resource.url and surface to extensionPoints[n] when extensionPoints[n] is an object', async () => { await inTemporaryDirectory(async (_tmpDir) => { - // Given const uiExtension = await testUIExtension({ devUUID: 'devUUID', configuration: { @@ -566,75 +401,44 @@ describe('getUIExtensionPayload', () => { network_access: false, block_progress: false, api_access: false, - collect_buyer_consent: { - sms_marketing: false, - customer_privacy: false, - }, - iframe: { - sources: [], - }, + collect_buyer_consent: {sms_marketing: false, customer_privacy: false}, + iframe: {sources: []}, }, extension_points: [ - { - target: 'Admin::Checkout::Editor::Settings', - module: './src/AdminCheckoutEditorSettings.js', - }, - { - target: 'admin.checkout.editor.settings', - module: './src/AdminCheckoutEditorSettings.js', - }, - { - target: 'Checkout::ShippingMethods::RenderAfter', - module: './src/CheckoutShippingMethodsRenderAfter.js', - }, + {target: 'Admin::Checkout::Editor::Settings', module: './src/AdminCheckoutEditorSettings.js'}, + {target: 'admin.checkout.editor.settings', module: './src/AdminCheckoutEditorSettings.js'}, + {target: 'Checkout::ShippingMethods::RenderAfter', module: './src/CheckoutShippingMethodsRenderAfter.js'}, ], }, }) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - const development: Partial = {} - - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, - currentDevelopmentPayload: development, + ...({} as ExtensionsPayloadStoreOptions), + currentDevelopmentPayload: {}, url: 'http://tunnel-url.com', }) - // Then expect(got.extensionPoints).toStrictEqual([ { target: 'Admin::Checkout::Editor::Settings', module: './src/AdminCheckoutEditorSettings.js', surface: 'admin', - root: { - url: 'http://tunnel-url.com/extensions/devUUID/Admin::Checkout::Editor::Settings', - }, - resource: { - url: '', - }, + root: {url: 'http://tunnel-url.com/extensions/devUUID/Admin::Checkout::Editor::Settings'}, + resource: {url: ''}, }, { target: 'admin.checkout.editor.settings', module: './src/AdminCheckoutEditorSettings.js', surface: 'admin', - root: { - url: 'http://tunnel-url.com/extensions/devUUID/admin.checkout.editor.settings', - }, - resource: { - url: '', - }, + root: {url: 'http://tunnel-url.com/extensions/devUUID/admin.checkout.editor.settings'}, + resource: {url: ''}, }, { target: 'Checkout::ShippingMethods::RenderAfter', module: './src/CheckoutShippingMethodsRenderAfter.js', surface: 'checkout', - root: { - url: 'http://tunnel-url.com/extensions/devUUID/Checkout::ShippingMethods::RenderAfter', - }, - resource: { - url: '', - }, + root: {url: 'http://tunnel-url.com/extensions/devUUID/Checkout::ShippingMethods::RenderAfter'}, + resource: {url: ''}, }, ]) }) @@ -642,48 +446,33 @@ describe('getUIExtensionPayload', () => { test('adds apiVersion when present in the configuration', async () => { await inTemporaryDirectory(async () => { - // Given - const apiVersion = '2023-01' const uiExtension = await testUIExtension({ devUUID: 'devUUID', configuration: { name: 'UI Extension', type: 'ui_extension', - api_version: apiVersion, + api_version: '2023-01', metafields: [], capabilities: { network_access: false, block_progress: false, api_access: false, - collect_buyer_consent: { - sms_marketing: false, - customer_privacy: false, - }, - iframe: { - sources: [], - }, + collect_buyer_consent: {sms_marketing: false, customer_privacy: false}, + iframe: {sources: []}, }, extension_points: [ - { - target: 'Admin::Checkout::Editor::Settings', - module: './src/AdminCheckoutEditorSettings.js', - }, + {target: 'Admin::Checkout::Editor::Settings', module: './src/AdminCheckoutEditorSettings.js'}, ], }, }) - const options: ExtensionsPayloadStoreOptions = {} as ExtensionsPayloadStoreOptions - const development: Partial = {} - - // When const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { - ...options, - currentDevelopmentPayload: development, + ...({} as ExtensionsPayloadStoreOptions), + currentDevelopmentPayload: {}, url: 'http://tunnel-url.com', }) - // Then - expect(got).toHaveProperty('apiVersion', apiVersion) + expect(got).toHaveProperty('apiVersion', '2023-01') }) }) }) diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index 37d08dfbfb8..fad6117648b 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -5,10 +5,8 @@ import {ExtensionsPayloadStoreOptions} from './payload/store.js' import {getUIExtensionResourceURL} from '../../../utilities/extensions/configuration.js' import {getUIExtensionRendererVersion} from '../../../models/app/app.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' -import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' -import {BuildAsset} from '../../../models/extensions/specification.js' import {NewExtensionPointSchemaType} from '../../../models/extensions/schemas.js' -import {fileLastUpdatedTimestamp} from '@shopify/cli-kit/node/fs' +import {fileExists, fileLastUpdatedTimestamp, readFile} from '@shopify/cli-kit/node/fs' import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' import {dirname, joinPath} from '@shopify/cli-kit/node/path' @@ -19,7 +17,7 @@ export type GetUIExtensionPayloadOptions = Omit { const {target, resource} = extensionPoint @@ -126,15 +127,16 @@ async function getExtensionPoints(extension: ExtensionInstance, url: string) { resource: resource || {url: ''}, } - if (!('build_manifest' in extensionPoint)) { + const manifestEntry = manifest?.[target] + if (!manifestEntry) { return payload } return { ...payload, - ...(await mapBuildManifestToPayload( - extensionPoint.build_manifest, - extensionPoint as NewExtensionPointSchemaType & {build_manifest: BuildManifest}, + ...(await mapManifestAssetsToPayload( + manifestEntry, + extensionPoint as unknown as NewExtensionPointSchemaType, url, extension, )), @@ -147,35 +149,84 @@ async function getExtensionPoints(extension: ExtensionInstance, url: string) { } /** - * Default asset mapper - adds asset to the assets object + * Reads and parses manifest.json from the extension's build output directory. + * Returns null if the file doesn't exist. + */ +async function readBundleManifest( + buildDirectory: string, +): Promise<{[target: string]: {[assetName: string]: unknown}} | null> { + const manifestPath = joinPath(buildDirectory, 'manifest.json') + if (!(await fileExists(manifestPath))) { + return null + } + const content = await readFile(manifestPath) + return JSON.parse(content) +} + +/** + * Default asset mapper - reads the source path from the extension point config + * and adds asset to the assets object */ async function defaultAssetMapper({ identifier, - asset, + extensionPoint, url, extension, }: AssetMapperContext): Promise> { - const payload = await getAssetPayload(identifier, asset, url, extension) + const filepath = (extensionPoint as Record)[identifier] + if (typeof filepath !== 'string') return {} + const payload = await getAssetPayload(identifier, filepath, url, extension) return { assets: {[payload.name]: payload}, } } /** - * Maps build manifest assets to payload format - * Each mapper returns a partial that gets merged into the extension point + * Intents asset mapper - iterates the extension point's intents array + * and resolves each intent's schema to an asset payload. + */ +async function intentsAssetMapper({ + extensionPoint, + url, + extension, +}: AssetMapperContext): Promise> { + if (!extensionPoint.intents) return {} + + const intents = await Promise.all( + extensionPoint.intents.map(async (intent) => ({ + ...intent, + schema: await getAssetPayload('schema', intent.schema, url, extension), + })), + ) + + return {intents} +} + +type AssetMapper = (context: AssetMapperContext) => Promise> + +/** + * Asset mappers registry - defines how each asset type should be handled. + * Assets not in this registry use the defaultAssetMapper. + */ +const ASSET_MAPPERS: {[key: string]: AssetMapper | undefined} = { + intents: intentsAssetMapper, +} + +/** + * Maps manifest entry to payload format. + * Uses the manifest entry to know which assets exist for a target, + * then reads source paths from the extension point config. */ -async function mapBuildManifestToPayload( - buildManifest: BuildManifest, - _extensionPoint: NewExtensionPointSchemaType & {build_manifest: BuildManifest}, +async function mapManifestAssetsToPayload( + manifestEntry: {[assetName: string]: unknown}, + extensionPoint: NewExtensionPointSchemaType, url: string, extension: ExtensionInstance, ): Promise> { - if (!buildManifest?.assets) return {} - const mappingResults = await Promise.all( - Object.entries(buildManifest.assets).map(async ([identifier, asset]) => { - return defaultAssetMapper({identifier, asset, url, extension}) + Object.keys(manifestEntry).map(async (identifier) => { + const context: AssetMapperContext = {identifier, extensionPoint, url, extension} + return ASSET_MAPPERS[identifier]?.(context) ?? defaultAssetMapper(context) }), ) @@ -196,10 +247,17 @@ export function isNewExtensionPointsSchema(extensionPoints: unknown): extensionP ) } -async function getAssetPayload(name: string, asset: BuildAsset, url: string, extension: ExtensionInstance) { +/** + * Builds an asset payload entry. + * + * `lastUpdated` is read from the source file in the extension directory so that + * payload updates only fire when the developer actually changes the source — not + * every time the build copies it fresh into the output directory. + */ +async function getAssetPayload(name: string, filepath: string, url: string, extension: ExtensionInstance) { return { name, - url: `${url}${joinPath('/assets/', asset.filepath)}`, - lastUpdated: (await fileLastUpdatedTimestamp(joinPath(dirname(extension.outputPath), asset.filepath))) ?? 0, + url: `${url}${joinPath('/assets/', filepath)}`, + lastUpdated: (await fileLastUpdatedTimestamp(joinPath(extension.directory, filepath))) ?? 0, } } 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 d82c4b0368d..0ba0c589bb1 100644 --- a/packages/app/src/cli/services/dev/extension/payload/models.ts +++ b/packages/app/src/cli/services/dev/extension/payload/models.ts @@ -1,5 +1,4 @@ import {Localization} from '../localization.js' -import {BuildManifest} from '../../../../models/extensions/specifications/ui_extension.js' import type {NewExtensionPointSchemaType, ApiVersionSchemaType} from '../../../../models/extensions/schemas.js' interface ExtensionsPayloadInterface { @@ -39,8 +38,7 @@ interface Asset { lastUpdated: number } -export interface DevNewExtensionPointSchema extends NewExtensionPointSchemaType { - build_manifest: BuildManifest +export interface DevNewExtensionPointSchema extends Omit { assets: { [name: string]: Asset } @@ -50,6 +48,13 @@ export interface DevNewExtensionPointSchema extends NewExtensionPointSchemaType resource: { url: string } + intents?: { + type: string + action: string + name?: string + description?: string + schema: Asset + }[] } interface SupportedFeatures { From 8bfa9702707d69b0013bafdd79a36c01906d0ef3 Mon Sep 17 00:00:00 2001 From: Trish Ta Date: Tue, 7 Apr 2026 19:11:34 -0400 Subject: [PATCH 2/3] Fix dev issues with manifest generation and files copying Include built assets in the manifest.json Allow serving static assets from the extensions directory --- .../specifications/ui_extension.test.ts | 148 ++---------------- .../extensions/specifications/ui_extension.ts | 59 ++----- .../src/cli/services/build/client-steps.ts | 17 +- .../services/build/steps/bundle-ui-step.ts | 44 +++++- .../build/steps/include-assets-step.test.ts | 129 +++++++++------ .../build/steps/include-assets-step.ts | 13 +- .../copy-config-key-entry.test.ts | 33 ++-- .../include-assets/copy-config-key-entry.ts | 63 ++++---- .../steps/include-assets/copy-source-entry.ts | 11 +- .../steps/include-assets/generate-manifest.ts | 58 +++++-- .../services/dev/extension/payload.test.ts | 131 +++++++++++++++- .../src/cli/services/dev/extension/payload.ts | 35 +++-- .../dev/extension/payload/store.test.ts | 47 ++++++ .../services/dev/extension/payload/store.ts | 2 +- .../dev/extension/server/middlewares.test.ts | 51 ++++-- .../dev/extension/server/middlewares.ts | 19 ++- 16 files changed, 511 insertions(+), 349 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.test.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.test.ts index c2dfcccb1d7..7e27bcece32 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.test.ts @@ -77,64 +77,6 @@ describe('ui_extension', async () => { }) } - async function setupToolsExtension( - tmpDir: string, - options: {tools?: string; instructions?: string; createFiles?: boolean} = {}, - ) { - await mkdir(joinPath(tmpDir, 'src')) - await touchFile(joinPath(tmpDir, 'src', 'ExtensionPointA.js')) - - if (options.createFiles) { - if (options.tools) { - await writeFile(joinPath(tmpDir, options.tools), '{"tools": []}') - } - if (options.instructions) { - await writeFile(joinPath(tmpDir, options.instructions), '# Instructions') - } - } - - const allSpecs = await loadLocalExtensionsSpecifications() - const specification = allSpecs.find((spec) => spec.identifier === 'ui_extension')! - - const targetConfig: any = { - target: 'EXTENSION::POINT::A', - module: './src/ExtensionPointA.js', - } - - if (options.tools) targetConfig.tools = options.tools - if (options.instructions) targetConfig.instructions = options.instructions - - const configuration = { - targeting: [targetConfig], - api_version: '2023-01' as const, - name: 'UI Extension', - description: 'This is an ordinary test extension', - type: 'ui_extension', - handle: 'test-ui-extension', - capabilities: { - block_progress: false, - network_access: false, - api_access: false, - collect_buyer_consent: { - customer_privacy: true, - sms_marketing: false, - }, - iframe: { - sources: [], - }, - }, - settings: {}, - } - - const parsed = specification.parseConfigurationObject(configuration) - if (parsed.state !== 'ok') { - throw new Error("Couldn't parse configuration") - } - - const result = await specification.validate?.(parsed.data, joinPath(tmpDir, 'shopify.extension.toml'), tmpDir) - return {result, tmpDir} - } - describe('validate()', () => { test('returns ok({}) if there are no errors', async () => { await inTemporaryDirectory(async (tmpDir) => { @@ -208,6 +150,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [{namespace: 'test', key: 'test'}], default_placement_reference: undefined, @@ -275,6 +218,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], default_placement_reference: 'PLACEMENT_REFERENCE1', @@ -338,6 +282,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], urls: {}, @@ -401,6 +346,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], default_placement_reference: undefined, @@ -467,6 +413,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], default_placement_reference: undefined, @@ -535,6 +482,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], default_placement_reference: undefined, @@ -603,6 +551,7 @@ describe('ui_extension', async () => { module: './src/ExtensionPointA.js', tools: './tools.json', instructions: undefined, + intents: undefined, metafields: [], default_placement_reference: undefined, capabilities: undefined, @@ -613,11 +562,6 @@ describe('ui_extension', async () => { filepath: 'test-ui-extension.js', module: './src/ExtensionPointA.js', }, - tools: { - filepath: 'test-ui-extension-tools-tools.json', - module: './tools.json', - static: true, - }, }, }, urls: {}, @@ -671,6 +615,7 @@ describe('ui_extension', async () => { module: './src/ExtensionPointA.js', tools: undefined, instructions: './instructions.md', + intents: undefined, metafields: [], default_placement_reference: undefined, capabilities: undefined, @@ -681,11 +626,6 @@ describe('ui_extension', async () => { filepath: 'test-ui-extension.js', module: './src/ExtensionPointA.js', }, - instructions: { - filepath: 'test-ui-extension-instructions-instructions.md', - module: './instructions.md', - static: true, - }, }, }, urls: {}, @@ -791,67 +731,6 @@ Please check the configuration in ${uiExtension.configurationPath}`), }) }) - test('shows an error when the tools file is missing', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const {result} = await setupToolsExtension(tmpDir, {tools: './tools.json'}) - - const notFoundPath = joinPath(tmpDir, './tools.json') - expect(result).toEqual( - err(`Couldn't find ${notFoundPath} - Please check the tools path for EXTENSION::POINT::A - -Please check the configuration in ${joinPath(tmpDir, 'shopify.extension.toml')}`), - ) - }) - }) - - test('shows an error when the instructions file is missing', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const {result} = await setupToolsExtension(tmpDir, {instructions: './instructions.md'}) - - const notFoundPath = joinPath(tmpDir, './instructions.md') - expect(result).toEqual( - err(`Couldn't find ${notFoundPath} - Please check the instructions path for EXTENSION::POINT::A - -Please check the configuration in ${joinPath(tmpDir, 'shopify.extension.toml')}`), - ) - }) - }) - - test('shows multiple errors when both tools and instructions files are missing', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const {result} = await setupToolsExtension(tmpDir, { - tools: './tools.json', - instructions: './instructions.md', - }) - - const toolsNotFoundPath = joinPath(tmpDir, './tools.json') - const instructionsNotFoundPath = joinPath(tmpDir, './instructions.md') - expect(result).toEqual( - err(`Couldn't find ${toolsNotFoundPath} - Please check the tools path for EXTENSION::POINT::A - -Couldn't find ${instructionsNotFoundPath} - Please check the instructions path for EXTENSION::POINT::A - -Please check the configuration in ${joinPath(tmpDir, 'shopify.extension.toml')}`), - ) - }) - }) - - test('succeeds when both tools and instructions files exist', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const {result} = await setupToolsExtension(tmpDir, { - tools: './tools.json', - instructions: './instructions.md', - createFiles: true, - }) - - expect(result).toStrictEqual(ok({})) - }) - }) - test('build_manifest includes both tools and instructions when both are present', async () => { const allSpecs = await loadLocalExtensionsSpecifications() const specification = allSpecs.find((spec) => spec.identifier === 'ui_extension')! @@ -899,6 +778,7 @@ Please check the configuration in ${joinPath(tmpDir, 'shopify.extension.toml')}` module: './src/ExtensionPointA.js', tools: './tools.json', instructions: './instructions.md', + intents: undefined, metafields: [], default_placement_reference: undefined, capabilities: undefined, @@ -909,16 +789,6 @@ Please check the configuration in ${joinPath(tmpDir, 'shopify.extension.toml')}` filepath: 'test-ui-extension.js', module: './src/ExtensionPointA.js', }, - tools: { - filepath: 'test-ui-extension-tools-tools.json', - module: './tools.json', - static: true, - }, - instructions: { - filepath: 'test-ui-extension-instructions-instructions.md', - module: './instructions.md', - static: true, - }, }, }, urls: {}, diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 7030fc95201..636ee1fb394 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -14,7 +14,7 @@ import {ExtensionInstance} from '../extension-instance.js' import {formatContent} from '../../../utilities/file-formatter.js' import {err, ok, Result} from '@shopify/cli-kit/node/result' import {copyFile, fileExists, readFile} from '@shopify/cli-kit/node/fs' -import {joinPath, basename, dirname} from '@shopify/cli-kit/node/path' +import {joinPath, dirname} from '@shopify/cli-kit/node/path' import {outputContent, outputToken, outputWarn} from '@shopify/cli-kit/node/output' import {zod} from '@shopify/cli-kit/node/schema' @@ -29,8 +29,6 @@ export interface BuildManifest { // Main asset is always required [AssetIdentifier.Main]: BuildAsset [AssetIdentifier.ShouldRender]?: BuildAsset - [AssetIdentifier.Tools]?: BuildAsset - [AssetIdentifier.Instructions]?: BuildAsset } } @@ -61,24 +59,6 @@ export const UIExtensionSchema = BaseSchema.extend({ }, } : null), - ...(targeting.tools - ? { - [AssetIdentifier.Tools]: { - filepath: `${config.handle}-${AssetIdentifier.Tools}-${basename(targeting.tools)}`, - module: targeting.tools, - static: true, - }, - } - : null), - ...(targeting.instructions - ? { - [AssetIdentifier.Instructions]: { - filepath: `${config.handle}-${AssetIdentifier.Instructions}-${basename(targeting.instructions)}`, - module: targeting.instructions, - static: true, - }, - } - : null), }, } @@ -93,6 +73,7 @@ export const UIExtensionSchema = BaseSchema.extend({ build_manifest: buildManifest, tools: targeting.tools, instructions: targeting.instructions, + intents: targeting.intents, } }) return {...config, extension_points: extensionPoints} @@ -108,7 +89,7 @@ const uiExtensionSpec = createExtensionSpecification({ { lifecycle: 'deploy', steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, + {id: 'bundle-ui', generatesAssetsManifest: true, name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, { id: 'include-ui-extension-assets', name: 'Include UI Extension Assets', @@ -118,21 +99,21 @@ const uiExtensionSpec = createExtensionSpecification({ inclusions: [ { type: 'configKey', - anchor: 'targeting[]', + anchor: 'extension_points[]', groupBy: 'target', - key: 'targeting[].tools', + key: 'extension_points[].tools', }, { type: 'configKey', - anchor: 'targeting[]', + anchor: 'extension_points[]', groupBy: 'target', - key: 'targeting[].instructions', + key: 'extension_points[].instructions', }, { type: 'configKey', - anchor: 'targeting[]', + anchor: 'extension_points[]', groupBy: 'target', - key: 'targeting[].intents[].schema', + key: 'extension_points[].intents[].schema', }, ], }, @@ -415,33 +396,13 @@ async function validateUIExtensionPointConfig( } for await (const extensionPoint of extensionPoints) { - const {module, target, build_manifest: buildManifest} = extensionPoint + const {module, target} = extensionPoint const missingModuleError = await checkForMissingPath(directory, module, target, 'module') if (missingModuleError) { errors.push(missingModuleError) } - const missingToolsError = await checkForMissingPath( - directory, - buildManifest?.assets[AssetIdentifier.Tools]?.module, - target, - AssetIdentifier.Tools, - ) - if (missingToolsError) { - errors.push(missingToolsError) - } - - const missingInstructionsError = await checkForMissingPath( - directory, - buildManifest?.assets[AssetIdentifier.Instructions]?.module, - target, - AssetIdentifier.Instructions, - ) - if (missingInstructionsError) { - errors.push(missingInstructionsError) - } - if (uniqueTargets.includes(target)) { duplicateTargets.push(target) } else { diff --git a/packages/app/src/cli/services/build/client-steps.ts b/packages/app/src/cli/services/build/client-steps.ts index 27bb601bd46..68e3e446004 100644 --- a/packages/app/src/cli/services/build/client-steps.ts +++ b/packages/app/src/cli/services/build/client-steps.ts @@ -21,15 +21,16 @@ interface IncludeAssetsStep extends BaseStep { readonly config: IncludeAssetsConfig } +/** Step with typed config specific to bundle_ui. */ +interface BundleUIStep extends BaseStep { + readonly type: 'bundle_ui' + readonly generatesAssetsManifest?: boolean + readonly config?: Record +} + /** Steps that don't require any config yet. */ interface NoConfigStep extends BaseStep { - readonly type: - | 'build_theme' - | 'bundle_theme' - | 'bundle_ui' - | 'copy_static_assets' - | 'build_function' - | 'create_tax_stub' + readonly type: 'build_theme' | 'bundle_theme' | 'copy_static_assets' | 'build_function' | 'create_tax_stub' readonly config?: Record } @@ -40,7 +41,7 @@ interface NoConfigStep extends BaseStep { * This is a discriminated union on `type`: each step type carries its own * typed `config`, so TypeScript catches config typos at compile time. */ -export type LifecycleStep = IncludeAssetsStep | NoConfigStep +export type LifecycleStep = IncludeAssetsStep | BundleUIStep | NoConfigStep /** * A group of steps scoped to a specific lifecycle phase. diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts index 0e33925e4cb..517d969207c 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts +++ b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts @@ -1,11 +1,51 @@ +import {mergeManifestEntries} from './include-assets/generate-manifest.js' import {buildUIExtension} from '../extension.js' +import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' import type {LifecycleStep, BuildContext} from '../client-steps.js' +interface ExtensionPointWithBuildManifest { + target: string + build_manifest: BuildManifest +} + /** * Executes a bundle_ui build step. * * Bundles the UI extension using esbuild, writing output to extension.outputPath. + * When `generatesAssetsManifest` is true, writes built asset entries (from + * build_manifest) into manifest.json so downstream steps can merge on top. + */ +export async function executeBundleUIStep(step: LifecycleStep, context: BuildContext): Promise { + await buildUIExtension(context.extension, context.options) + + if (!('generatesAssetsManifest' in step) || !step.generatesAssetsManifest) return + + const config = context.extension.configuration as Record + const extensionPoints = config.extension_points + if (!Array.isArray(extensionPoints) || !extensionPoints.every((ep) => typeof ep === 'object' && ep?.build_manifest)) + return + + const entries = extractBuiltAssetEntries(extensionPoints as ExtensionPointWithBuildManifest[]) + if (Object.keys(entries).length > 0) { + await mergeManifestEntries(context, entries) + } +} + +/** + * Extracts built asset filepaths from `build_manifest` on each extension point, + * grouped by target. Returns a map of target → `{assetName: filepath}`. */ -export async function executeBundleUIStep(_step: LifecycleStep, context: BuildContext): Promise { - return buildUIExtension(context.extension, context.options) +function extractBuiltAssetEntries(extensionPoints: {target: string; build_manifest: BuildManifest}[]): { + [target: string]: {[assetName: string]: string} +} { + const entries: {[target: string]: {[assetName: string]: string}} = {} + for (const {target, build_manifest: buildManifest} of extensionPoints) { + if (!buildManifest?.assets) continue + const assets: {[name: string]: string} = {} + for (const [name, asset] of Object.entries(buildManifest.assets)) { + if (asset?.filepath) assets[name] = asset.filepath + } + if (Object.keys(assets).length > 0) entries[target] = assets + } + return entries } 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 ac4e89dfaba..e9decd6b02d 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 @@ -259,8 +259,7 @@ describe('executeIncludeAssetsStep', () => { expect(fs.copyDirectoryContents).not.toHaveBeenCalled() }) - test('skips path that does not exist on disk but logs a warning', async () => { - // Given + test('throws an error when the referenced file does not exist on disk', async () => { const contextWithConfig = { ...mockContext, extension: { @@ -280,18 +279,68 @@ describe('executeIncludeAssetsStep', () => { }, } - // When - const result = await executeIncludeAssetsStep(step, contextWithConfig) + await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow( + `Couldn't find /test/extension/nonexistent\n Please check the path 'nonexistent' in your configuration`, + ) + }) - // Then — no error, logged warning - expect(result.filesCopied).toBe(0) - expect(mockStdout.write).toHaveBeenCalledWith( - expect.stringContaining("Warning: path 'nonexistent' does not exist"), + test('throws an error when an intent schema file does not exist on disk', async () => { + const contextWithConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + targeting: [ + { + target: 'admin.app.intent.link', + intents: [{type: 'application/email', action: 'edit', schema: './email-schema.json'}], + }, + ], + }, + } as unknown as ExtensionInstance, + } + + vi.mocked(fs.fileExists).mockResolvedValue(false) + + const step: LifecycleStep = { + id: 'copy-intents', + name: 'Copy Intents', + type: 'include_assets', + config: { + inclusions: [{type: 'configKey', key: 'targeting[].intents[].schema'}], + }, + } + + await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow( + `Couldn't find /test/extension/email-schema.json\n Please check the path './email-schema.json' in your configuration`, ) }) - test('renames output file to avoid collision when candidate path already exists', async () => { - // Given — tools.json already exists in the output dir; findUniqueDestPath must try tools-1.json + test('does not throw when intent config key is absent', async () => { + const contextWithConfig = { + ...mockContext, + extension: { + ...mockExtension, + configuration: { + targeting: [{target: 'admin.app.intent.link'}], + }, + } as unknown as ExtensionInstance, + } + + const step: LifecycleStep = { + id: 'copy-intents', + name: 'Copy Intents', + type: 'include_assets', + config: { + inclusions: [{type: 'configKey', key: 'targeting[].intents[].schema'}], + }, + } + + const result = await executeIncludeAssetsStep(step, contextWithConfig) + expect(result.filesCopied).toBe(0) + }) + + test('overwrites existing file on rebuild', async () => { const contextWithConfig = { ...mockContext, extension: { @@ -302,7 +351,7 @@ describe('executeIncludeAssetsStep', () => { vi.mocked(fs.fileExists).mockImplementation(async (path) => { const pathStr = String(path) - // Source file exists; first candidate output path is taken; suffixed path is free + // Source file exists; output file also exists from previous build return pathStr === '/test/extension/tools.json' || pathStr === '/test/output/tools.json' }) vi.mocked(fs.isDirectory).mockResolvedValue(false) @@ -318,30 +367,28 @@ describe('executeIncludeAssetsStep', () => { }, } - // When const result = await executeIncludeAssetsStep(step, contextWithConfig) - // Then — copied to tools-1.json, not tools.json - expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/tools.json', '/test/output/tools-1.json') + // Overwrites the existing file rather than creating tools-1.json + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/tools.json', '/test/output/tools.json') expect(result.filesCopied).toBe(1) }) - test('throws after exhausting 1000 rename attempts', async () => { - // Given — every candidate path (tools.json, tools-1.json … tools-1000.json) is taken + test('renames file to avoid collision when two different sources share the same basename', async () => { const contextWithConfig = { ...mockContext, extension: { ...mockExtension, - configuration: {tools: './tools.json'}, + configuration: {tools_a: './a/schema.json', tools_b: './b/schema.json'}, } as unknown as ExtensionInstance, } vi.mocked(fs.fileExists).mockImplementation(async (path) => { const pathStr = String(path) - // Source file exists; all 1001 output candidates are occupied - return pathStr.startsWith('/test/extension/') || pathStr.startsWith('/test/output/tools') + return pathStr === '/test/extension/a/schema.json' || pathStr === '/test/extension/b/schema.json' }) vi.mocked(fs.isDirectory).mockResolvedValue(false) + vi.mocked(fs.copyFile).mockResolvedValue() vi.mocked(fs.mkdir).mockResolvedValue() const step: LifecycleStep = { @@ -349,14 +396,18 @@ describe('executeIncludeAssetsStep', () => { name: 'Copy Tools', type: 'include_assets', config: { - inclusions: [{type: 'configKey', key: 'tools'}], + inclusions: [ + {type: 'configKey', key: 'tools_a'}, + {type: 'configKey', key: 'tools_b'}, + ], }, } - // When / Then - await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow( - "Unable to find unique destination path for 'tools.json' in '/test/output' after 1000 attempts", - ) + const result = await executeIncludeAssetsStep(step, contextWithConfig) + + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/a/schema.json', '/test/output/schema.json') + expect(fs.copyFile).toHaveBeenCalledWith('/test/extension/b/schema.json', '/test/output/schema-1.json') + expect(result.filesCopied).toBe(2) }) test('resolves array config value and copies each path', async () => { @@ -664,9 +715,7 @@ describe('executeIncludeAssetsStep', () => { beforeEach(() => { vi.mocked(fs.writeFile).mockResolvedValue() vi.mocked(fs.mkdir).mockResolvedValue() - // Source files exist; destination paths don't yet (so findUniqueDestPath - // resolves on the first candidate without looping). Individual tests can - // override for specific scenarios. + // Source files exist. Individual tests can override for specific scenarios. vi.mocked(fs.fileExists).mockImplementation( async (path) => typeof path === 'string' && path.startsWith('/test/extension'), ) @@ -869,8 +918,6 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - vi.mocked(fs.fileExists).mockResolvedValue(false) - // No generatesAssetsManifest field — defaults to false const step: LifecycleStep = { id: 'gen-manifest', @@ -1056,11 +1103,12 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - // Source files exist; output manifest.json already exists + // Source files exist; output manifest.json already exists from a prior step vi.mocked(fs.fileExists).mockImplementation(async (path) => { const pathStr = String(path) return pathStr === '/test/output/manifest.json' || pathStr.startsWith('/test/extension/') }) + vi.mocked(fs.readFile).mockResolvedValue(Buffer.from('{ "admin.intent.link": { "tools": "tools.json" } }')) vi.mocked(fs.glob).mockResolvedValue([]) const step: LifecycleStep = { @@ -1185,8 +1233,6 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - vi.mocked(fs.fileExists).mockResolvedValue(false) - const step: LifecycleStep = { id: 'gen-manifest', name: 'Generate Manifest', @@ -1358,9 +1404,7 @@ describe('executeIncludeAssetsStep', () => { ) }) - test('warns when a manifest entry contains an unresolved path because the source file was missing', async () => { - // Given — tools.json is referenced in config but does not exist on disk, - // so copyConfigKeyEntry skips it and it never enters pathMap. + test('throws when a referenced source file does not exist on disk', async () => { const contextWithConfig = { ...mockContext, extension: { @@ -1371,9 +1415,7 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - // Source file does not exist; output paths are free vi.mocked(fs.fileExists).mockResolvedValue(false) - vi.mocked(fs.glob).mockResolvedValue([]) const step: LifecycleStep = { id: 'gen-manifest', @@ -1392,18 +1434,9 @@ describe('executeIncludeAssetsStep', () => { }, } - // When - await executeIncludeAssetsStep(step, contextWithConfig) - - // Then — raw './tools.json' appears in manifest (not copied → not resolved), - // and a diagnostic warning is logged so the user knows a file is missing. - const writeFileCall = vi.mocked(fs.writeFile).mock.calls[0]! - const manifestContent = JSON.parse(writeFileCall[1] as string) - expect(manifestContent).toEqual({'admin.intent.link': {tools: './tools.json'}}) - expect(mockStdout.write).toHaveBeenCalledWith( - expect.stringContaining("manifest entry 'admin.intent.link' contains unresolved paths"), + await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow( + `Couldn't find /test/extension/tools.json\n Please check the path './tools.json' in your configuration`, ) - expect(mockStdout.write).toHaveBeenCalledWith(expect.stringContaining("path './tools.json' does not exist")) }) }) }) diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.ts b/packages/app/src/cli/services/build/steps/include-assets-step.ts index ed20363ec6b..7aa8a41bab1 100644 --- a/packages/app/src/cli/services/build/steps/include-assets-step.ts +++ b/packages/app/src/cli/services/build/steps/include-assets-step.ts @@ -128,11 +128,13 @@ export async function executeIncludeAssetsStep( const outputDir = extname(extension.outputPath) ? dirname(extension.outputPath) : extension.outputPath const aggregatedPathMap = new Map() + // Track basenames written across all configKey entries in this build to detect + // true conflicts (different sources, same basename) while allowing overwrites + // from previous builds. + const usedBasenames = new Set() - // configKey entries run sequentially: copyConfigKeyEntry uses findUniqueDestPath - // which checks filesystem state before writing. Running two configKey entries in - // parallel against the same output directory can cause both to see the same - // candidate path as free and silently overwrite each other. + // configKey entries run sequentially to avoid filesystem race conditions + // when multiple entries target the same output directory. let configKeyCount = 0 for (const entry of config.inclusions) { if (entry.type !== 'configKey') continue @@ -146,6 +148,7 @@ export async function executeIncludeAssetsStep( outputDir, context, destination: sanitizedDest, + usedBasenames, }) result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val)) configKeyCount += result.filesCopied @@ -198,7 +201,7 @@ export async function executeIncludeAssetsStep( if (config.generatesAssetsManifest) { const configKeyEntries = config.inclusions.filter((entry) => entry.type === 'configKey') - await generateManifestFile(configKeyEntries, context, outputDir, aggregatedPathMap, otherFiles) + await generateManifestFile(configKeyEntries, context, aggregatedPathMap, otherFiles) } return {filesCopied: counts.reduce((sum, count) => sum + (count ?? 0), 0)} diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts index a23265aab5f..d46d085abb9 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts @@ -58,23 +58,25 @@ describe('copyConfigKeyEntry', () => { }) }) - test('renames output file to avoid collision when candidate path already exists', async () => { + test('renames output file to avoid collision when two sources share the same basename', async () => { await inTemporaryDirectory(async (tmpDir) => { - await writeFile(joinPath(tmpDir, 'tools-a.json'), '{}') - await writeFile(joinPath(tmpDir, 'tools-b.json'), '{}') + const dirA = joinPath(tmpDir, 'a') + const dirB = joinPath(tmpDir, 'b') + await mkdir(dirA) + await mkdir(dirB) + await writeFile(joinPath(dirA, 'schema.json'), '{"a": true}') + await writeFile(joinPath(dirB, 'schema.json'), '{"b": true}') const outDir = joinPath(tmpDir, 'out') await mkdir(outDir) - // Pre-create the first candidate to force a rename - await writeFile(joinPath(outDir, 'tools-a.json'), 'existing') - const context = makeContext({files: ['tools-a.json', 'tools-b.json']}, mockStdout) + const context = makeContext({files: ['a/schema.json', 'b/schema.json']}, mockStdout) const result = await copyConfigKeyEntry({key: 'files', baseDir: tmpDir, outputDir: outDir, context}) expect(result.filesCopied).toBe(2) - // tools-a.json was taken, so the copy lands as tools-a-1.json - await expect(fileExists(joinPath(outDir, 'tools-a-1.json'))).resolves.toBe(true) - await expect(fileExists(joinPath(outDir, 'tools-b.json'))).resolves.toBe(true) + // Both have basename schema.json — second one gets renamed + await expect(fileExists(joinPath(outDir, 'schema.json'))).resolves.toBe(true) + await expect(fileExists(joinPath(outDir, 'schema-1.json'))).resolves.toBe(true) }) }) @@ -91,20 +93,15 @@ describe('copyConfigKeyEntry', () => { }) }) - test('skips with warning when path resolved from config does not exist on disk', async () => { + test('throws when path resolved from config does not exist on disk', async () => { await inTemporaryDirectory(async (tmpDir) => { const outDir = joinPath(tmpDir, 'out') await mkdir(outDir) - // 'nonexistent' directory is NOT created, so fileExists returns false naturally const context = makeContext({assets_dir: 'nonexistent'}, mockStdout) - const result = await copyConfigKeyEntry({key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context}) - - expect(result.filesCopied).toBe(0) - expect(result.pathMap.size).toBe(0) - expect(mockStdout.write).toHaveBeenCalledWith( - expect.stringContaining("Warning: path 'nonexistent' does not exist"), - ) + await expect( + copyConfigKeyEntry({key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context}), + ).rejects.toThrow(`Couldn't find`) }) }) diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts index aed94be0583..cf4ef145e20 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts @@ -1,6 +1,6 @@ import {joinPath, basename, relativePath, extname} from '@shopify/cli-kit/node/path' import {glob, copyFile, copyDirectoryContents, fileExists, mkdir, isDirectory} from '@shopify/cli-kit/node/fs' -import {outputDebug} from '@shopify/cli-kit/node/output' +import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import type {BuildContext} from '../../client-steps.js' /** @@ -11,10 +11,8 @@ import type {BuildContext} from '../../client-steps.js' * skipped silently with a log message. When `destination` is given, the * resolved directory is placed under `outputDir/destination`. * - * File sources are copied with `copyFile` using a unique destination name - * (via `findUniqueDestPath`) to prevent overwrites when multiple config values - * resolve to files with the same basename. Directory sources use - * `copyDirectoryContents`. + * File sources are copied with `copyFile` to the output directory. + * Directory sources use `copyDirectoryContents`. * * Returns `{filesCopied, pathMap}` where `pathMap` maps each raw config path * value to its output-relative location. File sources map to a single string. @@ -26,8 +24,9 @@ export async function copyConfigKeyEntry(config: { outputDir: string context: BuildContext destination?: string + usedBasenames?: Set }): Promise<{filesCopied: number; pathMap: Map}> { - const {key, baseDir, outputDir, context, destination} = config + const {key, baseDir, outputDir, context, destination, usedBasenames = new Set()} = config const {stdout} = context.options const value = getNestedValue(context.extension.configuration, key) let paths: string[] @@ -50,8 +49,7 @@ export async function copyConfigKeyEntry(config: { // should only be copied once; the pathMap entry is reused for all references. const uniquePaths = [...new Set(paths)] - // Process sequentially — findUniqueDestPath relies on filesystem state that - // would race if multiple copies ran in parallel against the same output dir. + // Process sequentially to avoid filesystem race conditions on shared output paths. const pathMap = new Map() let filesCopied = 0 @@ -60,8 +58,10 @@ export async function copyConfigKeyEntry(config: { const fullPath = joinPath(baseDir, sourcePath) const exists = await fileExists(fullPath) if (!exists) { - stdout.write(`Warning: path '${sourcePath}' does not exist, skipping\n`) - continue + throw new Error( + outputContent`Couldn't find ${outputToken.path(fullPath)}\n Please check the path '${sourcePath}' in your configuration` + .value, + ) } const sourceIsDir = await isDirectory(fullPath) @@ -69,17 +69,25 @@ export async function copyConfigKeyEntry(config: { const destDir = effectiveOutputDir if (sourceIsDir) { + // Glob the source directory (not the destination) to get the accurate file list. + // During dev, the include_assets step runs on every rebuild. If we glob the + // destination instead, it would pick up files accumulated from previous builds + // that may no longer exist in the source, inflating the file count and producing + // stale entries in the manifest's pathMap. + const sourceFiles = await glob(['**/*'], {cwd: fullPath, absolute: false}) await copyDirectoryContents(fullPath, destDir) - const copied = await glob(['**/*'], {cwd: destDir, absolute: false}) stdout.write(`Included '${sourcePath}'\n`) - const relFiles = copied.map((file) => relativePath(outputDir, joinPath(destDir, file))) + const relFiles = sourceFiles.map((file) => relativePath(outputDir, joinPath(destDir, file))) pathMap.set(sourcePath, relFiles) - filesCopied += copied.length + filesCopied += sourceFiles.length } else { await mkdir(destDir) - const uniqueDestPath = await findUniqueDestPath(destDir, basename(fullPath)) - await copyFile(fullPath, uniqueDestPath) - const outputRelative = relativePath(outputDir, uniqueDestPath) + const filename = basename(fullPath) + const destFilename = uniqueBasename(filename, usedBasenames) + usedBasenames.add(destFilename) + const destPath = joinPath(destDir, destFilename) + await copyFile(fullPath, destPath) + const outputRelative = relativePath(outputDir, destPath) stdout.write(`Included '${sourcePath}'\n`) pathMap.set(sourcePath, outputRelative) filesCopied += 1 @@ -91,25 +99,24 @@ export async function copyConfigKeyEntry(config: { } /** - * Returns a destination path for `filename` inside `dir` that does not already - * exist. If `dir/filename` is free, returns it as-is. Otherwise appends a - * counter before the extension: `name-1.ext`, `name-2.ext`, … + * Returns a unique filename given the set of basenames already used in this + * build. If `filename` hasn't been used, returns it as-is. Otherwise appends + * a counter: `name-1.ext`, `name-2.ext`, … */ -async function findUniqueDestPath(dir: string, filename: string): Promise { - const candidate = joinPath(dir, filename) - if (!(await fileExists(candidate))) return candidate +function uniqueBasename(filename: string, used: Set): string { + if (!used.has(filename)) return filename const ext = extname(filename) const base = ext ? filename.slice(0, -ext.length) : filename - let counter = 1 const maxAttempts = 1000 - while (counter <= maxAttempts) { - const next = joinPath(dir, `${base}-${counter}${ext}`) - // eslint-disable-next-line no-await-in-loop - if (!(await fileExists(next))) return next + let counter = 1 + while (used.has(`${base}-${counter}${ext}`)) { counter++ + if (counter > maxAttempts) { + throw new Error(`Unable to find unique basename for '${filename}' after ${maxAttempts} attempts`) + } } - throw new Error(`Unable to find unique destination path for '${filename}' in '${dir}' after ${maxAttempts} attempts`) + return `${base}-${counter}${ext}` } /** diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts index 9f153c73415..f573338b74b 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts @@ -36,12 +36,17 @@ export async function copySourceEntry( } if (sourceIsDir) { + // Glob the source directory (not the destination) to get the accurate file list. + // During dev, the include_assets step runs on every rebuild. If we glob the + // destination instead, it would pick up files accumulated from previous builds + // that may no longer exist in the source, inflating the file count and producing + // stale entries in the manifest. + const sourceFiles = await glob(['**/*'], {cwd: sourcePath, absolute: false}) await copyDirectoryContents(sourcePath, destPath) - const copied = await glob(['**/*'], {cwd: destPath, absolute: false}) options.stdout.write(logMsg) const destRelToOutput = relativePath(outputDir, destPath) - const outputPaths = destRelToOutput ? copied.map((file) => joinPath(destRelToOutput, file)) : copied - return {filesCopied: copied.length, outputPaths} + const outputPaths = destRelToOutput ? sourceFiles.map((file) => joinPath(destRelToOutput, file)) : sourceFiles + return {filesCopied: sourceFiles.length, outputPaths} } await mkdir(dirname(destPath)) 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 20cdc405ead..66f6e3a6777 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 {mkdir, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath, dirname, extname} from '@shopify/cli-kit/node/path' +import {fileExists, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' import type {BuildContext} from '../../client-steps.js' @@ -28,7 +28,6 @@ interface ConfigKeyManifestEntry { export async function generateManifestFile( configKeyEntries: ConfigKeyManifestEntry[], context: BuildContext, - outputDir: string, pathMap: Map, otherFiles: string[], ): Promise { @@ -96,10 +95,8 @@ export async function generateManifestFile( ) } - if (Object.prototype.hasOwnProperty.call(manifest, manifestKey)) { - options.stdout.write(`Warning: duplicate manifest key '${manifestKey}' — later entry overwrites earlier one\n`) - } - manifest[manifestKey] = shallowMerge(partials) + const existing = (manifest[manifestKey] ?? {}) as {[key: string]: unknown} + manifest[manifestKey] = shallowMerge([existing, ...partials]) } } @@ -112,10 +109,51 @@ export async function generateManifestFile( return } + await mergeManifestEntries(context, manifest) +} + +/** + * Merges new entries into the manifest.json for an extension. + * Reads the existing manifest if present and deep merges the new entries. + * This allows multiple build steps to contribute to the same manifest. + */ +export async function mergeManifestEntries(context: BuildContext, entries: {[key: string]: unknown}): Promise { + const outputPath = context.extension.outputPath + /** + * Resolves the output directory from an extension's outputPath. + * When outputPath is a file (has extension), uses dirname. Otherwise uses outputPath directly. + */ + const outputDir = extname(outputPath) ? dirname(outputPath) : outputPath + const manifestPath = joinPath(outputDir, 'manifest.json') - await mkdir(outputDir) - await writeFile(manifestPath, JSON.stringify(manifest, null, 2)) - outputDebug(`Generated manifest.json in ${outputDir}\n`, options.stdout) + + let existing: {[key: string]: unknown} = {} + if (await fileExists(manifestPath)) { + const content = await readFile(manifestPath) + existing = JSON.parse(content) + } else { + // Create the output directory + await mkdir(outputDir) + } + + // Deep merge: for each key, if both old and new are objects, merge their fields + for (const [key, value] of Object.entries(entries)) { + const existingValue = existing[key] + if ( + existingValue && + typeof existingValue === 'object' && + !Array.isArray(existingValue) && + typeof value === 'object' && + !Array.isArray(value) + ) { + existing[key] = {...(existingValue as Record), ...(value as Record)} + } else { + existing[key] = value + } + } + + await writeFile(manifestPath, JSON.stringify(existing, null, 2)) + outputDebug(`Updated manifest.json in ${outputDir}\n`, context.options.stdout) } /** diff --git a/packages/app/src/cli/services/dev/extension/payload.test.ts b/packages/app/src/cli/services/dev/extension/payload.test.ts index 566d0cd20cb..789d631cf39 100644 --- a/packages/app/src/cli/services/dev/extension/payload.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload.test.ts @@ -1,11 +1,10 @@ -import {UIExtensionPayload} from './payload/models.js' import {getUIExtensionPayload} from './payload.js' import {ExtensionsPayloadStoreOptions} from './payload/store.js' import {testUIExtension} from '../../../models/app/app.test-data.js' import * as appModel from '../../../models/app/app.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' -import {dirname, joinPath} from '@shopify/cli-kit/node/path' +import {dirname, extname, joinPath} from '@shopify/cli-kit/node/path' describe('getUIExtensionPayload', () => { beforeEach(() => { @@ -44,9 +43,9 @@ describe('getUIExtensionPayload', () => { sourceFiles: Record, ) { const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath) - const buildDir = dirname(extensionOutputPath) + const buildDir = extname(extensionOutputPath) ? dirname(extensionOutputPath) : extensionOutputPath await mkdir(buildDir) - await touchFile(extensionOutputPath) + if (extname(extensionOutputPath)) await touchFile(extensionOutputPath) await writeFile(joinPath(buildDir, 'manifest.json'), JSON.stringify(manifest)) for (const [filepath, content] of Object.entries(sourceFiles)) { @@ -183,6 +182,68 @@ describe('getUIExtensionPayload', () => { }) }) + test('maps main and should_render from build_manifest', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const buildManifest = { + assets: { + main: {module: './src/ExtensionPointA.js', filepath: 'test-ui-extension.js'}, + should_render: {module: './src/ShouldRender.js', filepath: 'test-ui-extension-conditions.js'}, + }, + } + + const uiExtension = await testUIExtension({ + directory: tmpDir, + configuration: { + name: 'test-ui-extension', + type: 'ui_extension', + extension_points: [ + { + target: 'CUSTOM_EXTENSION_POINT', + module: './src/ExtensionPointA.js', + build_manifest: buildManifest, + }, + ], + }, + devUUID: 'devUUID', + }) + + // Create source files so lastUpdated resolves + await mkdir(joinPath(tmpDir, 'src')) + await writeFile(joinPath(tmpDir, 'src', 'ExtensionPointA.js'), '// main') + await writeFile(joinPath(tmpDir, 'src', 'ShouldRender.js'), '// should render') + + await setupBuildOutput( + uiExtension, + tmpDir, + {CUSTOM_EXTENSION_POINT: {main: 'test-ui-extension.js', should_render: 'test-ui-extension-conditions.js'}}, + {}, + ) + + const got = await getUIExtensionPayload(uiExtension, tmpDir, { + ...createMockOptions(tmpDir, [uiExtension]), + currentDevelopmentPayload: {hidden: true, status: 'success'}, + }) + + expect(got.extensionPoints).toMatchObject([ + { + target: 'CUSTOM_EXTENSION_POINT', + assets: { + main: { + name: 'main', + url: 'http://tunnel-url.com/extensions/devUUID/assets/src/ExtensionPointA.js', + lastUpdated: expect.any(Number), + }, + should_render: { + name: 'should_render', + url: 'http://tunnel-url.com/extensions/devUUID/assets/src/ShouldRender.js', + lastUpdated: expect.any(Number), + }, + }, + }, + ]) + }) + }) + test('maps intents from manifest.json to asset payloads', async () => { await inTemporaryDirectory(async (tmpDir) => { const uiExtension = await testUIExtension({ @@ -244,6 +305,68 @@ describe('getUIExtensionPayload', () => { }) }) + test('returns extension points without assets when manifest.json does not exist', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const uiExtension = await testUIExtension({ + directory: tmpDir, + configuration: { + name: 'test-ui-extension', + type: 'ui_extension', + extension_points: [ + {target: 'CUSTOM_EXTENSION_POINT', module: './src/ExtensionPointA.js', tools: './tools.json'}, + ], + }, + devUUID: 'devUUID', + }) + + // No setupBuildOutput — manifest.json doesn't exist + const got = await getUIExtensionPayload(uiExtension, tmpDir, { + ...createMockOptions(tmpDir, [uiExtension]), + currentDevelopmentPayload: {hidden: true, status: 'success'}, + }) + + expect(got.extensionPoints).toMatchObject([ + { + target: 'CUSTOM_EXTENSION_POINT', + surface: expect.any(String), + root: {url: expect.any(String)}, + resource: {url: ''}, + }, + ]) + expect((got.extensionPoints as any[])[0].assets).toBeUndefined() + }) + }) + + test('returns extension points without assets when build directory does not exist', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const uiExtension = await testUIExtension({ + directory: tmpDir, + configuration: { + name: 'test-ui-extension', + type: 'ui_extension', + extension_points: [{target: 'CUSTOM_EXTENSION_POINT', module: './src/ExtensionPointA.js'}], + }, + devUUID: 'devUUID', + }) + + // Use a non-existent bundle path — parent directory doesn't exist + const got = await getUIExtensionPayload(uiExtension, joinPath(tmpDir, 'nonexistent', 'bundle'), { + ...createMockOptions(tmpDir, [uiExtension]), + currentDevelopmentPayload: {hidden: true, status: 'success'}, + }) + + expect(got.extensionPoints).toMatchObject([ + { + target: 'CUSTOM_EXTENSION_POINT', + surface: expect.any(String), + root: {url: expect.any(String)}, + resource: {url: ''}, + }, + ]) + expect((got.extensionPoints as any[])[0].assets).toBeUndefined() + }) + }) + test('returns the right payload for post-purchase extensions', async () => { await inTemporaryDirectory(async (tmpDir) => { const outputPath = joinPath(tmpDir, 'test-post-purchase-extension.js') diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index fad6117648b..6dc29b13a13 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -6,7 +6,8 @@ import {getUIExtensionResourceURL} from '../../../utilities/extensions/configura import {getUIExtensionRendererVersion} from '../../../models/app/app.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {NewExtensionPointSchemaType} from '../../../models/extensions/schemas.js' -import {fileExists, fileLastUpdatedTimestamp, readFile} from '@shopify/cli-kit/node/fs' +import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' +import {fileLastUpdatedTimestamp, readFile} from '@shopify/cli-kit/node/fs' import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' import {dirname, joinPath} from '@shopify/cli-kit/node/path' @@ -155,17 +156,20 @@ async function getExtensionPoints(extension: ExtensionInstance, url: string, bui async function readBundleManifest( buildDirectory: string, ): Promise<{[target: string]: {[assetName: string]: unknown}} | null> { - const manifestPath = joinPath(buildDirectory, 'manifest.json') - if (!(await fileExists(manifestPath))) { + try { + const manifestPath = joinPath(buildDirectory, 'manifest.json') + const content = await readFile(manifestPath) + return JSON.parse(content) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { return null } - const content = await readFile(manifestPath) - return JSON.parse(content) } /** - * Default asset mapper - reads the source path from the extension point config - * and adds asset to the assets object + * Default asset mapper - reads the source path from the extension point config, + * falling back to build_manifest.assets for compiled assets like main and + * should_render where the config field name doesn't match the asset identifier. */ async function defaultAssetMapper({ identifier, @@ -174,11 +178,20 @@ async function defaultAssetMapper({ extension, }: AssetMapperContext): Promise> { const filepath = (extensionPoint as Record)[identifier] - if (typeof filepath !== 'string') return {} - const payload = await getAssetPayload(identifier, filepath, url, extension) - return { - assets: {[payload.name]: payload}, + if (typeof filepath === 'string') { + const payload = await getAssetPayload(identifier, filepath, url, extension) + return {assets: {[payload.name]: payload}} + } + + const buildManifest = (extensionPoint as NewExtensionPointSchemaType & {build_manifest?: BuildManifest}) + .build_manifest + const asset = buildManifest?.assets?.[identifier as keyof typeof buildManifest.assets] + if (asset?.module) { + const payload = await getAssetPayload(identifier, asset.module, url, extension) + return {assets: {[payload.name]: payload}} } + + return {} } /** diff --git a/packages/app/src/cli/services/dev/extension/payload/store.test.ts b/packages/app/src/cli/services/dev/extension/payload/store.test.ts index 44e83ec1a69..a88bad5aca7 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.test.ts @@ -302,6 +302,53 @@ describe('ExtensionsPayloadStore()', () => { }) }) + test('replaces arrays in extension points instead of merging them', () => { + // Given — initial payload has intents with resolved schema (Asset objects) + const payload = { + extensions: [ + { + uuid: '123', + extensionPoints: [ + { + target: 'admin.app.intent.link', + resource: {url: ''}, + intents: [ + { + type: 'application/email', + action: 'edit', + schema: {name: 'schema', url: '/old-url', lastUpdated: 1}, + }, + ], + }, + ], + }, + ], + } as unknown as ExtensionsEndpointPayload + + const extensionsPayloadStore = new ExtensionsPayloadStore(payload, mockOptions) + + // When — update with new intents (simulating a rebuild) + extensionsPayloadStore.updateExtensions([ + { + uuid: '123', + extensionPoints: [ + { + target: 'admin.app.intent.link', + resource: {url: ''}, + intents: [ + {type: 'application/email', action: 'edit', schema: {name: 'schema', url: '/new-url', lastUpdated: 2}}, + ], + }, + ], + }, + ] as unknown as UIExtensionPayload[]) + + // Then — intents should be replaced, not accumulated + const extensionPoints = extensionsPayloadStore.getRawPayload().extensions[0]?.extensionPoints as any[] + expect(extensionPoints[0].intents).toHaveLength(1) + expect(extensionPoints[0].intents[0].schema.url).toBe('/new-url') + }) + test('informs event listeners of updated extensions', () => { // Given const payload = { 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 b009792fc9d..c485e6b4543 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.ts @@ -150,7 +150,7 @@ export class ExtensionsPayloadStore extends EventEmitter { return (destinationArray as DevNewExtensionPointSchema[]).map((extensionPoint) => { const extensionPointPayload = foundExtensionPointsPayloadMap[extensionPoint.target] if (extensionPointPayload) { - return deepMergeObjects(extensionPoint, extensionPointPayload) + return deepMergeObjects(extensionPoint, extensionPointPayload, (_dest, source) => source) } return extensionPoint }) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts index 69924d80cc9..862503f239b 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts @@ -17,7 +17,7 @@ import {AppEventWatcher} from '../../app-events/app-event-watcher.js' import {describe, expect, vi, test} from 'vitest' import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' import * as h3 from 'h3' -import {dirname, joinPath} from '@shopify/cli-kit/node/path' +import {joinPath} from '@shopify/cli-kit/node/path' import type {H3Event} from 'h3' @@ -220,25 +220,19 @@ describe('getExtensionAssetMiddleware()', () => { }) }) - test('returns the file for that asset path', async () => { + test('returns static asset from extension source directory', async () => { await inTemporaryDirectory(async (tmpDir: string) => { - const extension = await testUIExtension({}) - const outputPath = extension.getOutputPathForDirectory(tmpDir) + const extension = await testUIExtension({directory: tmpDir}) const options = getOptions({ devOptions: { extensions: [extension], - appWatcher: { - buildOutputPath: tmpDir, - } as unknown as AppEventWatcher, }, }) - const fileName = 'test-ui-extension.js' - - await mkdir(dirname(outputPath)) - await touchFile(outputPath) - await writeFile(outputPath, `content from ${fileName}`) + const fileName = 'tools.json' + await touchFile(joinPath(tmpDir, fileName)) + await writeFile(joinPath(tmpDir, fileName), '{"tools": []}') const event = getMockEvent({ params: { @@ -249,8 +243,39 @@ describe('getExtensionAssetMiddleware()', () => { const result = await getExtensionAssetMiddleware(options)(event) + expect(event.node.res.setHeader).toHaveBeenCalledWith('Content-Type', 'application/json') + expect(result).toBe('{"tools": []}') + }) + }) + + test('returns built asset from extension build output directory', async () => { + await inTemporaryDirectory(async (tmpDir: string) => { + const extension = await testUIExtension({directory: tmpDir}) + + const options = getOptions({ + devOptions: { + extensions: [extension], + }, + }) + + // Create the built output file at the outputRelativePath (e.g. dist/handle.js) + const outputDir = joinPath(tmpDir, 'dist') + await mkdir(outputDir) + const outputFile = joinPath(tmpDir, extension.outputRelativePath) + await touchFile(outputFile) + await writeFile(outputFile, 'compiled bundle content') + + const event = getMockEvent({ + params: { + extensionId: extension.devUUID, + assetPath: extension.outputRelativePath, + }, + }) + + const result = await getExtensionAssetMiddleware(options)(event) + expect(event.node.res.setHeader).toHaveBeenCalledWith('Content-Type', 'text/javascript') - expect(result).toBe(`content from ${fileName}`) + expect(result).toBe('compiled bundle content') }) }) }) 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 c7ab7e8967f..f63610a0693 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -5,7 +5,7 @@ import {getHTML} from '../templates.js' import {getWebSocketUrl} from '../../extension.js' import {fileExists, isDirectory, readFile, findPathUp} from '@shopify/cli-kit/node/fs' import {sendRedirect, defineEventHandler, getRequestHeader, getRouterParams, setResponseHeader} from 'h3' -import {joinPath, resolvePath, dirname, extname, moduleDirectory} from '@shopify/cli-kit/node/path' +import {joinPath, resolvePath, extname, moduleDirectory} from '@shopify/cli-kit/node/path' import {outputDebug} from '@shopify/cli-kit/node/output' import type {H3Event} from 'h3' @@ -66,7 +66,7 @@ export async function fileServerMiddleware(event: H3Event, options: {filePath: s return fileContent } -export function getExtensionAssetMiddleware({devOptions, getExtensions}: GetExtensionsMiddlewareOptions) { +export function getExtensionAssetMiddleware({getExtensions}: GetExtensionsMiddlewareOptions) { return defineEventHandler(async (event) => { const {extensionId, assetPath = ''} = getRouterParams(event) const extension = getExtensions().find((ext) => ext.devUUID === extensionId) @@ -78,14 +78,13 @@ export function getExtensionAssetMiddleware({devOptions, getExtensions}: GetExte }) } - const bundlePath = devOptions.appWatcher.buildOutputPath - const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath) - - const buildDirectory = dirname(extensionOutputPath) - - return fileServerMiddleware(event, { - filePath: joinPath(buildDirectory, assetPath), - }) + // Try the extension's build output first (for compiled assets), then fall back + // to the extension's source directory (for static assets like tools, instructions). + const buildPath = joinPath(extension.directory, extension.outputRelativePath) + if (await fileExists(buildPath)) { + return fileServerMiddleware(event, {filePath: buildPath}) + } + return fileServerMiddleware(event, {filePath: joinPath(extension.directory, assetPath)}) }) } From de5e9d916ed5ab455901eb886082be8538c9beff Mon Sep 17 00:00:00 2001 From: Trish Ta Date: Wed, 15 Apr 2026 14:58:05 -0400 Subject: [PATCH 3/3] Fix build step for ui extension to build to local directory first before copying over to the bundle Remove copy_static_assets client step completely. It was doing nothing for specifications that were not ui_extension. ui_extension now has its own steps for including static assets --- .../extensions/extension-instance.test.ts | 29 +-------------- .../models/extensions/extension-instance.ts | 10 ----- .../specification.integration.test.ts | 6 +-- .../cli/models/extensions/specification.ts | 5 --- .../specifications/checkout_post_purchase.ts | 5 +-- .../specifications/checkout_ui_extension.ts | 5 +-- .../specifications/pos_ui_extension.ts | 5 +-- .../specifications/product_subscription.ts | 5 +-- .../extensions/specifications/ui_extension.ts | 24 +----------- .../specifications/web_pixel_extension.ts | 5 +-- .../src/cli/services/build/client-steps.ts | 2 +- .../app/src/cli/services/build/extension.ts | 9 ++++- .../services/build/steps/bundle-ui-step.ts | 13 +++++-- .../build/steps/copy-static-assets-step.ts | 11 ------ .../app/src/cli/services/build/steps/index.ts | 4 -- .../services/dev/extension/payload.test.ts | 4 +- .../src/cli/services/dev/extension/payload.ts | 21 +++++++---- .../dev/extension/server/middlewares.test.ts | 37 +++++++++++++++++-- .../dev/extension/server/middlewares.ts | 12 +++--- 19 files changed, 84 insertions(+), 128 deletions(-) delete mode 100644 packages/app/src/cli/services/build/steps/copy-static-assets-step.ts diff --git a/packages/app/src/cli/models/extensions/extension-instance.test.ts b/packages/app/src/cli/models/extensions/extension-instance.test.ts index c3aa272a3de..3fea6b4a055 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -14,7 +14,7 @@ import { testSingleWebhookSubscriptionExtension, placeholderAppConfiguration, } from '../app/app.test-data.js' -import {ExtensionBuildOptions, buildUIExtension} from '../../services/build/extension.js' +import {ExtensionBuildOptions} from '../../services/build/extension.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {joinPath} from '@shopify/cli-kit/node/path' import {describe, expect, test, vi} from 'vitest' @@ -172,33 +172,6 @@ describe('build', async () => { }) }) - test('calls copyStaticAssets after buildUIExtension when building UI extensions', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const extensionInstance = await testUIExtension({ - type: 'ui_extension', - directory: tmpDir, - }) - - const copyStaticAssetsSpy = vi.spyOn(extensionInstance, 'copyStaticAssets').mockResolvedValue() - vi.mocked(buildUIExtension).mockResolvedValue() - - const options: ExtensionBuildOptions = { - stdout: new Writable({write: vi.fn()}), - stderr: new Writable({write: vi.fn()}), - app: testApp(), - environment: 'production', - } - - // When - await extensionInstance.build(options) - - // Then - expect(buildUIExtension).toHaveBeenCalledWith(extensionInstance, options) - expect(copyStaticAssetsSpy).toHaveBeenCalledOnce() - }) - }) - test('does not copy shopify.extension.toml file when bundling theme extensions', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 11e2f24b976..b68a22720cd 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -459,16 +459,6 @@ export class ExtensionInstance normalizePath(file)))] } - /** - * Copy static assets from the extension directory to the output path - * Used by both dev and deploy builds - */ - async copyStaticAssets(outputPath?: string) { - if (this.specification.copyStaticAssets) { - return this.specification.copyStaticAssets(this.configuration, this.directory, outputPath ?? this.outputPath) - } - } - /** * Rescans imports for this extension and updates the cached import paths * Returns true if the imports changed diff --git a/packages/app/src/cli/models/extensions/specification.integration.test.ts b/packages/app/src/cli/models/extensions/specification.integration.test.ts index 11f7912698d..f49d0712f3a 100644 --- a/packages/app/src/cli/models/extensions/specification.integration.test.ts +++ b/packages/app/src/cli/models/extensions/specification.integration.test.ts @@ -40,9 +40,9 @@ const testClientSteps: ClientSteps = [ lifecycle: 'deploy', steps: [ { - id: 'copy_static', - name: 'Copy static assets', - type: 'copy_static_assets', + id: 'bundle-ui', + name: 'Bundle UI Extension', + type: 'bundle_ui', }, ], }, diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index 1581072beec..4d9945d408e 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -133,11 +133,6 @@ export interface ExtensionSpecification>, ) => Promise - /** - * Copy static assets from the extension directory to the output path - */ - copyStaticAssets?: (configuration: TConfiguration, directory: string, outputPath: string) => Promise - /** * Custom watch configuration for dev sessions. * Return a DevSessionWatchConfig with paths to watch and optionally paths to ignore, diff --git a/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts b/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts index 6bc9241acb6..cfc32f4582e 100644 --- a/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts +++ b/packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts @@ -22,10 +22,7 @@ const checkoutPostPurchaseSpec = createExtensionSpecification({ clientSteps: [ { lifecycle: 'deploy', - steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, - ], + steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], deployConfig: async (config, _) => { diff --git a/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts index 43d2d159cda..bd6715fa762 100644 --- a/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts @@ -28,10 +28,7 @@ const checkoutSpec = createExtensionSpecification({ clientSteps: [ { lifecycle: 'deploy', - steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, - ], + steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], deployConfig: async (config, directory) => { diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts index d429c5e6846..05134ab44d8 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts @@ -20,10 +20,7 @@ const posUISpec = createExtensionSpecification({ clientSteps: [ { lifecycle: 'deploy', - steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, - ], + steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], deployConfig: async (config, directory) => { diff --git a/packages/app/src/cli/models/extensions/specifications/product_subscription.ts b/packages/app/src/cli/models/extensions/specifications/product_subscription.ts index dec7d6a7639..6efb44d4a48 100644 --- a/packages/app/src/cli/models/extensions/specifications/product_subscription.ts +++ b/packages/app/src/cli/models/extensions/specifications/product_subscription.ts @@ -21,10 +21,7 @@ const productSubscriptionSpec = createExtensionSpecification({ clientSteps: [ { lifecycle: 'deploy', - steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, - ], + steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], deployConfig: async (_, directory) => { diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 636ee1fb394..80a8de6b1f3 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -13,8 +13,8 @@ import {getExtensionPointTargetSurface} from '../../../services/dev/extension/ut import {ExtensionInstance} from '../extension-instance.js' import {formatContent} from '../../../utilities/file-formatter.js' import {err, ok, Result} from '@shopify/cli-kit/node/result' -import {copyFile, fileExists, readFile} from '@shopify/cli-kit/node/fs' -import {joinPath, dirname} from '@shopify/cli-kit/node/path' +import {fileExists, readFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' import {outputContent, outputToken, outputWarn} from '@shopify/cli-kit/node/output' import {zod} from '@shopify/cli-kit/node/schema' @@ -174,26 +174,6 @@ const uiExtensionSpec = createExtensionSpecification({ ...(assetsArray.length ? {assets: assetsArray} : {}), } }, - copyStaticAssets: async (config, directory, outputPath) => { - if (!isRemoteDomExtension(config)) return - - await Promise.all( - config.extension_points.flatMap((extensionPoint) => { - if (!('build_manifest' in extensionPoint)) return [] - - return Object.entries(extensionPoint.build_manifest.assets).map(([_, asset]) => { - if (asset.static && asset.module) { - const sourceFile = joinPath(directory, asset.module) - const outputFilePath = joinPath(dirname(outputPath), asset.filepath) - return copyFile(sourceFile, outputFilePath).catch((error) => { - throw new Error(`Failed to copy static asset ${asset.module} to ${outputFilePath}: ${error.message}`) - }) - } - return Promise.resolve() - }) - }), - ) - }, hasExtensionPointTarget: (config, requestedTarget) => { return ( config.extension_points?.find((extensionPoint) => { diff --git a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts b/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts index 66fcd908375..903fbaa8288 100644 --- a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts @@ -38,10 +38,7 @@ const webPixelSpec = createExtensionSpecification({ clientSteps: [ { lifecycle: 'deploy', - steps: [ - {id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}, - {id: 'copy-static-assets', name: 'Copy Static Assets', type: 'copy_static_assets', config: {}}, - ], + steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}], }, ], deployConfig: async (config, _) => { diff --git a/packages/app/src/cli/services/build/client-steps.ts b/packages/app/src/cli/services/build/client-steps.ts index 68e3e446004..07f6ef98c9a 100644 --- a/packages/app/src/cli/services/build/client-steps.ts +++ b/packages/app/src/cli/services/build/client-steps.ts @@ -30,7 +30,7 @@ interface BundleUIStep extends BaseStep { /** Steps that don't require any config yet. */ interface NoConfigStep extends BaseStep { - readonly type: 'build_theme' | 'bundle_theme' | 'copy_static_assets' | 'build_function' | 'create_tax_stub' + readonly type: 'build_theme' | 'bundle_theme' | 'build_function' | 'create_tax_stub' readonly config?: Record } diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index aaa10f2c0fb..9ec95061c6f 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -58,21 +58,25 @@ export interface ExtensionBuildOptions { /** * It builds the UI extensions. * @param options - Build options. + * @returns The local output path. */ -export async function buildUIExtension(extension: ExtensionInstance, options: ExtensionBuildOptions): Promise { +export async function buildUIExtension(extension: ExtensionInstance, options: ExtensionBuildOptions): Promise { options.stdout.write(`Bundling UI extension ${extension.localIdentifier}...`) const env = options.app.dotenv?.variables ?? {} if (options.appURL) { env.APP_URL = options.appURL } + // Always build into the extension's local directory (e.g. ext/dist/handle.js) + const localOutputPath = joinPath(extension.directory, extension.outputRelativePath) + const {main, assets} = extension.getBundleExtensionStdinContent() const startTime = performance.now() try { await bundleExtension({ minify: true, - outputPath: extension.outputPath, + outputPath: localOutputPath, stdin: { contents: main, resolveDir: extension.directory, @@ -122,6 +126,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex const duration = Math.round(performance.now() - startTime) const sizeInfo = await formatBundleSize(extension.outputPath) options.stdout.write(`${extension.localIdentifier} successfully built in ${duration}ms${sizeInfo}`) + return localOutputPath } type BuildFunctionExtensionOptions = ExtensionBuildOptions diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts index 517d969207c..efcc1d9d13c 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts +++ b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts @@ -1,6 +1,8 @@ import {mergeManifestEntries} from './include-assets/generate-manifest.js' import {buildUIExtension} from '../extension.js' import {BuildManifest} from '../../../models/extensions/specifications/ui_extension.js' +import {copyFile} from '@shopify/cli-kit/node/fs' +import {dirname} from '@shopify/cli-kit/node/path' import type {LifecycleStep, BuildContext} from '../client-steps.js' interface ExtensionPointWithBuildManifest { @@ -11,12 +13,15 @@ interface ExtensionPointWithBuildManifest { /** * Executes a bundle_ui build step. * - * Bundles the UI extension using esbuild, writing output to extension.outputPath. - * When `generatesAssetsManifest` is true, writes built asset entries (from - * build_manifest) into manifest.json so downstream steps can merge on top. + * Bundles the UI extension using esbuild into the extension's local directory + * and copies the output to the bundle. When `generatesAssetsManifest` is true, + * writes built asset entries (from build_manifest) into manifest.json so + * downstream steps can merge on top. */ export async function executeBundleUIStep(step: LifecycleStep, context: BuildContext): Promise { - await buildUIExtension(context.extension, context.options) + const localOutputPath = await buildUIExtension(context.extension, context.options) + // Copy the locally built files into the bundle + await copyFile(dirname(localOutputPath), dirname(context.extension.outputPath)) if (!('generatesAssetsManifest' in step) || !step.generatesAssetsManifest) return diff --git a/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts b/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts deleted file mode 100644 index 5bad6cea176..00000000000 --- a/packages/app/src/cli/services/build/steps/copy-static-assets-step.ts +++ /dev/null @@ -1,11 +0,0 @@ -import type {LifecycleStep, BuildContext} from '../client-steps.js' - -/** - * Executes a copy_static_assets build step. - * - * Copies static assets defined in the extension's build_manifest to the output directory. - * This is a no-op for extensions that do not define static assets. - */ -export async function executeCopyStaticAssetsStep(_step: LifecycleStep, context: BuildContext): Promise { - return context.extension.copyStaticAssets() -} diff --git a/packages/app/src/cli/services/build/steps/index.ts b/packages/app/src/cli/services/build/steps/index.ts index 6ca10e6e041..bf0a52f4f31 100644 --- a/packages/app/src/cli/services/build/steps/index.ts +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -2,7 +2,6 @@ import {executeIncludeAssetsStep} from './include-assets-step.js' import {executeBuildThemeStep} from './build-theme-step.js' import {executeBundleThemeStep} from './bundle-theme-step.js' import {executeBundleUIStep} from './bundle-ui-step.js' -import {executeCopyStaticAssetsStep} from './copy-static-assets-step.js' import {executeBuildFunctionStep} from './build-function-step.js' import {executeCreateTaxStubStep} from './create-tax-stub-step.js' import type {LifecycleStep, BuildContext} from '../client-steps.js' @@ -30,9 +29,6 @@ export async function executeStepByType(step: LifecycleStep, context: BuildConte case 'bundle_ui': return executeBundleUIStep(step, context) - case 'copy_static_assets': - return executeCopyStaticAssetsStep(step, context) - case 'build_function': return executeBuildFunctionStep(step, context) diff --git a/packages/app/src/cli/services/dev/extension/payload.test.ts b/packages/app/src/cli/services/dev/extension/payload.test.ts index 789d631cf39..f4115b73451 100644 --- a/packages/app/src/cli/services/dev/extension/payload.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload.test.ts @@ -230,12 +230,12 @@ describe('getUIExtensionPayload', () => { assets: { main: { name: 'main', - url: 'http://tunnel-url.com/extensions/devUUID/assets/src/ExtensionPointA.js', + url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js', lastUpdated: expect.any(Number), }, should_render: { name: 'should_render', - url: 'http://tunnel-url.com/extensions/devUUID/assets/src/ShouldRender.js', + url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension-conditions.js', lastUpdated: expect.any(Number), }, }, diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index 6dc29b13a13..18d2331041d 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -186,8 +186,8 @@ async function defaultAssetMapper({ const buildManifest = (extensionPoint as NewExtensionPointSchemaType & {build_manifest?: BuildManifest}) .build_manifest const asset = buildManifest?.assets?.[identifier as keyof typeof buildManifest.assets] - if (asset?.module) { - const payload = await getAssetPayload(identifier, asset.module, url, extension) + if (asset?.filepath) { + const payload = await getAssetPayload(identifier, asset.filepath, url, extension, asset.module) return {assets: {[payload.name]: payload}} } @@ -263,14 +263,21 @@ export function isNewExtensionPointsSchema(extensionPoints: unknown): extensionP /** * Builds an asset payload entry. * - * `lastUpdated` is read from the source file in the extension directory so that - * payload updates only fire when the developer actually changes the source — not - * every time the build copies it fresh into the output directory. + * @param sourcePath - Optional source file path for the timestamp. When provided + * (e.g. for compiled assets), the URL uses `filepath` (the build output name) + * while `lastUpdated` is read from `sourcePath` (the source module). For static + * assets, `filepath` is used for both. */ -async function getAssetPayload(name: string, filepath: string, url: string, extension: ExtensionInstance) { +async function getAssetPayload( + name: string, + filepath: string, + url: string, + extension: ExtensionInstance, + sourcePath?: string, +) { return { name, url: `${url}${joinPath('/assets/', filepath)}`, - lastUpdated: (await fileLastUpdatedTimestamp(joinPath(extension.directory, filepath))) ?? 0, + lastUpdated: (await fileLastUpdatedTimestamp(joinPath(extension.directory, sourcePath ?? filepath))) ?? 0, } } diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts index 862503f239b..a0863aef7ce 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts @@ -258,17 +258,17 @@ describe('getExtensionAssetMiddleware()', () => { }, }) - // Create the built output file at the outputRelativePath (e.g. dist/handle.js) + // Create the built output file in dist/ (e.g. dist/handle.js) const outputDir = joinPath(tmpDir, 'dist') await mkdir(outputDir) - const outputFile = joinPath(tmpDir, extension.outputRelativePath) + const outputFile = joinPath(outputDir, extension.outputFileName) await touchFile(outputFile) await writeFile(outputFile, 'compiled bundle content') const event = getMockEvent({ params: { extensionId: extension.devUUID, - assetPath: extension.outputRelativePath, + assetPath: extension.outputFileName, }, }) @@ -278,6 +278,37 @@ describe('getExtensionAssetMiddleware()', () => { expect(result).toBe('compiled bundle content') }) }) + + test('serves built asset over source file when both exist', async () => { + await inTemporaryDirectory(async (tmpDir: string) => { + const extension = await testUIExtension({directory: tmpDir}) + + const options = getOptions({ + devOptions: { + extensions: [extension], + }, + }) + + // Create both a source file and a built file with the same name + const fileName = extension.outputFileName + await writeFile(joinPath(tmpDir, fileName), 'source content') + const outputDir = joinPath(tmpDir, 'dist') + await mkdir(outputDir) + await writeFile(joinPath(outputDir, fileName), 'built content') + + const event = getMockEvent({ + params: { + extensionId: extension.devUUID, + assetPath: fileName, + }, + }) + + const result = await getExtensionAssetMiddleware(options)(event) + + // Built asset takes priority + expect(result).toBe('built content') + }) + }) }) describe('getExtensionPayloadMiddleware()', () => { 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 f63610a0693..3c5910bef8c 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -5,7 +5,7 @@ import {getHTML} from '../templates.js' import {getWebSocketUrl} from '../../extension.js' import {fileExists, isDirectory, readFile, findPathUp} from '@shopify/cli-kit/node/fs' import {sendRedirect, defineEventHandler, getRequestHeader, getRouterParams, setResponseHeader} from 'h3' -import {joinPath, resolvePath, extname, moduleDirectory} from '@shopify/cli-kit/node/path' +import {joinPath, resolvePath, dirname, extname, moduleDirectory} from '@shopify/cli-kit/node/path' import {outputDebug} from '@shopify/cli-kit/node/output' import type {H3Event} from 'h3' @@ -78,11 +78,11 @@ export function getExtensionAssetMiddleware({getExtensions}: GetExtensionsMiddle }) } - // Try the extension's build output first (for compiled assets), then fall back - // to the extension's source directory (for static assets like tools, instructions). - const buildPath = joinPath(extension.directory, extension.outputRelativePath) - if (await fileExists(buildPath)) { - return fileServerMiddleware(event, {filePath: buildPath}) + // Try the build output directory first (for compiled assets like dist/handle.js), + // then fall back to the extension's source directory (for static assets like tools, instructions). + const builtAssetPath = joinPath(dirname(joinPath(extension.directory, extension.outputRelativePath)), assetPath) + if (await fileExists(builtAssetPath)) { + return fileServerMiddleware(event, {filePath: builtAssetPath}) } return fileServerMiddleware(event, {filePath: joinPath(extension.directory, assetPath)}) })