diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts index e54a27ba93..5b5e8f8a33 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts @@ -29,7 +29,9 @@ export async function handleWatcherEvents( const appEvent: AppEvent = {app, extensionEvents: [], path: events[0].path, startTime: events[0].startTime} for (const event of otherEvents) { - const affectedExtensions = app.realExtensions.filter((ext) => ext.directory === event.extensionPath) + const affectedExtensions = event.extensionHandle + ? app.realExtensions.filter((ext) => ext.handle === event.extensionHandle) + : app.realExtensions.filter((ext) => ext.directory === event.extensionPath) const newEvent = handlers[event.type]({event, app: appEvent.app, extensions: affectedExtensions, options}) appEvent.extensionEvents.push(...newEvent.extensionEvents) } diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts index c13b48cdfc..c92a37bacb 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts @@ -25,9 +25,24 @@ vi.mock('../../../models/app/loader.js') vi.mock('./app-watcher-esbuild.js') // Extensions 1 and 1B simulate extensions defined in the same directory (same toml) -const extension1 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_1', uid: 'uid1'}) -const extension1B = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_1', uid: 'uid1B'}) -const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2', uid: 'uid2'}) +const extension1 = await testUIExtension({ + type: 'ui_extension', + handle: 'h1', + directory: '/extensions/ui_extension_1', + uid: 'uid1', +}) +const extension1B = await testUIExtension({ + type: 'ui_extension', + handle: 'h2', + directory: '/extensions/ui_extension_1', + uid: 'uid1B', +}) +const extension2 = await testUIExtension({ + type: 'ui_extension', + handle: 'h3', + directory: '/extensions/ui_extension_2', + uid: 'uid2', +}) const flowExtension = await testFlowActionExtension('/extensions/flow_action') const posExtension = await testAppConfigExtensions() const appAccessExtension = await testAppAccessConfigExtension() @@ -36,12 +51,14 @@ const webhookExtension = await testSingleWebhookSubscriptionExtension() // Simulate updated extensions const extension1Updated = await testUIExtension({ type: 'ui_extension', + handle: 'h1', name: 'updated_name1', directory: '/extensions/ui_extension_1', uid: 'uid1', }) const extension1BUpdated = await testUIExtension({ type: 'ui_extension', + handle: 'h2', name: 'updated_name1B', directory: '/extensions/ui_extension_1', uid: 'uid1B', @@ -212,6 +229,32 @@ const testCases: TestCase[] = [ {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', uid: 'uid1B'}}, ], }, + { + name: 'file_updated with extensionHandle targets only the specified extension', + fileWatchEvent: { + type: 'file_updated', + path: '/extensions/ui_extension_1/src/file.js', + extensionPath: '/extensions/ui_extension_1', + extensionHandle: 'h1', + startTime: [0, 0], + }, + initialExtensions: [extension1, extension1B, extension2, posExtension], + finalExtensions: [extension1, extension1B, extension2, posExtension], + extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}], + }, + { + name: 'file_created with extensionHandle targets only the specified extension', + fileWatchEvent: { + type: 'file_created', + path: '/extensions/ui_extension_1/src/new-file.js', + extensionPath: '/extensions/ui_extension_1', + extensionHandle: 'h2', + startTime: [0, 0], + }, + initialExtensions: [extension1, extension1B, extension2, posExtension], + finalExtensions: [extension1, extension1B, extension2, posExtension], + extensionEvents: [{type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', uid: 'uid1B'}}], + }, { name: 'app config updated with multiple extensions affected', fileWatchEvent: { diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts index ca479009e0..b69565d39b 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts @@ -76,6 +76,8 @@ interface TestCaseSingleEvent { fileSystemEvent: string path: string expectedEvent?: Omit & {startTime?: WatcherEvent['startTime']} + expectedEventCount?: number + expectedHandles?: string[] } /** @@ -103,7 +105,10 @@ const singleEventTestCases: TestCaseSingleEvent[] = [ type: 'file_updated', path: '/extensions/ui_extension_1/index.js', extensionPath: '/extensions/ui_extension_1', + extensionHandle: 'h1', }, + expectedEventCount: 2, + expectedHandles: ['h1', 'h2'], }, { name: 'change in toml', @@ -113,7 +118,10 @@ const singleEventTestCases: TestCaseSingleEvent[] = [ type: 'extensions_config_updated', path: '/extensions/ui_extension_1/shopify.ui.extension.toml', extensionPath: '/extensions/ui_extension_1', + extensionHandle: 'h1', }, + expectedEventCount: 2, + expectedHandles: ['h1', 'h2'], }, { name: 'change in app config', @@ -133,7 +141,10 @@ const singleEventTestCases: TestCaseSingleEvent[] = [ type: 'file_created', path: '/extensions/ui_extension_1/new-file.js', extensionPath: '/extensions/ui_extension_1', + extensionHandle: 'h1', }, + expectedEventCount: 2, + expectedHandles: ['h1', 'h2'], }, { name: 'delete a file', @@ -280,7 +291,7 @@ describe('file-watcher events', () => { test.each(singleEventTestCases)( 'The event $name returns the expected WatcherEvent', - async ({fileSystemEvent, path, expectedEvent}) => { + async ({fileSystemEvent, path, expectedEvent, expectedEventCount, expectedHandles}) => { // Given let eventHandler: any @@ -369,7 +380,8 @@ describe('file-watcher events', () => { throw new Error('Expected onChange to be called with events, but all calls had empty arrays') } - expect(actualEvents).toHaveLength(1) + const eventCount = expectedEventCount ?? 1 + expect(actualEvents).toHaveLength(eventCount) const actualEvent = actualEvents[0] expect(actualEvent.type).toBe(expectedEvent.type) @@ -377,6 +389,14 @@ describe('file-watcher events', () => { expect(actualEvent.extensionPath).toBe(normalizePath(expectedEvent.extensionPath)) expect(Array.isArray(actualEvent.startTime)).toBe(true) expect(actualEvent.startTime).toHaveLength(2) + + // Verify extensionHandle is set correctly on file-level events + if (expectedHandles) { + const actualHandles = actualEvents.map((event: WatcherEvent) => event.extensionHandle).sort() + expect(actualHandles).toEqual(expectedHandles.sort()) + } else if (expectedEvent.extensionHandle) { + expect(actualEvent.extensionHandle).toBe(expectedEvent.extensionHandle) + } }, {timeout: 1000, interval: 50}, ) diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.ts index fae4b59f19..e5995b140c 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.ts @@ -19,12 +19,12 @@ const FILE_DELETE_TIMEOUT_IN_MS = 500 /** * Event emitted by the file watcher * - * Includes the type of the event, the path of the file that triggered the event and the extension path that contains the file. - * path and extensionPath could be the same if the event is at the extension level (create, delete extension) + * Includes the type of the event, the path of the file that triggered the event and the extension handle that owns the file. + * For folder-level events (create, delete), extensionHandle is undefined since the extension may not exist yet. * * @typeParam type - The type of the event * @typeParam path - The path of the file that triggered the event - * @typeParam extensionPath - The path of the extension that contains the file + * @typeParam extensionHandle - The unique handle of the extension that owns the file * @typeParam startTime - The time when the event was triggered */ export interface WatcherEvent { @@ -37,6 +37,9 @@ export interface WatcherEvent { | 'extensions_config_updated' | 'app_config_deleted' path: string + /** The unique handle of the extension that owns this file. Undefined for folder-level events. */ + extensionHandle?: string + /** The directory path of the extension. Used for folder-level events (create/delete) where no extension handle exists yet. */ extensionPath: string startTime: StartTime } @@ -56,7 +59,7 @@ export class FileWatcher { private watcher?: FSWatcher private readonly debouncedEmit: () => void private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {} - // Map of file paths to the extensions that watch them + // Map of file paths to the extension handles that watch them private readonly extensionWatchedFiles = new Map>() constructor( @@ -162,15 +165,14 @@ export class FileWatcher { const allFiles = new Set() for (const {extension, watchedFiles} of extensionResults) { - const extensionDir = normalizePath(extension.directory) for (const file of watchedFiles) { const normalizedPath = normalizePath(file) allFiles.add(normalizedPath) - // Track which extensions watch this file - const extensionsSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set() - extensionsSet.add(extensionDir) - this.extensionWatchedFiles.set(normalizedPath, extensionsSet) + // Track which extension handles watch this file + const handlesSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set() + handlesSet.add(extension.handle) + this.extensionWatchedFiles.set(normalizedPath, handlesSet) } } @@ -204,13 +206,13 @@ export class FileWatcher { } // If the event is already in the list, don't push it again - // Check path, type, AND extensionPath to properly handle shared files + // Check path, type, AND extensionHandle to properly handle shared files if ( this.currentEvents.some( (extEvent) => extEvent.path === event.path && extEvent.type === event.type && - extEvent.extensionPath === event.extensionPath, + extEvent.extensionHandle === event.extensionHandle, ) ) return @@ -229,7 +231,9 @@ export class FileWatcher { private shouldIgnoreEvent(event: WatcherEvent) { if (event.type === 'extension_folder_deleted' || event.type === 'extension_folder_created') return false - const extension = this.app.realExtensions.find((ext) => ext.directory === event.extensionPath) + const extension = event.extensionHandle + ? this.app.realExtensions.find((ext) => ext.handle === event.extensionHandle) + : undefined const watchPaths = extension?.watchedFiles() const ignoreInstance = this.ignored[event.extensionPath] @@ -255,8 +259,8 @@ export class FileWatcher { if (isConfigAppPath) { this.handleEventForExtension(event, path, this.app.directory, startTime, false) } else { - const affectedExtensions = this.extensionWatchedFiles.get(normalizedPath) - const isUnknownExtension = affectedExtensions === undefined || affectedExtensions.size === 0 + const affectedHandles = this.extensionWatchedFiles.get(normalizedPath) + const isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0 if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) { // Ignore an event if it's not part of an existing extension @@ -265,8 +269,10 @@ export class FileWatcher { return } - for (const extensionPath of affectedExtensions ?? []) { - this.handleEventForExtension(event, path, extensionPath, startTime, false) + for (const handle of affectedHandles ?? []) { + const extension = this.app.realExtensions.find((ext) => ext.handle === handle) + const extensionPath = extension ? normalizePath(extension.directory) : this.app.directory + this.handleEventForExtension(event, path, extensionPath, startTime, false, handle) } if (isUnknownExtension) { this.handleEventForExtension(event, path, this.app.directory, startTime, true) @@ -281,6 +287,7 @@ export class FileWatcher { extensionPath: string, startTime: StartTime, isUnknownExtension: boolean, + extensionHandle?: string, ) { const isExtensionToml = path.endsWith('.extension.toml') const isConfigAppPath = path === this.app.configPath @@ -293,9 +300,9 @@ export class FileWatcher { break } if (isExtensionToml || isConfigAppPath) { - this.pushEvent({type: 'extensions_config_updated', path, extensionPath, startTime}) + this.pushEvent({type: 'extensions_config_updated', path, extensionPath, extensionHandle, startTime}) } else { - this.pushEvent({type: 'file_updated', path, extensionPath, startTime}) + this.pushEvent({type: 'file_updated', path, extensionPath, extensionHandle, startTime}) } break case 'add': @@ -303,7 +310,7 @@ export class FileWatcher { // If a toml file was added, a new extension(s) is being created. // We need to wait for the lock file to disappear before triggering the event. if (!isExtensionToml) { - this.pushEvent({type: 'file_created', path, extensionPath, startTime}) + this.pushEvent({type: 'file_created', path, extensionPath, extensionHandle, startTime}) break } let totalWaitedTime = 0 @@ -339,7 +346,7 @@ export class FileWatcher { setTimeout(() => { // If the extensionPath is not longer in the list, the extension was deleted while the timeout was running. if (!this.extensionPaths.includes(extensionPath)) return - this.pushEvent({type: 'file_deleted', path, extensionPath, startTime}) + this.pushEvent({type: 'file_deleted', path, extensionPath, extensionHandle, startTime}) // Force an emit because we are inside a timeout callback this.debouncedEmit() }, FILE_DELETE_TIMEOUT_IN_MS)