diff --git a/.changeset/faster-cli-startup.md b/.changeset/faster-cli-startup.md new file mode 100644 index 00000000000..8dc2652ac4b --- /dev/null +++ b/.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 diff --git a/packages/app/src/cli/hooks/public_metadata.test.ts b/packages/app/src/cli/hooks/public_metadata.test.ts index 1b52352b938..3e943750d37 100644 --- a/packages/app/src/cli/hooks/public_metadata.test.ts +++ b/packages/app/src/cli/hooks/public_metadata.test.ts @@ -3,14 +3,18 @@ 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 + describe('gatherPublicMetadata', () => { beforeEach(() => { vi.mocked(cwd).mockReturnValue('/some/app/dir') vi.mocked(localAppContext).mockResolvedValue({} as Awaited>) + setCurrentCommandId('app:dev') }) test('opportunistically enriches metadata from the current directory and returns the public metadata', async () => { @@ -18,7 +22,7 @@ describe('gatherPublicMetadata', () => { vi.spyOn(metadata, 'getAllPublicMetadata').mockReturnValueOnce({}).mockReturnValue({api_key: 'from-loader'}) // When - const result = await (gatherPublicMetadata as () => Promise)() + 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)() + 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)() + 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()) + }, + ) }) diff --git a/packages/app/src/cli/hooks/public_metadata.ts b/packages/app/src/cli/hooks/public_metadata.ts index 03b0a8bc7b2..07a7798b4e5 100644 --- a/packages/app/src/cli/hooks/public_metadata.ts +++ b/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 { let timer: ReturnType | 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((resolve) => { @@ -25,7 +40,9 @@ async function logAppContextMetadata(directory: string): Promise { } const gatherPublicMetadata: FanoutHookFunction<'public_command_metadata', '@shopify/app'> = async () => { - await logAppContextMetadata(cwd()) + if (isAppCommand(getCurrentCommandId())) { + await logAppContextMetadata(cwd()) + } return metadata.getAllPublicMetadata() } diff --git a/packages/cli-kit/src/private/node/session.test.ts b/packages/cli-kit/src/private/node/session.test.ts index d781fff189a..bfe7587daf7 100644 --- a/packages/cli-kit/src/private/node/session.test.ts +++ b/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, diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 3eb9f9e5ee6..51698daae69 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/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) diff --git a/packages/cli-kit/src/private/node/session/command-session-id.ts b/packages/cli-kit/src/private/node/session/command-session-id.ts new file mode 100644 index 00000000000..4acf1ebfb18 --- /dev/null +++ b/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 +} diff --git a/packages/cli-kit/src/public/node/analytics.ts b/packages/cli-kit/src/public/node/analytics.ts index dcaef13d51c..1b6d4a08045 100644 --- a/packages/cli-kit/src/public/node/analytics.ts +++ b/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 { 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)}`) } diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index 1da872612e0..90ad731c7cb 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/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 & {argv: string[]}> { let result = await super.parse(options, argv) result = await this.resultWithEnvironment(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 { + 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, diff --git a/packages/cli-kit/src/public/node/local-storage.ts b/packages/cli-kit/src/public/node/local-storage.ts index 9759267aab9..2c151737d6f 100644 --- a/packages/cli-kit/src/public/node/local-storage.ts +++ b/packages/cli-kit/src/public/node/local-storage.ts @@ -66,6 +66,17 @@ export class LocalStorage> { } } + /** + * 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). * diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index d9eade1f1cb..79427f90049 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/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 { 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] diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 449992bb4b8..78adcefbfa1 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/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') diff --git a/packages/cli-kit/src/public/node/session.ts b/packages/cli-kit/src/public/node/session.ts index 15be5d39cbf..3c7b589b07c 100644 --- a/packages/cli-kit/src/public/node/session.ts +++ b/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' /** diff --git a/packages/cli-kit/src/public/node/vendor/otel-js/utils/throttle.ts b/packages/cli-kit/src/public/node/vendor/otel-js/utils/throttle.ts index f100a38147f..8859663b7fc 100644 --- a/packages/cli-kit/src/public/node/vendor/otel-js/utils/throttle.ts +++ b/packages/cli-kit/src/public/node/vendor/otel-js/utils/throttle.ts @@ -61,6 +61,8 @@ export function throttle 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 } diff --git a/packages/cli/src/hooks/app-init.ts b/packages/cli/src/hooks/app-init.ts index 5cb417492a3..9c48fcf01ec 100644 --- a/packages/cli/src/hooks/app-init.ts +++ b/packages/cli/src/hooks/app-init.ts @@ -8,10 +8,12 @@ import {randomUUID} from 'crypto' * LocalStorage class only at call time, and uses Node's native crypto. */ const init: Hook<'init'> = async (_options) => { - // Lazy import to clear the command storage (equivalent to clearCachedCommandInfo) + // Lazy import to clear the command storage (equivalent to clearCachedCommandInfo). + // Clearing writes the file atomically (temp file + rename + fsync), so skip it when the store is + // already empty -- which is the case for every command that doesn't touch app command state. const {LocalStorage} = await import('@shopify/cli-kit/node/local-storage') const store = new LocalStorage({projectName: 'shopify-cli-app-command'}) - store.clear() + if (store.size > 0) store.clear() // Set a unique run ID so parallel commands don't collide in the cache process.env.COMMAND_RUN_ID = randomUUID()