Skip to content
Merged
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 @@ -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}`)
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/cli/services/build/client-steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
15 changes: 12 additions & 3 deletions packages/app/src/cli/services/build/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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({
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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})

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
}

Expand All @@ -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}))
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) => {
Expand Down Expand Up @@ -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> {
Expand Down
Loading
Loading