diff --git a/.changeset/file-watcher-external-roots.md b/.changeset/file-watcher-external-roots.md new file mode 100644 index 0000000000..6ee2834162 --- /dev/null +++ b/.changeset/file-watcher-external-roots.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Detect runtime-added files in extension watch paths outside the extension directory 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 4ca1f4d6d4..e6326cafde 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 @@ -957,6 +957,122 @@ describe('file-watcher events', () => { }) }) + describe('external watch roots', () => { + test('adds the static prefix of external watch globs to chokidar', async () => { + const sharedExtension = await testFunctionExtension({dir: '/extensions/with-external'}) + // Pretend this extension declares a watch path outside its directory + vi.spyOn(sharedExtension, 'devSessionWatchConfig', 'get').mockReturnValue({ + paths: ['/shared/lib/**/*.ts'], + }) + vi.spyOn(sharedExtension, 'watchPatterns').mockReturnValue({ + paths: ['/shared/lib/**/*.ts'], + ignore: [], + }) + mockExtensionWatchedFiles(sharedExtension, []) + + const app = testAppLinked({ + allExtensions: [sharedExtension], + directory: '/', + configPath: '/shopify.app.toml', + configuration: {...DEFAULT_CONFIG, extension_directories: ['/extensions']} as any, + }) + + let watchedPaths: string[] = [] + vi.spyOn(chokidar, 'watch').mockImplementation((paths) => { + watchedPaths = paths as string[] + return {on: vi.fn().mockReturnThis(), close: vi.fn().mockResolvedValue(undefined)} as any + }) + + const fileWatcher = new FileWatcher(app, outputOptions) + await fileWatcher.start() + + expect(watchedPaths).toContain('/shared/lib') + }) + + test('does not duplicate watch roots already covered by an extension directory', async () => { + const ext = await testFunctionExtension({dir: '/extensions/my-function'}) + vi.spyOn(ext, 'devSessionWatchConfig', 'get').mockReturnValue({ + paths: ['/extensions/my-function/src/**/*.rs'], + }) + vi.spyOn(ext, 'watchPatterns').mockReturnValue({ + paths: ['/extensions/my-function/src/**/*.rs'], + ignore: [], + }) + mockExtensionWatchedFiles(ext, []) + + const app = testAppLinked({ + allExtensions: [ext], + directory: '/', + configPath: '/shopify.app.toml', + configuration: {...DEFAULT_CONFIG, extension_directories: ['/extensions']} as any, + }) + + let watchedPaths: string[] = [] + vi.spyOn(chokidar, 'watch').mockImplementation((paths) => { + watchedPaths = paths as string[] + return {on: vi.fn().mockReturnThis(), close: vi.fn().mockResolvedValue(undefined)} as any + }) + + const fileWatcher = new FileWatcher(app, outputOptions) + await fileWatcher.start() + + // /extensions is already a chokidar watch root; the prefix /extensions/my-function/src + // is inside it and should not be added separately. + expect(watchedPaths).not.toContain('/extensions/my-function/src') + }) + + test('attributes runtime-added external files to extensions with explicit watch config', async () => { + const sharedExtension = await testFunctionExtension({dir: '/extensions/with-external'}) + vi.spyOn(sharedExtension, 'devSessionWatchConfig', 'get').mockReturnValue({ + paths: ['/shared/lib/**/*.ts'], + }) + vi.spyOn(sharedExtension, 'watchPatterns').mockReturnValue({ + paths: ['/shared/lib/**/*.ts'], + ignore: [], + }) + mockExtensionWatchedFiles(sharedExtension, []) + + const app = testAppLinked({ + allExtensions: [sharedExtension], + directory: '/', + configPath: '/shopify.app.toml', + configuration: {...DEFAULT_CONFIG, extension_directories: ['/extensions']} as any, + }) + + 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(app, outputOptions, 50) + const onChange = vi.fn() + fileWatcher.onChange(onChange) + await fileWatcher.start() + await flushPromises() + + await eventHandler('add', '/shared/lib/util.ts', undefined) + await sleep(0.15) + + 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(1) + expect(events[0].type).toBe('file_created') + expect(events[0].path).toBe('/shared/lib/util.ts') + expect(events[0].extensionHandle).toBe(sharedExtension.handle) + }, + {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 13189873fe..c748128e2f 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 @@ -16,6 +16,20 @@ const EXTENSION_CREATION_TIMEOUT_IN_MS = 60000 const EXTENSION_CREATION_CHECK_INTERVAL_IN_MS = 200 const FILE_DELETE_TIMEOUT_IN_MS = 500 +/** + * Returns the directory prefix of a glob pattern (the longest leading path with + * no wildcards), or the pattern itself if it has none. Patterns are expected to + * be absolute. An empty result means the pattern is too unrooted to be useful + * as a chokidar watch root. + */ +function staticGlobPrefix(pattern: string): string { + const match = /[*?[\](){}!]/.exec(pattern) + if (!match) return pattern + const head = pattern.slice(0, match.index) + const lastSlash = head.lastIndexOf('/') + return lastSlash <= 0 ? '' : head.slice(0, lastSlash) +} + /** * Event emitted by the file watcher * @@ -112,6 +126,11 @@ export class FileWatcher { const allWatchedFiles = this.getAllWatchedFiles() watchPaths.push(...allWatchedFiles) + // Watch the static prefix of every external watch glob so that NEW files + // created in those directories at runtime are surfaced by chokidar. Without + // this, chokidar would only see the specific files returned by globSync. + watchPaths.push(...this.getExternalWatchRoots(fullExtensionDirectories)) + this.close() // Create new watcher @@ -140,6 +159,27 @@ export class FileWatcher { this.options.signal.addEventListener('abort', this.close) } + /** + * Returns the static prefix of every extension watch glob that lives outside + * the directories chokidar already watches recursively. Used to surface NEW + * files created at runtime in externally-watched roots (e.g. an admin + * extension's `static_root` or a function's shared sources). + */ + private getExternalWatchRoots(coveredDirectories: string[]): string[] { + const covered = coveredDirectories.map((dir) => normalizePath(dir.replace(/\/\*+$/, ''))) + const roots = new Set() + for (const extension of this.app.realExtensions) { + if (!extension.devSessionWatchConfig) continue + for (const pattern of extension.watchPatterns().paths) { + const prefix = staticGlobPrefix(pattern) + if (!prefix) continue + if (covered.some((dir) => prefix === dir || prefix.startsWith(`${dir}/`))) continue + roots.add(prefix) + } + } + return Array.from(roots) + } + /** * Gets all files that need to be watched from all extensions */ @@ -247,16 +287,26 @@ export class FileWatcher { } /** - * 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. + * Returns the handles of every extension that should track the given path + * based on the extension's watch patterns. A path can match multiple + * extensions when they share a directory or watch patterns. + * + * Two attribution modes: + * - Inside the extension directory: default `**\/*` patterns apply. + * - Outside the extension directory: only extensions with explicit + * `devSessionWatchConfig` (e.g. admin `static_root`, a function pointing at + * shared sources) are eligible. */ 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 + const isInside = normalizedPath.startsWith(`${extensionDir}/`) + const hasExplicitConfig = extension.devSessionWatchConfig !== undefined + + if (extensionDir === this.app.directory && !hasExplicitConfig) continue + if (!isInside && !hasExplicitConfig) continue + if (this.pathMatchesWatchPatterns(normalizedPath, extension)) { owners.add(extension.handle) } @@ -265,14 +315,16 @@ export class FileWatcher { } /** - * Tests a path against an extension's watch patterns (paths included, ignore - * excluded). Patterns are matched relative to the extension directory. + * Tests a path against an extension's watch patterns. Patterns may be either + * absolute (e.g. function specs join with `extension.directory`) or relative + * to the extension directory (e.g. the default `**\/*`), so we try both forms. */ 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)) + const matches = (pattern: string) => matchGlob(normalizedPath, pattern) || matchGlob(relative, pattern) + if (ignorePatterns.some(matches)) return false + return paths.some(matches) } private handleEventForExtension(