Merge pull request #7252 from Shopify/04-13-refactor_remove_esbuildcontextmanager_from_app-event-watcher

refactor: remove ESBuildContextManager from app-event-watcher

Author: isaacroldan

packages/app/src/cli/models/extensions/extension-instance.ts

@@ -324,10 +324,6 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
       // eslint-disable-next-line no-await-in-loop
       const result = await executeStep(step, context)
       context.stepResults.set(step.id, result)
-
-      if (!result.success && !step.continueOnError) {
-        throw new Error(`Build step "${step.name}" failed: ${result.error?.message}`)
-      }
     }
   }
 

packages/app/src/cli/services/build/client-steps.ts

@@ -109,6 +109,7 @@ export async function executeStep(step: LifecycleStep, context: BuildContext): P
       }
     }
 
-    throw new Error(`Build step "${step.name}" failed: ${stepError.message}`)
+    stepError.message = `Build step "${step.name}" failed: ${stepError.message}`
+    throw stepError
   }
 }

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

@@ -68,6 +68,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex
 
   const {main, assets} = extension.getBundleExtensionStdinContent()
 
+  const startTime = performance.now()
   try {
     await bundleExtension({
       minify: true,
@@ -102,17 +103,25 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex
         }),
       )
     }
-  } catch (extensionBundlingError) {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  } catch (extensionBundlingError: any) {
     // this fails if the app's own source code is broken; wrap such that this isn't flagged as a CLI bug
-    throw new AbortError(
+    // Preserve esbuild errors array so the dev watcher can format actionable error messages
+    const errorMessage = (extensionBundlingError as Error).message ?? 'Unknown error occurred'
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const newError: any = new AbortError(
       `Failed to bundle extension ${extension.localIdentifier}. Please check the extension source code for errors.`,
+      errorMessage,
     )
+    newError.errors = extensionBundlingError.errors
+    throw newError
   }
 
   await extension.buildValidation()
 
+  const duration = Math.round(performance.now() - startTime)
   const sizeInfo = await formatBundleSize(extension.outputPath)
-  options.stdout.write(`${extension.localIdentifier} successfully built${sizeInfo}`)
+  options.stdout.write(`${extension.localIdentifier} successfully built in ${duration}ms${sizeInfo}`)
 }
 
 type BuildFunctionExtensionOptions = ExtensionBuildOptions

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

@@ -1,6 +1,5 @@
-import {AppEvent, AppEventWatcher, EventType, ExtensionEvent} from './app-event-watcher.js'
+import {AppEventWatcher, EventType, ExtensionEvent} from './app-event-watcher.js'
 import {OutputContextOptions, WatcherEvent, FileWatcher} from './file-watcher.js'
