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
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
26 changes: 21 additions & 5 deletions packages/app/src/cli/services/dev/app-events/file-watcher.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-case-declarations */
import {AppLinkedInterface} from '../../../models/app/app.js'
import {configurationFileNames} from '../../../constants.js'
import {dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
import {basename, dirname, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path'
import {FSWatcher} from 'chokidar'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {AbortSignal} from '@shopify/cli-kit/node/abort'
Expand Down Expand Up @@ -59,7 +59,7 @@ export class FileWatcher {
private watcher?: FSWatcher
private readonly debouncedEmit: () => void
private readonly ignored: {[key: string]: ignore.Ignore | undefined} = {}
// Map of file paths to the extension handles that watch them
// Map of watched paths (files and directories) to the extension handles that own them
private readonly extensionWatchedFiles = new Map<string, Set<string>>()

constructor(
Expand Down Expand Up @@ -159,18 +159,34 @@ export class FileWatcher {
for (const {extension, watchedFiles} of extensionResults) {
for (const file of watchedFiles) {
const normalizedPath = normalizePath(file)
allFiles.add(normalizedPath)

// Track which extension handles watch this file
// Track which extension handles watch this path
const handlesSet = this.extensionWatchedFiles.get(normalizedPath) ?? new Set()
handlesSet.add(extension.handle)
this.extensionWatchedFiles.set(normalizedPath, handlesSet)

// Only pass files to chokidar — directories are already covered by
// extension directory watchers
if (basename(normalizedPath).includes('.')) {
allFiles.add(normalizedPath)
}
}
}

return Array.from(allFiles)
}

private getExtensionHandlesForFilePath(normalizedPath: string): Set<string> | undefined {
const handles = this.extensionWatchedFiles.get(normalizedPath)
if (handles) return handles
for (const [watchedPath, pathHandles] of this.extensionWatchedFiles) {
if (normalizedPath.startsWith(`${watchedPath}/`)) {
return pathHandles
}
}
return undefined
}

/**
* Emits the accumulated events and resets the current events list.
* It also logs the number of events emitted and their paths for debugging purposes.
Expand Down Expand Up @@ -251,7 +267,7 @@ export class FileWatcher {
if (isConfigAppPath) {
this.handleEventForExtension(event, path, this.app.directory, startTime, false)
} else {
const affectedHandles = this.extensionWatchedFiles.get(normalizedPath)
const affectedHandles = this.getExtensionHandlesForFilePath(normalizedPath)
const isUnknownExtension = affectedHandles === undefined || affectedHandles.size === 0

if (isUnknownExtension && !isExtensionToml && !isConfigAppPath) {
Expand Down
Loading