Skip to content

Commit

Permalink
Add proper support for gitignore in AppWatcher
Browse files Browse the repository at this point in the history
  • Loading branch information
isaacroldan committed Nov 25, 2024
1 parent 1b4002b commit f675d44
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 66 deletions.
1 change: 1 addition & 0 deletions packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,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}
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
134 changes: 70 additions & 64 deletions packages/app/src/cli/services/dev/app-events/file-watcher.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -91,16 +79,13 @@ export class FileWatcher {
async start(): Promise<void> {
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,
})
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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})
Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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})
Expand All @@ -214,31 +228,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)
}
}
4 changes: 3 additions & 1 deletion packages/app/src/cli/services/dev/update-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 = {
Expand Down
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f675d44

Please sign in to comment.