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..cce121ea4d8 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. + */ +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..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 @@ -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,17 @@ 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. + // 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 + } + const extension = event.extensionHandle ? this.app.realExtensions.find((ext) => ext.handle === event.extensionHandle) : undefined @@ -251,8 +263,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 +297,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, @@ -339,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)