diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts index 0232a6f112..fc41427880 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts @@ -1,5 +1,5 @@ import {AppEvent, AppEventWatcher, EventType, ExtensionEvent} from './app-event-watcher.js' -import {OutputContextOptions, WatcherEvent, startFileWatcher} from './file-watcher.js' +import {OutputContextOptions, WatcherEvent, FileWatcher} from './file-watcher.js' import {ESBuildContextManager} from './app-watcher-esbuild.js' import { testAppAccessConfigExtension, @@ -11,6 +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 {describe, expect, test, vi} from 'vitest' import {AbortSignal} from '@shopify/cli-kit/node/abort' import {flushPromises} from '@shopify/cli-kit/node/promises' @@ -18,7 +19,6 @@ import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {Writable} from 'stream' -vi.mock('./file-watcher.js') vi.mock('../../../models/app/loader.js') vi.mock('./app-watcher-esbuild.js') @@ -235,7 +235,6 @@ describe('app-event-watcher when receiving a file event', () => { const mockedApp = testAppLinked({allExtensions: finalExtensions}) vi.mocked(loadApp).mockResolvedValue(mockedApp) vi.mocked(reloadApp).mockResolvedValue(mockedApp) - vi.mocked(startFileWatcher).mockImplementation(async (app, options, onChange) => onChange([fileWatchEvent])) const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') @@ -245,7 +244,9 @@ describe('app-event-watcher when receiving a file event', () => { configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, }) - const watcher = new AppEventWatcher(app, 'url', buildOutputPath, new MockESBuildContextManager()) + 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() @@ -305,7 +306,6 @@ describe('app-event-watcher build extension errors', () => { extensionPath: '/extensions/ui_extension_1', startTime: [0, 0], } - vi.mocked(startFileWatcher).mockImplementation(async (app, options, onChange) => onChange([fileWatchEvent])) // Given const esbuildError = { @@ -325,9 +325,10 @@ describe('app-event-watcher build extension errors', () => { allExtensions: [extension1], configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, }) + const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) // When - const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager) + const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher) const stderr = {write: vi.fn()} as unknown as Writable const stdout = {write: vi.fn()} as unknown as Writable @@ -358,7 +359,6 @@ describe('app-event-watcher build extension errors', () => { extensionPath: '/extensions/flow_action', startTime: [0, 0], } - vi.mocked(startFileWatcher).mockImplementation(async (app, options, onChange) => onChange([fileWatchEvent])) // Given const esbuildError = {message: 'Build failed'} @@ -371,7 +371,9 @@ describe('app-event-watcher build extension errors', () => { }) // When - const watcher = new AppEventWatcher(app, 'url', buildOutputPath, new MockESBuildContextManager()) + const mockManager = new MockESBuildContextManager() + const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) + const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher) const stderr = {write: vi.fn()} as unknown as Writable const stdout = {write: vi.fn()} as unknown as Writable @@ -403,3 +405,25 @@ class MockESBuildContextManager extends ESBuildContextManager { async updateContexts(appEvent: AppEvent) {} async deleteContexts(extensions: ExtensionInstance[]) {} } + +// Mock class for FileWatcher +// Used to trigger mocked file system events immediately after the watcher is started. +class MockFileWatcher extends FileWatcher { + private readonly events: WatcherEvent[] + private listener?: (events: WatcherEvent[]) => void + + constructor(app: AppInterface, options: OutputContextOptions, events: WatcherEvent[]) { + super(app, options) + this.events = events + } + + async start(): Promise { + if (this.listener) { + this.listener(this.events) + } + } + + onChange(listener: (events: WatcherEvent[]) => void) { + this.listener = listener + } +} diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 2657bd38c3..cac89dda8e 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -1,5 +1,5 @@ /* eslint-disable tsdoc/syntax */ -import {OutputContextOptions, startFileWatcher} from './file-watcher.js' +import {FileWatcher, OutputContextOptions} from './file-watcher.js' import {ESBuildContextManager} from './app-watcher-esbuild.js' import {handleWatcherEvents} from './app-event-watcher-handler.js' import {AppLinkedInterface} from '../../../models/app/app.js' @@ -96,12 +96,14 @@ export class AppEventWatcher extends EventEmitter { private started = false private ready = false private initialEvents: ExtensionEvent[] = [] + private fileWatcher?: FileWatcher constructor( app: AppLinkedInterface, appURL?: string, buildOutputPath?: string, esbuildManager?: ESBuildContextManager, + fileWatcher?: FileWatcher, ) { super() this.app = app @@ -117,6 +119,7 @@ export class AppEventWatcher extends EventEmitter { url: this.appURL ?? '', ...this.options, }) + this.fileWatcher = fileWatcher } async start(options?: OutputContextOptions, buildExtensionsFirst = true) { @@ -139,8 +142,8 @@ export class AppEventWatcher extends EventEmitter { await this.buildExtensions(this.initialEvents) } - // Start the file system watcher - await startFileWatcher(this.app, this.options, (events) => { + this.fileWatcher = this.fileWatcher ?? new FileWatcher(this.app, this.options) + this.fileWatcher.onChange((events) => { handleWatcherEvents(events, this.app, this.options) .then(async (appEvent) => { if (appEvent?.extensionEvents.length === 0) outputDebug('Change detected, but no extensions were affected') @@ -163,6 +166,7 @@ export class AppEventWatcher extends EventEmitter { this.options.stderr.write(`Error handling event: ${error.message}`) }) }) + await this.fileWatcher.start() this.ready = true this.emit('ready', {app: this.app, extensionEvents: this.initialEvents}) 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 0f461635f7..8ea8537248 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 @@ -1,4 +1,4 @@ -import {OutputContextOptions, WatcherEvent, startFileWatcher} from './file-watcher.js' +import {FileWatcher, OutputContextOptions, WatcherEvent} from './file-watcher.js' import { testApp, testAppAccessConfigExtension, @@ -198,7 +198,10 @@ describe('file-watcher events', () => { }) // When - await startFileWatcher(app, outputOptions, vi.fn()) + const fileWatcher = new FileWatcher(app, outputOptions) + fileWatcher.onChange(vi.fn()) + + await fileWatcher.start() // Then expect(watchSpy).toHaveBeenCalledWith([joinPath(dir, '/shopify.app.toml'), joinPath(dir, '/extensions')], { @@ -232,7 +235,10 @@ describe('file-watcher events', () => { // When const onChange = vi.fn() - await startFileWatcher(defaultApp, outputOptions, onChange) + const fileWatcher = new FileWatcher(defaultApp, outputOptions) + fileWatcher.onChange(onChange) + + await fileWatcher.start() // Then await flushPromises() @@ -260,7 +266,10 @@ describe('file-watcher events', () => { // When const onChange = vi.fn() - await startFileWatcher(defaultApp, outputOptions, onChange) + const fileWatcher = new FileWatcher(defaultApp, outputOptions) + fileWatcher.onChange(onChange) + + await fileWatcher.start() // Then await flushPromises() 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 d3971c3cf4..7e6cb1bd8a 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 @@ -10,6 +10,9 @@ import {fileExistsSync, matchGlob, readFileSync} from '@shopify/cli-kit/node/fs' import {debounce} from '@shopify/cli-kit/common/function' import {Writable} from 'stream' +const DEFAULT_DEBOUNCE_TIME_IN_MS = 200 +const EXTENSION_CREATION_TIMEOUT_IN_MS = 60000 +const EXTENSION_CREATION_CHECK_INTERVAL_IN_MS = 200 /** * Event emitted by the file watcher * @@ -41,48 +44,81 @@ export interface OutputContextOptions { signal: AbortSignal } -/** - * Watch for changes in the given app directory. - * - * It will watch for changes in the active config file and the extension directories. - * When possible, changes will be interpreted to detect new/deleted extensions - * - * Changes to toml files will be reported as different events to other file changes. - * - * @param app - The app to watch - * @param options - The output options - * @param onChange - The callback to call when a change is detected - */ -export async function startFileWatcher( - app: AppInterface, - options: OutputContextOptions, - onChange: (events: WatcherEvent[]) => void, -) { - const {default: chokidar} = await import('chokidar') +export class FileWatcher { + private currentEvents: WatcherEvent[] = [] + private extensionPaths: string[] + private readonly watchPaths: string[] + private readonly customGitIgnoredPatterns: string[] + private readonly app: AppInterface + private readonly options: OutputContextOptions + private onChangeCallback?: (events: WatcherEvent[]) => void + private watcher?: FSWatcher + private readonly debouncedEmit: () => void + + constructor(app: AppInterface, 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) + }) - const appConfigurationPath = app.configuration.path - const extensionDirectories = [...(app.configuration.extension_directories ?? ['extensions'])].map((directory) => { - return joinPath(app.directory, directory) - }) + this.watchPaths = [app.configuration.path, ...extensionDirectories] - let currentEvents: WatcherEvent[] = [] + // 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 300ms to avoid emitting too many events in a short period. - */ - const debouncedEmit = debounce(emitEvents, 300, {leading: true, trailing: true}) + /** + * 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.debouncedEmit = debounce(this.emitEvents.bind(this), debounceTime, {leading: true, trailing: true}) + } + + onChange(listener: (events: WatcherEvent[]) => void) { + this.onChangeCallback = listener + } + + async start(): Promise { + const {default: chokidar} = await import('chokidar') + + this.watcher = chokidar.watch(this.watchPaths, { + ignored: [ + '**/node_modules/**', + '**/.git/**', + '**/*.test.*', + '**/dist/**', + '**/*.swp', + '**/generated/**', + ...this.customGitIgnoredPatterns, + ], + persistent: true, + ignoreInitial: true, + }) + + this.watcher.on('all', this.handleFileEvent) + this.options.signal.addEventListener('abort', this.close) + } /** * Emits the accumulated events and resets the current events list. * It also logs the number of events emitted and their paths for debugging purposes. */ - function emitEvents() { - const events = currentEvents - currentEvents = [] + private readonly emitEvents = () => { + const events = this.currentEvents + this.currentEvents = [] const message = `🔉 ${events.length} EVENTS EMITTED in files: ${events.map((event) => event.path).join('\n')}` - outputDebug(message, options.stdout) - onChange(events) + outputDebug(message, this.options.stdout) + this.onChangeCallback?.(events) } /** @@ -91,73 +127,39 @@ export async function startFileWatcher( * * @param event - The event to be added */ - function pushEvent(event: WatcherEvent) { - const extension = app.realExtensions.find((ext) => ext.directory === event.extensionPath) + private pushEvent(event: WatcherEvent) { + const extension = this.app.realExtensions.find((ext) => ext.directory === event.extensionPath) const watchPaths = extension?.devSessionWatchPaths + // If the affected extension defines custom watch paths, ignore the event if it's not in the list if (watchPaths) { const isAValidWatchedPath = watchPaths.some((pattern) => matchGlob(event.path, pattern)) if (!isAValidWatchedPath) return } + // If the event is already in the list, don't push it again - if (currentEvents.some((extEvent) => extEvent.path === event.path && extEvent.type === event.type)) return - currentEvents.push(event) - debouncedEmit() + if (this.currentEvents.some((extEvent) => extEvent.path === event.path && extEvent.type === event.type)) return + this.currentEvents.push(event) + this.debouncedEmit() } - // 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 - let extensionPaths = app.realExtensions - .map((ext) => normalizePath(ext.directory)) - .filter((dir) => dir !== app.directory) - - // Watch the extensions root directories and the app configuration file, nothing else. - const watchPaths = [appConfigurationPath, ...extensionDirectories] - - // Read .gitignore files from extension directories and add the patterns to the ignored list - const customGitIgnoredPatterns = getCustomGitIgnorePatterns(extensionPaths) - - // Create watcher ignoring node_modules, git, test files, dist folders, vim swap files - // PENDING: Use .gitgnore from app and extensions to ignore files. - const watcher = chokidar.watch(watchPaths, { - ignored: [ - '**/node_modules/**', - '**/.git/**', - '**/*.test.*', - '**/dist/**', - '**/*.swp', - '**/generated/**', - ...customGitIgnoredPatterns, - ], - persistent: true, - ignoreInitial: true, - }) - - // Start chokidar watcher for 'all' events - watcher.on('all', (event, path) => { + private readonly handleFileEvent = (event: string, path: string) => { const startTime = startHRTime() - const isConfigAppPath = path === appConfigurationPath + const isConfigAppPath = path === this.app.configuration.path const extensionPath = - extensionPaths.find((dir) => isSubpath(dir, path)) ?? (isConfigAppPath ? app.directory : 'unknown') + this.extensionPaths.find((dir) => isSubpath(dir, path)) ?? (isConfigAppPath ? this.app.directory : 'unknown') const isToml = path.endsWith('.toml') - outputDebug(`🌀: ${event} ${path.replace(app.directory, '')}\n`) + outputDebug(`🌀: ${event} ${path.replace(this.app.directory, '')}\n`) - if (extensionPath === 'unknown' && !isToml) { - // 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 - } + if (extensionPath === 'unknown' && !isToml) return switch (event) { case 'change': if (isToml) { - pushEvent({type: 'extensions_config_updated', path, extensionPath, startTime}) + this.pushEvent({type: 'extensions_config_updated', path, extensionPath, startTime}) } else { - pushEvent({type: 'file_updated', path, extensionPath, startTime}) + this.pushEvent({type: 'file_updated', path, extensionPath, startTime}) } break case 'add': @@ -165,7 +167,7 @@ export async function startFileWatcher( // 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) { - pushEvent({type: 'file_created', path, extensionPath, startTime}) + this.pushEvent({type: 'file_created', path, extensionPath, startTime}) break } let totalWaitedTime = 0 @@ -173,34 +175,35 @@ export async function startFileWatcher( const intervalId = setInterval(() => { if (fileExistsSync(joinPath(realPath, configurationFileNames.lockFile))) { outputDebug(`Waiting for extension to complete creation: ${path}\n`) - totalWaitedTime += 500 + totalWaitedTime += EXTENSION_CREATION_CHECK_INTERVAL_IN_MS } else { clearInterval(intervalId) - extensionPaths.push(realPath) - pushEvent({type: 'extension_folder_created', path: realPath, extensionPath, startTime}) + this.extensionPaths.push(realPath) + this.pushEvent({type: 'extension_folder_created', path: realPath, extensionPath, startTime}) } - if (totalWaitedTime >= 20000) { + if (totalWaitedTime >= EXTENSION_CREATION_TIMEOUT_IN_MS) { clearInterval(intervalId) - options.stderr.write(`Extension creation detection timeout at path: ${path}\nYou might need to restart dev`) + this.options.stderr.write( + `Extension creation detection timeout at path: ${path}\nYou might need to restart dev`, + ) } - }, 200) + }, EXTENSION_CREATION_CHECK_INTERVAL_IN_MS) break case 'unlink': // Ignore shoplock files if (path.endsWith(configurationFileNames.lockFile)) break if (isConfigAppPath) { - pushEvent({type: 'app_config_deleted', path, extensionPath, startTime}) + this.pushEvent({type: 'app_config_deleted', path, extensionPath, startTime}) } else if (isToml) { // When a toml is deleted, we can consider every extension in that folder was deleted. - extensionPaths = extensionPaths.filter((extPath) => extPath !== extensionPath) - pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) + this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath) + this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) } else { - // This could be an extension delete event, Wait 500ms to see if the toml is deleted or not. setTimeout(() => { // If the extensionPath is not longer in the list, the extension was deleted while the timeout was running. - if (!extensionPaths.includes(extensionPath)) return - pushEvent({type: 'file_deleted', path, extensionPath, startTime}) + if (!this.extensionPaths.includes(extensionPath)) return + this.pushEvent({type: 'file_deleted', path, extensionPath, startTime}) }, 500) } break @@ -209,38 +212,33 @@ export async function startFileWatcher( case 'unlinkDir': break } - }) - - listenForAbortOnWatcher(watcher, options) -} + } -const listenForAbortOnWatcher = (watcher: FSWatcher, options: OutputContextOptions) => { - options.signal.addEventListener('abort', () => { - outputDebug(`Closing file watcher`, options.stdout) - watcher - .close() - .then(() => outputDebug(`File watching closed`, options.stdout)) - .catch((error: Error) => outputDebug(`File watching failed to close: ${error.message}`, options.stderr)) - }) -} + /** + * 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() + } -/** - * Returns the custom gitignore patterns for the given extension directories. - * - * @param extensionDirectories - The extension directories to get the custom gitignore patterns from - * @returns The custom gitignore patterns - */ -function getCustomGitIgnorePatterns(extensionDirectories: string[]): string[] { - return extensionDirectories - .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)) + } }