Skip to content

Commit e3a34f3

Browse files
committed
Use extension handle to identify extensions in file-watcher
1 parent dbcb9d6 commit e3a34f3

File tree

3 files changed

+40
-26
lines changed

3 files changed

+40
-26
lines changed

packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ export async function handleWatcherEvents(
2929
const appEvent: AppEvent = {app, extensionEvents: [], path: events[0].path, startTime: events[0].startTime}
3030

3131
for (const event of otherEvents) {
32-
const affectedExtensions = app.realExtensions.filter((ext) => ext.directory === event.extensionPath)
32+
const affectedExtensions = event.extensionHandle
33+
? app.realExtensions.filter((ext) => ext.handle === event.extensionHandle)
34+
: app.realExtensions.filter((ext) => ext.directory === event.extensionPath)
3335
const newEvent = handlers[event.type]({event, app: appEvent.app, extensions: affectedExtensions, options})
3436
appEvent.extensionEvents.push(...newEvent.extensionEvents)
3537
}

packages/app/src/cli/services/dev/app-events/file-watcher.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ interface TestCaseSingleEvent {
7676
fileSystemEvent: string
7777
path: string
7878
expectedEvent?: Omit<WatcherEvent, 'startTime'> & {startTime?: WatcherEvent['startTime']}
79+
expectedEventCount?: number
7980
}
8081

8182
/**
@@ -104,6 +105,7 @@ const singleEventTestCases: TestCaseSingleEvent[] = [
104105
path: '/extensions/ui_extension_1/index.js',
105106
extensionPath: '/extensions/ui_extension_1',
106107
},
108+
expectedEventCount: 2,
107109
},
108110
{
109111
name: 'change in toml',
@@ -114,6 +116,7 @@ const singleEventTestCases: TestCaseSingleEvent[] = [
114116
path: '/extensions/ui_extension_1/shopify.ui.extension.toml',
115117
extensionPath: '/extensions/ui_extension_1',
116118
},
119+
expectedEventCount: 2,
117120
},
118121
{
119122
name: 'change in app config',
@@ -134,6 +137,7 @@ const singleEventTestCases: TestCaseSingleEvent[] = [
134137
path: '/extensions/ui_extension_1/new-file.js',
135138
extensionPath: '/extensions/ui_extension_1',
136139
},
140+
expectedEventCount: 2,
137141
},
138142
{
139143
name: 'delete a file',
@@ -280,7 +284,7 @@ describe('file-watcher events', () => {
280284

281285
test.each(singleEventTestCases)(
282286
'The event $name returns the expected WatcherEvent',
283-
async ({fileSystemEvent, path, expectedEvent}) => {
287+
async ({fileSystemEvent, path, expectedEvent, expectedEventCount}) => {
284288
// Given
285289
let eventHandler: any
286290

@@ -369,7 +373,8 @@ describe('file-watcher events', () => {
369373
throw new Error('Expected onChange to be called with events, but all calls had empty arrays')
370374
}
371375

372-
expect(actualEvents).toHaveLength(1)
376+
const eventCount = expectedEventCount ?? 1
377+
expect(actualEvents).toHaveLength(eventCount)
373378
const actualEvent = actualEvents[0]
374379

375380
expect(actualEvent.type).toBe(expectedEvent.type)

packages/app/src/cli/services/dev/app-events/file-watcher.ts

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ const FILE_DELETE_TIMEOUT_IN_MS = 500
1919
/**
2020
* Event emitted by the file watcher
2121
*
22-
* Includes the type of the event, the path of the file that triggered the event and the extension path that contains the file.
23-
* path and extensionPath could be the same if the event is at the extension level (create, delete extension)
22+
* Includes the type of the event, the path of the file that triggered the event and the extension handle that owns the file.
23+
* For folder-level events (create, delete), extensionHandle is undefined since the extension may not exist yet.
2424
*
2525
* @typeParam type - The type of the event
2626
* @typeParam path - The path of the file that triggered the event
27-
* @typeParam extensionPath - The path of the extension that contains the file
27+
* @typeParam extensionHandle - The unique handle of the extension that owns the file
2828
* @typeParam startTime - The time when the event was triggered
2929
*/
3030
export interface WatcherEvent {
@@ -37,6 +37,9 @@ export interface WatcherEvent {
3737
| 'extensions_config_updated'
3838
| 'app_config_deleted'
3939
path: string
40+
/** The unique handle of the extension that owns this file. Undefined for folder-level events. */
41+
extensionHandle?: string
42+
/** The directory path of the extension. Used for folder-level events (create/delete) where no extension handle exists yet. */
4043
extensionPath: string
4144
startTime: StartTime
4245
}
@@ -56,7 +59,7 @@ export class FileWatcher {
5659
private watcher?: FSWatcher
5760
private readonly debouncedEmit: () => void
5861
private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {}
59-
// Map of file paths to the extensions that watch them
62+
// Map of file paths to the extension handles that watch them
6063
private readonly extensionWatchedFiles = new Map<string, Set<string>>()
6164

6265
constructor(
@@ -155,22 +158,21 @@ export class FileWatcher {
155158
private getAllWatchedFiles(): string[] {
156159
this.extensionWatchedFiles.clear()
157160

158-
const extensionResults = this.app.nonConfigExtensions.map((extension) => ({
161+
const extensionResults = this.app.realExtensions.map((extension) => ({
159162
extension,
160163
watchedFiles: extension.watchedFiles(),
161164
}))
162165

163166
const allFiles = new Set<string>()
164167
for (const {extension, watchedFiles} of extensionResults) {
165-
const extensionDir = normalizePath(extension.directory)
166168
for (const file of watchedFiles) {
167169
const normalizedPath = normalizePath(file)
168170
allFiles.add(normalizedPath)
169171

170-
// Track which extensions watch this file
171-
const extensionsSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set()
172-
extensionsSet.add(extensionDir)
173-
this.extensionWatchedFiles.set(normalizedPath, extensionsSet)
172+
// Track which extension handles watch this file
173+
const handlesSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set()
174+
handlesSet.add(extension.handle)
175+
this.extensionWatchedFiles.set(normalizedPath, handlesSet)
174176
}
175177
}
176178

@@ -204,13 +206,13 @@ export class FileWatcher {
204206
}
205207

206208
// If the event is already in the list, don't push it again
207-
// Check path, type, AND extensionPath to properly handle shared files
209+
// Check path, type, AND extensionHandle to properly handle shared files
208210
if (
209211
this.currentEvents.some(
210212
(extEvent) =>
211213
extEvent.path === event.path &&
212214
extEvent.type === event.type &&
213-
extEvent.extensionPath === event.extensionPath,
215+
extEvent.extensionHandle === event.extensionHandle,
214216
)
215217
)
216218
return
@@ -229,15 +231,17 @@ export class FileWatcher {
229231
private shouldIgnoreEvent(event: WatcherEvent) {
230232
if (event.type === 'extension_folder_deleted' || event.type === 'extension_folder_created') return false
231233

232-
const extension = this.app.realExtensions.find((ext) => ext.directory === event.extensionPath)
234+
const extension = event.extensionHandle
235+
? this.app.realExtensions.find((ext) => ext.handle === event.extensionHandle)
236+
: undefined
233237
const watchPaths = extension?.watchedFiles()
234-
const ignoreInstance = this.ignored[event.extensionPath]
238+
const ignoreInstance = extension ? this.ignored[extension.directory] : undefined
235239

236240
if (watchPaths) {
237241
const isAValidWatchedPath = watchPaths.some((pattern) => matchGlob(event.path, pattern))
238242
return !isAValidWatchedPath
239243
} else if (ignoreInstance) {
240-
const relative = relativePath(event.extensionPath, event.path)
244+
const relative = relativePath(extension!.directory, event.path)
241245
return ignoreInstance.ignores(relative)
242246
}
243247

@@ -255,8 +259,8 @@ export class FileWatcher {
255259
if (isConfigAppPath) {
256260
this.handleEventForExtension(event, path, this.app.directory, startTime, false)
257261
} else {
258-
const affectedExtensions = this.extensionWatchedFiles.get(normalizedPath)
259-
const isUnknownExtension = affectedExtensions === undefined || affectedExtensions.size === 0
262+
const affectedHandles = this.extensionWatchedFiles.get(normalizedPath)
263+
const isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0
260264

261265
if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {
262266
// Ignore an event if it's not part of an existing extension
@@ -265,8 +269,10 @@ export class FileWatcher {
265269
return
266270
}
267271

268-
for (const extensionPath of affectedExtensions ?? []) {
269-
this.handleEventForExtension(event, path, extensionPath, startTime, false)
272+
for (const handle of affectedHandles ?? []) {
273+
const extension = this.app.realExtensions.find((ext) => ext.handle === handle)
274+
const extensionPath = extension ? normalizePath(extension.directory) : this.app.directory
275+
this.handleEventForExtension(event, path, extensionPath, startTime, false, handle)
270276
}
271277
if (isUnknownExtension) {
272278
this.handleEventForExtension(event, path, this.app.directory, startTime, true)
@@ -281,6 +287,7 @@ export class FileWatcher {
281287
extensionPath: string,
282288
startTime: StartTime,
283289
isUnknownExtension: boolean,
290+
extensionHandle?: string,
284291
) {
285292
const isExtensionToml = path.endsWith('.extension.toml')
286293
const isConfigAppPath = path === this.app.configPath
@@ -293,17 +300,17 @@ export class FileWatcher {
293300
break
294301
}
295302
if (isExtensionToml || isConfigAppPath) {
296-
this.pushEvent({type: 'extensions_config_updated', path, extensionPath, startTime})
303+
this.pushEvent({type: 'extensions_config_updated', path, extensionPath, extensionHandle, startTime})
297304
} else {
298-
this.pushEvent({type: 'file_updated', path, extensionPath, startTime})
305+
this.pushEvent({type: 'file_updated', path, extensionPath, extensionHandle, startTime})
299306
}
300307
break
301308
case 'add':
302309
// If it's a normal non-toml file, just report a file_created event.
303310
// If a toml file was added, a new extension(s) is being created.
304311
// We need to wait for the lock file to disappear before triggering the event.
305312
if (!isExtensionToml) {
306-
this.pushEvent({type: 'file_created', path, extensionPath, startTime})
313+
this.pushEvent({type: 'file_created', path, extensionPath, extensionHandle, startTime})
307314
break
308315
}
309316
let totalWaitedTime = 0
@@ -339,7 +346,7 @@ export class FileWatcher {
339346
setTimeout(() => {
340347
// If the extensionPath is not longer in the list, the extension was deleted while the timeout was running.
341348
if (!this.extensionPaths.includes(extensionPath)) return
342-
this.pushEvent({type: 'file_deleted', path, extensionPath, startTime})
349+
this.pushEvent({type: 'file_deleted', path, extensionPath, extensionHandle, startTime})
343350
// Force an emit because we are inside a timeout callback
344351
this.debouncedEmit()
345352
}, FILE_DELETE_TIMEOUT_IN_MS)

0 commit comments

Comments
 (0)