-import {ESBuildContextManager} from './app-watcher-esbuild.js'
 import {
   testAppAccessConfigExtension,
   testAppConfigExtensions,
@@ -22,7 +21,6 @@ import {joinPath} from '@shopify/cli-kit/node/path'
 import {Writable} from 'stream'
 
 vi.mock('../../../models/app/loader.js')
-vi.mock('./app-watcher-esbuild.js')
 
 // Extensions 1 and 1B simulate extensions defined in the same directory (same toml)
 const extension1 = await testUIExtension({
@@ -336,6 +334,24 @@ describe('app-event-watcher', () => {
     stdout = {write: vi.fn()}
     stderr = {write: vi.fn()}
     abortController = new AbortController()
+
+    // Mock buildForBundle on all test extensions so the watcher doesn't attempt real builds
+    const allExtensions = [
+      extension1,
+      extension1B,
+      extension2,
+      extension1Updated,
+      extension1BUpdated,
+      flowExtension,
+      posExtension,
+      posExtensionUpdated,
+      appAccessExtension,
+      webhookExtension,
+    ]
+    for (const ext of allExtensions) {
+      vi.spyOn(ext, 'buildForBundle').mockResolvedValue()
+      vi.spyOn(ext, 'rescanImports').mockResolvedValue(false)
+    }
   })
 
   afterEach(() => {
@@ -360,9 +376,8 @@ describe('app-event-watcher', () => {
             configuration: testAppConfiguration,
           })
 
-          const mockManager = new MockESBuildContextManager()
           const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-          const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+          const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
           const emitSpy = vi.spyOn(watcher, 'emit')
           await watcher.start({stdout, stderr, signal: abortController.signal})
 
@@ -433,9 +448,8 @@ describe('app-event-watcher', () => {
         })
         const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes')
 
-        const mockManager = new MockESBuildContextManager()
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
 
         // When
@@ -468,9 +482,8 @@ describe('app-event-watcher', () => {
           configuration: testAppConfiguration,
         })
 
-        const mockManager = new MockESBuildContextManager()
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
 
         // When
@@ -503,9 +516,8 @@ describe('app-event-watcher', () => {
           configuration: testAppConfiguration,
         })
 
-        const mockManager = new MockESBuildContextManager()
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
 
         // When
@@ -535,9 +547,8 @@ describe('app-event-watcher', () => {
         })
         const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes')
 
-        const mockManager = new MockESBuildContextManager()
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
 
         // When
@@ -571,19 +582,18 @@ describe('app-event-watcher', () => {
           ],
         }
 
-        const mockManager = new MockESBuildContextManager()
-        mockManager.rebuildContext = vi.fn().mockRejectedValueOnce(esbuildError)
-
         const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle')
         const app = testAppLinked({
           allExtensions: [extension1],
           configPath: 'shopify.app.custom.toml',
           configuration: testAppConfiguration,
         })
+        // First call succeeds (initial build on start), second call fails (file watcher triggered build)
+        vi.spyOn(extension1, 'buildForBundle').mockResolvedValueOnce().mockRejectedValueOnce(esbuildError)
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
 
         // When
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
         const stderr = {write: vi.fn()} as unknown as Writable
         const stdout = {write: vi.fn()} as unknown as Writable
@@ -627,9 +637,9 @@ describe('app-event-watcher', () => {
         })
 
         // When
-        const mockManager = new MockESBuildContextManager()
+
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
         const stderr = {write: vi.fn()} as unknown as Writable
         const stdout = {write: vi.fn()} as unknown as Writable
@@ -661,13 +671,14 @@ describe('app-event-watcher', () => {
           configPath: 'shopify.app.custom.toml',
           configuration: testAppConfiguration,
         })
+
+        // Make rescanImports throw to simulate an uncaught error in the watcher pipeline
+        vi.spyOn(extension1, 'rescanImports').mockRejectedValueOnce(uncaughtError)
+
         const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
 
         // When
-        const mockManager = new MockESBuildContextManager()
-        mockManager.updateContexts = vi.fn().mockRejectedValueOnce(uncaughtError)
-
-        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher)
         const emitSpy = vi.spyOn(watcher, 'emit')
         const stderr = {write: vi.fn()} as unknown as Writable
         const stdout = {write: vi.fn()} as unknown as Writable
@@ -684,26 +695,6 @@ describe('app-event-watcher', () => {
     })
   })
 })
