Add proper support for gitignore in AppWatcher (#4917)

### WHY are these changes introduced?

Fixes issues with file watching and extension creation detection during `app dev`.

### WHAT is this pull request doing?

- Improves file watching by using the `ignore` package to properly handle `.gitignore` files in extensions. If an extension defines it, anything in it will be ignored by the app watcher.
- Fixes extension creation mid-dev by updating the file watcher when the app is reloaded.

### How to test your changes?

1. Create a new app with some extensions
2. Add a `.gitignore` file to an extension directory
3. Start dev and verify:
   - File changes matching `.gitignore` patterns are properly ignored
   - New extensions are properly detected mid-dev

### Checklist

- [x] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [x] I've considered possible documentation changes

packages/app/src/cli/models/app/app.test-data.ts

@@ -500,6 +500,7 @@ function defaultFunctionConfiguration(): FunctionConfigType {
     type: 'product_discounts',
     build: {
       command: 'echo "hello world"',
+      watch: ['src/**/*.rs'],
     },
     api_version: '2022-07',
     configuration_ui: true,

packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts

@@ -119,7 +119,7 @@ async function ReloadAppHandler({event, app}: HandlerInput): Promise<AppEvent> {
   const deletedEvents = diff.deleted.map((ext) => ({type: EventType.Deleted, extension: ext}))
   const updatedEvents = diff.updated.map((ext) => ({type: EventType.Updated, extension: ext}))
   const extensionEvents = [...createdEvents, ...deletedEvents, ...updatedEvents]
-  return {app: newApp, extensionEvents, startTime: event.startTime, path: event.path}
+  return {app: newApp, extensionEvents, startTime: event.startTime, path: event.path, appWasReloaded: true}
 }
 
 /*

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

@@ -11,7 +11,7 @@ import {
 } from '../../../models/app/app.test-data.js'
 import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
 import {loadApp, reloadApp} from '../../../models/app/loader.js'
-import {AppInterface} from '../../../models/app/app.js'
+import {AppLinkedInterface} from '../../../models/app/app.js'
 import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
 import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort'
 import {flushPromises} from '@shopify/cli-kit/node/promises'
@@ -260,7 +260,6 @@ describe('app-event-watcher', () => {
 
           const mockManager = new MockESBuildContextManager()
           const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-
           const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
           const emitSpy = vi.spyOn(watcher, 'emit')
           await watcher.start({stdout, stderr, signal: abortController.signal})
@@ -290,6 +289,7 @@ describe('app-event-watcher', () => {
             extensionEvents: expect.arrayContaining(extensionEvents),
             startTime: expect.anything(),
             path: expect.anything(),
+            appWasReloaded: needsAppReload,
           })
 
           const initialEvents = app.realExtensions.map((eve) => ({
@@ -426,7 +426,7 @@ class MockFileWatcher extends FileWatcher {
   private readonly events: WatcherEvent[]
   private listener?: (events: WatcherEvent[]) => void
 
-  constructor(app: AppInterface, options: OutputContextOptions, events: WatcherEvent[]) {
+  constructor(app: AppLinkedInterface, options: OutputContextOptions, events: WatcherEvent[]) {
     super(app, options)
     this.events = events
   }

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

@@ -80,6 +80,7 @@ export interface AppEvent {
   extensionEvents: ExtensionEvent[]
   path: string
   startTime: [number, number]
+  appWasReloaded?: boolean
 }
 
 type ExtensionBuildResult = {status: 'ok'; handle: string} | {status: 'error'; error: string; handle: string}
@@ -150,6 +151,7 @@ export class AppEventWatcher extends EventEmitter {
           if (!appEvent || appEvent.extensionEvents.length === 0) return
 
           this.app = appEvent.app
+          if (appEvent.appWasReloaded) this.fileWatcher?.updateApp(this.app)
           await this.esbuildManager.updateContexts(appEvent)
 
           // Find affected created/updated extensions and build them

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

@@ -1,9 +1,9 @@
 import {FileWatcher, OutputContextOptions, WatcherEvent} from './file-watcher.js'
 import {
-  testApp,
   testAppAccessConfigExtension,
   testAppConfigExtensions,
   testAppLinked,
+  testFunctionExtension,
   testUIExtension,
 } from '../../../models/app/app.test-data.js'
 import {flushPromises} from '@shopify/cli-kit/node/promises'
@@ -12,10 +12,12 @@ import chokidar from 'chokidar'
 import {AbortSignal} from '@shopify/cli-kit/node/abort'
 import {inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
+import {sleep} from '@shopify/cli-kit/node/system'
 
 const extension1 = await testUIExtension({type: 'ui_extension', handle: 'h1', directory: '/extensions/ui_extension_1'})
 const extension1B = await testUIExtension({type: 'ui_extension', handle: 'h2', directory: '/extensions/ui_extension_1'})
 const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2'})
+const functionExtension = await testFunctionExtension({dir: '/extensions/my-function'})
 const posExtension = await testAppConfigExtensions()
 const appAccessExtension = await testAppAccessConfigExtension()
 
@@ -30,7 +32,7 @@ interface TestCaseSingleEvent {
   name: string
   fileSystemEvent: string
   path: string
-  expectedEvent: WatcherEvent
+  expectedEvent?: WatcherEvent
 }
 
 /**
@@ -127,6 +129,12 @@ const singleEventTestCases: TestCaseSingleEvent[] = [
       startTime: expect.any(Array),
     },
   },
+  {
+    name: 'change in function extension is ignored if not in watch list',
+    fileSystemEvent: 'change',
+    path: '/extensions/my-function/src/cargo.lock',
+    expectedEvent: undefined,
+  },
 ]
 
 const multiEventTestCases: TestCaseMultiEvent[] = [
@@ -167,8 +175,8 @@ const multiEventTestCases: TestCaseMultiEvent[] = [
 ]
 
 const outputOptions: OutputContextOptions = {stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()}
-const defaultApp = testApp({
-  allExtensions: [extension1, extension1B, extension2, posExtension, appAccessExtension],
+const defaultApp = testAppLinked({
+  allExtensions: [extension1, extension1B, extension2, posExtension, appAccessExtension, functionExtension],
   directory: '/',
   configuration: {scopes: '', extension_directories: ['/extensions'], path: '/shopify.app.toml'},
 })
@@ -212,9 +220,7 @@ describe('file-watcher events', () => {
           '**/dist/**',
           '**/*.swp',
           '**/generated/**',
-          joinPath(dir, '/extensions/ext1/a_folder'),
-          joinPath(dir, '/extensions/ext1/a_file.txt'),
-          joinPath(dir, '/extensions/ext1/**/nested/**'),
+          '**/.gitignore',
         ],
         ignoreInitial: true,
         persistent: true,
@@ -244,12 +250,17 @@ describe('file-watcher events', () => {
       await flushPromises()
 
       // use waitFor to so that we can test the debouncers and timeouts
-      await vi.waitFor(
-        () => {
-          expect(onChange).toHaveBeenCalledWith([expectedEvent])
-        },
-        {timeout: 2000, interval: 100},
-      )
+      if (expectedEvent) {
+        await vi.waitFor(
+          () => {
+            expect(onChange).toHaveBeenCalledWith([expectedEvent])
+          },
+          {timeout: 2000, interval: 100},
+        )
+      } else {
+        await sleep(0.01)
+        expect(onChange).not.toHaveBeenCalled()
+      }
     },
   )
 

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

@@ -1,13 +1,14 @@
 /* eslint-disable no-case-declarations */
-import {AppInterface} from '../../../models/app/app.js'
+import {AppLinkedInterface} from '../../../models/app/app.js'
 import {configurationFileNames} from '../../../constants.js'
-import {dirname, isSubpath, joinPath, normalizePath} from '@shopify/cli-kit/node/path'
+import {dirname, isSubpath, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
 import {FSWatcher} from 'chokidar'
 import {outputDebug} from '@shopify/cli-kit/node/output'
 import {AbortSignal} from '@shopify/cli-kit/node/abort'
 import {startHRTime, StartTime} from '@shopify/cli-kit/node/hrtime'
 import {fileExistsSync, matchGlob, readFileSync} from '@shopify/cli-kit/node/fs'
 import {debounce} from '@shopify/cli-kit/common/function'
+import ignore from 'ignore'
 import {Writable} from 'stream'
 
 const DEFAULT_DEBOUNCE_TIME_IN_MS = 200
@@ -48,42 +49,29 @@ export interface OutputContextOptions {
 
 export class FileWatcher {
   private currentEvents: WatcherEvent[] = []
-  private extensionPaths: string[]
-  private readonly watchPaths: string[]
-  private readonly customGitIgnoredPatterns: string[]
-  private readonly app: AppInterface
+  private extensionPaths: string[] = []
+  private app: AppLinkedInterface
   private readonly options: OutputContextOptions
   private onChangeCallback?: (events: WatcherEvent[]) => void
   private watcher?: FSWatcher
   private readonly debouncedEmit: () => void
+  private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {}
 
-  constructor(app: AppInterface, options: OutputContextOptions, debounceTime: number = DEFAULT_DEBOUNCE_TIME_IN_MS) {
+  constructor(
+    app: AppLinkedInterface,
+    options: OutputContextOptions,
+    debounceTime: number = DEFAULT_DEBOUNCE_TIME_IN_MS,
+  ) {
     this.app = app
     this.options = options
 
-    // Current active extension paths (not defined in the main app configuration file)
-    // If a change happens outside of these paths, it will be ignored unless is for a new extension being created
-    // When a new extension is created, the path is added to this list
-    // When an extension is deleted, the path is removed from this list
-    // For every change, the corresponding extensionPath will be also reported in the event
-    this.extensionPaths = app.realExtensions
-      .map((ext) => normalizePath(ext.directory))
-      .filter((dir) => dir !== app.directory)
-
-    const extensionDirectories = [...(app.configuration.extension_directories ?? ['extensions'])].map((directory) => {
-      return joinPath(app.directory, directory)
-    })
-
-    this.watchPaths = [app.configuration.path, ...extensionDirectories]
-
-    // Read .gitignore files from extension directories and add the patterns to the ignored list
-    this.customGitIgnoredPatterns = this.getCustomGitIgnorePatterns()
-
     /**
      * Debounced function to emit the accumulated events.
-     * This function will be called at most once every 500ms to avoid emitting too many events in a short period.
+     * This function will be called at most once every DEFAULT_DEBOUNCE_TIME_IN_MS
+     * to avoid emitting too many events in a short period.
      */
     this.debouncedEmit = debounce(this.emitEvents.bind(this), debounceTime, {leading: true, trailing: true})
+    this.updateApp(app)
   }
 
   onChange(listener: (events: WatcherEvent[]) => void) {
@@ -93,15 +81,20 @@ export class FileWatcher {
   async start(): Promise<void> {
     const {default: chokidar} = await import('chokidar')
 
-    this.watcher = chokidar.watch(this.watchPaths, {
+    const extensionDirectories = [...(this.app.configuration.extension_directories ?? ['extensions'])]
+    const fullExtensionDirectories = extensionDirectories.map((directory) => joinPath(this.app.directory, directory))
+
+    const watchPaths = [this.app.configuration.path, ...fullExtensionDirectories]
+
+    this.watcher = chokidar.watch(watchPaths, {
       ignored: [
         '**/node_modules/**',
         '**/.git/**',
         '**/*.test.*',
         '**/dist/**',
         '**/*.swp',
         '**/generated/**',
-        ...this.customGitIgnoredPatterns,
+        '**/.gitignore',
       ],
       persistent: true,
       ignoreInitial: true,
@@ -111,6 +104,16 @@ export class FileWatcher {
     this.options.signal.addEventListener('abort', this.close)
   }
 
+  updateApp(app: AppLinkedInterface) {
+    this.app = app
+    this.extensionPaths = this.app.realExtensions
+      .map((ext) => normalizePath(ext.directory))
+      .filter((dir) => dir !== this.app.directory)
+    this.extensionPaths.forEach((path) => {
+      this.ignored[path] ??= this.createIgnoreInstance(path)
+    })
+  }
+
   /**
    * Emits the accumulated events and resets the current events list.
    * It also logs the number of events emitted and their paths for debugging purposes.
@@ -132,11 +135,22 @@ export class FileWatcher {
   private pushEvent(event: WatcherEvent) {
     const extension = this.app.realExtensions.find((ext) => ext.directory === event.extensionPath)
     const watchPaths = extension?.devSessionWatchPaths
+    const ignoreInstance = this.ignored[event.extensionPath]
 
     // If the affected extension defines custom watch paths, ignore the event if it's not in the list
+    // ELSE, if the extension has a custom gitignore file, ignore the event if it matches the patterns
+    // Explicit watch paths have priority over custom gitignore files
     if (watchPaths) {
       const isAValidWatchedPath = watchPaths.some((pattern) => matchGlob(event.path, pattern))
       if (!isAValidWatchedPath) return
+    } else if (ignoreInstance) {
+      const relative = relativePath(event.extensionPath, event.path)
+      if (ignoreInstance.ignores(relative)) return
+    }
+
+    // If the event is for a new extension folder, create a new ignore instance
+    if (event.type === 'extension_folder_created') {
+      this.ignored[event.path] = this.createIgnoreInstance(event.path)
     }
 
     // If the event is already in the list, don't push it again
@@ -150,15 +164,25 @@ export class FileWatcher {
     const isConfigAppPath = path === this.app.configuration.path
     const extensionPath =
       this.extensionPaths.find((dir) => isSubpath(dir, path)) ?? (isConfigAppPath ? this.app.directory : 'unknown')
-    const isToml = path.endsWith('.toml')
+    const isExtensionToml = path.endsWith('.extension.toml')
+    const isUnknownExtension = extensionPath === 'unknown'
 
     outputDebug(`🌀: ${event} ${path.replace(this.app.directory, '')}\n`)
 
-    if (extensionPath === 'unknown' && !isToml) return
+    if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {
+      // Ignore an event if it's not part of an existing extension
+      // Except if it is a toml file (either app config or extension config)
+      return
+    }
 
     switch (event) {
       case 'change':
-        if (isToml) {
+        if (isUnknownExtension) {
+          // If the extension path is unknown, it means the extension was just created.
+          // We need to wait for the lock file to disappear before triggering the event.
+          return
+        }
+        if (isExtensionToml || isConfigAppPath) {
           this.pushEvent({type: 'extensions_config_updated', path, extensionPath, startTime})
         } else {
           this.pushEvent({type: 'file_updated', path, extensionPath, startTime})
@@ -168,7 +192,7 @@ export class FileWatcher {
         // If it's a normal non-toml file, just report a file_created event.
         // If a toml file was added, a new extension(s) is being created.
         // We need to wait for the lock file to disappear before triggering the event.
-        if (!isToml) {
+        if (!isExtensionToml) {
           this.pushEvent({type: 'file_created', path, extensionPath, startTime})
           break
         }
@@ -185,9 +209,7 @@ export class FileWatcher {
           }
           if (totalWaitedTime >= EXTENSION_CREATION_TIMEOUT_IN_MS) {
             clearInterval(intervalId)
-            this.options.stderr.write(
-              `Extension creation detection timeout at path: ${path}\nYou might need to restart dev`,
-            )
+            this.options.stderr.write(`Error loading new extension at path: ${path}.\n Please restart the process.`)
           }
         }, EXTENSION_CREATION_CHECK_INTERVAL_IN_MS)
         break
@@ -197,7 +219,7 @@ export class FileWatcher {
 
         if (isConfigAppPath) {
           this.pushEvent({type: 'app_config_deleted', path, extensionPath, startTime})
-        } else if (isToml) {
+        } else if (isExtensionToml) {
           // When a toml is deleted, we can consider every extension in that folder was deleted.
           this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath)
           this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime})
@@ -216,31 +238,23 @@ export class FileWatcher {
     }
   }
 
-  /**
-   * Returns the custom gitignore patterns for the given extension directories.
-   *
-   * @returns The custom gitignore patterns
-   */
-  private getCustomGitIgnorePatterns(): string[] {
-    return this.extensionPaths
-      .map((dir) => {
-        const gitIgnorePath = joinPath(dir, '.gitignore')
-        if (!fileExistsSync(gitIgnorePath)) return []
-        const gitIgnoreContent = readFileSync(gitIgnorePath).toString()
-        return gitIgnoreContent
-          .split('\n')
-          .map((pattern) => pattern.trim())
-          .filter((pattern) => pattern !== '' && !pattern.startsWith('#'))
-          .map((pattern) => joinPath(dir, pattern))
-      })
-      .flat()
-  }
-
   private readonly close = () => {
     outputDebug(`Closing file watcher`, this.options.stdout)
     this.watcher
       ?.close()
       .then(() => outputDebug(`File watching closed`, this.options.stdout))
       .catch((error: Error) => outputDebug(`File watching failed to close: ${error.message}`, this.options.stderr))
   }
+
+  // Creates an "Ignore" instance for the given path if a .gitignore file exists, otherwise undefined
+  private createIgnoreInstance(path: string): ignore.Ignore | undefined {
+    const gitIgnorePath = joinPath(path, '.gitignore')
+    if (!fileExistsSync(gitIgnorePath)) return undefined
+    const gitIgnoreContent = readFileSync(gitIgnorePath)
+      .toString()
+      .split('\n')
+      .map((pattern) => pattern.trim())
+      .filter((pattern) => pattern !== '' && !pattern.startsWith('#'))
+    return ignore.default().add(gitIgnoreContent)
+  }
 }