Skip to content

Commit 8e21010

Browse files
committed
add-directories-to-watched-files
1 parent 95c92ff commit 8e21010

7 files changed

Lines changed: 353 additions & 7 deletions

File tree

packages/app/src/cli/models/extensions/extension-instance.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,81 @@ describe('watchedFiles', async () => {
717717
vi.mocked(extractImportPathsRecursively).mockReset()
718718
})
719719
})
720+
721+
test('includes additional watched paths registered via addWatchedPath', async () => {
722+
await inTemporaryDirectory(async (tmpDir) => {
723+
const extensionInstance = await testUIExtension({
724+
directory: tmpDir,
725+
entrySourceFilePath: joinPath(tmpDir, 'src', 'index.ts'),
726+
})
727+
728+
const srcDir = joinPath(tmpDir, 'src')
729+
await mkdir(srcDir)
730+
await writeFile(joinPath(srcDir, 'index.ts'), 'code')
731+
732+
vi.mocked(extractImportPathsRecursively).mockImplementation((filePath) => [filePath])
733+
734+
const assetsDir = joinPath(tmpDir, 'assets')
735+
await mkdir(assetsDir)
736+
extensionInstance.addWatchedPath(assetsDir)
737+
738+
const watchedFiles = extensionInstance.watchedFiles()
739+
740+
expect(watchedFiles).toContain(assetsDir)
741+
742+
vi.mocked(extractImportPathsRecursively).mockReset()
743+
})
744+
})
745+
746+
test('clearWatchedPaths removes all registered paths', async () => {
747+
await inTemporaryDirectory(async (tmpDir) => {
748+
const extensionInstance = await testUIExtension({
749+
directory: tmpDir,
750+
entrySourceFilePath: joinPath(tmpDir, 'src', 'index.ts'),
751+
})
752+
753+
const srcDir = joinPath(tmpDir, 'src')
754+
await mkdir(srcDir)
755+
await writeFile(joinPath(srcDir, 'index.ts'), 'code')
756+
757+
vi.mocked(extractImportPathsRecursively).mockImplementation((filePath) => [filePath])
758+
759+
extensionInstance.addWatchedPath(joinPath(tmpDir, 'assets'))
760+
extensionInstance.clearWatchedPaths()
761+
762+
const watchedFiles = extensionInstance.watchedFiles()
763+
764+
expect(watchedFiles).not.toContain(joinPath(tmpDir, 'assets'))
765+
766+
vi.mocked(extractImportPathsRecursively).mockReset()
767+
})
768+
})
769+
770+
test('getWatchedPaths returns registered paths', async () => {
771+
await inTemporaryDirectory(async (tmpDir) => {
772+
const extensionInstance = await testUIExtension({directory: tmpDir})
773+
774+
const assetsDir = joinPath(tmpDir, 'assets')
775+
extensionInstance.addWatchedPath(assetsDir)
776+
777+
const paths = extensionInstance.getWatchedPaths()
778+
779+
expect(paths.has(assetsDir)).toBe(true)
780+
expect(paths.size).toBe(1)
781+
})
782+
})
783+
784+
test('addWatchedPath deduplicates identical paths', async () => {
785+
await inTemporaryDirectory(async (tmpDir) => {
786+
const extensionInstance = await testUIExtension({directory: tmpDir})
787+
788+
const assetsDir = joinPath(tmpDir, 'assets')
789+
extensionInstance.addWatchedPath(assetsDir)
790+
extensionInstance.addWatchedPath(assetsDir)
791+
792+
expect(extensionInstance.getWatchedPaths().size).toBe(1)
793+
})
794+
})
720795
})
721796

