Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/file-watcher-detect-new-files.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/app': patch
---

Detect files added during a running dev session
46 changes: 30 additions & 16 deletions packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
147 changes: 147 additions & 0 deletions packages/app/src/cli/services/dev/app-events/file-watcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 59 additions & 2 deletions packages/app/src/cli/services/dev/app-events/file-watcher.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Comment on lines +269 to +278
}

if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {
// Ignore an event if it's not part of an existing extension
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
Loading