From f675d44cf35f040c4a8e6f8a9f3c97eda1a143ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 25 Nov 2024 16:22:35 +0100 Subject: [PATCH] Add proper support for gitignore in AppWatcher --- packages/app/package.json | 1 + .../app-events/app-event-watcher-handler.ts | 2 +- .../dev/app-events/app-event-watcher.ts | 2 + .../services/dev/app-events/file-watcher.ts | 134 +++++++++--------- .../src/cli/services/dev/update-extension.ts | 4 +- pnpm-lock.yaml | 8 ++ 6 files changed, 85 insertions(+), 66 deletions(-) diff --git a/packages/app/package.json b/packages/app/package.json index b1d9c54e505..867222571ae 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -67,6 +67,7 @@ "graphql-request": "5.2.0", "h3": "0.7.21", "http-proxy": "1.18.1", + "ignore": "6.0.2", "proper-lockfile": "4.1.2", "react": "^18.2.0", "react-dom": "18.2.0", diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts index 00770aa1521..44a1228c0eb 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts @@ -120,7 +120,7 @@ async function ReloadAppHandler({event, app}: HandlerInput): Promise { 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} } /* 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 cac89dda8e0..efd01f6116e 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 @@ -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 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 33bb7183092..cb39b030a7a 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 @@ -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 @@ -46,42 +47,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) + this.updateApp(app) } onChange(listener: (events: WatcherEvent[]) => void) { @@ -91,16 +79,13 @@ export class FileWatcher { 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, - ], + 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/**'], persistent: true, ignoreInitial: true, }) @@ -109,6 +94,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. @@ -130,11 +125,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 @@ -148,15 +154,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}) @@ -166,7 +182,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 } @@ -183,9 +199,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 @@ -195,7 +209,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}) @@ -214,26 +228,6 @@ 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 @@ -241,4 +235,16 @@ export class FileWatcher { .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) + } } diff --git a/packages/app/src/cli/services/dev/update-extension.ts b/packages/app/src/cli/services/dev/update-extension.ts index 7227be9774b..7a799bfe2e5 100644 --- a/packages/app/src/cli/services/dev/update-extension.ts +++ b/packages/app/src/cli/services/dev/update-extension.ts @@ -38,6 +38,8 @@ export async function updateExtensionDraft({ bundlePath, }: UpdateExtensionDraftOptions) { let encodedFile: string | undefined + const outputPath = extension.getOutputPathForDirectory(bundlePath) + if (extension.features.includes('esbuild')) { const outputPath = extension.getOutputPathForDirectory(bundlePath) const content = await readFile(outputPath) @@ -58,7 +60,7 @@ export async function updateExtensionDraft({ serialized_script: encodedFile, } if (extension.isFunctionExtension) { - const compiledFiles = await readFile(extension.outputPath, {encoding: 'base64'}) + const compiledFiles = await readFile(outputPath, {encoding: 'base64'}) draftableConfig.uploaded_files = {'dist/index.wasm': compiledFiles} } const extensionInput: ExtensionUpdateDraftMutationVariables = { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a564cf327ca..019671fb1bc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -200,6 +200,9 @@ importers: http-proxy: specifier: 1.18.1 version: 1.18.1 + ignore: + specifier: 6.0.2 + version: 6.0.2 proper-lockfile: specifier: 4.1.2 version: 4.1.2 @@ -11287,6 +11290,11 @@ packages: resolution: {integrity: sha512-5Fytz/IraMjqpwfd34ke28PTVMjZjJG2MPn5t7OE4eUCUNf8BAa7b5WUS9/Qvr6mwOQS7Mk6vdsMno5he+T8Xw==} engines: {node: '>= 4'} + /ignore@6.0.2: + resolution: {integrity: sha512-InwqeHHN2XpumIkMvpl/DCJVrAHgCsG5+cn1XlnLWGwtZBm8QJfSusItfrwx81CTp5agNZqpKU2J/ccC5nGT4A==} + engines: {node: '>= 4'} + dev: false + /immutable@3.7.6: resolution: {integrity: sha512-AizQPcaofEtO11RZhPPHBOJRdo/20MKQF9mBLnVkBoyHi1/zXK8fzVdnEpSV9gxqtnh6Qomfp3F0xT5qP/vThw==} engines: {node: '>=0.8.0'}