Skip to content

Commit 514b959

Browse files
isaacroldanClaude Code
andcommitted
refactor: remove ESBuildContextManager from app-event-watcher
Co-authored-by: Claude Code <claude-code@anthropic.com>
1 parent abc5566 commit 514b959

File tree

4 files changed

+18
-447
lines changed

4 files changed

+18
-447
lines changed

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

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import {AppEvent, AppEventWatcher, EventType, ExtensionEvent} from './app-event-watcher.js'
1+
import {AppEventWatcher, EventType, ExtensionEvent} from './app-event-watcher.js'
22
import {OutputContextOptions, WatcherEvent, FileWatcher} from './file-watcher.js'
3-
import {ESBuildContextManager} from './app-watcher-esbuild.js'
43
import {
54
testAppAccessConfigExtension,
65
testAppConfigExtensions,
@@ -22,7 +21,6 @@ import {joinPath} from '@shopify/cli-kit/node/path'
2221
import {Writable} from 'stream'
2322

2423
vi.mock('../../../models/app/loader.js')
25-
vi.mock('./app-watcher-esbuild.js')
2624

2725
// Extensions 1 and 1B simulate extensions defined in the same directory (same toml)
2826
const extension1 = await testUIExtension({
@@ -360,9 +358,8 @@ describe('app-event-watcher', () => {
360358
configuration: testAppConfiguration,
361359
})
362360

363-
const mockManager = new MockESBuildContextManager()
364361
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
365-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
362+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
366363
const emitSpy = vi.spyOn(watcher, 'emit')
367364
await watcher.start({stdout, stderr, signal: abortController.signal})
368365

@@ -433,9 +430,8 @@ describe('app-event-watcher', () => {
433430
})
434431
const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes')
435432

436-
const mockManager = new MockESBuildContextManager()
437433
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
438-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
434+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
439435
const emitSpy = vi.spyOn(watcher, 'emit')
440436

441437
// When
@@ -468,9 +464,8 @@ describe('app-event-watcher', () => {
468464
configuration: testAppConfiguration,
469465
})
470466

471-
const mockManager = new MockESBuildContextManager()
472467
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
473-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
468+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
474469
const emitSpy = vi.spyOn(watcher, 'emit')
475470

476471
// When
@@ -503,9 +498,8 @@ describe('app-event-watcher', () => {
503498
configuration: testAppConfiguration,
504499
})
505500

506-
const mockManager = new MockESBuildContextManager()
507501
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
508-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
502+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
509503
const emitSpy = vi.spyOn(watcher, 'emit')
510504

511505
// When
@@ -535,9 +529,8 @@ describe('app-event-watcher', () => {
535529
})
536530
const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes')
537531

538-
const mockManager = new MockESBuildContextManager()
539532
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
540-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
533+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
541534
const emitSpy = vi.spyOn(watcher, 'emit')
542535

543536
// When
@@ -571,19 +564,17 @@ describe('app-event-watcher', () => {
571564
],
572565
}
573566

574-
const mockManager = new MockESBuildContextManager()
575-
mockManager.rebuildContext = vi.fn().mockRejectedValueOnce(esbuildError)
576-
577567
const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle')
578568
const app = testAppLinked({
579569
allExtensions: [extension1],
580570
configPath: 'shopify.app.custom.toml',
581571
configuration: testAppConfiguration,
582572
})
573+
extension1.buildForBundle = vi.fn().mockRejectedValueOnce(esbuildError)
583574
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
584575

585576
// When
586-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
577+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
587578
const emitSpy = vi.spyOn(watcher, 'emit')
588579
const stderr = {write: vi.fn()} as unknown as Writable
589580
const stdout = {write: vi.fn()} as unknown as Writable
@@ -627,9 +618,9 @@ describe('app-event-watcher', () => {
627618
})
628619

629620
// When
630-
const mockManager = new MockESBuildContextManager()
621+
631622
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
632-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
623+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
633624
const emitSpy = vi.spyOn(watcher, 'emit')
634625
const stderr = {write: vi.fn()} as unknown as Writable
635626
const stdout = {write: vi.fn()} as unknown as Writable
@@ -661,13 +652,14 @@ describe('app-event-watcher', () => {
661652
configPath: 'shopify.app.custom.toml',
662653
configuration: testAppConfiguration,
663654
})
655+
656+
// Make rescanImports throw to simulate an uncaught error in the watcher pipeline
657+
extension1.rescanImports = vi.fn().mockRejectedValueOnce(uncaughtError)
658+
664659
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
665660

666661
// When
667-
const mockManager = new MockESBuildContextManager()
668-
mockManager.updateContexts = vi.fn().mockRejectedValueOnce(uncaughtError)
669-
670-
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
662+
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
671663
const emitSpy = vi.spyOn(watcher, 'emit')
672664
const stderr = {write: vi.fn()} as unknown as Writable
673665
const stdout = {write: vi.fn()} as unknown as Writable
@@ -684,26 +676,6 @@ describe('app-event-watcher', () => {
684676
})
685677
})
686678
})
687-
// Mock class for ESBuildContextManager
688-
// It handles the ESBuild contexts for the extensions that are being watched
689-
class MockESBuildContextManager extends ESBuildContextManager {
690-
contexts = {
691-
// The keys are the extension handles, the values are the ESBuild contexts mocked
692-
uid1: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
693-
uid1B: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
694-
uid2: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
695-
'test-ui-extension': [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
696-
}
697-
698-
constructor() {
699-
super({dotEnvVariables: {}, url: 'url', outputPath: 'outputPath'})
700-
}
701-
702-
async createContexts(extensions: ExtensionInstance[]) {}
703-
async updateContexts(appEvent: AppEvent) {}
704-
async deleteContexts(extensions: ExtensionInstance[]) {}
705-
}
706-
707679
// Mock class for FileWatcher
708680
// Used to trigger mocked file system events immediately after the watcher is started.
709681
class MockFileWatcher extends FileWatcher {

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

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
/* eslint-disable tsdoc/syntax */
22
import {FileWatcher, OutputContextOptions} from './file-watcher.js'
3-
import {ESBuildContextManager} from './app-watcher-esbuild.js'
43
import {handleWatcherEvents} from './app-event-watcher-handler.js'
54
import {AppLinkedInterface} from '../../../models/app/app.js'
65
import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
76
import {ExtensionBuildOptions} from '../../build/extension.js'
8-
import {formatBundleSize} from '../../build/bundle-size.js'
97
import {outputDebug} from '@shopify/cli-kit/node/output'
108
import {AbortSignal} from '@shopify/cli-kit/node/abort'
119
import {joinPath} from '@shopify/cli-kit/node/path'
@@ -95,7 +93,6 @@ export class AppEventWatcher extends EventEmitter {
9593
private app: AppLinkedInterface
9694
private options: OutputContextOptions
9795
private readonly appURL?: string
98-
private readonly esbuildManager: ESBuildContextManager
9996
private started = false
10097
private ready = false
10198
private initialEvents: ExtensionEvent[] = []
@@ -105,7 +102,6 @@ export class AppEventWatcher extends EventEmitter {
105102
app: AppLinkedInterface,
106103
appURL?: string,
107104
buildOutputPath?: string,
108-
esbuildManager?: ESBuildContextManager,
109105
fileWatcher?: FileWatcher,
110106
) {
111107
super()
@@ -114,14 +110,6 @@ export class AppEventWatcher extends EventEmitter {
114110
this.buildOutputPath = buildOutputPath ?? joinPath(app.directory, '.shopify', 'dev-bundle')
115111
// Default options, to be overwritten by the start method
116112
this.options = {stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()}
117-
this.esbuildManager =
118-
esbuildManager ??
119-
new ESBuildContextManager({
120-
outputPath: this.buildOutputPath,
121-
dotEnvVariables: this.app.dotenv?.variables ?? {},
122-
url: this.appURL ?? '',
123-
...this.options,
124-
})
125113
this.fileWatcher = fileWatcher
126114
}
127115

@@ -130,15 +118,11 @@ export class AppEventWatcher extends EventEmitter {
130118
this.started = true
131119

132120
this.options = options ?? this.options
133-
this.esbuildManager.setAbortSignal(this.options.signal)
134121

135122
// If there is a previous build folder, delete it
136123
if (await fileExists(this.buildOutputPath)) await rmdir(this.buildOutputPath, {force: true})
137124
await mkdir(this.buildOutputPath)
138125

139-
// Start the esbuild bundler for extensions that require it
140-
await this.esbuildManager.createContexts(this.app.realExtensions.filter((ext) => ext.isESBuildExtension))
141-
142126
// Initial build of all extensions
143127
if (buildExtensionsFirst) {
144128
this.initialEvents = this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext}))
@@ -154,8 +138,6 @@ export class AppEventWatcher extends EventEmitter {
154138

155139
this.app = appEvent.app
156140
if (appEvent.appWasReloaded) this.fileWatcher?.updateApp(this.app)
157-
await this.esbuildManager.updateContexts(appEvent)
158-
159141
await this.rescanImports(appEvent)
160142

161143
// Find affected created/updated extensions and build them
@@ -237,9 +219,7 @@ export class AppEventWatcher extends EventEmitter {
237219
}
238220

239221
/**
240-
* Builds all given extensions.
241-
* ESBuild extensions will be built using their own ESBuild context, other extensions will be built using the default
242-
* buildForBundle method.
222+
* Builds all given extensions using the default buildForBundle method.
243223
*/
244224
private async buildExtensions(extensionEvents: ExtensionEvent[]) {
245225
// Group events by extension to handle multiple events for the same extension
@@ -258,13 +238,7 @@ export class AppEventWatcher extends EventEmitter {
258238
const promises = groupedExtensionEvents.map(async ({extension: ext, events}) => {
259239
return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => {
260240
try {
261-
if (this.esbuildManager.contexts?.[ext.uid]?.length) {
262-
await this.esbuildManager.rebuildContext(ext)
263-
const sizeInfo = await formatBundleSize(ext.outputPath)
264-
this.options.stdout.write(`Build successful${sizeInfo}`)
265-
} else {
266-
await this.buildExtension(ext)
267-
}
241+
await this.buildExtension(ext)
268242
// Update all events for this extension with success result
269243
const buildResult = {status: 'ok' as const, uid: ext.uid}
270244
events.forEach((event) => {
@@ -309,7 +283,7 @@ export class AppEventWatcher extends EventEmitter {
309283
}
310284

311285
/**
312-
* Build a single non-esbuild extension using the default buildForBundle method.
286+
* Build a single extension using the default buildForBundle method.
313287
* @param extension - The extension to build
314288
*/
315289
private async buildExtension(extension: ExtensionInstance): Promise<void> {

0 commit comments

Comments
 (0)