restore theme dev analytics

.changeset/fix-theme-dev-analytics.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix `theme dev` analytics being dropped on Ctrl+C. The command called `process.exit()` synchronously inside the keypress handler, which skipped the oclif post-run hook where analytics normally fire. The fix emits the event inline before exit.

packages/theme/src/cli/commands/theme/dev.ts

@@ -166,6 +166,7 @@ You can run this command only in a directory that matches the [default Shopify t
 
     await dev({
       adminSession,
+      commandConfig: this.config,
       directory: flags.path,
       store: flags.store,
       password: flags.password,

packages/theme/src/cli/services/dev.test.ts

@@ -1,9 +1,17 @@
-import {openURLSafely, renderLinks, createKeypressHandler} from './dev.js'
-import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest'
+import {dev, openURLSafely, renderLinks, createKeypressHandler, reportDevAnalytics} from './dev.js'
+import {setupDevServer} from '../utilities/theme-environment/theme-environment.js'
+import {hasRequiredThemeDirectories} from '../utilities/theme-fs.js'
+import {isStorefrontPasswordProtected} from '../utilities/theme-environment/storefront-session.js'
+import {initializeDevServerSession} from '../utilities/theme-environment/dev-server-session.js'
+import {describe, expect, test, vi, beforeEach, afterEach, type MockInstance} from 'vitest'
 import {buildTheme} from '@shopify/cli-kit/node/themes/factories'
 import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
 import {renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui'
 import {openURL} from '@shopify/cli-kit/node/system'
+import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics'
+import {addPublicMetadata, addSensitiveMetadata} from '@shopify/cli-kit/node/metadata'
+import {getAvailableTCPPort, checkPortAvailability} from '@shopify/cli-kit/node/tcp'
+import {Config} from '@oclif/core'
 
 vi.mock('@shopify/cli-kit/node/ui')
 vi.mock('@shopify/cli-kit/node/colors', () => ({
@@ -16,6 +24,33 @@ vi.mock('@shopify/cli-kit/node/colors', () => ({
 vi.mock('@shopify/cli-kit/node/system', () => ({
   openURL: vi.fn(),
 }))
+vi.mock('@shopify/cli-kit/node/analytics', () => ({
+  reportAnalyticsEvent: vi.fn(),
+}))
+vi.mock('@shopify/cli-kit/node/metadata', () => ({
+  addPublicMetadata: vi.fn(),
+  addSensitiveMetadata: vi.fn(),
+}))
+vi.mock('@shopify/cli-kit/node/tcp', () => ({
+  getAvailableTCPPort: vi.fn(),
+  checkPortAvailability: vi.fn(),
+}))
+vi.mock('../utilities/theme-environment/theme-environment.js', () => ({
+  setupDevServer: vi.fn(),
+}))
+vi.mock('../utilities/theme-fs.js', () => ({
+  hasRequiredThemeDirectories: vi.fn(),
+  mountThemeFileSystem: vi.fn().mockReturnValue({}),
+}))
+vi.mock('../utilities/theme-fs-empty.js', () => ({
+  emptyThemeExtFileSystem: vi.fn().mockReturnValue({}),
+}))
+vi.mock('../utilities/theme-environment/storefront-session.js', () => ({
+  isStorefrontPasswordProtected: vi.fn(),
+}))
+vi.mock('../utilities/theme-environment/dev-server-session.js', () => ({
+  initializeDevServerSession: vi.fn(),
+}))
 
 const store = 'my-store.myshopify.com'
 const theme = buildTheme({id: 123, name: 'My Theme', role: DEVELOPMENT_THEME_ROLE})!
@@ -124,7 +159,7 @@ describe('createKeypressHandler', () => {
 
   test('opens localhost when "t" is pressed', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('t', {name: 't'})
@@ -135,7 +170,7 @@ describe('createKeypressHandler', () => {
 
   test('opens theme preview when "p" is pressed', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('p', {name: 'p'})
@@ -146,7 +181,7 @@ describe('createKeypressHandler', () => {
 
   test('opens theme editor when "e" is pressed', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('e', {name: 'e'})
@@ -157,7 +192,7 @@ describe('createKeypressHandler', () => {
 
   test('opens gift card preview when "g" is pressed', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('g', {name: 'g'})
@@ -169,7 +204,7 @@ describe('createKeypressHandler', () => {
   test('appends preview path to theme editor URL when lastRequestedPath is not "/"', () => {
     // Given
     const ctxWithPath = {lastRequestedPath: '/products/test-product'}
-    const handler = createKeypressHandler(urls, ctxWithPath)
+    const handler = createKeypressHandler(urls, ctxWithPath, vi.fn())
 
     // When
     handler('e', {name: 'e'})
@@ -182,7 +217,7 @@ describe('createKeypressHandler', () => {
 
   test('debounces rapid keypresses - only opens URL once during debounce window', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('t', {name: 't'})
@@ -197,7 +232,7 @@ describe('createKeypressHandler', () => {
 
   test('allows keypresses after debounce period expires', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('t', {name: 't'})
@@ -220,7 +255,7 @@ describe('createKeypressHandler', () => {
 
   test('debounces different keys during the same debounce window', () => {
     // Given
-    const handler = createKeypressHandler(urls, ctx)
+    const handler = createKeypressHandler(urls, ctx, vi.fn())
 
     // When
     handler('t', {name: 't'})
@@ -232,4 +267,90 @@ describe('createKeypressHandler', () => {
     expect(openURL).toHaveBeenCalledTimes(1)
     expect(openURL).toHaveBeenCalledWith(urls.local)
   })
+
+})
+
+describe('dev() Ctrl-C analytics', () => {
+  const mockConfig = {} as unknown as Config
+  const adminSession = {storeFqdn: 'test.myshopify.com', token: 'x'}
+
+  let exitSpy: MockInstance
+  let resolveBackgroundJob: () => void
+
+  const baseOptions = {
+    adminSession,
+    commandConfig: mockConfig,
+    directory: '/tmp/theme',
+    store: 'test.myshopify.com',
+    open: false,
+    theme,
+    force: false,
+    'theme-editor-sync': false,
+    'live-reload': 'hot-reload' as const,
+    'error-overlay': 'default' as const,
+    noDelete: false,
+    ignore: [],
+    only: [],
+  }
+
+  beforeEach(() => {
+    vi.mocked(hasRequiredThemeDirectories).mockResolvedValue(true)
+    vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false)
+    vi.mocked(initializeDevServerSession).mockResolvedValue({
+      storeFqdn: adminSession.storeFqdn,
+      token: adminSession.token,
+    } as any)
+    vi.mocked(getAvailableTCPPort).mockResolvedValue(9292)
+    vi.mocked(checkPortAvailability).mockResolvedValue(true)
+
+    const backgroundJobPromise = new Promise<void>((resolve) => {
+      resolveBackgroundJob = resolve
+    })
+    vi.mocked(setupDevServer).mockReturnValue({
+      serverStart: vi.fn().mockResolvedValue(undefined),
+      renderDevSetupProgress: vi.fn().mockResolvedValue(undefined),
+      backgroundJobPromise,
+      resolveBackgroundJob: resolveBackgroundJob!,
+      dispatchEvent: vi.fn(),
+    } as any)
+
+    exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never)
+  })
+
+  afterEach(() => {
+    vi.clearAllMocks()
+    exitSpy.mockRestore()
+  })
+
+  test('Ctrl-C path: reports analytics once, even if reportDevAnalytics is invoked again', async () => {
+    const devPromise = dev(baseOptions)
+
+    // Flush microtasks so the Promise.all is awaiting backgroundJobPromise.
+    await new Promise((resolve) => setImmediate(resolve))
+
+    // Simulate Ctrl-C by resolving the background job.
+    resolveBackgroundJob()
+    await devPromise
+
+    expect(reportAnalyticsEvent).toHaveBeenCalledTimes(1)
+    expect(reportAnalyticsEvent).toHaveBeenCalledWith({config: mockConfig, exitMode: 'ok'})
+
+    expect(addPublicMetadata).toHaveBeenCalledTimes(1)
+    const publicMetadataFn = vi.mocked(addPublicMetadata).mock.calls[0]![0] as () => Record<string, unknown>
+    expect(publicMetadataFn()).toEqual({store_fqdn_hash: expect.any(String)})
+
+    expect(addSensitiveMetadata).toHaveBeenCalledTimes(1)
+    const sensitiveMetadataFn = vi.mocked(addSensitiveMetadata).mock.calls[0]![0] as () => Record<string, unknown>
+    expect(sensitiveMetadataFn()).toEqual({store_fqdn: adminSession.storeFqdn})
+
+    expect(exitSpy).toHaveBeenCalledWith(0)
+
+    const reportOrder = vi.mocked(reportAnalyticsEvent).mock.invocationCallOrder[0]!
+    const exitOrder = exitSpy.mock.invocationCallOrder[0]!
+    expect(reportOrder).toBeLessThan(exitOrder)
+
+    await reportDevAnalytics(mockConfig, adminSession as any)
+
+    expect(reportAnalyticsEvent).toHaveBeenCalledTimes(1)
+  })
 })

packages/theme/src/cli/services/dev.ts

@@ -14,15 +14,22 @@ import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {openURL} from '@shopify/cli-kit/node/system'
 import {debounce} from '@shopify/cli-kit/common/function'
+import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics'
+import {addPublicMetadata, addSensitiveMetadata} from '@shopify/cli-kit/node/metadata'
+import {hashString} from '@shopify/cli-kit/node/crypto'
 import chalk from '@shopify/cli-kit/node/colors'
+import {Config} from '@oclif/core'
 
 import readline from 'readline'
 
 const DEFAULT_HOST = '127.0.0.1'
 const DEFAULT_PORT = '9292'
 
+let hasReportedAnalyticsEvent = false
+
 interface DevOptions {
   adminSession: AdminSession
+  commandConfig: Config
   directory: string
   store: string
   password?: string
@@ -128,11 +135,14 @@ export async function dev(options: DevOptions) {
     },
   }
 
-  const {serverStart, renderDevSetupProgress, backgroundJobPromise} = setupDevServer(options.theme, ctx)
+  const {serverStart, renderDevSetupProgress, backgroundJobPromise, resolveBackgroundJob} = setupDevServer(
+    options.theme,
+    ctx,
+  )
 
   readline.emitKeypressEvents(process.stdin)
 
-  const keypressHandler = createKeypressHandler(urls, ctx)
+  const keypressHandler = createKeypressHandler(urls, ctx, resolveBackgroundJob)
   process.stdin.on('keypress', keypressHandler)
 
   await Promise.all([
@@ -149,17 +159,36 @@ export async function dev(options: DevOptions) {
         }
       }),
   ])
+
+  await reportDevAnalytics(options.commandConfig, options.adminSession)
+
+  process.exit(0)
+}
+
+export async function reportDevAnalytics(config: Config, session: AdminSession): Promise<void> {
+  if (hasReportedAnalyticsEvent) return
+  hasReportedAnalyticsEvent = true
+
+  try {
+    await addPublicMetadata(() => ({store_fqdn_hash: hashString(session.storeFqdn)}))
+    await addSensitiveMetadata(() => ({store_fqdn: session.storeFqdn}))
+    await reportAnalyticsEvent({config, exitMode: 'ok'})
+  } catch (_error) {
+    // Analytics must never block exit.
+  }
 }
 
 export function createKeypressHandler(
   urls: {local: string; giftCard: string; themeEditor: string; preview: string},
   ctx: {lastRequestedPath: string},
+  onClose: () => void,
 ) {
   const debouncedOpenURL = debounce(openURLSafely, 100, {leading: true, trailing: false})
 
   return (_str: string, key: {ctrl?: boolean; name?: string}) => {
     if (key.ctrl && key.name === 'c') {
-      process.exit()
+      onClose()
+      return
     }
 
     switch (key.name) {
@@ -182,6 +211,7 @@ export function createKeypressHandler(
       case 'g':
         debouncedOpenURL(urls.giftCard, 'gift card preview')
         break
+      case undefined:
       default:
         break
     }

packages/theme/src/cli/utilities/theme-environment/theme-environment.ts

@@ -18,7 +18,11 @@ import type {Checksum, Theme} from '@shopify/cli-kit/node/themes/types'
 import type {DevServerContext} from './types.js'
 
 export function setupDevServer(theme: Theme, ctx: DevServerContext) {
-  const {promise: backgroundJobPromise, reject: rejectBackgroundJob} = promiseWithResolvers<never>()
+  const {
+    promise: backgroundJobPromise,
+    resolve: resolveBackgroundJob,
+    reject: rejectBackgroundJob,
+  } = promiseWithResolvers<void>()
 
   const watcherPromise = setupInMemoryTemplateWatcher(theme, ctx)
   const envSetup = ensureThemeEnvironmentSetup(theme, ctx, rejectBackgroundJob)
@@ -33,6 +37,7 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) {
     dispatchEvent: server.dispatch,
     renderDevSetupProgress: envSetup.renderProgress,
     backgroundJobPromise,
+    resolveBackgroundJob,
   }
 }