Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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',
Expand Down Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ interface TestCaseSingleEvent {
fileSystemEvent: string
path: string
expectedEvent?: Omit<WatcherEvent, 'startTime'> & {startTime?: WatcherEvent['startTime']}
expectedEventCount?: number
expectedHandles?: string[]
}

/**
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -369,14 +380,23 @@ 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)
expect(actualEvent.path).toBe(normalizePath(expectedEvent.path))
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},
)
Expand Down
47 changes: 27 additions & 20 deletions packages/app/src/cli/services/dev/app-events/file-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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<string, Set<string>>()

constructor(
Expand Down Expand Up @@ -162,15 +165,14 @@ export class FileWatcher {

const allFiles = new Set<string>()
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)
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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]

Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -293,17 +300,17 @@ 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':
// If it's a normal non-toml file, just report a file_created event.
// 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
Expand Down Expand Up @@ -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)
Expand Down
Loading