From f8eabc1e1dff1f8c0c5ba77f30fe0456718ff0d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 7 May 2026 15:52:43 +0200 Subject: [PATCH 1/3] Detect new files added during dev session in file watcher The file watcher resolved file ownership statically at start time using extension.watchedFiles(), which expanded globs into a literal list of paths. Files created at runtime weren't in that list, so chokidar's add events were dropped by the "not watched by any extension" check. Now attribute unknown 'add' events to an owning extension by directory containment and watch-pattern matching, register the new path in the dynamic map, and short-circuit shouldIgnoreEvent for paths already tracked. Adds ExtensionInstance.watchPatterns() exposing the raw paths + ignore patterns. --- .changeset/file-watcher-detect-new-files.md | 5 + .../models/extensions/extension-instance.ts | 46 ++++-- .../dev/app-events/file-watcher.test.ts | 147 ++++++++++++++++++ .../services/dev/app-events/file-watcher.ts | 52 ++++++- 4 files changed, 232 insertions(+), 18 deletions(-) create mode 100644 .changeset/file-watcher-detect-new-files.md diff --git a/.changeset/file-watcher-detect-new-files.md b/.changeset/file-watcher-detect-new-files.md new file mode 100644 index 00000000000..166fd054f53 --- /dev/null +++ b/.changeset/file-watcher-detect-new-files.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Detect files added during a running dev session diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index e266cb54c0c..0c091b46b4a 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -26,6 +26,20 @@ import { import {isTruthy} from '@shopify/cli-kit/node/context/utilities' import {uniq} from '@shopify/cli-kit/common/array' +/** + * Default ignore patterns for files watched in an extension directory. Used when + * the extension does not provide a custom devSessionWatchConfig. + */ +export const DEFAULT_WATCH_IGNORE = [ + '**/node_modules/**', + '**/.git/**', + '**/*.test.*', + '**/dist/**', + '**/*.swp', + '**/generated/**', + '**/.gitignore', +] + /** * Class that represents an instance of a local extension * Before creating this class we've validated that: @@ -423,6 +437,19 @@ export class ExtensionInstance + const {paths, ignore} = this.watchPatterns() + const files = paths.flatMap((pattern) => globSync(pattern, { cwd: this.directory, absolute: true, @@ -455,7 +469,7 @@ export class ExtensionInstance { }) }) + describe('runtime file discovery', () => { + test('files added at runtime inside an existing extension trigger file_created', async () => { + // Given: extension knows about index.js but NOT runtime-added.js + mockExtensionWatchedFiles(extension1, ['/extensions/ui_extension_1/index.js']) + mockExtensionWatchedFiles(extension1B, ['/extensions/ui_extension_1/index.js']) + mockExtensionWatchedFiles(extension2, []) + mockExtensionWatchedFiles(functionExtension, []) + mockExtensionWatchedFiles(posExtension, []) + mockExtensionWatchedFiles(appAccessExtension, []) + + const testApp = { + ...defaultApp, + allExtensions: defaultApp.allExtensions, + nonConfigExtensions: defaultApp.allExtensions.filter((ext) => !ext.isAppConfigExtension), + realExtensions: defaultApp.allExtensions, + } + + let eventHandler: any + const mockWatcher = { + on: vi.fn((event: string, listener: any) => { + if (event === 'all') eventHandler = listener + return mockWatcher + }), + close: vi.fn(() => Promise.resolve()), + } + vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any) + vi.mocked(fileExistsSync).mockReturnValue(false) + + const fileWatcher = new FileWatcher(testApp, outputOptions, 50) + const onChange = vi.fn() + fileWatcher.onChange(onChange) + await fileWatcher.start() + await flushPromises() + + // When: a file the extension didn't pre-register is created on disk + await eventHandler('add', '/extensions/ui_extension_1/runtime-added.js', undefined) + await sleep(0.15) + + // Then: it's attributed to the owning extensions and emitted + await vi.waitFor( + () => { + const events = onChange.mock.calls.find((call) => call[0].length > 0)?.[0] + if (!events) throw new Error('no events emitted') + expect(events).toHaveLength(2) + for (const event of events) { + expect(event.type).toBe('file_created') + expect(event.path).toBe('/extensions/ui_extension_1/runtime-added.js') + expect(event.extensionPath).toBe('/extensions/ui_extension_1') + } + const handles = events.map((event: WatcherEvent) => event.extensionHandle).sort() + expect(handles).toEqual(['h1', 'h2']) + }, + {timeout: 1000, interval: 50}, + ) + }) + + test('files added at runtime outside any extension are ignored', async () => { + mockExtensionWatchedFiles(extension1, []) + mockExtensionWatchedFiles(extension1B, []) + mockExtensionWatchedFiles(extension2, []) + mockExtensionWatchedFiles(functionExtension, []) + mockExtensionWatchedFiles(posExtension, []) + mockExtensionWatchedFiles(appAccessExtension, []) + + const testApp = { + ...defaultApp, + allExtensions: defaultApp.allExtensions, + nonConfigExtensions: defaultApp.allExtensions.filter((ext) => !ext.isAppConfigExtension), + realExtensions: defaultApp.allExtensions, + } + + let eventHandler: any + const mockWatcher = { + on: vi.fn((event: string, listener: any) => { + if (event === 'all') eventHandler = listener + return mockWatcher + }), + close: vi.fn(() => Promise.resolve()), + } + vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any) + vi.mocked(fileExistsSync).mockReturnValue(false) + + const fileWatcher = new FileWatcher(testApp, outputOptions, 50) + const onChange = vi.fn() + fileWatcher.onChange(onChange) + await fileWatcher.start() + await flushPromises() + + await eventHandler('add', '/some/random/path/file.js', undefined) + await sleep(0.15) + + const hasNonEmptyCall = onChange.mock.calls.some((call) => call[0].length > 0) + expect(hasNonEmptyCall).toBe(false) + }) + + test('subsequent change/unlink on a runtime-discovered file are not dropped', async () => { + mockExtensionWatchedFiles(extension1, ['/extensions/ui_extension_1/index.js']) + mockExtensionWatchedFiles(extension1B, ['/extensions/ui_extension_1/index.js']) + mockExtensionWatchedFiles(extension2, []) + mockExtensionWatchedFiles(functionExtension, []) + mockExtensionWatchedFiles(posExtension, []) + mockExtensionWatchedFiles(appAccessExtension, []) + + const testApp = { + ...defaultApp, + allExtensions: defaultApp.allExtensions, + nonConfigExtensions: defaultApp.allExtensions.filter((ext) => !ext.isAppConfigExtension), + realExtensions: defaultApp.allExtensions, + } + + let eventHandler: any + const mockWatcher = { + on: vi.fn((event: string, listener: any) => { + if (event === 'all') eventHandler = listener + return mockWatcher + }), + close: vi.fn(() => Promise.resolve()), + } + vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any) + vi.mocked(fileExistsSync).mockReturnValue(false) + + const fileWatcher = new FileWatcher(testApp, outputOptions, 50) + const onChange = vi.fn() + fileWatcher.onChange(onChange) + await fileWatcher.start() + await flushPromises() + + // Discover the file via 'add' + await eventHandler('add', '/extensions/ui_extension_1/runtime-added.js', undefined) + await sleep(0.1) + + // Now fire a 'change' on the same path; should produce a file_updated event + onChange.mockClear() + await eventHandler('change', '/extensions/ui_extension_1/runtime-added.js', undefined) + await sleep(0.1) + + await vi.waitFor( + () => { + const events = onChange.mock.calls.find((call) => call[0].length > 0)?.[0] + if (!events) throw new Error('no change events emitted') + expect(events.some((event: WatcherEvent) => event.type === 'file_updated')).toBe(true) + }, + {timeout: 1000, interval: 50}, + ) + }) + }) + describe('refreshWatchedFiles', () => { test('closes and recreates the watcher with updated paths', async () => { // Given 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 6b022e82cf2..e78fc8dad3d 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 @@ -1,5 +1,6 @@ /* eslint-disable no-case-declarations */ import {AppLinkedInterface} from '../../../models/app/app.js' +import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {configurationFileNames} from '../../../constants.js' import {dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path' import {FSWatcher} from 'chokidar' @@ -223,6 +224,12 @@ export class FileWatcher { private shouldIgnoreEvent(event: WatcherEvent) { if (event.type === 'extension_folder_deleted' || event.type === 'extension_folder_created') return false + // If this path is already tracked for this handle (either pre-registered or + // discovered at runtime), accept without re-checking against the static list. + if (event.extensionHandle && this.extensionWatchedFiles.get(event.path)?.has(event.extensionHandle)) { + return false + } + const extension = event.extensionHandle ? this.app.realExtensions.find((ext) => ext.handle === event.extensionHandle) : undefined @@ -251,8 +258,20 @@ export class FileWatcher { if (isConfigAppPath) { this.handleEventForExtension(event, path, this.app.directory, startTime, false) } else { - const affectedHandles = this.extensionWatchedFiles.get(normalizedPath) - const isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0 + let affectedHandles = this.extensionWatchedFiles.get(normalizedPath) + let isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0 + + // For 'add' events on paths we don't yet track, try to attribute them to an + // existing extension by directory containment + watch pattern matching. This + // is what allows files created during a running dev session to be picked up. + if (isUnknownExtension && event === 'add' && !isExtensionToml) { + const discovered = this.discoverFileOwners(normalizedPath) + if (discovered.size > 0) { + this.extensionWatchedFiles.set(normalizedPath, discovered) + affectedHandles = discovered + isUnknownExtension = false + } + } if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) { // Ignore an event if it's not part of an existing extension @@ -273,6 +292,35 @@ export class FileWatcher { this.debouncedEmit() } + /** + * Returns the handles of every extension that should track the given path, + * based on directory containment and the extension's watch patterns. + * A path can be owned by multiple extensions when they share a directory. + */ + private discoverFileOwners(normalizedPath: string): Set { + const owners = new Set() + for (const extension of this.app.realExtensions) { + const extensionDir = normalizePath(extension.directory) + if (extensionDir === this.app.directory) continue + if (!normalizedPath.startsWith(`${extensionDir}/`)) continue + if (this.pathMatchesWatchPatterns(normalizedPath, extension)) { + owners.add(extension.handle) + } + } + return owners + } + + /** + * Tests a path against an extension's watch patterns (paths included, ignore + * excluded). Patterns are matched relative to the extension directory. + */ + private pathMatchesWatchPatterns(normalizedPath: string, extension: ExtensionInstance): boolean { + const {paths, ignore: ignorePatterns} = extension.watchPatterns() + const relative = relativePath(normalizePath(extension.directory), normalizedPath) + if (ignorePatterns.some((pattern) => matchGlob(relative, pattern))) return false + return paths.some((pattern) => matchGlob(relative, pattern)) + } + private handleEventForExtension( event: string, path: string, From 2ef08a33b7a9b4bfbd7846eca433bf62f36fe7c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 7 May 2026 18:06:44 +0200 Subject: [PATCH 2/3] Unexport DEFAULT_WATCH_IGNORE to satisfy knip knip flags the constant as an unused export since it's only consumed inside extension-instance.ts. Make it a module-local const. --- packages/app/src/cli/models/extensions/extension-instance.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 0c091b46b4a..cce121ea4d8 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -30,7 +30,7 @@ import {uniq} from '@shopify/cli-kit/common/array' * Default ignore patterns for files watched in an extension directory. Used when * the extension does not provide a custom devSessionWatchConfig. */ -export const DEFAULT_WATCH_IGNORE = [ +const DEFAULT_WATCH_IGNORE = [ '**/node_modules/**', '**/.git/**', '**/*.test.*', From 3f7b6f1ccab0b948236ef3cc08caa23a7859f4a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 7 May 2026 18:09:11 +0200 Subject: [PATCH 3/3] Fix Windows path mismatch and clean up map on unlink Two review-comment fixes from #7491: - shouldIgnoreEvent looked up event.path raw, but extensionWatchedFiles is keyed by normalizePath(file). On Windows, chokidar emits backslash-separated paths and the lookup would miss, causing runtime-discovered files to be re-filtered by the static-list check. Normalize before the lookup. - Runtime discovery added entries to extensionWatchedFiles but never removed them on unlink. In a long-running dev session the map grew unbounded with stale ownership data. Delete the normalized path from the map after pushing file_deleted; subsequent timeouts for other handles are no-ops on the now-missing key. --- .../src/cli/services/dev/app-events/file-watcher.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 e78fc8dad3d..cbc86472baa 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 @@ -226,7 +226,12 @@ export class FileWatcher { // If this path is already tracked for this handle (either pre-registered or // discovered at runtime), accept without re-checking against the static list. - if (event.extensionHandle && this.extensionWatchedFiles.get(event.path)?.has(event.extensionHandle)) { + // The map is keyed by normalized paths, so normalize before the lookup — + // chokidar can emit backslash-separated paths on Windows. + if ( + event.extensionHandle && + this.extensionWatchedFiles.get(normalizePath(event.path))?.has(event.extensionHandle) + ) { return false } @@ -387,6 +392,10 @@ export class FileWatcher { // 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, extensionHandle, startTime}) + // Drop the path from our ownership map so it doesn't leak across a + // long-running dev session. Subsequent timeouts for other handles + // are no-ops on the now-missing key. + this.extensionWatchedFiles.delete(normalizePath(path)) // Force an emit because we are inside a timeout callback this.debouncedEmit() }, FILE_DELETE_TIMEOUT_IN_MS)