Merge pull request #4809 from Shopify/consistent-dev-ux-improvements

UX Improvements for `app dev` in app management

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

@@ -9,6 +9,7 @@ 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 EventEmitter from 'events'
 
 /**
@@ -55,9 +56,9 @@ Examples:
  * - Created: The extension was created
  */
 export enum EventType {
-  Updated,
-  Deleted,
-  Created,
+  Updated = 'changed',
+  Deleted = 'deleted',
+  Created = 'created',
 }
 
 export interface ExtensionEvent {
@@ -200,18 +201,20 @@ export class AppEventWatcher extends EventEmitter {
    */
   private async buildExtensions(extensions: ExtensionInstance[]): Promise<ExtensionBuildResult[]> {
     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]) {
+            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 {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
         }
-        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)

packages/app/src/cli/services/dev/extension/localization.test.ts

@@ -5,7 +5,7 @@ import * as output from '@shopify/cli-kit/node/output'
 import {describe, expect, vi, test} from 'vitest'
 import {mkdir, writeFile, inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
-import {outputInfo} from '@shopify/cli-kit/node/output'
+import {outputDebug} from '@shopify/cli-kit/node/output'
 
 async function testGetLocalization(tmpDir: string, currentLocalization?: Localization) {
   const mockOptions = {} as unknown as ExtensionDevOptions
@@ -142,16 +142,16 @@ describe('when there are locale files', () => {
   })
   test('outputs message when there are no JSON errors', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
-      vi.spyOn(output, 'outputInfo')
+      vi.spyOn(output, 'outputDebug')
 
       await mkdir(joinPath(tmpDir, 'locales'))
       await writeFile(joinPath(tmpDir, 'locales', 'en.json'), '{"greeting": "Hi!"}')
       await writeFile(joinPath(tmpDir, 'locales', 'fr.json'), '{"greeting": "Bonjour!"}')
 
       await testGetLocalization(tmpDir)
 
-      expect(outputInfo).toHaveBeenCalledWith(expect.stringContaining('mock-name'), undefined)
-      expect(outputInfo).toHaveBeenCalledWith(expect.stringContaining(tmpDir), undefined)
+      expect(outputDebug).toHaveBeenLastCalledWith(expect.stringContaining('mock-name'), undefined)
+      expect(outputDebug).toHaveBeenLastCalledWith(expect.stringContaining(tmpDir), undefined)
     })
   })
 

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 {outputDebug, 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)
+    outputDebug(`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'

packages/app/src/cli/services/dev/processes/dev-session.ts

@@ -2,7 +2,7 @@ import {BaseProcess, DevProcessFunction} from './types.js'
 import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js'
 import {AppLinkedInterface} from '../../../models/app/app.js'
 import {getExtensionUploadURL} from '../../deploy/upload.js'
-import {AppEventWatcher, EventType} from '../app-events/app-event-watcher.js'
+import {AppEvent, AppEventWatcher} 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'
@@ -37,8 +37,17 @@ export interface DevSessionProcess extends BaseProcess<DevSessionOptions> {
   type: 'dev-session'
 }
 
+interface DevSessionResult {
+  status: 'updated' | 'created' | 'aborted' | 'error'
+  error?: string
+}
+
 let bundleControllers: AbortController[] = []
 
+// Current status of the dev session
+// Since the watcher can emit events before the dev session is ready, we need to keep track of the status
+let devSessionStatus: 'idle' | 'initializing' | 'ready' = 'idle'
+
 export async function setupDevSessionProcess({
   app,
   apiKey,
@@ -47,7 +56,7 @@ export async function setupDevSessionProcess({
 }: Omit<DevSessionOptions, 'extensions'>): Promise<DevSessionProcess | undefined> {
   return {
     type: 'dev-session',
-    prefix: 'extensions',
+    prefix: 'dev-session',
     function: pushUpdatesForDevSession,
     options: {
       app,
@@ -75,7 +84,7 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
 
   const processOptions = {...options, stderr, stdout, signal, bundlePath: appWatcher.buildOutputPath, app}
 
-  await printWarning('[BETA] Starting Dev Session', processOptions.stdout)
+  await printLogMessage('Preparing dev session', processOptions.stdout)
 
   appWatcher
     .onEvent(async (event) => {
@@ -84,45 +93,60 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
       // Remove aborted controllers from array:
       bundleControllers = bundleControllers.filter((controller) => !controller.signal.aborted)
 
-      event.extensionEvents.map((eve) => {
-        switch (eve.type) {
-          case EventType.Created:
-            processOptions.stdout.write(`✅ Extension created ->> ${eve.extension.handle}`)
-            break
-          case EventType.Deleted:
-            processOptions.stdout.write(`❌ Extension deleted ->> ${eve.extension.handle}`)
-            break
-          case EventType.Updated:
-            processOptions.stdout.write(`🔄 Extension Updated ->> ${eve.extension.handle}`)
-            break
-        }
+      // For each extension event, print a message to the terminal
+      // eslint-disable-next-line @typescript-eslint/no-misused-promises
+      event.extensionEvents.forEach(async (eve) => {
+        const outputPrefix = eve.extension.isAppConfigExtension ? 'app-config' : eve.extension.handle
+        const message = `${eve.extension.isAppConfigExtension ? 'App config' : 'Extension'} ${eve.type}`
+        await useConcurrentOutputContext({outputPrefix, stripAnsi: false}, () => processOptions.stdout.write(message))
       })
 
       const networkStartTime = startHRTime()
       await performActionWithRetryAfterRecovery(async () => {
-        const result = await bundleExtensionsAndUpload({...processOptions, app: event.app}, true)
+        const result = await bundleExtensionsAndUpload({...processOptions, app: event.app})
+        await handleDevSessionResult(result, processOptions, event)
         const endTime = endHRTimeInMs(event.startTime)
         const endNetworkTime = endHRTimeInMs(networkStartTime)
-        if (result) {
-          processOptions.stdout.write(`✅ Session updated [Network: ${endNetworkTime}ms -- Total: ${endTime}ms]`)
-        } else {
-          processOptions.stdout.write(
-            `❌ Session update aborted (new change detected) [Network: ${endNetworkTime}ms -- Total: ${endTime}ms]`,
-          )
-        }
+        outputDebug(`✅ Event handled [Network: ${endNetworkTime}ms -- Total: ${endTime}ms]`, processOptions.stdout)
       }, refreshToken)
     })
     .onStart(async () => {
       await performActionWithRetryAfterRecovery(async () => {
-        await bundleExtensionsAndUpload(processOptions, false)
-        await printWarning('[BETA] Dev session ready, watching for changes in your app', processOptions.stdout)
+        const result = await bundleExtensionsAndUpload(processOptions)
+        await handleDevSessionResult(result, processOptions)
       }, refreshToken)
     })
 
   // Start watching for changes in the app
   await appWatcher.start()
 }
 
+async function handleDevSessionResult(
+  result: DevSessionResult,
+  processOptions: DevSessionProcessOptions,
+  event?: AppEvent,
+) {
+  if (result.status === 'updated') {
+    await printSuccess(`✅ Updated`, processOptions.stdout)
+    const scopeChanges = event?.extensionEvents.find((eve) => eve.extension.handle === 'app-access')
+    if (scopeChanges) {
+      await printWarning(`🔄 Action required`, processOptions.stdout)
+      const message = outputContent`${outputToken.yellow(`└  Scopes updated`)}. ${outputToken.link(
+        'Open app to accept scopes.',
+        'https://shopify.dev/docs/apps/build/app-scopes/scopes-overview',
+      )}`
+      await printWarning(message.value, processOptions.stdout)
+    }
+  } else if (result.status === 'created') {
+    await printSuccess(`✅ Ready, watching for changes in your app `, processOptions.stdout)
+  } else if (result.status === 'aborted') {
+    outputDebug('❌ Session update aborted (new change detected)', processOptions.stdout)
+  } else {
+    await printError(`❌ Error`, processOptions.stderr)
+    await printError(`└  ${result.error}`, processOptions.stderr)
+  }
+}
+
 /**
  * Bundle all extensions and upload them to the developer platform
  * Generate a new manifest in the bundle folder, zip it and upload it to GCS.
@@ -131,13 +155,18 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
  * @param options - The options for the process
  * @param updating - Whether the dev session is being updated or created
  */
-async function bundleExtensionsAndUpload(options: DevSessionProcessOptions, updating: boolean) {
+async function bundleExtensionsAndUpload(options: DevSessionProcessOptions): Promise<DevSessionResult> {
+  // If the dev session is still initializing, ignore this event
+  if (devSessionStatus === 'initializing') return {status: 'aborted'}
+  // If the dev session is idle, set the status to initializing
+  if (devSessionStatus === 'idle') devSessionStatus = 'initializing'
+
   // Every new bundle process gets its own controller. This way we can cancel any previous one if a new change
   // is detected even when multiple events are triggered very quickly (which causes weird edge cases)
   const currentBundleController = new AbortController()
   bundleControllers.push(currentBundleController)
 
-  if (currentBundleController.signal.aborted) return false
+  if (currentBundleController.signal.aborted) return {status: 'aborted'}
   outputDebug('Bundling and uploading extensions', options.stdout)
   const bundleZipPath = joinPath(dirname(options.bundlePath), `bundle.zip`)
 
@@ -147,22 +176,22 @@ async function bundleExtensionsAndUpload(options: DevSessionProcessOptions, upda
   await writeFile(manifestPath, JSON.stringify(appManifest, null, 2))
 
   // Create zip file with everything
-  if (currentBundleController.signal.aborted) return false
+  if (currentBundleController.signal.aborted) return {status: 'aborted'}
   await zip({
     inputDirectory: options.bundlePath,
     outputZipPath: bundleZipPath,
   })
 
   // Get a signed URL to upload the zip file
-  if (currentBundleController.signal.aborted) return false
+  if (currentBundleController.signal.aborted) return {status: 'aborted'}
   const signedURL = await getExtensionUploadURL(options.developerPlatformClient, {
     apiKey: options.appId,
     organizationId: options.organizationId,
     id: options.appId,
   })
 
   // Upload the zip file
-  if (currentBundleController.signal.aborted) return false
+  if (currentBundleController.signal.aborted) return {status: 'aborted'}
   const form = formData()
   const buffer = readFileSync(bundleZipPath)
   form.append('my_upload', buffer)
@@ -175,24 +204,26 @@ async function bundleExtensionsAndUpload(options: DevSessionProcessOptions, upda
   const payload = {shopFqdn: options.storeFqdn, appId: options.appId, assetsUrl: signedURL}
 
   // Create or update the dev session
-  if (currentBundleController.signal.aborted) return false
+  if (currentBundleController.signal.aborted) return {status: 'aborted'}
   try {
-    if (updating) {
+    if (devSessionStatus === 'ready') {
       await options.developerPlatformClient.devSessionUpdate(payload)
+      return {status: 'updated'}
     } else {
       await options.developerPlatformClient.devSessionCreate(payload)
+      // eslint-disable-next-line require-atomic-updates
+      devSessionStatus = 'ready'
+      return {status: 'created'}
     }
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
   } catch (error: any) {
     if (error.statusCode === 401) {
       // Re-throw the error so the recovery procedure can be executed
       throw new Error('Unauthorized')
     } else {
-      options.stderr.write(`❌ ${updating ? 'Update' : 'Create'} Dev Session Error`)
-      await printError(`${error.message}`, options.stderr)
+      return {status: 'error', error: error.message}
     }
   }
-  return true
 }
 
 async function printWarning(message: string, stdout: Writable) {
@@ -203,8 +234,13 @@ async function printError(message: string, stdout: Writable) {
   await printLogMessage(outputContent`${outputToken.errorText(message)}`.value, stdout)
 }
 
+async function printSuccess(message: string, stdout: Writable) {
+  await printLogMessage(outputContent`${outputToken.green(message)}`.value, stdout)
+}
+
+// Helper function to print to terminal using output context with stripAnsi disabled.
 async function printLogMessage(message: string, stdout: Writable) {
-  await useConcurrentOutputContext({outputPrefix: 'extensions', stripAnsi: false}, () => {
+  await useConcurrentOutputContext({outputPrefix: 'dev-session', stripAnsi: false}, () => {
     stdout.write(message)
   })
 }

packages/app/src/cli/services/dev/processes/setup-dev-processes.test.ts

@@ -324,7 +324,7 @@ describe('setup-dev-processes', () => {
 
     expect(res.processes[2]).toMatchObject({
       type: 'dev-session',
-      prefix: 'extensions',
+      prefix: 'dev-session',
       function: pushUpdatesForDevSession,
       options: {
         app: localApp,

packages/app/src/cli/services/dev/ui/components/Dev.tsx

@@ -214,6 +214,7 @@ const Dev: FunctionComponent<DevProps> = ({
         prefixColumnSize={calculatePrefixColumnSize(errorHandledProcesses, app.extensions)}
         abortSignal={abortController.signal}
         keepRunningAfterProcessesResolve={true}
+        useAlternativeColorPalette={app.developerPlatformClient.supportsDevSessions}
       />
       {/* eslint-disable-next-line no-negated-condition */}
       {!isAborted ? (

packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts

@@ -112,7 +112,7 @@ import {
 } from '@shopify/cli-kit/node/api/business-platform'
 import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version'
 import {versionSatisfies} from '@shopify/cli-kit/node/node-package-manager'
-import {outputWarn} from '@shopify/cli-kit/node/output'
+import {outputDebug} from '@shopify/cli-kit/node/output'
 import {developerDashboardFqdn} from '@shopify/cli-kit/node/context/fqdn'
 
 const TEMPLATE_JSON_URL = 'https://cdn.shopify.com/static/cli/extensions/templates.json'
@@ -734,7 +734,7 @@ export class AppManagementClient implements DeveloperPlatformClient {
   }
 
   async sendSampleWebhook(_input: SendSampleWebhookVariables): Promise<SendSampleWebhookSchema> {
-    outputWarn('⚠️ sendSampleWebhook is not implemented')
+    outputDebug('⚠️ sendSampleWebhook is not implemented')
     return {
       sendSampleWebhook: {
         samplePayload: '',
@@ -746,7 +746,7 @@ export class AppManagementClient implements DeveloperPlatformClient {
   }
 
   async apiVersions(): Promise<PublicApiVersionsSchema> {
-    outputWarn('⚠️ apiVersions is not implemented')
+    outputDebug('⚠️ apiVersions is not implemented')
     return {publicApiVersions: ['unstable']}
   }
 
@@ -763,7 +763,7 @@ export class AppManagementClient implements DeveloperPlatformClient {
   }
 
   async updateURLs(_input: UpdateURLsVariables): Promise<UpdateURLsSchema> {
-    outputWarn('⚠️ updateURLs is not implemented')
+    outputDebug('⚠️ updateURLs is not implemented')
     return {appUpdate: {userErrors: []}}
   }