From c767bd6e9e2f635667f55f15a2634ebf4c641615 Mon Sep 17 00:00:00 2001 From: Trish Ta Date: Tue, 31 Mar 2026 11:17:07 -0400 Subject: [PATCH 1/2] 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 19ce4209c5bc3a945b4279fd6dc949d04f0ddc7b Mon Sep 17 00:00:00 2001 From: Trish Ta Date: Tue, 7 Apr 2026 19:11:34 -0400 Subject: [PATCH 2/2] 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 | 147 ++---------------- .../extensions/specifications/ui_extension.ts | 56 ++----- .../build/steps/include-assets-step.test.ts | 126 +++++++++------ .../build/steps/include-assets-step.ts | 32 ++-- .../include-assets/copy-config-key-entry.ts | 52 ++++--- .../steps/include-assets/generate-manifest.ts | 35 ++++- .../services/dev/extension/payload.test.ts | 6 +- .../dev/extension/payload/store.test.ts | 47 ++++++ .../services/dev/extension/payload/store.ts | 2 +- .../dev/extension/server/middlewares.ts | 11 +- 10 files changed, 240 insertions(+), 274 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..e06b4e67436 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,63 +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 () => { @@ -208,6 +151,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 +219,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 +283,7 @@ describe('ui_extension', async () => { target: 'EXTENSION::POINT::A', tools: undefined, instructions: undefined, + intents: undefined, module: './src/ExtensionPointA.js', metafields: [], urls: {}, @@ -401,6 +347,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 +414,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 +483,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 +552,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 +563,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 +616,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 +627,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 +732,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 +779,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 +790,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..5c5569bb914 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' @@ -61,24 +61,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 +75,7 @@ export const UIExtensionSchema = BaseSchema.extend({ build_manifest: buildManifest, tools: targeting.tools, instructions: targeting.instructions, + intents: targeting.intents, } }) return {...config, extension_points: extensionPoints} @@ -115,24 +98,25 @@ const uiExtensionSpec = createExtensionSpecification({ type: 'include_assets', config: { generatesAssetsManifest: true, + builtAssets: {anchor: 'extension_points[]', groupBy: 'target', key: 'build_manifest'}, 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 +399,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/steps/include-assets-step.test.ts b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts index ac4e89dfaba..2f1c24a09d6 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', @@ -1185,8 +1232,6 @@ describe('executeIncludeAssetsStep', () => { } as unknown as ExtensionInstance, } - vi.mocked(fs.fileExists).mockResolvedValue(false) - const step: LifecycleStep = { id: 'gen-manifest', name: 'Generate Manifest', @@ -1358,9 +1403,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 +1414,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 +1433,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..c6940f4357c 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 @@ -2,7 +2,7 @@ import {generateManifestFile} from './include-assets/generate-manifest.js' import {copyByPattern} from './include-assets/copy-by-pattern.js' import {copySourceEntry} from './include-assets/copy-source-entry.js' import {copyConfigKeyEntry} from './include-assets/copy-config-key-entry.js' -import {joinPath, dirname, extname, sanitizeRelativePath} from '@shopify/cli-kit/node/path' +import {joinPath, dirname, sanitizeRelativePath} from '@shopify/cli-kit/node/path' import {z} from 'zod' import type {LifecycleStep, BuildContext} from '../client-steps.js' @@ -73,9 +73,22 @@ const InclusionEntrySchema = z.discriminatedUnion('type', [PatternEntrySchema, S * with `anchor` and `groupBy` produce structured manifest entries; `pattern` * and `static` entries contribute their paths under a `"files"` key. */ +/** + * Built assets config — records compiled outputs in the manifest without copying. + * + * Reads `build_manifest.assets` from each item in the anchor array and merges + * each asset's `filepath` into the manifest entry keyed by `groupBy`. + */ +const BuiltAssetsConfigSchema = z.object({ + anchor: z.string(), + groupBy: z.string(), + key: z.string(), +}) + const IncludeAssetsConfigSchema = z .object({ inclusions: z.array(InclusionEntrySchema), + builtAssets: BuiltAssetsConfigSchema.optional(), generatesAssetsManifest: z.boolean().default(false), }) .strict() @@ -123,16 +136,16 @@ export async function executeIncludeAssetsStep( ): Promise<{filesCopied: number}> { const config = IncludeAssetsConfigSchema.parse(step.config) const {extension, options} = context - // When outputPath is a file (e.g. index.js, index.wasm), the output directory is its - // parent. When outputPath has no extension, it IS the output directory. - const outputDir = extname(extension.outputPath) ? dirname(extension.outputPath) : extension.outputPath + const outputDir = dirname(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 +159,7 @@ export async function executeIncludeAssetsStep( outputDir, context, destination: sanitizedDest, + usedBasenames, }) result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val)) configKeyCount += result.filesCopied @@ -198,7 +212,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, outputDir, aggregatedPathMap, otherFiles, config.builtAssets) } 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.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts index aed94be0583..d0ff69f0940 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) @@ -77,9 +77,12 @@ export async function copyConfigKeyEntry(config: { filesCopied += copied.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 +94,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/generate-manifest.ts b/packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts index 20cdc405ead..90d5492bd26 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 @@ -25,12 +25,19 @@ interface ConfigKeyManifestEntry { * @param pathMap - Map from raw config path values to their output-relative * paths, as recorded during the copy phase by `copyConfigKeyEntry`. */ +interface BuiltAssetsConfig { + anchor: string + groupBy: string + key: string +} + export async function generateManifestFile( configKeyEntries: ConfigKeyManifestEntry[], context: BuildContext, outputDir: string, pathMap: Map, otherFiles: string[], + builtAssets?: BuiltAssetsConfig, ): Promise { const {extension, options} = context @@ -47,7 +54,7 @@ export async function generateManifestFile( } } - if (anchoredIncs.length === 0 && rootIncs.length === 0 && otherFiles.length === 0) return + if (anchoredIncs.length === 0 && rootIncs.length === 0 && otherFiles.length === 0 && !builtAssets) return const manifest: {[key: string]: unknown} = {} @@ -103,6 +110,32 @@ export async function generateManifestFile( } } + // Merge built assets (e.g. compiled JS bundles) into grouped manifest entries. + // Reads build_manifest.assets from each anchor item and records each asset's filepath. + if (builtAssets) { + const anchorValue = getNestedValue(extension.configuration, builtAssets.anchor) + if (Array.isArray(anchorValue)) { + for (const item of anchorValue) { + if (item === null || typeof item !== 'object' || Array.isArray(item)) continue + const typedItem = item as {[key: string]: unknown} + + const manifestKey = typedItem[builtAssets.groupBy] + if (typeof manifestKey !== 'string') continue + + const buildManifest = typedItem[builtAssets.key] as {assets?: {[name: string]: {filepath: string}}} | undefined + if (!buildManifest?.assets) continue + + const existing = (manifest[manifestKey] ?? {}) as {[key: string]: unknown} + for (const [name, asset] of Object.entries(buildManifest.assets)) { + if (asset?.filepath) { + existing[name] = asset.filepath + } + } + manifest[manifestKey] = existing + } + } + } + if (otherFiles.length > 0) { manifest.files = otherFiles } 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..0c56694424a 100644 --- a/packages/app/src/cli/services/dev/extension/payload.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload.test.ts @@ -5,7 +5,7 @@ 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 +44,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)) { 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.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.ts index c7ab7e8967f..18b8de152ee 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -80,12 +80,15 @@ 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 build output directory first (for compiled bundles), then fall back + // to the extension's source directory (for static assets like tools, instructions). + const buildPath = joinPath(buildDirectory, assetPath) + if (await fileExists(buildPath)) { + return fileServerMiddleware(event, {filePath: buildPath}) + } + return fileServerMiddleware(event, {filePath: joinPath(extension.directory, assetPath)}) }) }