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
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,81 @@ describe('watchedFiles', async () => {
vi.mocked(extractImportPathsRecursively).mockReset()
})
})

test('includes additional watched paths registered via addWatchedPath', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const extensionInstance = await testUIExtension({
directory: tmpDir,
entrySourceFilePath: joinPath(tmpDir, 'src', 'index.ts'),
})

const srcDir = joinPath(tmpDir, 'src')
await mkdir(srcDir)
await writeFile(joinPath(srcDir, 'index.ts'), 'code')

vi.mocked(extractImportPathsRecursively).mockImplementation((filePath) => [filePath])

const assetsDir = joinPath(tmpDir, 'assets')
await mkdir(assetsDir)
extensionInstance.addWatchedPath(assetsDir)

const watchedFiles = extensionInstance.watchedFiles()

expect(watchedFiles).toContain(assetsDir)

vi.mocked(extractImportPathsRecursively).mockReset()
})
})

test('clearWatchedPaths removes all registered paths', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const extensionInstance = await testUIExtension({
directory: tmpDir,
entrySourceFilePath: joinPath(tmpDir, 'src', 'index.ts'),
})

const srcDir = joinPath(tmpDir, 'src')
await mkdir(srcDir)
await writeFile(joinPath(srcDir, 'index.ts'), 'code')

vi.mocked(extractImportPathsRecursively).mockImplementation((filePath) => [filePath])

extensionInstance.addWatchedPath(joinPath(tmpDir, 'assets'))
extensionInstance.clearWatchedPaths()

const watchedFiles = extensionInstance.watchedFiles()

expect(watchedFiles).not.toContain(joinPath(tmpDir, 'assets'))

vi.mocked(extractImportPathsRecursively).mockReset()
})
})

test('getWatchedPaths returns registered paths', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const extensionInstance = await testUIExtension({directory: tmpDir})

const assetsDir = joinPath(tmpDir, 'assets')
extensionInstance.addWatchedPath(assetsDir)

const paths = extensionInstance.getWatchedPaths()

expect(paths.has(assetsDir)).toBe(true)
expect(paths.size).toBe(1)
})
})

test('addWatchedPath deduplicates identical paths', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const extensionInstance = await testUIExtension({directory: tmpDir})

const assetsDir = joinPath(tmpDir, 'assets')
extensionInstance.addWatchedPath(assetsDir)
extensionInstance.addWatchedPath(assetsDir)

expect(extensionInstance.getWatchedPaths().size).toBe(1)
})
})
})

