Merge pull request #7491 from Shopify/file-watcher-detect-new-files

Detect new files added during dev session in file watcher

Author: isaacroldan

.changeset/file-watcher-detect-new-files.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Detect files added during a running dev session

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,28 +437,28 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
     await this.specification.contributeToSharedTypeFile?.(this, typeDefinitionsByFile)
   }
 
+  /**
+   * Returns the raw watch patterns (paths + ignore) for this extension, without
+   * expanding globs against the file system. The file watcher uses this to decide
+   * whether a path created at runtime should be tracked by this extension.
+   */
+  watchPatterns(): {paths: string[]; ignore: string[]} {
+    const watchConfig = this.devSessionWatchConfig
+    return {
+      paths: watchConfig?.paths ?? ['**/*'],
+      ignore: watchConfig?.ignore ?? DEFAULT_WATCH_IGNORE,
+    }
+  }
+
   /**
    * Returns all files that need to be watched for this extension
    * This includes files in the extension directory (respecting watch paths and gitignore)
    * as well as any imported files from outside the extension directory
    */
   watchedFiles(): string[] {
     const watchedFiles: string[] = []
-
-    const defaultIgnore = [
-      '**/node_modules/**',
-      '**/.git/**',
-      '**/*.test.*',
-      '**/dist/**',
-      '**/*.swp',
-      '**/generated/**',
-      '**/.gitignore',
-    ]
-    const watchConfig = this.devSessionWatchConfig
-
-    const patterns = watchConfig?.paths ?? ['**/*']
-    const ignore = watchConfig?.ignore ?? defaultIgnore
-    const files = patterns.flatMap((pattern) =>
+    const {paths, ignore} = this.watchPatterns()
+    const files = paths.flatMap((pattern) =>
       globSync(pattern, {
         cwd: this.directory,
         absolute: true,
@@ -455,7 +469,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
     watchedFiles.push(...files.flat())
 
     // Add imported files from outside the extension directory unless custom watch config is defined
-    if (!watchConfig) {
+    if (!this.devSessionWatchConfig) {
       const importedFiles = this.scanImports()
       watchedFiles.push(...importedFiles)
     }

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

@@ -810,6 +810,153 @@ describe('file-watcher events', () => {
     })
   })
 
+  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

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<string> {
+    const owners = new Set<string>()
+    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)