Skip to content

Commit

Permalink
Merge pull request #4817 from Shopify/improve-error-logging-in-app-wa…
Browse files Browse the repository at this point in the history
…tcher

Use AppWatcher events for dev-console, improve error logs for `dev`
  • Loading branch information
isaacroldan authored Nov 20, 2024
2 parents c1f5bf3 + daa1bd3 commit 2450a80
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 372 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@
],
"project": "**/*.{ts,tsx}!",
"ignoreBinaries": [
"bundle",
"open"
],
"vite": {
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/commands/app/import-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export default class ImportExtensions extends AppCommand {
renderFatalError(new AbortError('Invalid migration choice'))
process.exit(1)
}

await importExtensions({
...appContext,
extensionTypes: migrationChoice.extensionTypes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,15 @@ describe('app-event-watcher when receiving a file event', () => {
path: expect.anything(),
})

expect(emitSpy).toHaveBeenCalledWith('ready', app)
const initialEvents = app.realExtensions.map((eve) => ({
type: EventType.Updated,
extension: eve,
buildResult: {status: 'ok', handle: eve.handle},
}))
expect(emitSpy).toHaveBeenCalledWith('ready', {
app,
extensionEvents: expect.arrayContaining(initialEvents),
})

if (needsAppReload) {
expect(loadApp).toHaveBeenCalledWith({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export class AppEventWatcher extends EventEmitter {
private readonly esbuildManager: ESBuildContextManager
private started = false
private ready = false
private initialEvents: ExtensionEvent[] = []

constructor(
app: AppLinkedInterface,
Expand Down Expand Up @@ -133,7 +134,8 @@ export class AppEventWatcher extends EventEmitter {
await this.esbuildManager.createContexts(this.app.realExtensions.filter((ext) => ext.isESBuildExtension))

// Initial build of all extensions
await this.buildExtensions(this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext})))
this.initialEvents = this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext}))
await this.buildExtensions(this.initialEvents)

// Start the file system watcher
await startFileWatcher(this.app, this.options, (events) => {
Expand Down Expand Up @@ -161,7 +163,7 @@ export class AppEventWatcher extends EventEmitter {
})

this.ready = true
this.emit('ready', this.app)
this.emit('ready', {app: this.app, extensionEvents: this.initialEvents})
}

/**
Expand All @@ -183,9 +185,10 @@ export class AppEventWatcher extends EventEmitter {
* @param listener - The listener function to add
* @returns The AppEventWatcher instance
*/
onStart(listener: (app: AppLinkedInterface) => Promise<void> | void) {
onStart(listener: (appEvent: AppEvent) => Promise<void> | void) {
if (this.ready) {
listener(this.app)?.catch(() => {})
const event: AppEvent = {app: this.app, extensionEvents: this.initialEvents, startTime: [0, 0], path: ''}
listener(event)?.catch(() => {})
} else {
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.once('ready', listener)
Expand Down
23 changes: 4 additions & 19 deletions packages/app/src/cli/services/dev/extension.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import * as store from './extension/payload/store.js'
import * as server from './extension/server.js'
import * as websocket from './extension/websocket.js'
import * as bundler from './extension/bundler.js'
import {devUIExtensions, ExtensionDevOptions} from './extension.js'
import {ExtensionsEndpointPayload} from './extension/payload/models.js'
import {WebsocketConnection} from './extension/websocket/models.js'
import {AppEventWatcher} from './app-events/app-event-watcher.js'
import {testAppLinked} from '../../models/app/app.test-data.js'
import {describe, test, vi, expect} from 'vitest'
import {Server} from 'http'

describe('devUIExtensions()', () => {
const serverCloseSpy = vi.fn()
const websocketCloseSpy = vi.fn()
const bundlerCloseSpy = vi.fn()
const app = testAppLinked()

const options = {
mock: 'options',
Expand All @@ -20,6 +22,7 @@ describe('devUIExtensions()', () => {
stdout: process.stdout,
stderr: process.stderr,
checkoutCartUrl: 'mock/path/from/extensions',
appWatcher: new AppEventWatcher(app, 'url', 'path'),
} as unknown as ExtensionDevOptions

function spyOnEverything() {
Expand All @@ -37,9 +40,6 @@ describe('devUIExtensions()', () => {
vi.spyOn(websocket, 'setupWebsocketConnection').mockReturnValue({
close: websocketCloseSpy,
} as unknown as WebsocketConnection)
vi.spyOn(bundler, 'setupBundlerAndFileWatcher').mockResolvedValue({
close: bundlerCloseSpy,
})
}

test('initializes the payload store', async () => {
Expand Down Expand Up @@ -85,20 +85,6 @@ describe('devUIExtensions()', () => {
})
})

test('initializes the bundler and file watcher', async () => {
// GIVEN
spyOnEverything()

// WHEN
await devUIExtensions(options)

// THEN
expect(bundler.setupBundlerAndFileWatcher).toHaveBeenCalledWith({
devOptions: options,
payloadStore: {mock: 'payload-store'},
})
})

test('closes the http server, websocket and bundler when the process aborts', async () => {
// GIVEN
spyOnEverything()
Expand All @@ -114,7 +100,6 @@ describe('devUIExtensions()', () => {

abortEventCallback()

expect(bundlerCloseSpy).toHaveBeenCalledOnce()
expect(websocketCloseSpy).toHaveBeenCalledOnce()
expect(serverCloseSpy).toHaveBeenCalledOnce()
})
Expand Down
22 changes: 18 additions & 4 deletions packages/app/src/cli/services/dev/extension.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {setupWebsocketConnection} from './extension/websocket.js'
import {setupBundlerAndFileWatcher} from './extension/bundler.js'
import {setupHTTPServer} from './extension/server.js'
import {ExtensionsPayloadStore, getExtensionsPayloadStoreRawPayload} from './extension/payload/store.js'
import {AppEvent, AppEventWatcher} from './app-events/app-event-watcher.js'
import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
import {AbortSignal} from '@shopify/cli-kit/node/abort'
import {outputDebug} from '@shopify/cli-kit/node/output'
Expand Down Expand Up @@ -101,14 +101,20 @@ export interface ExtensionDevOptions {
* This is exposed in the JSON payload for clients connecting to the Dev Server
*/
manifestVersion: string

/**
* The app watcher that emits events when the app is updated
*/
appWatcher: AppEventWatcher
}

export async function devUIExtensions(options: ExtensionDevOptions): Promise<void> {
const payloadStoreOptions = {
...options,
websocketURL: getWebSocketUrl(options.url),
}
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadStoreOptions)
const bundlePath = options.appWatcher.buildOutputPath
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadStoreOptions, bundlePath)
const payloadStore = new ExtensionsPayloadStore(payloadStoreRawPayload, payloadStoreOptions)

outputDebug(`Setting up the UI extensions HTTP server...`, options.stdout)
Expand All @@ -117,11 +123,19 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
outputDebug(`Setting up the UI extensions Websocket server...`, options.stdout)
const websocketConnection = setupWebsocketConnection({...options, httpServer, payloadStore})
outputDebug(`Setting up the UI extensions bundler and file watching...`, options.stdout)
const fileWatcher = await setupBundlerAndFileWatcher({devOptions: options, payloadStore})

const eventHandler = async ({extensionEvents}: AppEvent) => {
for (const event of extensionEvents) {
const status = event.buildResult?.status === 'ok' ? 'success' : 'error'
// eslint-disable-next-line no-await-in-loop
await payloadStore.updateExtension(event.extension, options, bundlePath, {status})
}
}

options.appWatcher.onEvent(eventHandler).onStart(eventHandler)

options.signal.addEventListener('abort', () => {
outputDebug('Closing the UI extensions dev server...')
fileWatcher.close()
websocketConnection.close()
httpServer.close()
})
Expand Down
Loading

0 comments on commit 2450a80

Please sign in to comment.