-// Mock class for ESBuildContextManager
-// It handles the ESBuild contexts for the extensions that are being watched
-class MockESBuildContextManager extends ESBuildContextManager {
-  contexts = {
-    // The keys are the extension handles, the values are the ESBuild contexts mocked
-    uid1: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
-    uid1B: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
-    uid2: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
-    'test-ui-extension': [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}],
-  }
-
-  constructor() {
-    super({dotEnvVariables: {}, url: 'url', outputPath: 'outputPath'})
-  }
-
-  async createContexts(extensions: ExtensionInstance[]) {}
-  async updateContexts(appEvent: AppEvent) {}
-  async deleteContexts(extensions: ExtensionInstance[]) {}
-}
-
 // Mock class for FileWatcher
 // Used to trigger mocked file system events immediately after the watcher is started.
 class MockFileWatcher extends FileWatcher {

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

@@ -1,11 +1,9 @@
 /* eslint-disable tsdoc/syntax */
 import {FileWatcher, OutputContextOptions} from './file-watcher.js'
-import {ESBuildContextManager} from './app-watcher-esbuild.js'
 import {handleWatcherEvents} from './app-event-watcher-handler.js'
 import {AppLinkedInterface} from '../../../models/app/app.js'
 import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
 import {ExtensionBuildOptions} from '../../build/extension.js'
-import {formatBundleSize} from '../../build/bundle-size.js'
 import {outputDebug} from '@shopify/cli-kit/node/output'
 import {AbortSignal} from '@shopify/cli-kit/node/abort'
 import {joinPath} from '@shopify/cli-kit/node/path'
@@ -95,33 +93,18 @@ export class AppEventWatcher extends EventEmitter {
   private app: AppLinkedInterface
   private options: OutputContextOptions
   private readonly appURL?: string
-  private readonly esbuildManager: ESBuildContextManager
   private started = false
   private ready = false
   private initialEvents: ExtensionEvent[] = []
   private fileWatcher?: FileWatcher
 
-  constructor(
-    app: AppLinkedInterface,
-    appURL?: string,
-    buildOutputPath?: string,
-    esbuildManager?: ESBuildContextManager,
-    fileWatcher?: FileWatcher,
-  ) {
+  constructor(app: AppLinkedInterface, appURL?: string, buildOutputPath?: string, fileWatcher?: FileWatcher) {
     super()
     this.app = app
     this.appURL = appURL
     this.buildOutputPath = buildOutputPath ?? joinPath(app.directory, '.shopify', 'dev-bundle')
     // Default options, to be overwritten by the start method
     this.options = {stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()}
-    this.esbuildManager =
-      esbuildManager ??
-      new ESBuildContextManager({
-        outputPath: this.buildOutputPath,
-        dotEnvVariables: this.app.dotenv?.variables ?? {},
-        url: this.appURL ?? '',
-        ...this.options,
-      })
     this.fileWatcher = fileWatcher
   }
 
@@ -130,15 +113,11 @@ export class AppEventWatcher extends EventEmitter {
     this.started = true
 
     this.options = options ?? this.options
-    this.esbuildManager.setAbortSignal(this.options.signal)
 
     // If there is a previous build folder, delete it
     if (await fileExists(this.buildOutputPath)) await rmdir(this.buildOutputPath, {force: true})
     await mkdir(this.buildOutputPath)
 
-    // Start the esbuild bundler for extensions that require it
-    await this.esbuildManager.createContexts(this.app.realExtensions.filter((ext) => ext.isESBuildExtension))
-
     // Initial build of all extensions
     if (buildExtensionsFirst) {
       this.initialEvents = this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext}))
@@ -154,8 +133,6 @@ export class AppEventWatcher extends EventEmitter {
 
           this.app = appEvent.app
           if (appEvent.appWasReloaded) this.fileWatcher?.updateApp(this.app)
-          await this.esbuildManager.updateContexts(appEvent)
-
           await this.rescanImports(appEvent)
 
           // Find affected created/updated extensions and build them
@@ -237,9 +214,7 @@ export class AppEventWatcher extends EventEmitter {
   }
 
   /**
-   * Builds all given extensions.
-   * ESBuild extensions will be built using their own ESBuild context, other extensions will be built using the default
-   * buildForBundle method.
+   * Builds all given extensions using the default buildForBundle method.
    */
   private async buildExtensions(extensionEvents: ExtensionEvent[]) {
     // Group events by extension to handle multiple events for the same extension
@@ -258,13 +233,7 @@ export class AppEventWatcher extends EventEmitter {
     const promises = groupedExtensionEvents.map(async ({extension: ext, events}) => {
       return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => {
         try {
-          if (this.esbuildManager.contexts?.[ext.uid]?.length) {
-            await this.esbuildManager.rebuildContext(ext)
-            const sizeInfo = await formatBundleSize(ext.outputPath)
-            this.options.stdout.write(`Build successful${sizeInfo}`)
-          } else {
-            await this.buildExtension(ext)
-          }
+          await this.buildExtension(ext)
           // Update all events for this extension with success result
           const buildResult = {status: 'ok' as const, uid: ext.uid}
           events.forEach((event) => {
@@ -309,7 +278,7 @@ export class AppEventWatcher extends EventEmitter {
   }
 
   /**
-   * Build a single non-esbuild extension using the default buildForBundle method.
+   * Build a single extension using the default buildForBundle method.
    * @param extension - The extension to build
    */
   private async buildExtension(extension: ExtensionInstance): Promise<void> {