722797
describe('rescanImports', async () => {

packages/app/src/cli/models/extensions/extension-instance.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
5151
specification: ExtensionSpecification
5252
uid: string
5353
private cachedImportPaths?: string[]
54+
private readonly additionalWatchedPaths = new Set<string>()
5455

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

464+
watchedFiles.push(...this.additionalWatchedPaths)
465+
463466
return [...new Set(watchedFiles.map((file) => normalizePath(file)))]
464467
}
465468

469+
addWatchedPath(absolutePath: string): void {
470+
this.additionalWatchedPaths.add(normalizePath(absolutePath))
471+
}
472+
473+
getWatchedPaths(): ReadonlySet<string> {
474+
return this.additionalWatchedPaths
475+
}
476+
477+
clearWatchedPaths(): void {
478+
this.additionalWatchedPaths.clear()
479+
}
480+
466481
/**
467482
* Rescans imports for this extension and updates the cached import paths
468483
* Returns true if the imports changed

packages/app/src/cli/services/build/steps/include-assets-step.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ describe('executeIncludeAssetsStep', () => {
1818
mockExtension = {
1919
directory: '/test/extension',
2020
outputPath: '/test/output/extension.js',
21-
} as ExtensionInstance
21+
addWatchedPath: vi.fn(),
22+
} as unknown as ExtensionInstance
2223

2324
mockContext = {
2425
extension: mockExtension,
@@ -232,6 +233,72 @@ describe('executeIncludeAssetsStep', () => {
232233
expect(result.filesCopied).toBe(2)
233234
})
234235

236+
test('registers directory source paths as watched paths on the extension', async () => {
237+
// Given
238+
const addWatchedPath = vi.fn()
239+
const contextWithConfig = {
240+
...mockContext,
241+
extension: {
242+
...mockExtension,
243+
configuration: {static_root: 'public'},
244+
addWatchedPath,
245+
} as unknown as ExtensionInstance,
246+
}
247+
248+
vi.mocked(fs.fileExists).mockResolvedValue(true)
249+
vi.mocked(fs.isDirectory).mockResolvedValue(true)
250+
vi.mocked(fs.copyDirectoryContents).mockResolvedValue()
251+
vi.mocked(fs.glob).mockResolvedValue(['index.html', 'logo.png'])
252+
253+
const step: LifecycleStep = {
254+
id: 'copy-static',
255+
name: 'Copy Static',
256+
type: 'include_assets',
257+
config: {
258+
inclusions: [{type: 'configKey', key: 'static_root'}],
259+
},
260+
}
261+
262+
// When
263+
await executeIncludeAssetsStep(step, contextWithConfig)
264+
265+
// Then — directory source is registered as a watched path
266+
expect(addWatchedPath).toHaveBeenCalledWith('/test/extension/public')
267+
})
268+
269+
test('does not register file source paths as watched paths', async () => {
270+
// Given
271+
const addWatchedPath = vi.fn()
272+
const contextWithConfig = {
273+
...mockContext,
274+
extension: {
275+
...mockExtension,
276+
configuration: {schema_path: 'src/schema.json'},
277+
addWatchedPath,
278+
} as unknown as ExtensionInstance,
279+
}
280+
281+
vi.mocked(fs.fileExists).mockResolvedValue(true)
282+
vi.mocked(fs.isDirectory).mockResolvedValue(false)
283+
vi.mocked(fs.copyFile).mockResolvedValue()
284+
vi.mocked(fs.mkdir).mockResolvedValue()
285+
286+
const step: LifecycleStep = {
287+
id: 'copy-schema',
288+
name: 'Copy Schema',
289+
type: 'include_assets',
290+
config: {
291+
inclusions: [{type: 'configKey', key: 'schema_path'}],
292+
},
293+
}
294+
295+
// When
296+
await executeIncludeAssetsStep(step, contextWithConfig)
297+
298+
// Then — file sources are not registered as watched paths
299+
expect(addWatchedPath).not.toHaveBeenCalled()
300+
})
301+
235302
test('skips silently when configKey is absent from config', async () => {
236303
// Given — configuration has no static_root
237304
const contextWithoutConfig = {

packages/app/src/cli/services/build/steps/include-assets-step.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,12 @@ export async function executeIncludeAssetsStep(
151151
usedBasenames,
152152
preserveFilePaths: entry.preserveFilePaths,
153153
})
154-
result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
154+
result.pathMap.forEach((val, key) => {
155+
aggregatedPathMap.set(key, val)
156+
if (Array.isArray(val)) {
157+
extension.addWatchedPath(joinPath(extension.directory, key))
158+
}
159+
})
155160
configKeyCount += result.filesCopied
156161
}
157162

packages/app/src/cli/services/dev/app-events/app-event-watcher.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,13 @@ export class AppEventWatcher extends EventEmitter {
139139
const buildableEvents = appEvent.extensionEvents.filter((extEvent) => extEvent.type !== EventType.Deleted)
140140

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

144150
// Generate the extension types after building the extensions so new imports are included
145151
// Skip if the app was reloaded, as generateExtensionTypes was already called during reload

packages/app/src/cli/services/dev/app-events/file-watcher.test.ts

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,100 @@ describe('file-watcher events', () => {
810810
})
811811
})
812812

813+
describe('directory watched path handle resolution', () => {
814+
test('resolves extension handle for files inside a watched directory', async () => {
815+
// Given — extension with a watched directory (e.g., assets/)
816+
const assetsDir = '/extensions/ui_extension_1/assets'
817+
const ext = await testUIExtension({
818+
type: 'ui_extension',
819+
handle: 'my-ext',
820+
directory: '/extensions/ui_extension_1',
821+
})
822+
// watchedFiles() returns individual files AND the directory path.
823+
// shouldIgnoreEvent calls matchGlob against this list, so include
824+
// a glob pattern for the directory so files inside it are not filtered out.
825+
mockExtensionWatchedFiles(ext, ['/extensions/ui_extension_1/index.js', `${assetsDir}/**/*`, assetsDir])
826+
827+
const app = testAppLinked({
828+
allExtensions: [ext],
829+
directory: '/',
830+
configPath: '/shopify.app.toml',
831+
configuration: {
832+
...DEFAULT_CONFIG,
833+
extension_directories: ['/extensions'],
834+
} as any,
835+
})
836+
837+
let eventHandler: any
838+
const mockWatcher = {
839+
on: vi.fn((event: string, listener: any) => {
840+
if (event === 'all') eventHandler = listener
841+
return mockWatcher
842+
}),
843+
close: vi.fn(() => Promise.resolve()),
844+
}
845+
vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any)
846+
847+
const onChange = vi.fn()
848+
const fileWatcher = new FileWatcher(app, outputOptions, 50)
849+
fileWatcher.onChange(onChange)
850+
await fileWatcher.start()
851+
852+
// When — a file inside the watched directory changes
853+
await eventHandler('change', `${assetsDir}/logo.png`)
854+
await sleep(0.15)
855+
856+
// Then — event is emitted with the correct extension handle
857+
await vi.waitFor(
858+
() => {
859+
expect(onChange).toHaveBeenCalled()
860+
const events = onChange.mock.calls.find((call) => call[0].length > 0)?.[0]
861+
expect(events).toBeDefined()
862+
expect(events[0].type).toBe('file_updated')
863+
expect(events[0].extensionHandle).toBe('my-ext')
864+
},
865+
{timeout: 1000, interval: 50},
866+
)
867+
})
868+
869+
test('does not pass watched directories to chokidar', async () => {
870+
// Given — extension with both files and a directory in watchedFiles
871+
const ext = await testUIExtension({
872+
type: 'ui_extension',
873+
handle: 'my-ext',
874+
directory: '/extensions/ui_extension_1',
875+
})
876+
mockExtensionWatchedFiles(ext, ['/extensions/ui_extension_1/index.js', '/extensions/ui_extension_1/assets'])
877+
878+
const app = testAppLinked({
879+
allExtensions: [ext],
880+
directory: '/',
881+
configPath: '/shopify.app.toml',
882+
configuration: {
883+
...DEFAULT_CONFIG,
884+
extension_directories: ['/extensions'],
885+
} as any,
886+
})
887+
888+
let watchedPaths: string[] = []
889+
vi.spyOn(chokidar, 'watch').mockImplementation((paths) => {
890+
watchedPaths = paths as string[]
891+
return {
892+
on: vi.fn().mockReturnThis(),
893+
close: vi.fn().mockResolvedValue(undefined),
894+
} as any
895+
})
896+
897+
// When
898+
const fileWatcher = new FileWatcher(app, outputOptions)
899+
await fileWatcher.start()
900+
901+
// Then — files are passed to chokidar but directories are not
902+
expect(watchedPaths).toContain('/extensions/ui_extension_1/index.js')
903+
expect(watchedPaths).not.toContain('/extensions/ui_extension_1/assets')
904+
})
905+
})
906+
813907
describe('refreshWatchedFiles', () => {
814908
test('closes and recreates the watcher with updated paths', async () => {
815909
// Given
@@ -844,4 +938,67 @@ describe('file-watcher events', () => {
844938
expect(watchedPaths[1]).toEqual(watchedPaths[0])
845939
})
846940
})
941+
942+
describe('file deletion events', () => {
943+
test('file_deleted event is emitted even when the file is already gone from disk', async () => {
944+
// Given: an extension with a watched file
945+
const ext = await testUIExtension({
946+
type: 'ui_extension',
947+
handle: 'del_ext',
948+
directory: '/extensions/del_ext',
949+
})
950+
const filePath = '/extensions/del_ext/assets/image.png'
951+
// Simulate real behavior: watchedFiles() returns the file at start,
952+
// but after deletion it no longer appears in the glob results
953+
let watchedFilesResult: string[] = [filePath]
954+
vi.spyOn(ext, 'watchedFiles').mockImplementation(() => watchedFilesResult)
955+
956+
const testApp = testAppLinked({
957+
allExtensions: [ext],
958+
directory: '/',
959+
configPath: '/shopify.app.toml',
960+
configuration: {
961+
...DEFAULT_CONFIG,
962+
extension_directories: ['/extensions'],
963+
} as any,
964+
})
965+
966+
let eventHandler: any
967+
const mockWatcher = {
968+
on: vi.fn((event: string, listener: any) => {
969+
if (event === 'all') eventHandler = listener
970+
return mockWatcher
971+
}),
972+
close: vi.fn().mockResolvedValue(undefined),
973+
}
974+
vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any)
975+
vi.mocked(fileExistsSync).mockReturnValue(false)
976+
977+
const fileWatcher = new FileWatcher(testApp, outputOptions, 50)
978+
const onChange = vi.fn()
979+
fileWatcher.onChange(onChange)
980+
await fileWatcher.start()
981+
await flushPromises()
982+
983+
// Simulate the file being deleted from disk — watchedFiles() no longer returns it
984+
watchedFilesResult = []
985+
await eventHandler('unlink', filePath, undefined)
986+
await sleep(0.15)
987+
988+
// Then: the file_deleted event should still be emitted
989+
await vi.waitFor(
990+
() => {
991+
expect(onChange).toHaveBeenCalled()
992+
const calls = onChange.mock.calls
993+
const actualEvents = calls.find((call) => call[0].length > 0)?.[0]
994+
expect(actualEvents).toBeDefined()
995+
expect(actualEvents).toHaveLength(1)
996+
expect(actualEvents[0].type).toBe('file_deleted')
997+
expect(actualEvents[0].path).toBe(normalizePath(filePath))
998+
expect(actualEvents[0].extensionHandle).toBe('del_ext')
999+
},
1000+
{timeout: 1000, interval: 50},
1001+
)
1002+
})
1003+
})
8471004
})

0 commit comments

Comments
 (0)