Skip to content

Commit 5ce5d81

Browse files
Hook app asset file watcher into the existing app event watcher list and fix upload assets on change
Co-authored-by: Trish Ta <trish.ta@shopify.com>
1 parent a349d95 commit 5ce5d81

6 files changed

Lines changed: 90 additions & 67 deletions

File tree

packages/app/src/cli/models/extensions/specifications/admin.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {createContractBasedModuleSpecification} from '../specification.js'
33
const adminSpecificationSpec = createContractBasedModuleSpecification({
44
identifier: 'admin',
55
uidStrategy: 'single',
6+
experience: 'configuration',
67
transformRemoteToLocal: (remoteContent) => {
78
return {
89
admin: {
@@ -24,7 +25,8 @@ const adminSpecificationSpec = createContractBasedModuleSpecification({
2425
name: 'Hosted App Copy Files',
2526
type: 'include_assets',
2627
config: {
27-
generatesAssetsManifest: true,
28+
// Remove this until we fix the bug related to recreating the manifest during dev
29+
generatesAssetsManifest: false,
2830
inclusions: [
2931
{
3032
type: 'configKey',

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export async function handleWatcherEvents(
3232
const affectedExtensions = app.realExtensions.filter((ext) => ext.directory === event.extensionPath)
3333
const newEvent = handlers[event.type]({event, app: appEvent.app, extensions: affectedExtensions, options})
3434
appEvent.extensionEvents.push(...newEvent.extensionEvents)
35+
if (newEvent.appAssetsUpdated) appEvent.appAssetsUpdated = true
3536
}
3637

3738
return appEvent
@@ -61,6 +62,7 @@ const handlers: {[key in WatcherEvent['type']]: Handler} = {
6162
file_deleted: FileChangeHandler,
6263
file_updated: FileChangeHandler,
6364
app_config_deleted: AppConfigDeletedHandler,
65+
app_asset_updated: AppAssetUpdatedHandler,
6466
// These two are processed manually to avoid multiple reloads
6567
extension_folder_created: EmptyHandler,
6668
extensions_config_updated: EmptyHandler,
@@ -91,6 +93,17 @@ function FileChangeHandler({event, app, extensions}: HandlerInput): AppEvent {
9193
return {app, extensionEvents: events, startTime: event.startTime, path: event.path}
9294
}
9395

96+
/**
97+
* When a file inside an app asset directory (e.g. static_root) is updated:
98+
* Find the owning extension via extensionPath and return an Updated event so it gets rebuilt.
99+
* The rebuild runs the include_assets step which copies the changed files into the bundle.
100+
*/
101+
function AppAssetUpdatedHandler({event, app}: HandlerInput): AppEvent {
102+
const adminExtension = app.realExtensions.find((ext) => ext.specification.identifier === 'admin')
103+
const events: ExtensionEvent[] = adminExtension ? [{type: EventType.Updated, extension: adminExtension}] : []
104+
return {app, extensionEvents: events, startTime: event.startTime, path: event.path, appAssetsUpdated: true}
105+
}
106+
94107
/**
95108
* When an event doesn't require any action, return the same app and an empty event.
96109
*/

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ export interface AppEvent {
8282
path: string
8383
startTime: [number, number]
8484
appWasReloaded?: boolean
85+
appAssetsUpdated?: boolean
8586
}
8687

8788
type ExtensionBuildResult = {status: 'ok'; uid: string} | {status: 'error'; error: string; file?: string; uid: string}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,15 +263,15 @@ describe('file-watcher events', () => {
263263

264264
// Then
265265
expect(watchSpy).toHaveBeenCalledWith([joinPath(dir, '/shopify.app.toml'), joinPath(dir, '/extensions')], {
266-
ignored: [
266+
ignored: expect.arrayContaining([
267267
'**/node_modules/**',
268268
'**/.git/**',
269269
'**/*.test.*',
270-
'**/dist/**',
271270
'**/*.swp',
272271
'**/generated/**',
273272
'**/.gitignore',
274-
],
273+
expect.any(Function),
274+
]),
275275
ignoreInitial: true,
276276
persistent: true,
277277
})

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

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable no-case-declarations */
22
import {AppLinkedInterface} from '../../../models/app/app.js'
33
import {configurationFileNames} from '../../../constants.js'
4+
import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
45
import {dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
56
import {FSWatcher} from 'chokidar'
67
import {outputDebug} from '@shopify/cli-kit/node/output'
@@ -9,6 +10,7 @@ import {startHRTime, StartTime} from '@shopify/cli-kit/node/hrtime'
910
import {fileExistsSync, matchGlob, mkdir, readFileSync} from '@shopify/cli-kit/node/fs'
1011
import {debounce} from '@shopify/cli-kit/common/function'
1112
import ignore from 'ignore'
13+
import {getPathValue} from '@shopify/cli-kit/common/object'
1214
import {Writable} from 'stream'
1315

1416
const DEFAULT_DEBOUNCE_TIME_IN_MS = 200
@@ -36,6 +38,7 @@ export interface WatcherEvent {
3638
| 'file_deleted'
3739
| 'extensions_config_updated'
3840
| 'app_config_deleted'
41+
| 'app_asset_updated'
3942
path: string
4043
extensionPath: string
4144
startTime: StartTime
@@ -58,6 +61,8 @@ export class FileWatcher {
5861
private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {}
5962
// Map of file paths to the extensions that watch them
6063
private readonly extensionWatchedFiles = new Map<string, Set<string>>()
64+
// Map of asset directory path to the extension directory that owns it
65+
private appAssetToExtensionDir = new Map<string, string>()
6166

6267
constructor(
6368
app: AppLinkedInterface,
@@ -104,7 +109,9 @@ export class FileWatcher {
104109
}),
105110
)
106111

112+
this.appAssetToExtensionDir = this.resolveAppAssetWatchPaths(this.app.realExtensions)
107113
const watchPaths = [this.app.configPath, ...fullExtensionDirectories]
114+
Array.from(this.appAssetToExtensionDir.keys()).forEach((key) => watchPaths.push(key))
108115

109116
// Get all watched files from extensions
110117
const allWatchedFiles = this.getAllWatchedFiles()
@@ -114,15 +121,24 @@ export class FileWatcher {
114121

115122
// Create new watcher
116123
const {default: chokidar} = await import('chokidar')
124+
const appAssetDirs = [...this.appAssetToExtensionDir.keys()]
117125
this.watcher = chokidar.watch(watchPaths, {
118126
ignored: [
119127
'**/node_modules/**',
120128
'**/.git/**',
121129
'**/*.test.*',
122-
'**/dist/**',
123130
'**/*.swp',
124131
'**/generated/**',
125132
'**/.gitignore',
133+
// Ignore files inside dist/ directories, unless the path falls under a watched
134+
// app asset directory (e.g. static_root may point to a dist/ folder).
135+
// Non-dist paths are never ignored here (return false). For dist paths, we only
136+
// allow them through if they are inside one of the app asset directories.
137+
(filePath: string) => {
138+
const normalized = normalizePath(filePath)
139+
if (!normalized.includes('/dist/') && !normalized.endsWith('/dist')) return false
140+
return !appAssetDirs.some((assetDir) => normalized.startsWith(assetDir))
141+
},
126142
],
127143
persistent: true,
128144
ignoreInitial: true,
@@ -177,6 +193,36 @@ export class FileWatcher {
177193
return Array.from(allFiles)
178194
}
179195

196+
/**
197+
* Resolves app asset directories that should be watched.
198+
* Returns a map of absolute asset directory path → owning extension directory.
199+
*/
200+
private resolveAppAssetWatchPaths(allExtensions: ExtensionInstance[]): Map<string, string> {
201+
const result = new Map<string, string>()
202+
const adminExtension = allExtensions.find((ext) => ext.specification.identifier === 'admin')
203+
if (adminExtension) {
204+
const staticRootPath = getPathValue<string>(adminExtension.configuration, 'admin.static_root')
205+
if (staticRootPath) {
206+
const absolutePath = joinPath(adminExtension.directory, staticRootPath)
207+
result.set(normalizePath(absolutePath), normalizePath(adminExtension.directory))
208+
}
209+
}
210+
return result
211+
}
212+
213+
/**
214+
* Checks if a file path is inside any app asset directory.
215+
* Returns the owning extension directory if found, undefined otherwise.
216+
*/
217+
private findAppAssetExtensionDir(filePath: string): string | undefined {
218+
for (const [assetDir, extensionDir] of this.appAssetToExtensionDir) {
219+
if (filePath.startsWith(assetDir)) {
220+
return extensionDir
221+
}
222+
}
223+
return undefined
224+
}
225+
180226
/**
181227
* Emits the accumulated events and resets the current events list.
182228
* It also logs the number of events emitted and their paths for debugging purposes.
@@ -227,7 +273,12 @@ export class FileWatcher {
227273
* Explicit watch paths have priority over custom gitignore files
228274
*/
229275
private shouldIgnoreEvent(event: WatcherEvent) {
230-
if (event.type === 'extension_folder_deleted' || event.type === 'extension_folder_created') return false
276+
if (
277+
event.type === 'extension_folder_deleted' ||
278+
event.type === 'extension_folder_created' ||
279+
event.type === 'app_asset_updated'
280+
)
281+
return false
231282

232283
const extension = this.app.realExtensions.find((ext) => ext.directory === event.extensionPath)
233284
const watchPaths = extension?.watchedFiles()
@@ -258,6 +309,16 @@ export class FileWatcher {
258309
const affectedExtensions = this.extensionWatchedFiles.get(normalizedPath)
259310
const isUnknownExtension = affectedExtensions === undefined || affectedExtensions.size === 0
260311

312+
// Check if the file is inside an app asset directory (e.g. static_root)
313+
const appAssetExtensionDir = this.findAppAssetExtensionDir(normalizedPath)
314+
if (appAssetExtensionDir) {
315+
if (event === 'change' || event === 'add' || event === 'unlink') {
316+
this.pushEvent({type: 'app_asset_updated', path, extensionPath: appAssetExtensionDir, startTime})
317+
}
318+
this.debouncedEmit()
319+
return
320+
}
321+
261322
if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {
262323
// Ignore an event if it's not part of an existing extension
263324
// Except if it is a toml file (either app config or extension config)

packages/app/src/cli/services/dev/extension.ts

Lines changed: 7 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,8 @@ import {AbortSignal} from '@shopify/cli-kit/node/abort'
1313
import {outputDebug} from '@shopify/cli-kit/node/output'
1414
import {joinPath} from '@shopify/cli-kit/node/path'
1515
import {getPathValue} from '@shopify/cli-kit/common/object'
16-
import {fileExists} from '@shopify/cli-kit/node/fs'
1716
import {DotEnvFile} from '@shopify/cli-kit/node/dot-env'
1817
import {Writable} from 'stream'
19-
import type {FSWatcher} from 'chokidar'
2018

2119
interface AppAssets {
2220
[key: string]: string
@@ -170,64 +168,16 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
170168
const websocketConnection = setupWebsocketConnection({...payloadOptions, httpServer, payloadStore})
171169
outputDebug(`Setting up the UI extensions bundler and file watching...`, payloadOptions.stdout)
172170

173-
// Set up asset directory watchers
174-
let assetWatchers: FSWatcher[] = []
175-
const debounceTimers = new Map<string, ReturnType<typeof setTimeout>>()
176-
177-
function createDebouncedAssetUpdater(assetKey: string) {
178-
return () => {
179-
const existing = debounceTimers.get(assetKey)
180-
if (existing) clearTimeout(existing)
181-
debounceTimers.set(
182-
assetKey,
183-
setTimeout(() => {
184-
debounceTimers.delete(assetKey)
185-
payloadStore.updateAppAssetTimestamp(assetKey)
186-
}, 200),
187-
)
188-
}
189-
}
190-
191-
function clearDebounceTimers() {
192-
for (const timer of debounceTimers.values()) {
193-
clearTimeout(timer)
194-
}
195-
debounceTimers.clear()
196-
}
197-
198-
async function startAssetWatchers(assets: Record<string, string>) {
199-
// Clear pending debounce timers and close existing watchers
200-
clearDebounceTimers()
201-
await Promise.all(assetWatchers.map((watcher) => watcher.close()))
202-
// eslint-disable-next-line require-atomic-updates
203-
assetWatchers = []
204-
205-
const {default: chokidar} = await import('chokidar')
206-
for (const [assetKey, directoryPath] of Object.entries(assets)) {
207-
const exists = await fileExists(directoryPath)
208-
if (!exists) continue
209-
210-
const watcher = chokidar.watch(directoryPath, {ignoreInitial: true})
211-
watcher.on('all', createDebouncedAssetUpdater(assetKey))
212-
assetWatchers.push(watcher)
213-
}
214-
}
215-
216-
const initialAppAssets = payloadOptions.appAssets
217-
if (initialAppAssets && Object.keys(initialAppAssets).length > 0) {
218-
await startAssetWatchers(initialAppAssets)
219-
}
220-
221-
const eventHandler = async ({appWasReloaded, app, extensionEvents}: AppEvent) => {
171+
const eventHandler = async ({appWasReloaded, app, extensionEvents, appAssetsUpdated}: AppEvent) => {
222172
if (appWasReloaded) {
223173
extensions = app.allExtensions.filter((ext) => ext.isPreviewable)
174+
}
224175

225-
// Re-resolve app assets in case admin extension was added/removed/changed
226-
const newAppAssets = resolveAppAssets(app.allExtensions)
227-
const hasAssets = Object.keys(newAppAssets).length > 0
228-
payloadOptions.appAssets = hasAssets ? newAppAssets : undefined
229-
payloadStore.updateAppAssets(payloadOptions.appAssets, payloadOptions.url)
230-
await startAssetWatchers(hasAssets ? newAppAssets : {})
176+
if (appAssetsUpdated && payloadOptions.appAssets) {
177+
for (const assetKey of Object.keys(payloadOptions.appAssets)) {
178+
payloadStore.updateAppAssetTimestamp(assetKey)
179+
}
180+
return
231181
}
232182

233183
for (const event of extensionEvents) {
@@ -264,10 +214,6 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
264214

265215
payloadOptions.signal.addEventListener('abort', () => {
266216
outputDebug('Closing the UI extensions dev server...')
267-
clearDebounceTimers()
268-
for (const watcher of assetWatchers) {
269-
watcher.close().catch(() => {})
270-
}
271217
websocketConnection.close()
272218
httpServer.close()
273219
})

0 commit comments

Comments
 (0)