From 054b1febaf2dde86561529dd88f013dc8f9b1cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 7 May 2026 17:59:12 +0200 Subject: [PATCH] Detect runtime-added files in external extension watch paths Extensions can declare watch globs outside their own directory via devSessionWatchConfig.paths (e.g. an admin extension's static_root, or a function pointing at a shared sources tree). Today, files added there at runtime aren't picked up: chokidar only watches the specific files returned by globSync at start, not their parent directories. This change: - Computes the static prefix of every external watch glob (the longest leading path with no wildcards) and adds those prefixes to chokidar's watch list, deduped against the extension directories chokidar already watches recursively. - Loosens discoverFileOwners to attribute paths by watch-pattern match rather than directory containment alone. Outside-dir paths are only attributed to extensions that explicitly opt in via devSessionWatchConfig, so the default '**/*' pattern doesn't pollute attribution for config-style extensions whose directory is the app root. - Updates pathMatchesWatchPatterns to match against both the absolute path and the path relative to the extension directory, since devSessionWatchConfig patterns can be either form. --- .changeset/file-watcher-external-roots.md | 5 + .../dev/app-events/file-watcher.test.ts | 116 ++++++++++++++++++ .../services/dev/app-events/file-watcher.ts | 70 +++++++++-- 3 files changed, 182 insertions(+), 9 deletions(-) create mode 100644 .changeset/file-watcher-external-roots.md 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(