describe('rescanImports', async () => {
Expand Down
15 changes: 15 additions & 0 deletions packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
specification: ExtensionSpecification
uid: string
private cachedImportPaths?: string[]
private readonly additionalWatchedPaths = new Set<string>()

get graphQLType() {
return (this.specification.graphQLType ?? this.specification.identifier).toUpperCase()
Expand Down Expand Up @@ -460,9 +461,23 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
watchedFiles.push(...importedFiles)
}

watchedFiles.push(...this.additionalWatchedPaths)

return [...new Set(watchedFiles.map((file) => normalizePath(file)))]
}

addWatchedPath(absolutePath: string): void {
this.additionalWatchedPaths.add(normalizePath(absolutePath))
}

getWatchedPaths(): ReadonlySet<string> {
return this.additionalWatchedPaths
}

clearWatchedPaths(): void {
this.additionalWatchedPaths.clear()
}

/**
* Rescans imports for this extension and updates the cached import paths
* Returns true if the imports changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ describe('executeIncludeAssetsStep', () => {
mockExtension = {
directory: '/test/extension',
outputPath: '/test/output/extension.js',
} as ExtensionInstance
addWatchedPath: vi.fn(),
} as unknown as ExtensionInstance

mockContext = {
extension: mockExtension,
Expand Down Expand Up @@ -232,6 +233,72 @@ describe('executeIncludeAssetsStep', () => {
expect(result.filesCopied).toBe(2)
})

test('registers directory source paths as watched paths on the extension', async () => {
// Given
const addWatchedPath = vi.fn()
const contextWithConfig = {
...mockContext,
extension: {
...mockExtension,
configuration: {static_root: 'public'},
addWatchedPath,
} as unknown as ExtensionInstance,
}

vi.mocked(fs.fileExists).mockResolvedValue(true)
vi.mocked(fs.isDirectory).mockResolvedValue(true)
vi.mocked(fs.copyDirectoryContents).mockResolvedValue()
vi.mocked(fs.glob).mockResolvedValue(['index.html', 'logo.png'])

const step: LifecycleStep = {
id: 'copy-static',
name: 'Copy Static',
type: 'include_assets',
config: {
inclusions: [{type: 'configKey', key: 'static_root'}],
},
}

// When
await executeIncludeAssetsStep(step, contextWithConfig)

// Then — directory source is registered as a watched path
expect(addWatchedPath).toHaveBeenCalledWith('/test/extension/public')
})

test('does not register file source paths as watched paths', async () => {
// Given
const addWatchedPath = vi.fn()
const contextWithConfig = {
...mockContext,
extension: {
...mockExtension,
configuration: {schema_path: 'src/schema.json'},
addWatchedPath,
} as unknown as ExtensionInstance,
}

vi.mocked(fs.fileExists).mockResolvedValue(true)
vi.mocked(fs.isDirectory).mockResolvedValue(false)
vi.mocked(fs.copyFile).mockResolvedValue()
vi.mocked(fs.mkdir).mockResolvedValue()

const step: LifecycleStep = {
id: 'copy-schema',
name: 'Copy Schema',
type: 'include_assets',
config: {
inclusions: [{type: 'configKey', key: 'schema_path'}],
},
}

// When
await executeIncludeAssetsStep(step, contextWithConfig)

// Then — file sources are not registered as watched paths
expect(addWatchedPath).not.toHaveBeenCalled()
})

test('skips silently when configKey is absent from config', async () => {
// Given — configuration has no static_root
const contextWithoutConfig = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ export async function executeIncludeAssetsStep(
usedBasenames,
preserveFilePaths: entry.preserveFilePaths,
})
result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
result.pathMap.forEach((val, key) => {
aggregatedPathMap.set(key, val)
if (Array.isArray(val)) {
extension.addWatchedPath(joinPath(extension.directory, key))
}
})
configKeyCount += result.filesCopied
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,13 @@ export class AppEventWatcher extends EventEmitter {
const buildableEvents = appEvent.extensionEvents.filter((extEvent) => extEvent.type !== EventType.Deleted)

// Build the created/updated extensions and update the extension events with the build result
const watchedPathsBefore = new Set(buildableEvents.flatMap((ev) => [...ev.extension.getWatchedPaths()]))
await this.buildExtensions(buildableEvents)
const watchedPathsAfter = new Set(buildableEvents.flatMap((ev) => [...ev.extension.getWatchedPaths()]))
const watchedPathsChanged =
watchedPathsBefore.size !== watchedPathsAfter.size ||
[...watchedPathsAfter].some((watchedPath) => !watchedPathsBefore.has(watchedPath))
if (watchedPathsChanged) await this.fileWatcher?.start()

// Generate the extension types after building the extensions so new imports are included
// Skip if the app was reloaded, as generateExtensionTypes was already called during reload
Expand Down
157 changes: 157 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,100 @@ describe('file-watcher events', () => {
})
})

describe('directory watched path handle resolution', () => {
test('resolves extension handle for files inside a watched directory', async () => {
// Given — extension with a watched directory (e.g., assets/)
const assetsDir = '/extensions/ui_extension_1/assets'
const ext = await testUIExtension({
type: 'ui_extension',
handle: 'my-ext',
directory: '/extensions/ui_extension_1',
})
// watchedFiles() returns individual files AND the directory path.
// shouldIgnoreEvent calls matchGlob against this list, so include
// a glob pattern for the directory so files inside it are not filtered out.
mockExtensionWatchedFiles(ext, ['/extensions/ui_extension_1/index.js', `${assetsDir}/**/*`, assetsDir])

const app = testAppLinked({
allExtensions: [ext],
directory: '/',
configPath: '/shopify.app.toml',
configuration: {
...DEFAULT_CONFIG,
extension_directories: ['/extensions'],
} as any,
})

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)

const onChange = vi.fn()
const fileWatcher = new FileWatcher(app, outputOptions, 50)
fileWatcher.onChange(onChange)
await fileWatcher.start()

// When — a file inside the watched directory changes
await eventHandler('change', `${assetsDir}/logo.png`)
await sleep(0.15)

// Then — event is emitted with the correct extension handle
await vi.waitFor(
() => {
expect(onChange).toHaveBeenCalled()
const events = onChange.mock.calls.find((call) => call[0].length > 0)?.[0]
expect(events).toBeDefined()
expect(events[0].type).toBe('file_updated')
expect(events[0].extensionHandle).toBe('my-ext')
},
{timeout: 1000, interval: 50},
)
})

test('does not pass watched directories to chokidar', async () => {
// Given — extension with both files and a directory in watchedFiles
const ext = await testUIExtension({
type: 'ui_extension',
handle: 'my-ext',
directory: '/extensions/ui_extension_1',
})
mockExtensionWatchedFiles(ext, ['/extensions/ui_extension_1/index.js', '/extensions/ui_extension_1/assets'])

const app = testAppLinked({
allExtensions: [ext],
directory: '/',
configPath: '/shopify.app.toml',
configuration: {
...DEFAULT_CONFIG,
extension_directories: ['/extensions'],
} as any,
})

let watchedPaths: string[] = []
vi.spyOn(chokidar, 'watch').mockImplementation((paths) => {
watchedPaths = paths as string[]
return {
on: vi.fn().mockReturnThis(),
close: vi.fn().mockResolvedValue(undefined),
} as any
})

// When
const fileWatcher = new FileWatcher(app, outputOptions)
await fileWatcher.start()

// Then — files are passed to chokidar but directories are not
expect(watchedPaths).toContain('/extensions/ui_extension_1/index.js')
expect(watchedPaths).not.toContain('/extensions/ui_extension_1/assets')
})
})

describe('refreshWatchedFiles', () => {
test('closes and recreates the watcher with updated paths', async () => {
// Given
Expand Down Expand Up @@ -844,4 +938,67 @@ describe('file-watcher events', () => {
expect(watchedPaths[1]).toEqual(watchedPaths[0])
})
})

describe('file deletion events', () => {
test('file_deleted event is emitted even when the file is already gone from disk', async () => {
// Given: an extension with a watched file
const ext = await testUIExtension({
type: 'ui_extension',
handle: 'del_ext',
directory: '/extensions/del_ext',
})
const filePath = '/extensions/del_ext/assets/image.png'
// Simulate real behavior: watchedFiles() returns the file at start,
// but after deletion it no longer appears in the glob results
let watchedFilesResult: string[] = [filePath]
vi.spyOn(ext, 'watchedFiles').mockImplementation(() => watchedFilesResult)

const testApp = testAppLinked({
allExtensions: [ext],
directory: '/',
configPath: '/shopify.app.toml',
configuration: {
...DEFAULT_CONFIG,
extension_directories: ['/extensions'],
} as any,
})

let eventHandler: any
const mockWatcher = {
on: vi.fn((event: string, listener: any) => {
if (event === 'all') eventHandler = listener
return mockWatcher
}),
close: vi.fn().mockResolvedValue(undefined),
}
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()

// Simulate the file being deleted from disk — watchedFiles() no longer returns it
watchedFilesResult = []
await eventHandler('unlink', filePath, undefined)
await sleep(0.15)

// Then: the file_deleted event should still be emitted
await vi.waitFor(
() => {
expect(onChange).toHaveBeenCalled()
const calls = onChange.mock.calls
const actualEvents = calls.find((call) => call[0].length > 0)?.[0]
expect(actualEvents).toBeDefined()
expect(actualEvents).toHaveLength(1)
expect(actualEvents[0].type).toBe('file_deleted')
expect(actualEvents[0].path).toBe(normalizePath(filePath))
expect(actualEvents[0].extensionHandle).toBe('del_ext')
},
{timeout: 1000, interval: 50},
)
})
})
})
Loading
Loading