From 0b1b1a0e8b59987c944504502aa6eb238e78b3bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 5 Nov 2024 11:10:41 +0100 Subject: [PATCH 01/16] Improve error logging int app watcher --- .../dev/app-events/app-event-watcher.ts | 35 ++++++++++++------- .../dev/app-events/app-watcher-esbuild.ts | 1 + .../app/src/cli/services/extensions/bundle.ts | 6 ++++ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 004bd19edee..e3358777fb5 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -9,6 +9,8 @@ import {outputDebug} from '@shopify/cli-kit/node/output' import {AbortSignal} from '@shopify/cli-kit/node/abort' import {joinPath} from '@shopify/cli-kit/node/path' import {fileExists, mkdir, rmdir} from '@shopify/cli-kit/node/fs' +import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' +import {formatMessagesSync, Message} from 'esbuild' import EventEmitter from 'events' /** @@ -179,20 +181,29 @@ export class AppEventWatcher extends EventEmitter { */ private async buildExtensions(extensions: ExtensionInstance[]): Promise { const promises = extensions.map(async (ext) => { - try { - if (this.esbuildManager.contexts[ext.handle]) { - const result = await this.esbuildManager.contexts[ext.handle]?.rebuild() - if (result?.errors?.length) throw new Error(result?.errors.map((err) => err.text).join('\n')) - } else { - await this.buildExtension(ext) + return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { + try { + if (this.esbuildManager.contexts[ext.handle]) { + await this.esbuildManager.contexts[ext.handle]?.rebuild() + } else { + await this.buildExtension(ext) + } + return {status: 'ok', handle: ext.handle} as const + // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any + } catch (error: any) { + const errors: Message[] = error.errors ?? [] + if (errors.length) { + const formattedErrors = formatMessagesSync(errors, {kind: 'error', color: true}) + formattedErrors.forEach((error) => { + this.options.stderr.write(error) + }) + } else { + this.options.stderr.write(error.message) + } + return {status: 'error', error: error.message, handle: ext.handle} as const } - return {status: 'ok', handle: ext.handle} as const - // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any - } catch (error: any) { - return {status: 'error', error: error.message, handle: ext.handle} as const - } + }) }) - // ESBuild errors are already logged by the ESBuild bundler return Promise.all(promises) } diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts index 06d1d2d0ac1..dc2cbb98885 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts @@ -57,6 +57,7 @@ export class ESBuildContextManager { resolveDir: extension.directory, loader: 'tsx', }, + logLevel: 'silent', stderr: this.stderr ?? process.stderr, stdout: this.stdout ?? process.stdout, sourceMaps: true, diff --git a/packages/app/src/cli/services/extensions/bundle.ts b/packages/app/src/cli/services/extensions/bundle.ts index 9f1b93ab224..ba391b3ba74 100644 --- a/packages/app/src/cli/services/extensions/bundle.ts +++ b/packages/app/src/cli/services/extensions/bundle.ts @@ -49,6 +49,11 @@ interface BundleOptions { * Whether or not to generate source maps. */ sourceMaps?: boolean + + /** + * Whether or not to log messages to the console. + */ + logLevel?: 'silent' | 'error' } /** @@ -137,6 +142,7 @@ export function getESBuildOptions(options: BundleOptions, processEnv = process.e bundle: true, define, jsx: 'automatic', + logLevel: options.logLevel ?? 'error', loader: { '.esnext': 'ts', '.js': 'jsx', From 944dbeb9ef2f22030f637165569f2efac9563a29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 5 Nov 2024 12:43:20 +0100 Subject: [PATCH 02/16] Create shared app watcher for all dev processes --- .../app/src/cli/commands/app/demo/watcher.ts | 3 +- .../app-events/app-event-watcher-handler.ts | 6 ++-- .../dev/app-events/app-event-watcher.ts | 12 +++++--- .../dev/app-events/app-watcher-esbuild.ts | 19 +++++------- .../dev/processes/app-watcher-process.ts | 27 +++++++++++++++++ .../cli/services/dev/processes/dev-session.ts | 10 ++----- .../dev/processes/setup-dev-processes.ts | 29 ++++++++++++------- 7 files changed, 68 insertions(+), 38 deletions(-) create mode 100644 packages/app/src/cli/services/dev/processes/app-watcher-process.ts diff --git a/packages/app/src/cli/commands/app/demo/watcher.ts b/packages/app/src/cli/commands/app/demo/watcher.ts index 538a18df98d..5fdc9a4db4f 100644 --- a/packages/app/src/cli/commands/app/demo/watcher.ts +++ b/packages/app/src/cli/commands/app/demo/watcher.ts @@ -6,6 +6,7 @@ import colors from '@shopify/cli-kit/node/colors' import {globalFlags} from '@shopify/cli-kit/node/cli' import {outputInfo} from '@shopify/cli-kit/node/output' import {endHRTimeInMs} from '@shopify/cli-kit/node/hrtime' +import {AbortSignal} from '@shopify/cli-kit/node/abort' export default class DemoWatcher extends AppCommand { static summary = 'Watch and prints out changes to an app.' @@ -27,7 +28,7 @@ export default class DemoWatcher extends AppCommand { }) const watcher = new AppEventWatcher(app) - await watcher.start() + await watcher.start({stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()}) outputInfo(`Watching for changes in ${app.name}...`) watcher.onEvent(async ({app: _newApp, extensionEvents, startTime, path}) => { diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts index dc0c606c396..fe56d781a1f 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts @@ -127,7 +127,7 @@ async function ReloadAppHandler({event, app, options}: HandlerInput): Promise { +export async function reloadApp(app: AppLinkedInterface, options?: OutputContextOptions): Promise { const start = startHRTime() try { const newApp = await loadApp({ @@ -136,11 +136,11 @@ export async function reloadApp(app: AppLinkedInterface, options: OutputContextO userProvidedConfigName: basename(app.configuration.path), remoteFlags: app.remoteFlags, }) - outputDebug(`App reloaded [${endHRTimeInMs(start)}ms]`, options.stdout) + outputDebug(`App reloaded [${endHRTimeInMs(start)}ms]`, options?.stdout) return newApp as AppLinkedInterface // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any } catch (error: any) { - outputWarn(`Error reloading app: ${error.message}`, options.stderr) + outputWarn(`Error reloading app: ${error.message}`, options?.stderr) return app } } diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index e3358777fb5..0f172049b69 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -88,16 +88,17 @@ type ExtensionBuildResult = {status: 'ok'; handle: string} | {status: 'error'; e export class AppEventWatcher extends EventEmitter { buildOutputPath: string private app: AppLinkedInterface - private readonly options: OutputContextOptions + private options: OutputContextOptions private readonly appURL?: string private readonly esbuildManager: ESBuildContextManager - constructor(app: AppLinkedInterface, appURL?: string, options?: OutputContextOptions, buildOutputPath?: string) { + constructor(app: AppLinkedInterface, appURL?: string, buildOutputPath?: string) { super() this.app = app this.appURL = appURL this.buildOutputPath = buildOutputPath ?? joinPath(app.directory, '.shopify', 'bundle') - this.options = options ?? {stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()} + // Default options, to be overwritten by the start method + this.options = {stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()} this.esbuildManager = new ESBuildContextManager({ outputPath: this.buildOutputPath, dotEnvVariables: this.app.dotenv?.variables ?? {}, @@ -106,7 +107,10 @@ export class AppEventWatcher extends EventEmitter { }) } - async start() { + async start(options: OutputContextOptions) { + this.options = options + this.esbuildManager.setAbortSignal(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) diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts index dc2cbb98885..fec1fab6528 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts @@ -3,15 +3,11 @@ import {ExtensionInstance} from '../../../models/extensions/extension-instance.j import {getESBuildOptions} from '../../extensions/bundle.js' import {BuildContext, context as esContext} from 'esbuild' import {AbortSignal} from '@shopify/cli-kit/node/abort' -import {Writable} from 'stream' export interface DevAppWatcherOptions { dotEnvVariables: {[key: string]: string} url: string outputPath: string - stderr?: Writable - stdout?: Writable - signal?: AbortSignal } /** @@ -23,20 +19,18 @@ export class ESBuildContextManager { outputPath: string dotEnvVariables: {[key: string]: string} url: string - stderr?: Writable - stdout?: Writable signal?: AbortSignal constructor(options: DevAppWatcherOptions) { this.dotEnvVariables = options.dotEnvVariables this.url = options.url this.outputPath = options.outputPath - this.stderr = options.stderr - this.stdout = options.stdout - this.signal = options.signal this.contexts = {} + } - options.signal?.addEventListener('abort', async () => { + setAbortSignal(signal: AbortSignal) { + this.signal = signal + this.signal?.addEventListener('abort', async () => { const allDispose = Object.values(this.contexts).map((context) => context.dispose()) await Promise.all(allDispose) }) @@ -58,8 +52,9 @@ export class ESBuildContextManager { loader: 'tsx', }, logLevel: 'silent', - stderr: this.stderr ?? process.stderr, - stdout: this.stdout ?? process.stdout, + // stdout and stderr are mandatory, but not actually used + stderr: process.stderr, + stdout: process.stdout, sourceMaps: true, }) diff --git a/packages/app/src/cli/services/dev/processes/app-watcher-process.ts b/packages/app/src/cli/services/dev/processes/app-watcher-process.ts new file mode 100644 index 00000000000..d1c2badb81d --- /dev/null +++ b/packages/app/src/cli/services/dev/processes/app-watcher-process.ts @@ -0,0 +1,27 @@ +import {BaseProcess, DevProcessFunction} from './types.js' +import {AppEventWatcher} from '../app-events/app-event-watcher.js' + +interface AppWatcherProcessOptions { + appWatcher: AppEventWatcher +} + +export interface AppWatcherProcess extends BaseProcess { + type: 'app-watcher' +} + +export async function setupAppWatcherProcess(options: AppWatcherProcessOptions): Promise { + return { + type: 'app-watcher', + prefix: `extensions`, + options, + function: launchAppWatcher, + } +} + +export const launchAppWatcher: DevProcessFunction = async ( + {stdout, stderr, abortSignal}, + options: AppWatcherProcessOptions, +) => { + const {appWatcher} = options + await appWatcher.start({stdout, stderr, signal: abortSignal}) +} diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index 148c3407f8e..84591d1c061 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -3,7 +3,6 @@ import {DeveloperPlatformClient} from '../../../utilities/developer-platform-cli import {AppLinkedInterface} from '../../../models/app/app.js' import {getExtensionUploadURL} from '../../deploy/upload.js' import {AppEventWatcher, EventType} from '../app-events/app-event-watcher.js' -import {reloadApp} from '../app-events/app-event-watcher-handler.js' import {readFileSync, writeFile} from '@shopify/cli-kit/node/fs' import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {AbortSignal} from '@shopify/cli-kit/node/abort' @@ -22,6 +21,7 @@ interface DevSessionOptions { app: AppLinkedInterface organizationId: string appId: string + appWatcher: AppEventWatcher } interface DevSessionProcessOptions extends DevSessionOptions { @@ -61,17 +61,12 @@ export const pushUpdatesForDevSession: DevProcessFunction = a {stderr, stdout, abortSignal: signal}, options, ) => { - const {developerPlatformClient} = options - - // Reload the app before starting the dev session, at this point the configuration has changed (e.g. application_url) - const app = await reloadApp(options.app, {stderr, stdout, signal}) + const {developerPlatformClient, appWatcher} = options const refreshToken = async () => { return developerPlatformClient.refreshToken() } - const appWatcher = new AppEventWatcher(app, options.url, {stderr, stdout, signal}) - const processOptions = {...options, stderr, stdout, signal, bundlePath: appWatcher.buildOutputPath} outputWarn('-----> Using DEV SESSIONS <-----') @@ -115,7 +110,6 @@ export const pushUpdatesForDevSession: DevProcessFunction = a }) // Start watching for changes in the app - await appWatcher.start() processOptions.stdout.write(`Dev session ready, watching for changes in your app`) } diff --git a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts index daf59d48898..321bcd77273 100644 --- a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts +++ b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts @@ -7,6 +7,7 @@ import {GraphiQLServerProcess, setupGraphiQLServerProcess} from './graphiql.js' import {WebProcess, setupWebProcesses} from './web.js' import {DevSessionProcess, setupDevSessionProcess} from './dev-session.js' import {AppLogsSubscribeProcess, setupAppLogsPollingProcess} from './app-logs-polling.js' +import {AppWatcherProcess, setupAppWatcherProcess} from './app-watcher-process.js' import {environmentVariableNames} from '../../../constants.js' import {AppLinkedInterface, getAppScopes, WebType} from '../../../models/app/app.js' @@ -16,6 +17,8 @@ import {getProxyingWebServer} from '../../../utilities/app/http-reverse-proxy.js import {buildAppURLForWeb} from '../../../utilities/app/app-url.js' import {PartnersURLs} from '../urls.js' import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' +import {reloadApp} from '../app-events/app-event-watcher-handler.js' +import {AppEventWatcher} from '../app-events/app-event-watcher.js' import {getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' import {isTruthy} from '@shopify/cli-kit/node/context/utilities' import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment' @@ -34,6 +37,7 @@ type DevProcessDefinition = | GraphiQLServerProcess | DevSessionProcess | AppLogsSubscribeProcess + | AppWatcherProcess export type DevProcesses = DevProcessDefinition[] @@ -82,15 +86,19 @@ export async function setupDevProcesses({ const shouldRenderGraphiQL = !isTruthy(env[environmentVariableNames.disableGraphiQLExplorer]) const shouldPerformAppLogPolling = localApp.allExtensions.some((extension) => extension.isFunctionExtension) + const reloadedApp = await reloadApp(localApp) + const appWatcher = new AppEventWatcher(reloadedApp, network.proxyUrl) + const processes = [ + await setupAppWatcherProcess({appWatcher}), ...(await setupWebProcesses({ - webs: localApp.webs, + webs: reloadedApp.webs, proxyUrl: network.proxyUrl, frontendPort: network.frontendPort, backendPort: network.backendPort, apiKey, apiSecret, - scopes: getAppScopes(localApp.configuration), + scopes: getAppScopes(reloadedApp.configuration), })), shouldRenderGraphiQL ? await setupGraphiQLServerProcess({ @@ -104,31 +112,32 @@ export async function setupDevProcesses({ }) : undefined, await setupPreviewableExtensionsProcess({ - allExtensions: localApp.allExtensions, + allExtensions: reloadedApp.allExtensions, storeFqdn, storeId, apiKey, subscriptionProductUrl: commandOptions.subscriptionProductUrl, checkoutCartUrl: commandOptions.checkoutCartUrl, proxyUrl: network.proxyUrl, - appName: localApp.name, - appDotEnvFile: localApp.dotenv, + appName: reloadedApp.name, + appDotEnvFile: reloadedApp.dotenv, grantedScopes: remoteApp.grantedScopes, appId: remoteApp.id, - appDirectory: localApp.directory, + appDirectory: reloadedApp.directory, }), developerPlatformClient.supportsDevSessions ? await setupDevSessionProcess({ - app: localApp, + app: reloadedApp, apiKey, developerPlatformClient, url: network.proxyUrl, appId: remoteApp.id, organizationId: remoteApp.organizationId, storeFqdn, + appWatcher, }) : await setupDraftableExtensionsProcess({ - localApp, + localApp: reloadedApp, remoteApp, apiKey, developerPlatformClient, @@ -136,13 +145,13 @@ export async function setupDevProcesses({ }), await setupPreviewThemeAppExtensionsProcess({ remoteApp, - localApp, + localApp: reloadedApp, storeFqdn, theme: commandOptions.theme, themeExtensionPort: commandOptions.themeExtensionPort, }), setupSendUninstallWebhookProcess({ - webs: localApp.webs, + webs: reloadedApp.webs, backendPort: network.backendPort, frontendPort: network.frontendPort, developerPlatformClient, From 11d7ddfcc51bf07c322fb5b31e7fa3ec0cc014f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 5 Nov 2024 16:45:04 +0100 Subject: [PATCH 03/16] Make the extension server use the new app-watcher --- .../dev/app-events/app-event-watcher.ts | 21 ++++--- .../app/src/cli/services/dev/extension.ts | 21 +++++-- .../src/cli/services/dev/extension/bundler.ts | 61 +++++++++---------- .../services/dev/extension/localization.ts | 4 +- .../src/cli/services/dev/extension/payload.ts | 22 ++++--- .../services/dev/extension/payload/store.ts | 12 +++- .../dev/extension/server/middlewares.ts | 8 ++- .../dev/processes/previewable-extension.ts | 4 ++ .../dev/processes/setup-dev-processes.ts | 1 + 9 files changed, 91 insertions(+), 63 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 0f172049b69..d0087b35bed 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -65,6 +65,7 @@ export enum EventType { export interface ExtensionEvent { type: EventType extension: ExtensionInstance + buildResult?: ExtensionBuildResult } /** @@ -119,7 +120,7 @@ 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) + await this.buildExtensions(this.app.realExtensions.map((ext) => ({type: EventType.Created, extension: ext}))) // Start the file system watcher await startFileWatcher(this.app, this.options, (events) => { @@ -134,11 +135,12 @@ export class AppEventWatcher extends EventEmitter { await this.esbuildManager.updateContexts(appEvent) // Find affected created/updated extensions and build them - const createdOrUpdatedExtensions = appEvent.extensionEvents - .filter((extEvent) => extEvent.type !== EventType.Deleted) - .map((extEvent) => extEvent.extension) + const createdOrUpdatedExtensionsEvents = appEvent.extensionEvents.filter( + (extEvent) => extEvent.type !== EventType.Deleted, + ) - await this.buildExtensions(createdOrUpdatedExtensions) + // Build the created/updated extensions and update the extension events with the build result + await this.buildExtensions(createdOrUpdatedExtensionsEvents) // Find deleted extensions and delete their previous build output const deletedExtensions = appEvent.extensionEvents @@ -183,8 +185,9 @@ export class AppEventWatcher extends EventEmitter { * ESBuild extensions will be built using their own ESBuild context, other extensions will be built using the default * buildForBundle method. */ - private async buildExtensions(extensions: ExtensionInstance[]): Promise { - const promises = extensions.map(async (ext) => { + private async buildExtensions(extensionEvents: ExtensionEvent[]) { + const promises = extensionEvents.map(async (extEvent) => { + const ext = extEvent.extension return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { try { if (this.esbuildManager.contexts[ext.handle]) { @@ -192,7 +195,7 @@ export class AppEventWatcher extends EventEmitter { } else { await this.buildExtension(ext) } - return {status: 'ok', handle: ext.handle} as const + extEvent.buildResult = {status: 'ok', handle: ext.handle} // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any } catch (error: any) { const errors: Message[] = error.errors ?? [] @@ -204,7 +207,7 @@ export class AppEventWatcher extends EventEmitter { } else { this.options.stderr.write(error.message) } - return {status: 'error', error: error.message, handle: ext.handle} as const + extEvent.buildResult = {status: 'error', error: error.message, handle: ext.handle} } }) }) diff --git a/packages/app/src/cli/services/dev/extension.ts b/packages/app/src/cli/services/dev/extension.ts index 7b785653a72..935d117925f 100644 --- a/packages/app/src/cli/services/dev/extension.ts +++ b/packages/app/src/cli/services/dev/extension.ts @@ -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 {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' @@ -101,6 +101,11 @@ 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 { @@ -108,7 +113,8 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise { + 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}) + outputDebug(`Extension ${event.extension.handle} updated`, options.stdout) + } + }) options.signal.addEventListener('abort', () => { outputDebug('Closing the UI extensions dev server...') - fileWatcher.close() websocketConnection.close() httpServer.close() }) diff --git a/packages/app/src/cli/services/dev/extension/bundler.ts b/packages/app/src/cli/services/dev/extension/bundler.ts index 89e05619148..ea70f2eb1b0 100644 --- a/packages/app/src/cli/services/dev/extension/bundler.ts +++ b/packages/app/src/cli/services/dev/extension/bundler.ts @@ -7,7 +7,6 @@ import {reloadExtensionConfig} from '../update-extension.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {ExtensionBuildOptions} from '../../build/extension.js' import {AbortController, AbortSignal} from '@shopify/cli-kit/node/abort' -import {joinPath} from '@shopify/cli-kit/node/path' import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' import {FSWatcher} from 'chokidar' import micromatch from 'micromatch' @@ -21,7 +20,6 @@ export interface FileWatcherOptions { } export async function setupBundlerAndFileWatcher(options: FileWatcherOptions) { - const {default: chokidar} = await import('chokidar') const abortController = new AbortController() const bundlers: Promise[] = [] @@ -57,9 +55,6 @@ export async function setupBundlerAndFileWatcher(options: FileWatcherOptions) { ) try { - await options.payloadStore.updateExtension(extension, options.devOptions, { - status: error ? 'error' : 'success', - }) // eslint-disable-next-line no-catch-all/no-catch-all } catch { // ESBuild handles error output @@ -69,34 +64,34 @@ export async function setupBundlerAndFileWatcher(options: FileWatcherOptions) { }), ) - const localeWatcher = chokidar - .watch(joinPath(extension.directory, 'locales', '**.json')) - .on('change', (_event, path) => { - outputDebug(`Locale file at path ${path} changed`, options.devOptions.stdout) - options.payloadStore - .updateExtension(extension, options.devOptions) - .then((_closed) => { - outputDebug(`Notified extension ${extension.devUUID} about the locale change.`, options.devOptions.stdout) - }) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .catch((_: any) => {}) - }) - - abortController.signal.addEventListener('abort', () => { - outputDebug(`Closing locale file watching for extension with ID ${extension.devUUID}`, options.devOptions.stdout) - localeWatcher - .close() - .then(() => { - outputDebug(`Locale file watching closed for extension with ${extension.devUUID}`, options.devOptions.stdout) - }) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .catch((error: any) => { - outputDebug( - `Locale file watching failed to close for extension with ${extension.devUUID}: ${error.message}`, - options.devOptions.stderr, - ) - }) - }) + // const localeWatcher = chokidar + // .watch(joinPath(extension.directory, 'locales', '**.json')) + // .on('change', (_event, path) => { + // outputDebug(`Locale file at path ${path} changed`, options.devOptions.stdout) + // options.payloadStore + // .updateExtension(extension, options.devOptions) + // .then((_closed) => { + // outputDebug(`Notified extension ${extension.devUUID} about the locale change.`, options.devOptions.stdout) + // }) + // // eslint-disable-next-line @typescript-eslint/no-explicit-any + // .catch((_: any) => {}) + // }) + + // abortController.signal.addEventListener('abort', () => { + // outputDebug(`Closing locale file watching for extension with ID ${extension.devUUID}`, options.devOptions.stdout) + // localeWatcher + // .close() + // .then(() => { + // outputDebug(`Locale file watching closed for extension with ${extension.devUUID}`, options.devOptions.stdout) + // }) + // // eslint-disable-next-line @typescript-eslint/no-explicit-any + // .catch((error: any) => { + // outputDebug( + // `Locale file watching failed to close for extension with ${extension.devUUID}: ${error.message}`, + // options.devOptions.stderr, + // ) + // }) + // }) }) await Promise.all(bundlers) diff --git a/packages/app/src/cli/services/dev/extension/localization.ts b/packages/app/src/cli/services/dev/extension/localization.ts index bb0110f71f8..f9ead4d5e3e 100644 --- a/packages/app/src/cli/services/dev/extension/localization.ts +++ b/packages/app/src/cli/services/dev/extension/localization.ts @@ -4,7 +4,7 @@ import {ExtensionInstance} from '../../../models/extensions/extension-instance.j import {joinPath} from '@shopify/cli-kit/node/path' import {readFile, glob} from '@shopify/cli-kit/node/fs' import {ExtendableError} from '@shopify/cli-kit/node/error' -import {outputInfo, outputWarn} from '@shopify/cli-kit/node/output' +import {outputWarn} from '@shopify/cli-kit/node/output' type Locale = string @@ -57,7 +57,7 @@ export async function getLocalization( }), ) localization.lastUpdated = Date.now() - outputInfo(`Parsed locales for extension ${extension.handle} at ${extension.directory}`, options.stdout) + // outputInfo(`Parsed locales for extension ${extension.handle} at ${extension.directory}`, options.stdout) // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all } catch (error: any) { status = 'error' diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index 13344169c31..79b0c9a224a 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -15,9 +15,11 @@ export type GetUIExtensionPayloadOptions = ExtensionDevOptions & { export async function getUIExtensionPayload( extension: ExtensionInstance, + bundlePath: string, options: GetUIExtensionPayloadOptions, ): Promise { return useConcurrentOutputContext({outputPrefix: extension.outputPrefix}, async () => { + const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath) const url = `${options.url}/extensions/${extension.devUUID}` const {localization, status: localizationStatus} = await getLocalization(extension, options) @@ -27,19 +29,19 @@ export async function getUIExtensionPayload( main: { name: 'main', url: `${url}/assets/${extension.outputFileName}`, - lastUpdated: (await fileLastUpdatedTimestamp(extension.outputPath)) ?? 0, + lastUpdated: (await fileLastUpdatedTimestamp(extensionOutputPath)) ?? 0, }, }, capabilities: { - blockProgress: extension.configuration.capabilities?.block_progress || false, - networkAccess: extension.configuration.capabilities?.network_access || false, - apiAccess: extension.configuration.capabilities?.api_access || false, + blockProgress: extension.configuration.capabilities?.block_progress ?? false, + networkAccess: extension.configuration.capabilities?.network_access ?? false, + apiAccess: extension.configuration.capabilities?.api_access ?? false, collectBuyerConsent: { - smsMarketing: extension.configuration.capabilities?.collect_buyer_consent?.sms_marketing || false, - customerPrivacy: extension.configuration.capabilities?.collect_buyer_consent?.customer_privacy || false, + smsMarketing: extension.configuration.capabilities?.collect_buyer_consent?.sms_marketing ?? false, + customerPrivacy: extension.configuration.capabilities?.collect_buyer_consent?.customer_privacy ?? false, }, iframe: { - sources: extension.configuration.capabilities?.iframe?.sources || [], + sources: extension.configuration.capabilities?.iframe?.sources ?? [], }, }, development: { @@ -48,10 +50,10 @@ export async function getUIExtensionPayload( root: { url, }, - hidden: options.currentDevelopmentPayload?.hidden || false, + hidden: options.currentDevelopmentPayload?.hidden ?? false, localizationStatus, - status: options.currentDevelopmentPayload?.status || 'success', - ...(options.currentDevelopmentPayload || {status: 'success'}), + status: options.currentDevelopmentPayload?.status ?? 'success', + ...(options.currentDevelopmentPayload ?? {status: 'success'}), }, extensionPoints: getExtensionPoints(extension.configuration.extension_points, url), localization: localization ?? null, diff --git a/packages/app/src/cli/services/dev/extension/payload/store.ts b/packages/app/src/cli/services/dev/extension/payload/store.ts index f813ac7a406..4cc62aea707 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.ts @@ -17,6 +17,7 @@ export enum ExtensionsPayloadStoreEvent { export async function getExtensionsPayloadStoreRawPayload( options: ExtensionsPayloadStoreOptions, + bundlePath: string, ): Promise { return { app: { @@ -37,7 +38,11 @@ export async function getExtensionsPayloadStoreRawPayload( url: new URL('/extensions/dev-console', options.url).toString(), }, store: options.storeFqdn, - extensions: await Promise.all(options.extensions.map((extension) => getUIExtensionPayload(extension, options))), + extensions: await Promise.all( + options.extensions.map((extension) => { + return getUIExtensionPayload(extension, bundlePath, options) + }), + ), } } @@ -131,6 +136,7 @@ export class ExtensionsPayloadStore extends EventEmitter { async updateExtension( extension: ExtensionInstance, options: ExtensionDevOptions, + bundlePath: string, development?: Partial, ) { const payloadExtensions = this.rawPayload.extensions @@ -144,9 +150,9 @@ export class ExtensionsPayloadStore extends EventEmitter { return } - payloadExtensions[index] = await getUIExtensionPayload(extension, { + payloadExtensions[index] = await getUIExtensionPayload(extension, bundlePath, { ...this.options, - currentDevelopmentPayload: development || {status: payloadExtensions[index]?.development.status}, + currentDevelopmentPayload: development ?? {status: payloadExtensions[index]?.development.status}, currentLocalizationPayload: payloadExtensions[index]?.localization, }) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.ts index 6eee1d5f8f8..823510d5fbb 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.ts @@ -86,7 +86,10 @@ export function getExtensionAssetMiddleware({devOptions}: GetExtensionsMiddlewar }) } - const buildDirectory = extension.outputPath.replace(extension.outputFileName, '') + const bundlePath = devOptions.appWatcher.buildOutputPath + const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath) + + const buildDirectory = extensionOutputPath.replace(extension.outputFileName, '') return fileServerMiddleware(request, response, next, { filePath: joinPath(buildDirectory, assetPath), @@ -183,6 +186,7 @@ export function getExtensionPayloadMiddleware({devOptions}: GetExtensionsMiddlew return } } + const bundlePath = devOptions.appWatcher.buildOutputPath response.setHeader('content-type', 'application/json') response.end( @@ -201,7 +205,7 @@ export function getExtensionPayloadMiddleware({devOptions}: GetExtensionsMiddlew url: new URL('/extensions/dev-console', devOptions.url).toString(), }, store: devOptions.storeFqdn, - extension: await getUIExtensionPayload(extension, devOptions), + extension: await getUIExtensionPayload(extension, bundlePath, devOptions), }), ) } diff --git a/packages/app/src/cli/services/dev/processes/previewable-extension.ts b/packages/app/src/cli/services/dev/processes/previewable-extension.ts index d8a647b4d0d..a2cfeec5504 100644 --- a/packages/app/src/cli/services/dev/processes/previewable-extension.ts +++ b/packages/app/src/cli/services/dev/processes/previewable-extension.ts @@ -2,6 +2,7 @@ import {BaseProcess, DevProcessFunction} from './types.js' import {devUIExtensions} from '../extension.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {buildCartURLIfNeeded} from '../extension/utilities.js' +import {AppEventWatcher} from '../app-events/app-event-watcher.js' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' @@ -22,6 +23,7 @@ interface PreviewableExtensionOptions { appId?: string grantedScopes: string[] previewableExtensions: ExtensionInstance[] + appWatcher: AppEventWatcher } export interface PreviewableExtensionProcess extends BaseProcess { @@ -44,6 +46,7 @@ export const launchPreviewableExtensionProcess: DevProcessFunction { await devUIExtensions({ @@ -64,6 +67,7 @@ export const launchPreviewableExtensionProcess: DevProcessFunction Date: Fri, 8 Nov 2024 17:03:27 +0100 Subject: [PATCH 04/16] Remove setupBundlerAndFileWatcher --- .../services/dev/extension/bundler.test.ts | 210 +----------------- .../src/cli/services/dev/extension/bundler.ts | 85 ------- 2 files changed, 2 insertions(+), 293 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension/bundler.test.ts b/packages/app/src/cli/services/dev/extension/bundler.test.ts index 551e37f9905..d093af6cc36 100644 --- a/packages/app/src/cli/services/dev/extension/bundler.test.ts +++ b/packages/app/src/cli/services/dev/extension/bundler.test.ts @@ -1,16 +1,5 @@ -import { - FileWatcherOptions, - SetupExtensionWatcherOptions, - setupBundlerAndFileWatcher, - setupExtensionWatcher, -} from './bundler.js' -import * as bundle from '../../extensions/bundle.js' -import { - testUIExtension, - testFunctionExtension, - testApp, - testAppConfigExtensions, -} from '../../../models/app/app.test-data.js' +import {SetupExtensionWatcherOptions, setupExtensionWatcher} from './bundler.js' +import {testFunctionExtension, testApp, testAppConfigExtensions} from '../../../models/app/app.test-data.js' import {reloadExtensionConfig} from '../update-extension.js' import {FunctionConfigType} from '../../../models/extensions/specifications/function.js' import * as extensionBuild from '../../../services/build/extension.js' @@ -18,7 +7,6 @@ import {ExtensionInstance} from '../../../models/extensions/extension-instance.j import {BaseConfigType} from '../../../models/extensions/schemas.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import chokidar from 'chokidar' -import {BuildResult} from 'esbuild' import {AbortController, AbortSignal} from '@shopify/cli-kit/node/abort' import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' import {flushPromises} from '@shopify/cli-kit/node/promises' @@ -32,53 +20,6 @@ vi.mock('../update-extension.js') vi.mock('../../../services/build/extension.js') vi.mock('../update-extension.js') -async function testBundlerAndFileWatcher() { - const extension1 = await testUIExtension({ - devUUID: '1', - directory: 'directory/path/1', - configuration: { - handle: 'my-handle', - name: 'test-ui-extension', - type: 'product_subscription', - metafields: [], - }, - }) - - const extension2 = await testUIExtension({ - devUUID: '2', - directory: 'directory/path/2', - configuration: { - handle: 'my-handle-2', - name: 'test-ui-extension', - type: 'product_subscription', - metafields: [], - }, - }) - - const fileWatcherOptions = { - devOptions: { - extensions: [extension1, extension2], - appDotEnvFile: { - variables: { - SOME_KEY: 'SOME_VALUE', - }, - }, - url: 'mock/url', - stderr: { - mockStdErr: 'STD_ERR', - }, - stdout: { - mockStdOut: 'STD_OUT', - }, - }, - payloadStore: { - updateExtension: vi.fn(() => Promise.resolve(undefined)), - }, - } as unknown as FileWatcherOptions - await setupBundlerAndFileWatcher(fileWatcherOptions) - return fileWatcherOptions -} - function functionConfiguration(): FunctionConfigType { return { name: 'foo', @@ -90,153 +31,6 @@ function functionConfiguration(): FunctionConfigType { } } -describe('setupBundlerAndFileWatcher()', () => { - test("call 'bundleExtension' for each extension", async () => { - vi.spyOn(bundle, 'bundleExtension').mockResolvedValue(undefined) - vi.spyOn(chokidar, 'watch').mockReturnValue({ - on: vi.fn() as any, - } as any) - - await testBundlerAndFileWatcher() - - expect(bundle.bundleExtension).toHaveBeenCalledWith( - expect.objectContaining({ - minify: false, - outputPath: 'directory/path/1/dist/my-handle.js', - stdin: { - contents: "import './src/index.js';", - resolveDir: 'directory/path/1', - loader: 'tsx', - }, - environment: 'development', - env: { - SOME_KEY: 'SOME_VALUE', - APP_URL: 'mock/url', - }, - stderr: { - mockStdErr: 'STD_ERR', - }, - stdout: { - mockStdOut: 'STD_OUT', - }, - }), - ) - - expect(bundle.bundleExtension).toHaveBeenCalledWith( - expect.objectContaining({ - minify: false, - outputPath: 'directory/path/2/dist/my-handle-2.js', - stdin: { - contents: "import './src/index.js';", - resolveDir: 'directory/path/2', - loader: 'tsx', - }, - environment: 'development', - env: { - SOME_KEY: 'SOME_VALUE', - APP_URL: 'mock/url', - }, - stderr: { - mockStdErr: 'STD_ERR', - }, - stdout: { - mockStdOut: 'STD_OUT', - }, - }), - ) - }) - - test("Call 'updateExtension' with status success when no error occurs", async () => { - // GIVEN - vi.spyOn(bundle, 'bundleExtension').mockResolvedValue(undefined) - vi.spyOn(chokidar, 'watch').mockReturnValue({ - on: vi.fn() as any, - } as any) - const fileWatcherOptions = await testBundlerAndFileWatcher() - - // WHEN - const bundleExtensionFn = bundle.bundleExtension as any - bundleExtensionFn.mock.calls[0][0].watch() - - // THEN - expect(fileWatcherOptions.payloadStore.updateExtension).toHaveBeenCalledWith( - fileWatcherOptions.devOptions.extensions[0], - fileWatcherOptions.devOptions, - {status: 'success'}, - ) - }) - - test("Call 'updateExtension' with status error when an error occurs", async () => { - // GIVEN - vi.spyOn(bundle, 'bundleExtension').mockResolvedValue(undefined) - vi.spyOn(chokidar, 'watch').mockReturnValue({ - on: vi.fn() as any, - } as any) - const fileWatcherOptions = await testBundlerAndFileWatcher() - - // WHEN - const buildFailure = { - errors: ['error'] as any, - warnings: [], - outputFiles: [], - metafile: {} as any, - mangleCache: {}, - } as BuildResult - const bundleExtensionFn = bundle.bundleExtension as any - bundleExtensionFn.mock.calls[0][0].watch(buildFailure) - - // THEN - expect(fileWatcherOptions.payloadStore.updateExtension).toHaveBeenCalledWith( - fileWatcherOptions.devOptions.extensions[0], - fileWatcherOptions.devOptions, - {status: 'error'}, - ) - }) - - test('Watches the locales directory for change events', async () => { - // GIVEN - const chokidarOnSpy = vi.fn() as any - vi.spyOn(bundle, 'bundleExtension').mockResolvedValue(undefined) - vi.spyOn(chokidar, 'watch').mockReturnValue({ - on: chokidarOnSpy, - } as any) - - // WHEN - await testBundlerAndFileWatcher() - - // THEN - expect(chokidar.watch).toHaveBeenCalledWith('directory/path/1/locales/**.json') - expect(chokidar.watch).toHaveBeenCalledWith('directory/path/2/locales/**.json') - expect(chokidarOnSpy).toHaveBeenCalledTimes(2) - expect(chokidarOnSpy).toHaveBeenCalledWith('change', expect.any(Function)) - }) - - test('Updates the extension when a locale changes', async () => { - const chokidarOnSpy = vi.fn() as any - - // GIVEN - vi.spyOn(bundle, 'bundleExtension').mockResolvedValue(undefined) - vi.spyOn(chokidar, 'watch').mockReturnValue({ - on: chokidarOnSpy, - } as any) - - // WHEN - const fileWatcherOptions = await testBundlerAndFileWatcher() - chokidarOnSpy.mock.calls[0][1]() - chokidarOnSpy.mock.calls[1][1]() - - // THEN - expect(fileWatcherOptions.payloadStore.updateExtension).toHaveBeenCalledWith( - fileWatcherOptions.devOptions.extensions[0], - fileWatcherOptions.devOptions, - ) - expect(fileWatcherOptions.payloadStore.updateExtension).toHaveBeenCalledWith( - fileWatcherOptions.devOptions.extensions[1], - fileWatcherOptions.devOptions, - ) - }) -}) - describe('setupExtensionWatcher', () => { beforeEach(() => { const config = {type: 'type', name: 'name', path: 'path', metafields: []} diff --git a/packages/app/src/cli/services/dev/extension/bundler.ts b/packages/app/src/cli/services/dev/extension/bundler.ts index ea70f2eb1b0..a24d5f7b761 100644 --- a/packages/app/src/cli/services/dev/extension/bundler.ts +++ b/packages/app/src/cli/services/dev/extension/bundler.ts @@ -1,6 +1,5 @@ import {ExtensionsPayloadStore} from './payload/store.js' import {ExtensionDevOptions} from '../extension.js' -import {bundleExtension} from '../../extensions/bundle.js' import {AppInterface} from '../../../models/app/app.js' import {reloadExtensionConfig} from '../update-extension.js' @@ -19,90 +18,6 @@ export interface FileWatcherOptions { payloadStore: ExtensionsPayloadStore } -export async function setupBundlerAndFileWatcher(options: FileWatcherOptions) { - const abortController = new AbortController() - - const bundlers: Promise[] = [] - - const extensions = options.devOptions.extensions.filter((ext) => ext.isESBuildExtension) - - // eslint-disable-next-line @typescript-eslint/no-misused-promises - extensions.forEach(async (extension) => { - bundlers.push( - bundleExtension({ - minify: false, - outputPath: extension.outputPath, - environment: 'development', - env: { - ...(options.devOptions.appDotEnvFile?.variables ?? {}), - APP_URL: options.devOptions.url, - }, - stdin: { - contents: extension.getBundleExtensionStdinContent(), - resolveDir: extension.directory, - loader: 'tsx', - }, - stderr: options.devOptions.stderr, - stdout: options.devOptions.stdout, - watchSignal: abortController.signal, - watch: async (result) => { - const error = (result?.errors?.length ?? 0) > 0 - outputDebug( - `The Javascript bundle of the UI extension with ID ${extension.devUUID} has ${ - error ? 'an error' : 'changed' - }`, - error ? options.devOptions.stderr : options.devOptions.stdout, - ) - - try { - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // ESBuild handles error output - } - }, - sourceMaps: true, - }), - ) - - // const localeWatcher = chokidar - // .watch(joinPath(extension.directory, 'locales', '**.json')) - // .on('change', (_event, path) => { - // outputDebug(`Locale file at path ${path} changed`, options.devOptions.stdout) - // options.payloadStore - // .updateExtension(extension, options.devOptions) - // .then((_closed) => { - // outputDebug(`Notified extension ${extension.devUUID} about the locale change.`, options.devOptions.stdout) - // }) - // // eslint-disable-next-line @typescript-eslint/no-explicit-any - // .catch((_: any) => {}) - // }) - - // abortController.signal.addEventListener('abort', () => { - // outputDebug(`Closing locale file watching for extension with ID ${extension.devUUID}`, options.devOptions.stdout) - // localeWatcher - // .close() - // .then(() => { - // outputDebug(`Locale file watching closed for extension with ${extension.devUUID}`, options.devOptions.stdout) - // }) - // // eslint-disable-next-line @typescript-eslint/no-explicit-any - // .catch((error: any) => { - // outputDebug( - // `Locale file watching failed to close for extension with ${extension.devUUID}: ${error.message}`, - // options.devOptions.stderr, - // ) - // }) - // }) - }) - - await Promise.all(bundlers) - - return { - close: () => { - abortController.abort() - }, - } -} - export interface SetupExtensionWatcherOptions { extension: ExtensionInstance app: AppInterface From d247da4e88c94bffa2bd74fceef493a363681ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 8 Nov 2024 17:04:56 +0100 Subject: [PATCH 05/16] Make output options optional --- packages/app/src/cli/commands/app/demo/watcher.ts | 3 +-- .../src/cli/services/dev/app-events/app-event-watcher.ts | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/commands/app/demo/watcher.ts b/packages/app/src/cli/commands/app/demo/watcher.ts index 5fdc9a4db4f..538a18df98d 100644 --- a/packages/app/src/cli/commands/app/demo/watcher.ts +++ b/packages/app/src/cli/commands/app/demo/watcher.ts @@ -6,7 +6,6 @@ import colors from '@shopify/cli-kit/node/colors' import {globalFlags} from '@shopify/cli-kit/node/cli' import {outputInfo} from '@shopify/cli-kit/node/output' import {endHRTimeInMs} from '@shopify/cli-kit/node/hrtime' -import {AbortSignal} from '@shopify/cli-kit/node/abort' export default class DemoWatcher extends AppCommand { static summary = 'Watch and prints out changes to an app.' @@ -28,7 +27,7 @@ export default class DemoWatcher extends AppCommand { }) const watcher = new AppEventWatcher(app) - await watcher.start({stdout: process.stdout, stderr: process.stderr, signal: new AbortSignal()}) + await watcher.start() outputInfo(`Watching for changes in ${app.name}...`) watcher.onEvent(async ({app: _newApp, extensionEvents, startTime, path}) => { diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 8555394abec..509b9ac66bf 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -110,12 +110,12 @@ export class AppEventWatcher extends EventEmitter { }) } - async start(options: OutputContextOptions) { + async start(options?: OutputContextOptions) { if (this.started) return this.started = true - this.options = options - this.esbuildManager.setAbortSignal(options.signal) + 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}) From 6d1af71f5b4491be8aa037d3843474caec805314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 8 Nov 2024 17:06:49 +0100 Subject: [PATCH 06/16] Reload app doesnt need stdout --- .../dev/app-events/app-event-watcher-handler.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts index fe56d781a1f..00770aa1521 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts @@ -4,7 +4,7 @@ import {appDiff} from './app-diffing.js' import {AppLinkedInterface} from '../../../models/app/app.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {loadApp} from '../../../models/app/loader.js' -import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' +import {outputDebug} from '@shopify/cli-kit/node/output' import {AbortError} from '@shopify/cli-kit/node/error' import {endHRTimeInMs, startHRTime} from '@shopify/cli-kit/node/hrtime' import {basename} from '@shopify/cli-kit/node/path' @@ -113,8 +113,8 @@ function AppConfigDeletedHandler(_input: HandlerInput): AppEvent { * - When the app.toml is updated * - When an extension toml is updated */ -async function ReloadAppHandler({event, app, options}: HandlerInput): Promise { - const newApp = await reloadApp(app, options) +async function ReloadAppHandler({event, app}: HandlerInput): Promise { + const newApp = await reloadApp(app) const diff = appDiff(app, newApp, true) const createdEvents = diff.created.map((ext) => ({type: EventType.Created, extension: ext})) const deletedEvents = diff.deleted.map((ext) => ({type: EventType.Deleted, extension: ext})) @@ -127,7 +127,7 @@ async function ReloadAppHandler({event, app, options}: HandlerInput): Promise { +export async function reloadApp(app: AppLinkedInterface): Promise { const start = startHRTime() try { const newApp = await loadApp({ @@ -136,11 +136,10 @@ export async function reloadApp(app: AppLinkedInterface, options?: OutputContext userProvidedConfigName: basename(app.configuration.path), remoteFlags: app.remoteFlags, }) - outputDebug(`App reloaded [${endHRTimeInMs(start)}ms]`, options?.stdout) + outputDebug(`App reloaded [${endHRTimeInMs(start)}ms]`) return newApp as AppLinkedInterface - // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any + // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { - outputWarn(`Error reloading app: ${error.message}`, options?.stderr) - return app + throw new Error(`Error reloading app: ${error.message}`) } } From 4bf069a0883b33f3f3ed7e9bd44427398f4fbdb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 8 Nov 2024 17:22:22 +0100 Subject: [PATCH 07/16] Some cleanup --- .../src/cli/services/dev/extension/payload.ts | 18 +++++++++--------- .../services/dev/extension/payload/store.ts | 6 +----- .../dev/processes/app-watcher-process.ts | 14 +++++++++++++- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index 79b0c9a224a..d395b7508f7 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -33,15 +33,15 @@ export async function getUIExtensionPayload( }, }, capabilities: { - blockProgress: extension.configuration.capabilities?.block_progress ?? false, - networkAccess: extension.configuration.capabilities?.network_access ?? false, - apiAccess: extension.configuration.capabilities?.api_access ?? false, + blockProgress: extension.configuration.capabilities?.block_progress || false, + networkAccess: extension.configuration.capabilities?.network_access || false, + apiAccess: extension.configuration.capabilities?.api_access || false, collectBuyerConsent: { - smsMarketing: extension.configuration.capabilities?.collect_buyer_consent?.sms_marketing ?? false, - customerPrivacy: extension.configuration.capabilities?.collect_buyer_consent?.customer_privacy ?? false, + smsMarketing: extension.configuration.capabilities?.collect_buyer_consent?.sms_marketing || false, + customerPrivacy: extension.configuration.capabilities?.collect_buyer_consent?.customer_privacy || false, }, iframe: { - sources: extension.configuration.capabilities?.iframe?.sources ?? [], + sources: extension.configuration.capabilities?.iframe?.sources || [], }, }, development: { @@ -50,10 +50,10 @@ export async function getUIExtensionPayload( root: { url, }, - hidden: options.currentDevelopmentPayload?.hidden ?? false, + hidden: options.currentDevelopmentPayload?.hidden || false, localizationStatus, - status: options.currentDevelopmentPayload?.status ?? 'success', - ...(options.currentDevelopmentPayload ?? {status: 'success'}), + status: options.currentDevelopmentPayload?.status || 'success', + ...(options.currentDevelopmentPayload || {status: 'success'}), }, extensionPoints: getExtensionPoints(extension.configuration.extension_points, url), localization: localization ?? null, diff --git a/packages/app/src/cli/services/dev/extension/payload/store.ts b/packages/app/src/cli/services/dev/extension/payload/store.ts index 4cc62aea707..cdca6844272 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.ts @@ -38,11 +38,7 @@ export async function getExtensionsPayloadStoreRawPayload( url: new URL('/extensions/dev-console', options.url).toString(), }, store: options.storeFqdn, - extensions: await Promise.all( - options.extensions.map((extension) => { - return getUIExtensionPayload(extension, bundlePath, options) - }), - ), + extensions: await Promise.all(options.extensions.map((ext) => getUIExtensionPayload(ext, bundlePath, options))), } } diff --git a/packages/app/src/cli/services/dev/processes/app-watcher-process.ts b/packages/app/src/cli/services/dev/processes/app-watcher-process.ts index d1c2badb81d..518b46ac01e 100644 --- a/packages/app/src/cli/services/dev/processes/app-watcher-process.ts +++ b/packages/app/src/cli/services/dev/processes/app-watcher-process.ts @@ -9,10 +9,22 @@ export interface AppWatcherProcess extends BaseProcess type: 'app-watcher' } +/** + * Sets up the app watcher process. + * + * This process just STARTS the app watcher, the watcher object is created before and shared with all other processes + * so they can receive events from it. + * + * The "start" is done in a process so that it can receive the stdout, stderr and abortSignal of the concurrent output context + * that is shared with the other processes. + * + * @param options - The options for the app watcher process. + * @returns The app watcher process. + */ export async function setupAppWatcherProcess(options: AppWatcherProcessOptions): Promise { return { type: 'app-watcher', - prefix: `extensions`, + prefix: `dev-session`, options, function: launchAppWatcher, } From 9b0a79affb9dc51cc9ae39453e0e978a9187ab99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 11 Nov 2024 13:04:07 +0100 Subject: [PATCH 08/16] Fix tests in middleware --- .../dev/extension/server/middlewares.test.ts | 24 +++++++++---------- .../dev/processes/setup-dev-processes.ts | 1 + 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts index 9eaa096f754..97ef8e82bc2 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts @@ -16,7 +16,7 @@ import {testUIExtension} from '../../../../models/app/app.test-data.js' import {describe, expect, vi, test} from 'vitest' import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' import * as h3 from 'h3' -import {joinPath} from '@shopify/cli-kit/node/path' +import {dirname, joinPath} from '@shopify/cli-kit/node/path' vi.mock('h3', async () => { const actual: any = await vi.importActual('h3') @@ -234,23 +234,20 @@ describe('getExtensionAssetMiddleware()', () => { test('returns the file for that asset path', async () => { await inTemporaryDirectory(async (tmpDir: string) => { const response = getMockResponse() - const devUUID = '123abc' const fileName = 'test-ui-extension.js' - const outputPath = joinPath(tmpDir, devUUID, fileName) + const extension = await testUIExtension({}) + const outputPath = extension.getOutputPathForDirectory(tmpDir) const options = { devOptions: { - extensions: [ - { - devUUID, - outputPath, - outputFileName: fileName, - }, - ], + extensions: [extension], + appWatcher: { + buildOutputPath: tmpDir, + }, }, payloadStore: {}, } as unknown as GetExtensionsMiddlewareOptions - await mkdir(joinPath(tmpDir, devUUID)) + await mkdir(dirname(outputPath)) await touchFile(outputPath) await writeFile(outputPath, `content from ${fileName}`) @@ -258,7 +255,7 @@ describe('getExtensionAssetMiddleware()', () => { getMockRequest({ context: { params: { - extensionId: devUUID, + extensionId: extension.devUUID, assetPath: fileName, }, }, @@ -420,6 +417,9 @@ describe('getExtensionPayloadMiddleware()', () => { }), ], manifestVersion: '3', + appWatcher: { + buildOutputPath: 'mock-build-output-path', + }, }, payloadStore: {}, } as unknown as GetExtensionsMiddlewareOptions diff --git a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts index b9dc562a110..2e4d19720a8 100644 --- a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts +++ b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts @@ -86,6 +86,7 @@ export async function setupDevProcesses({ const shouldRenderGraphiQL = !isTruthy(env[environmentVariableNames.disableGraphiQLExplorer]) const shouldPerformAppLogPolling = localApp.allExtensions.some((extension) => extension.isFunctionExtension) + // At this point, the toml file has changed, we need to reload the app before actually starting dev const reloadedApp = await reloadApp(localApp) const appWatcher = new AppEventWatcher(reloadedApp, network.proxyUrl) From 9f5a8e4c91c57e6033a574e470cc24a5f922c91b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 11 Nov 2024 13:48:25 +0100 Subject: [PATCH 09/16] Fix more tests --- .../cli/services/dev/extension/payload.test.ts | 10 +++++----- .../src/cli/services/dev/extension/payload.ts | 2 +- .../dev/extension/payload/store.test.ts | 18 +++++++++--------- .../services/dev/extension/payload/store.ts | 4 ++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension/payload.test.ts b/packages/app/src/cli/services/dev/extension/payload.test.ts index a748362ef8a..285759d9b21 100644 --- a/packages/app/src/cli/services/dev/extension/payload.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload.test.ts @@ -45,7 +45,7 @@ describe('getUIExtensionPayload', () => { devUUID: 'devUUID', }) - const options: ExtensionDevOptions = { + const options: Omit = { signal, stdout, stderr, @@ -69,7 +69,7 @@ describe('getUIExtensionPayload', () => { } // When - const got = await getUIExtensionPayload(uiExtension, { + const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { ...options, currentDevelopmentPayload: development, }) @@ -128,7 +128,7 @@ describe('getUIExtensionPayload', () => { const development: Partial = {} // When - const got = await getUIExtensionPayload(uiExtension, { + const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { ...options, currentDevelopmentPayload: development, }) @@ -195,7 +195,7 @@ describe('getUIExtensionPayload', () => { const development: Partial = {} // When - const got = await getUIExtensionPayload(uiExtension, { + const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { ...options, currentDevelopmentPayload: development, url: 'http://tunnel-url.com', @@ -276,7 +276,7 @@ describe('getUIExtensionPayload', () => { const development: Partial = {} // When - const got = await getUIExtensionPayload(uiExtension, { + const got = await getUIExtensionPayload(uiExtension, 'mock-bundle-path', { ...options, currentDevelopmentPayload: development, url: 'http://tunnel-url.com', diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index d395b7508f7..5cbd10f9155 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -8,7 +8,7 @@ import {ExtensionInstance} from '../../../models/extensions/extension-instance.j import {fileLastUpdatedTimestamp} from '@shopify/cli-kit/node/fs' import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' -export type GetUIExtensionPayloadOptions = ExtensionDevOptions & { +export type GetUIExtensionPayloadOptions = Omit & { currentDevelopmentPayload?: Partial currentLocalizationPayload?: UIExtensionPayload['localization'] } diff --git a/packages/app/src/cli/services/dev/extension/payload/store.test.ts b/packages/app/src/cli/services/dev/extension/payload/store.test.ts index 8ac6e56a393..cd9a5229da8 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.test.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.test.ts @@ -27,7 +27,7 @@ describe('getExtensionsPayloadStoreRawPayload()', () => { } as unknown as ExtensionsPayloadStoreOptions // When - const rawPayload = await getExtensionsPayloadStoreRawPayload(options) + const rawPayload = await getExtensionsPayloadStoreRawPayload(options, 'mock-bundle-path') // Then expect(rawPayload).toMatchObject({ @@ -252,10 +252,10 @@ describe('ExtensionsPayloadStore()', () => { const updatedExtension = {devUUID: '123', updated: 'extension'} as unknown as ExtensionInstance // When - await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, {hidden: true}) + await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, 'mock-bundle-path', {hidden: true}) // Then - expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, { + expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, 'mock-bundle-path', { ...mockOptions, currentDevelopmentPayload: {hidden: true}, }) @@ -279,10 +279,10 @@ describe('ExtensionsPayloadStore()', () => { const updatedExtension = {devUUID: '123', updated: 'extension'} as unknown as ExtensionInstance // When - await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions) + await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, 'mock-bundle-path') // Then - expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, { + expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, 'mock-bundle-path', { ...mockOptions, currentDevelopmentPayload: { status: 'success', @@ -310,10 +310,10 @@ describe('ExtensionsPayloadStore()', () => { const updatedExtension = {devUUID: '123', updated: 'extension'} as unknown as ExtensionInstance // When - await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions) + await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, 'mock-bundle-path') // Then - expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, { + expect(payload.getUIExtensionPayload).toHaveBeenCalledWith(updatedExtension, 'mock-bundle-path', { ...mockOptions, currentDevelopmentPayload: { status: 'success', @@ -338,7 +338,7 @@ describe('ExtensionsPayloadStore()', () => { extensionsPayloadStore.on(ExtensionsPayloadStoreEvent.Update, onUpdateSpy) // When - await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions) + await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, 'mock-bundle-path') // Then expect(onUpdateSpy).toHaveBeenCalledWith(['123']) @@ -358,7 +358,7 @@ describe('ExtensionsPayloadStore()', () => { extensionsPayloadStore.on(ExtensionsPayloadStoreEvent.Update, onUpdateSpy) // When - await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions) + await extensionsPayloadStore.updateExtension(updatedExtension, mockOptions, 'mock-bundle-path') // Then expect(initialRawPayload).toStrictEqual(extensionsPayloadStore.getRawPayload()) diff --git a/packages/app/src/cli/services/dev/extension/payload/store.ts b/packages/app/src/cli/services/dev/extension/payload/store.ts index cdca6844272..7969839c91c 100644 --- a/packages/app/src/cli/services/dev/extension/payload/store.ts +++ b/packages/app/src/cli/services/dev/extension/payload/store.ts @@ -16,7 +16,7 @@ export enum ExtensionsPayloadStoreEvent { } export async function getExtensionsPayloadStoreRawPayload( - options: ExtensionsPayloadStoreOptions, + options: Omit, bundlePath: string, ): Promise { return { @@ -131,7 +131,7 @@ export class ExtensionsPayloadStore extends EventEmitter { async updateExtension( extension: ExtensionInstance, - options: ExtensionDevOptions, + options: Omit, bundlePath: string, development?: Partial, ) { From 727f1283b86c268c32fc1026d16e8163afa5ad5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 11 Nov 2024 17:48:58 +0100 Subject: [PATCH 10/16] Remove duplicated app watcher process --- .../app/src/cli/services/dev/processes/setup-dev-processes.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts index 0ed8859f02b..5cf563395e1 100644 --- a/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts +++ b/packages/app/src/cli/services/dev/processes/setup-dev-processes.ts @@ -91,7 +91,6 @@ export async function setupDevProcesses({ const appWatcher = new AppEventWatcher(reloadedApp, network.proxyUrl) const processes = [ - await setupAppWatcherProcess({appWatcher}), ...(await setupWebProcesses({ webs: reloadedApp.webs, proxyUrl: network.proxyUrl, From 8b046d0444f7b9987c73d320a72ad6ffbe60db9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 14 Nov 2024 16:07:37 +0100 Subject: [PATCH 11/16] Report initial events in app watcher on start --- .../dev/app-events/app-event-watcher.ts | 14 +++++++---- .../app/src/cli/services/dev/extension.ts | 24 ++++++++++++------- .../cli/services/dev/processes/dev-session.ts | 4 ++++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 6461d400279..5e80bdcb40c 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -94,6 +94,7 @@ export class AppEventWatcher extends EventEmitter { private readonly esbuildManager: ESBuildContextManager private started = false private ready = false + private initialEvents: ExtensionEvent[] = [] constructor( app: AppLinkedInterface, @@ -132,7 +133,10 @@ 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) + // const anyError = initialEvents.some((extEvent) => extEvent.buildResult?.status === 'error') + // if (anyError) return // Start the file system watcher await startFileWatcher(this.app, this.options, (events) => { @@ -160,7 +164,7 @@ export class AppEventWatcher extends EventEmitter { }) this.ready = true - this.emit('ready', this.app) + this.emit('ready', this.app, this.initialEvents) } /** @@ -182,9 +186,9 @@ export class AppEventWatcher extends EventEmitter { * @param listener - The listener function to add * @returns The AppEventWatcher instance */ - onStart(listener: (app: AppLinkedInterface) => Promise | void) { + onStart(listener: (app: AppLinkedInterface, initialEvents: ExtensionEvent[]) => Promise | void) { if (this.ready) { - listener(this.app)?.catch(() => {}) + listener(this.app, this.initialEvents)?.catch(() => {}) } else { // eslint-disable-next-line @typescript-eslint/no-misused-promises this.once('ready', listener) @@ -219,7 +223,7 @@ export class AppEventWatcher extends EventEmitter { return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { try { if (this.esbuildManager.contexts[ext.handle]) { - await this.esbuildManager.contexts[ext.handle]?.rebuild() + const result = await this.esbuildManager.contexts[ext.handle]?.rebuild() } else { await this.buildExtension(ext) } diff --git a/packages/app/src/cli/services/dev/extension.ts b/packages/app/src/cli/services/dev/extension.ts index 935d117925f..5501d35fc49 100644 --- a/packages/app/src/cli/services/dev/extension.ts +++ b/packages/app/src/cli/services/dev/extension.ts @@ -124,14 +124,22 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise { - 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}) - outputDebug(`Extension ${event.extension.handle} updated`, options.stdout) - } - }) + options.appWatcher + .onEvent(async ({extensionEvents}) => { + 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}) + outputDebug(`Extension ${event.extension.handle} updated`, options.stdout) + } + }) + .onStart(async (_, initialEvents) => { + for (const event of initialEvents) { + 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.signal.addEventListener('abort', () => { outputDebug('Closing the UI extensions dev server...') diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index d8b1e1f0902..06536abcdb6 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -89,6 +89,10 @@ export const pushUpdatesForDevSession: DevProcessFunction = a return } + // If there are any errors build errors, don't update the dev session + const anyError = event.extensionEvents.some((eve) => eve.buildResult?.status === 'error') + if (anyError) return + // Cancel any ongoing bundle and upload process bundleControllers.forEach((controller) => controller.abort()) // Remove aborted controllers from array: From 8177fe93a1f0a27f3eb82380a8d6d798e118d8dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 18 Nov 2024 15:20:33 +0100 Subject: [PATCH 12/16] Handle initial state in extension server --- .../dev/app-events/app-event-watcher.ts | 7 ++--- .../app/src/cli/services/dev/extension.ts | 27 +++++++------------ .../cli/services/dev/processes/dev-session.ts | 4 +-- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 6d8ec536ff5..99c10f6f5be 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -165,7 +165,7 @@ export class AppEventWatcher extends EventEmitter { }) this.ready = true - this.emit('ready', this.app, this.initialEvents) + this.emit('ready', {app: this.app, extensionEvents: this.initialEvents}) } /** @@ -187,9 +187,10 @@ export class AppEventWatcher extends EventEmitter { * @param listener - The listener function to add * @returns The AppEventWatcher instance */ - onStart(listener: (app: AppLinkedInterface, initialEvents: ExtensionEvent[]) => Promise | void) { + onStart(listener: (appEvent: AppEvent) => Promise | void) { if (this.ready) { - listener(this.app, this.initialEvents)?.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) diff --git a/packages/app/src/cli/services/dev/extension.ts b/packages/app/src/cli/services/dev/extension.ts index 5501d35fc49..957eccee106 100644 --- a/packages/app/src/cli/services/dev/extension.ts +++ b/packages/app/src/cli/services/dev/extension.ts @@ -1,7 +1,7 @@ import {setupWebsocketConnection} from './extension/websocket.js' import {setupHTTPServer} from './extension/server.js' import {ExtensionsPayloadStore, getExtensionsPayloadStoreRawPayload} from './extension/payload/store.js' -import {AppEventWatcher} from './app-events/app-event-watcher.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' @@ -124,22 +124,15 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise { - 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}) - outputDebug(`Extension ${event.extension.handle} updated`, options.stdout) - } - }) - .onStart(async (_, initialEvents) => { - for (const event of initialEvents) { - const status = event.buildResult?.status === 'ok' ? 'success' : 'error' - // eslint-disable-next-line no-await-in-loop - await payloadStore.updateExtension(event.extension, options, bundlePath, {status}) - } - }) + 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...') diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index 06536abcdb6..0e2c70d65a1 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -115,9 +115,9 @@ export const pushUpdatesForDevSession: DevProcessFunction = a outputDebug(`✅ Event handled [Network: ${endNetworkTime}ms -- Total: ${endTime}ms]`, processOptions.stdout) }, refreshToken) }) - .onStart(async (app) => { + .onStart(async (event) => { await performActionWithRetryAfterRecovery(async () => { - const result = await bundleExtensionsAndUpload({...processOptions, app}) + const result = await bundleExtensionsAndUpload({...processOptions, app: event.app}) await handleDevSessionResult(result, processOptions) }, refreshToken) }) From e868a459cbeb4dda2d5129adad880e7e42d64735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 18 Nov 2024 16:24:16 +0100 Subject: [PATCH 13/16] Remove outdated tests --- .../app/src/cli/services/dev/extension.test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension.test.ts b/packages/app/src/cli/services/dev/extension.test.ts index 1972d7ca55a..461749bbb9f 100644 --- a/packages/app/src/cli/services/dev/extension.test.ts +++ b/packages/app/src/cli/services/dev/extension.test.ts @@ -1,7 +1,6 @@ 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' @@ -37,9 +36,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 () => { @@ -85,20 +81,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() From cbc79e2e2d5f5e6076c4f54f5847433cd8d484fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 19 Nov 2024 11:56:38 +0100 Subject: [PATCH 14/16] Some cleanup --- packages/app/src/cli/commands/app/import-extensions.ts | 1 + .../app/src/cli/services/dev/app-events/app-event-watcher.ts | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/app/src/cli/commands/app/import-extensions.ts b/packages/app/src/cli/commands/app/import-extensions.ts index 100e470b364..e2b05631cf1 100644 --- a/packages/app/src/cli/commands/app/import-extensions.ts +++ b/packages/app/src/cli/commands/app/import-extensions.ts @@ -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, diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 99c10f6f5be..d2f4c16ac5c 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -136,8 +136,6 @@ export class AppEventWatcher extends EventEmitter { // Initial build of all extensions this.initialEvents = this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext})) await this.buildExtensions(this.initialEvents) - // const anyError = initialEvents.some((extEvent) => extEvent.buildResult?.status === 'error') - // if (anyError) return // Start the file system watcher await startFileWatcher(this.app, this.options, (events) => { @@ -225,7 +223,7 @@ export class AppEventWatcher extends EventEmitter { return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { try { if (this.esbuildManager.contexts[ext.handle]) { - const result = await this.esbuildManager.contexts[ext.handle]?.rebuild() + await this.esbuildManager.contexts[ext.handle]?.rebuild() } else { await this.buildExtension(ext) } From 4104a35949d92c982bc7d0750a45e3a96cfd822c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 19 Nov 2024 18:17:02 +0100 Subject: [PATCH 15/16] Remove unused interface --- packages/app/src/cli/services/dev/extension/bundler.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/app/src/cli/services/dev/extension/bundler.ts b/packages/app/src/cli/services/dev/extension/bundler.ts index a24d5f7b761..e5ecdbb1062 100644 --- a/packages/app/src/cli/services/dev/extension/bundler.ts +++ b/packages/app/src/cli/services/dev/extension/bundler.ts @@ -1,6 +1,3 @@ -import {ExtensionsPayloadStore} from './payload/store.js' -import {ExtensionDevOptions} from '../extension.js' - import {AppInterface} from '../../../models/app/app.js' import {reloadExtensionConfig} from '../update-extension.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' @@ -13,11 +10,6 @@ import {deepCompare} from '@shopify/cli-kit/common/object' import {Writable} from 'stream' import {AsyncResource} from 'async_hooks' -export interface FileWatcherOptions { - devOptions: ExtensionDevOptions - payloadStore: ExtensionsPayloadStore -} - export interface SetupExtensionWatcherOptions { extension: ExtensionInstance app: AppInterface From e53a7e3ed26104d25a7efff28e560c3db6c0c5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 19 Nov 2024 18:26:44 +0100 Subject: [PATCH 16/16] Solve knip issues --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 61abbda6f59..a2bf1f12feb 100644 --- a/package.json +++ b/package.json @@ -149,7 +149,8 @@ "ignoreBinaries": [ "packages/features/snapshots/regenerate.sh", "bin/*", - "istanbul-merge" + "istanbul-merge", + "gh" ], "ignoreDependencies": [ "@nx/workspace", @@ -261,7 +262,6 @@ ], "project": "**/*.{ts,tsx}!", "ignoreBinaries": [ - "bundle", "open" ], "vite": {