From 524fd69572d8fb983c39bab9d62c7e13005268f2 Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Thu, 2 Apr 2026 09:21:04 -0400 Subject: [PATCH] add debounce on file watcher for theme app extensions --- .../theme-ext-fs.test.ts | 157 +++++++++++++++++- .../theme-ext-environment/theme-ext-fs.ts | 31 +++- 2 files changed, 184 insertions(+), 4 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.test.ts b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.test.ts index b41ac516f54..81c12c72b42 100644 --- a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.test.ts @@ -1,6 +1,11 @@ import {mountThemeExtensionFileSystem} from './theme-ext-fs.js' -import {test, describe, expect} from 'vitest' +import * as themeFsModule from '../theme-fs.js' +import * as checksumModule from '../asset-checksum.js' +import {test, describe, expect, vi, beforeEach, afterEach} from 'vitest' import {dirname, joinPath} from '@shopify/cli-kit/node/path' +import * as systemModule from '@shopify/cli-kit/node/system' +import chokidar from 'chokidar' +import EventEmitter from 'node:events' import {fileURLToPath} from 'node:url' import type {Checksum, ThemeAsset} from '@shopify/cli-kit/node/themes/types' @@ -172,6 +177,156 @@ describe('theme-ext-fs', () => { }) }) + describe('startWatcher debounce', () => { + const root = joinPath(locationOfThisFile, '../fixtures/theme-ext') + + beforeEach(() => { + vi.useFakeTimers() + const mockWatcher = new EventEmitter() + vi.spyOn(chokidar, 'watch').mockImplementation((_) => { + return mockWatcher as any + }) + vi.spyOn(themeFsModule, 'readThemeFile').mockResolvedValue('mock content') + vi.spyOn(checksumModule, 'calculateChecksum').mockReturnValue('mock-checksum') + vi.spyOn(systemModule, 'sleep').mockResolvedValue(undefined as any) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + test('triggers handler after debounce period (add event)', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const addHandler = vi.fn() + themeFileSystem.addEventListener('add', addHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + watcher.emit('add', joinPath(root, 'blocks/new_block.liquid')) + + expect(addHandler).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(250) + + expect(addHandler).toHaveBeenCalledOnce() + expect(addHandler).toHaveBeenCalledWith(expect.objectContaining({fileKey: 'blocks/new_block.liquid'})) + }) + + test('triggers delete handler after debounce period (unlink event)', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const unlinkHandler = vi.fn() + themeFileSystem.addEventListener('unlink', unlinkHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + watcher.emit('unlink', joinPath(root, 'blocks/star_rating.liquid')) + + expect(unlinkHandler).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(250) + + expect(unlinkHandler).toHaveBeenCalledOnce() + expect(unlinkHandler).toHaveBeenCalledWith(expect.objectContaining({fileKey: 'blocks/star_rating.liquid'})) + expect(themeFileSystem.files.has('blocks/star_rating.liquid')).toBe(false) + }) + + test('collapses rapid duplicate events into single handler call', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const changeHandler = vi.fn() + themeFileSystem.addEventListener('change', changeHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + const filePath = joinPath(root, 'blocks/star_rating.liquid') + + watcher.emit('change', filePath) + watcher.emit('change', filePath) + watcher.emit('change', filePath) + watcher.emit('change', filePath) + watcher.emit('change', filePath) + + await vi.advanceTimersByTimeAsync(250) + + expect(changeHandler).toHaveBeenCalledOnce() + }) + + test('debounces different files independently', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const changeHandler = vi.fn() + themeFileSystem.addEventListener('change', changeHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + watcher.emit('change', joinPath(root, 'blocks/star_rating.liquid')) + watcher.emit('change', joinPath(root, 'snippets/stars.liquid')) + + await vi.advanceTimersByTimeAsync(250) + + expect(changeHandler).toHaveBeenCalledTimes(2) + }) + + test('calls correct handler per event type', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const addHandler = vi.fn() + const unlinkHandler = vi.fn() + themeFileSystem.addEventListener('add', addHandler) + themeFileSystem.addEventListener('unlink', unlinkHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + watcher.emit('add', joinPath(root, 'blocks/new_block.liquid')) + watcher.emit('unlink', joinPath(root, 'snippets/stars.liquid')) + + await vi.advanceTimersByTimeAsync(250) + + expect(addHandler).toHaveBeenCalledOnce() + expect(addHandler).toHaveBeenCalledWith(expect.objectContaining({fileKey: 'blocks/new_block.liquid'})) + expect(unlinkHandler).toHaveBeenCalledOnce() + expect(unlinkHandler).toHaveBeenCalledWith(expect.objectContaining({fileKey: 'snippets/stars.liquid'})) + }) + + test('resets debounce timer on new event for same file', async () => { + const themeFileSystem = mountThemeExtensionFileSystem(root) + await themeFileSystem.ready() + + const changeHandler = vi.fn() + themeFileSystem.addEventListener('change', changeHandler) + + await themeFileSystem.startWatcher() + + const watcher = chokidar.watch('') as unknown as EventEmitter + const filePath = joinPath(root, 'blocks/star_rating.liquid') + + watcher.emit('change', filePath) + + await vi.advanceTimersByTimeAsync(200) + expect(changeHandler).not.toHaveBeenCalled() + + watcher.emit('change', filePath) + + await vi.advanceTimersByTimeAsync(200) + expect(changeHandler).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(50) + expect(changeHandler).toHaveBeenCalledOnce() + }) + }) + function fsEntry({key, checksum}: Checksum): [string, ThemeAsset] { return [ key, diff --git a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.ts b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.ts index bb168f4984f..82cb4119942 100644 --- a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.ts +++ b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-fs.ts @@ -22,6 +22,8 @@ const THEME_EXT_DIRECTORY_PATTERNS = [ 'snippets/**/*.liquid', ] +const THEME_EXT_FILE_EVENT_DEBOUNCE_TIME_IN_MS = 250 + export function mountThemeExtensionFileSystem(root: string): ThemeExtensionFileSystem { const files = new Map() const unsyncedFileKeys = new Set() @@ -121,10 +123,33 @@ export function mountThemeExtensionFileSystem(root: string): ThemeExtensionFileS ignoreInitial: true, }) + const pendingEvents = new Map() + + const queueFsEvent = (eventName: 'add' | 'change' | 'unlink', filePath: string) => { + const fileKey = relativePath(root, filePath) + const eventKey = `${fileKey}:${eventName}` + + const pending = pendingEvents.get(eventKey) + if (pending) { + clearTimeout(pending) + } + + const timeout = setTimeout(() => { + pendingEvents.delete(eventKey) + if (eventName === 'unlink') { + handleFileDelete(filePath) + } else { + handleFileUpdate(eventName, filePath) + } + }, THEME_EXT_FILE_EVENT_DEBOUNCE_TIME_IN_MS) + + pendingEvents.set(eventKey, timeout) + } + watcher - .on('add', handleFileUpdate.bind(null, 'add')) - .on('change', handleFileUpdate.bind(null, 'change')) - .on('unlink', handleFileDelete.bind(null)) + .on('add', queueFsEvent.bind(null, 'add')) + .on('change', queueFsEvent.bind(null, 'change')) + .on('unlink', queueFsEvent.bind(null, 'unlink')) }, } }