Skip to content

Commit 92ac699

Browse files
committed
add-directories-to-watched-files
1 parent 9f31416 commit 92ac699

7 files changed

Lines changed: 288 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: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,103 @@ 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, [
877+
'/extensions/ui_extension_1/index.js',
878+
'/extensions/ui_extension_1/assets',
879+
])
880+
881+
const app = testAppLinked({
882+
allExtensions: [ext],
883+
directory: '/',
884+
configPath: '/shopify.app.toml',
885+
configuration: {
886+
...DEFAULT_CONFIG,
887+
extension_directories: ['/extensions'],
888+
} as any,
889+
})
890+
891+
let watchedPaths: string[] = []
892+
vi.spyOn(chokidar, 'watch').mockImplementation((paths) => {
893+
watchedPaths = paths as string[]
894+
return {
895+
on: vi.fn().mockReturnThis(),
896+
close: vi.fn().mockResolvedValue(undefined),
897+
} as any
898+
})
899+
900+
// When
901+
const fileWatcher = new FileWatcher(app, outputOptions)
902+
await fileWatcher.start()
903+
904+
// Then — files are passed to chokidar but directories are not
905+
expect(watchedPaths).toContain('/extensions/ui_extension_1/index.js')
906+
expect(watchedPaths).not.toContain('/extensions/ui_extension_1/assets')
907+
})
908+
})
909+
813910
describe('refreshWatchedFiles', () => {
814911
test('closes and recreates the watcher with updated paths', async () => {
815912
// Given

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable no-case-declarations */
22
import {AppLinkedInterface} from '../../../models/app/app.js'
33
import {configurationFileNames} from '../../../constants.js'
4-
import {dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
4+
import {basename, dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
55
import {FSWatcher} from 'chokidar'
66
import {outputDebug} from '@shopify/cli-kit/node/output'
77
import {AbortSignal} from '@shopify/cli-kit/node/abort'
@@ -59,7 +59,7 @@ export class FileWatcher {
5959
private watcher?: FSWatcher
6060
private readonly debouncedEmit: () => void
6161
private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {}
62-
// Map of file paths to the extension handles that watch them
62+
// Map of watched paths (files and directories) to the extension handles that own them
6363
private readonly extensionWatchedFiles = new Map<string, Set<string>>()
6464

6565
constructor(
@@ -159,18 +159,34 @@ export class FileWatcher {
159159
for (const {extension, watchedFiles} of extensionResults) {
160160
for (const file of watchedFiles) {
161161
const normalizedPath = normalizePath(file)
162-
allFiles.add(normalizedPath)
163162

164-
// Track which extension handles watch this file
163+
// Track which extension handles watch this path
165164
const handlesSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set()
166165
handlesSet.add(extension.handle)
167166
this.extensionWatchedFiles.set(normalizedPath, handlesSet)
167+
168+
// Only pass files to chokidar — directories are already covered by
169+
// extension directory watchers
170+
if (basename(normalizedPath).includes('.')) {
171+
allFiles.add(normalizedPath)
172+
}
168173
}
169174
}
170175

171176
return Array.from(allFiles)
172177
}
173178

179+
private getExtensionHandlesForFilePath(normalizedPath: string): Set<string> | undefined {
180+
const handles = this.extensionWatchedFiles.get(normalizedPath)
181+
if (handles) return handles
182+
for (const [watchedPath, pathHandles] of this.extensionWatchedFiles) {
183+
if (normalizedPath.startsWith(`${watchedPath}/`)) {
184+
return pathHandles
185+
}
186+
}
187+
return undefined
188+
}
189+
174190
/**
175191
* Emits the accumulated events and resets the current events list.
176192
* It also logs the number of events emitted and their paths for debugging purposes.
@@ -251,7 +267,7 @@ export class FileWatcher {
251267
if (isConfigAppPath) {
252268
this.handleEventForExtension(event, path, this.app.directory, startTime, false)
253269
} else {
254-
const affectedHandles = this.extensionWatchedFiles.get(normalizedPath)
270+
const affectedHandles = this.getExtensionHandlesForFilePath(normalizedPath)
255271
const isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0
256272

257273
if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {

0 commit comments

Comments
 (0)