Halve CLI startup time by not loading the app for non-app commands

.changeset/faster-cli-startup.md

@@ -0,0 +1,7 @@
+---
+'@shopify/cli-kit': patch
+'@shopify/app': patch
+'@shopify/cli': patch
+---
+
+Reduce CLI startup time by only loading the app when running `app` commands

packages/app/src/cli/hooks/public_metadata.test.ts

@@ -3,22 +3,26 @@ import {localAppContext} from '../services/app-context.js'
 import metadata from '../metadata.js'
 import {describe, expect, test, vi, beforeEach} from 'vitest'
 import {cwd} from '@shopify/cli-kit/node/path'
+import {setCurrentCommandId} from '@shopify/cli-kit/node/global-context'
 
 vi.mock('../services/app-context.js')
 vi.mock('@shopify/cli-kit/node/path')
 
+const gather = gatherPublicMetadata as () => Promise<unknown>
+
 describe('gatherPublicMetadata', () => {
   beforeEach(() => {
     vi.mocked(cwd).mockReturnValue('/some/app/dir')
     vi.mocked(localAppContext).mockResolvedValue({} as Awaited<ReturnType<typeof localAppContext>>)
+    setCurrentCommandId('app:dev')
   })
 
   test('opportunistically enriches metadata from the current directory and returns the public metadata', async () => {
     // Given
     vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue({api_key: 'from-loader'})
 
     // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+    const result = await gather()
 
     // Then
     expect(localAppContext).toHaveBeenCalledWith({directory: '/some/app/dir', skipPrompts: true})
@@ -30,7 +34,7 @@ describe('gatherPublicMetadata', () => {
     vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({api_key: 'already-set'})
 
     // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+    const result = await gather()
 
     // Then
     expect(localAppContext).not.toHaveBeenCalled()
@@ -43,10 +47,38 @@ describe('gatherPublicMetadata', () => {
     vi.mocked(localAppContext).mockRejectedValue(new Error('not an app'))
 
     // When
-    const result = await (gatherPublicMetadata as () => Promise<unknown>)()
+    const result = await gather()
 
     // Then
     expect(localAppContext).toHaveBeenCalledOnce()
     expect(result).toEqual(metadata.getAllPublicMetadata())
   })
+
+  test('loads the app for the top-level app command', async () => {
+    // Given
+    vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({})
+    setCurrentCommandId('app')
+
+    // When
+    await gather()
+
+    // Then
+    expect(localAppContext).toHaveBeenCalledOnce()
+  })
+
+  test.each(['version', 'theme:dev', 'store:create', 'apps:something', ''])(
+    'does not load the app for the non-app command %s',
+    async (commandId) => {
+      // Given
+      vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValue({})
+      setCurrentCommandId(commandId)
+
+      // When
+      const result = await gather()
+
+      // Then
+      expect(localAppContext).not.toHaveBeenCalled()
+      expect(result).toEqual(metadata.getAllPublicMetadata())
+    },
+  )
 })

packages/app/src/cli/hooks/public_metadata.ts

@@ -1,15 +1,30 @@
 import metadata from '../metadata.js'
-import {localAppContext} from '../services/app-context.js'
 import {FanoutHookFunction} from '@shopify/cli-kit/node/plugins'
 import {cwd} from '@shopify/cli-kit/node/path'
+import {getCurrentCommandId} from '@shopify/cli-kit/node/global-context'
 
 const APP_CONTEXT_METADATA_TIMEOUT_MS = 3000
 
+/**
+ * Loading an app to gather `app_*` analytics only makes sense for `app` commands. Every other
+ * command (`version`, `theme *`, `store *`, ...) would load the whole app graph — which reaches
+ * theme-check and its ohm-js Liquid grammar — to report metadata it can't produce anyway.
+ *
+ * The command id is the canonical oclif id (`app:dev`), set by cli-kit's BaseCommand. It is empty
+ * for commands that don't extend BaseCommand, which are never app commands.
+ */
+function isAppCommand(commandId: string): boolean {
+  return commandId === 'app' || commandId.startsWith('app:')
+}
+
 async function logAppContextMetadata(directory: string): Promise<void> {
   let timer: ReturnType<typeof setTimeout> | undefined
   try {
     if (metadata.getAllPublicMetadata().api_key !== undefined) return
 
+    // Imported lazily so that non-app commands never pay for the app graph.
+    const {localAppContext} = await import('../services/app-context.js')
+
     await Promise.race([
       localAppContext({directory, skipPrompts: true}),
       new Promise<void>((resolve) => {
@@ -25,7 +40,9 @@ async function logAppContextMetadata(directory: string): Promise<void> {
 }
 
 const gatherPublicMetadata: FanoutHookFunction<'public_command_metadata', '@shopify/app'> = async () => {
-  await logAppContextMetadata(cwd())
+  if (isAppCommand(getCurrentCommandId())) {
+    await logAppContextMetadata(cwd())
+  }
   return metadata.getAllPublicMetadata()
 }
 

packages/cli-kit/src/private/node/session.test.ts

@@ -4,10 +4,10 @@ import {
   getLastSeenUserIdAfterAuth,
   OAuthApplications,
   OAuthSession,
-  setCommandSessionId,
   setLastSeenAuthMethod,
   setLastSeenUserIdAfterAuth,
 } from './session.js'
+import {setCommandSessionId} from './session/command-session-id.js'
 import {
   exchangeAccessForApplicationTokens,
   exchangeCustomPartnerToken,

packages/cli-kit/src/private/node/session.ts

@@ -13,6 +13,7 @@ import {IdentityToken, Session, Sessions} from './session/schema.js'
 import * as sessionStore from './session/store.js'
 import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js'
 import {isThemeAccessSession} from './api/rest.js'
+import {getCommandSessionId} from './session/command-session-id.js'
 import {getCurrentSessionId, setCurrentSessionId} from './conf-store.js'
 import {UserEmailQueryString, UserEmailQuery} from './api/graphql/business-platform-destinations/user-email.js'
 import {outputContent, outputToken, outputDebug, outputCompleted} from '../../public/node/output.js'
@@ -118,7 +119,6 @@ type AuthMethod = 'partners_token' | 'device_auth' | 'theme_access_token' | 'cus
 
 let userId: undefined | string
 let authMethod: AuthMethod = 'none'
-let commandSessionId: string | undefined
 
 /**
  * Retrieves a stable user identifier for analytics, or `'unknown'` if none applies.
@@ -180,10 +180,6 @@ export function setLastSeenAuthMethod(method: AuthMethod) {
   authMethod = method
 }
 
-export function setCommandSessionId(sessionId: string | undefined) {
-  commandSessionId = sessionId
-}
-
 export interface EnsureAuthenticatedAdditionalOptions {
   noPrompt?: boolean
   forceRefresh?: boolean
@@ -215,6 +211,7 @@ export async function ensureAuthenticated(
 
   const sessions = (await sessionStore.fetch()) ?? {}
 
+  const commandSessionId = getCommandSessionId()
   let currentSessionId = forceNewSession ? undefined : (commandSessionId ?? getCurrentSessionId())
   if (!currentSessionId && !commandSessionId) {
     const userIds = Object.keys(sessions[fqdn] ?? {})
@@ -265,7 +262,7 @@ ${outputToken.json(applications)}
   // Save the new session info if it has changed
   if (!isEmpty(newSession)) {
     await sessionStore.store(updatedSessions)
-    if (!commandSessionId) setCurrentSessionId(newSessionId)
+    if (!getCommandSessionId()) setCurrentSessionId(newSessionId)
   }
 
   const tokens = await tokensFor(applications, completeSession)

packages/cli-kit/src/private/node/session/command-session-id.ts

@@ -0,0 +1,25 @@
+/**
+ * The session selected for the current command process via `--auth-alias`.
+ *
+ * This lives in its own dependency-free module so that resetting it (the overwhelmingly common
+ * case, where no alias was passed) does not require loading the session/identity/API graph.
+ */
+let commandSessionId: string | undefined
+
+/**
+ * Get the session id selected for the current command, if any.
+ *
+ * @returns The selected session id, or undefined when no alias was selected.
+ */
+export function getCommandSessionId(): string | undefined {
+  return commandSessionId
+}
+
+/**
+ * Select a stored session for the current command process.
+ *
+ * @param sessionId - The session id to select, or undefined to clear the selection.
+ */
+export function setCommandSessionId(sessionId: string | undefined): void {
+  commandSessionId = sessionId
+}

packages/cli-kit/src/public/node/analytics.ts

@@ -1,4 +1,4 @@
-import {alwaysLogAnalytics, alwaysLogMetrics, analyticsDisabled, isShopify} from './context/local.js'
+import {alwaysLogAnalytics, alwaysLogMetrics, analyticsDisabled, isShopify, isVerbose} from './context/local.js'
 import * as metadata from './metadata.js'
 import {publishMonorailEvent, MONORAIL_COMMAND_TOPIC} from './monorail.js'
 import {fanoutHooks} from './plugins.js'
@@ -44,6 +44,16 @@ interface ReportAnalyticsEventOptions {
  */
 export async function reportAnalyticsEvent(options: ReportAnalyticsEventOptions): Promise<void> {
   try {
+    const skipMonorailAnalytics = !alwaysLogAnalytics() && analyticsDisabled()
+    const skipMetricAnalytics = !alwaysLogMetrics() && analyticsDisabled()
+
+    // Building the payload fans out `public_command_metadata` to every plugin, which for app
+    // commands loads the app. When neither destination will receive anything there is nothing to
+    // build it for -- unless the user asked to see it, in which case we still build it to log it.
+    if (skipMonorailAnalytics && skipMetricAnalytics && !isVerbose()) {
+      return
+    }
+
     const payload = await buildPayload(options)
     if (payload === undefined) {
       // Nothing to log
@@ -63,8 +73,6 @@ export async function reportAnalyticsEvent(options: ReportAnalyticsEventOptions)
       return
     }
 
-    const skipMonorailAnalytics = !alwaysLogAnalytics() && analyticsDisabled()
-    const skipMetricAnalytics = !alwaysLogMetrics() && analyticsDisabled()
     if (skipMonorailAnalytics || skipMetricAnalytics) {
       outputDebug(outputContent`Skipping command analytics, payload: ${outputToken.json(payload)}`)
     }

packages/cli-kit/src/public/node/base-command.ts

@@ -2,7 +2,6 @@ import {isDevelopment} from './context/local.js'
 import {addPublicMetadata} from './metadata.js'
 import {AbortError} from './error.js'
 import {outputContent, outputResult, outputToken} from './output.js'
-import {setCurrentSessionAlias} from './session.js'
 import {terminalSupportsPrompting} from './system.js'
 import {hashString} from './crypto.js'
 import {isTruthy} from './context/utilities.js'
@@ -106,7 +105,7 @@ abstract class BaseCommand extends Command {
   ): Promise<ParserOutput<TFlags, TGlobalFlags, TArgs> & {argv: string[]}> {
     let result = await super.parse<TFlags, TGlobalFlags, TArgs>(options, argv)
     result = await this.resultWithEnvironment<TFlags, TGlobalFlags, TArgs>(result, options, argv)
-    await setCurrentSessionAlias(result.flags['auth-alias'])
+    await this.selectSessionAlias(result.flags['auth-alias'])
     await addFromParsedFlags(result.flags)
     return {...result, ...{argv: result.argv as string[]}}
   }
@@ -133,6 +132,22 @@ This flag is required in non-interactive terminal environments, such as a CI env
     })
   }
 
+  /**
+   * Resolving an alias needs the session/identity/API graph, so it is imported on demand. Almost no
+   * invocation passes `--auth-alias`, and clearing the selection only needs the tiny state module.
+   *
+   * @param alias - The account alias passed via `--auth-alias`, if any.
+   */
+  private async selectSessionAlias(alias?: string): Promise<void> {
+    if (alias) {
+      const {setCurrentSessionAlias} = await import('./session.js')
+      await setCurrentSessionAlias(alias)
+      return
+    }
+    const {setCommandSessionId} = await import('../../private/node/session/command-session-id.js')
+    setCommandSessionId(undefined)
+  }
+
   private async resultWithEnvironment<
     TFlags extends FlagOutput & {path?: string; verbose?: boolean},
     TGlobalFlags extends FlagOutput,

packages/cli-kit/src/public/node/local-storage.ts

@@ -66,6 +66,17 @@ export class LocalStorage<T extends Record<string, any>> {
     }
   }
 
+  /**
+   * The number of values held in the local storage.
+   *
+   * Useful to avoid an unnecessary write when there is nothing to clear.
+   *
+   * @returns The number of stored keys.
+   */
+  get size(): number {
+    return this.config.size
+  }
+
   /**
    * Clear the local storage (delete all values).
    *

packages/cli-kit/src/public/node/notifications-system.ts

@@ -13,6 +13,12 @@ import {NotificationKey, NotificationsKey, cacheRetrieve, cacheStore} from '../.
 
 const URL = 'https://cdn.shopify.com/static/cli/notifications.json'
 const EMPTY_CACHE_MESSAGE = 'Cache is empty'
+/**
+ * How long a cached notifications payload is considered fresh enough to skip the background refresh.
+ * Refreshing spawns a whole extra CLI process, so doing it on literally every command is wasteful for
+ * a static CDN document.
+ */
+const NOTIFICATIONS_REFRESH_INTERVAL_MS = 60 * 60 * 1000
 const COMMANDS_TO_SKIP = [
   'notifications:list',
   'notifications:generate',
@@ -166,6 +172,18 @@ async function cacheNotifications(notifications: string): Promise<void> {
   outputDebug(`Notifications from ${url()} stored in the cache`)
 }
 
+/**
+ * Whether the cached notifications payload is recent enough that refreshing it can be skipped.
+ *
+ * @returns True when a cached payload exists and is younger than the refresh interval.
+ */
+function notificationsCacheIsFresh(): boolean {
+  const cacheKey: NotificationsKey = `notifications-${url()}`
+  const cached = cacheRetrieve(cacheKey)
+  if (cached?.value === undefined) return false
+  return Date.now() - cached.timestamp < NOTIFICATIONS_REFRESH_INTERVAL_MS
+}
+
 /**
  * Fetch notifications in background as a detached process.
  *
@@ -180,6 +198,10 @@ export function fetchNotificationsInBackground(
 ): void {
   if (skipNotifications(currentCommand, environment)) return
   if (!argv[0] || !argv[1]) return
+  if (notificationsCacheIsFresh()) {
+    outputDebug('Notifications cache is still fresh, skipping background refresh')
+    return
+  }
 
   // Run the Shopify command the same way as the current execution
   const nodeBinary = argv[0]

packages/cli-kit/src/public/node/session.test.ts

@@ -14,12 +14,8 @@ import {
 import {nonRandomUUID} from './crypto.js'
 import {getAppAutomationToken} from './environment.js'
 import {shopifyFetch} from './http.js'
-import {
-  ensureAuthenticated,
-  setCommandSessionId,
-  setLastSeenAuthMethod,
-  setLastSeenUserIdAfterAuth,
-} from '../../private/node/session.js'
+import {ensureAuthenticated, setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../../private/node/session.js'
+import {setCommandSessionId} from '../../private/node/session/command-session-id.js'
 import * as sessionStore from '../../private/node/session/store.js'
 import {ApplicationToken} from '../../private/node/session/schema.js'
 import {
@@ -39,6 +35,7 @@ const partnersToken: ApplicationToken = {
 }
 
 vi.mock('../../private/node/session.js')
+vi.mock('../../private/node/session/command-session-id.js')
 vi.mock('../../private/node/session/exchange.js')
 vi.mock('../../private/node/session/store.js')
 vi.mock('./environment.js')

packages/cli-kit/src/public/node/session.ts

@@ -17,10 +17,10 @@ import {
   PartnersAPIScope,
   StorefrontRendererScope,
   ensureAuthenticated,
-  setCommandSessionId,
   setLastSeenAuthMethod,
   setLastSeenUserIdAfterAuth,
 } from '../../private/node/session.js'
+import {setCommandSessionId} from '../../private/node/session/command-session-id.js'
 import {isThemeAccessSession} from '../../private/node/api/rest.js'
 
 /**

packages/cli-kit/src/public/node/vendor/otel-js/utils/throttle.ts

@@ -61,6 +61,8 @@ export function throttle<T extends (...args: unknown[]) => unknown>(
       lastArgs = null
     } else if (!timeout && trailing !== false) {
       timeout = setTimeout(later, remaining)
+      // A trailing metrics export must never hold the CLI process open waiting to fire.
+      timeout.unref?.()
     }
     return result
   }