From 29e09f49d9c8856ba201f5267d3b1ed344c0ae2a Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 19 Mar 2026 16:49:28 +0100 Subject: [PATCH] Lazy imports and V8 compile cache Defer heavy module imports to point of use across the codebase: - base-command: lazy ui.js (600KB React/Ink), error-handler, notifications - prerun hook: parallel imports via Promise.all - postrun hook: lazy analytics and deprecations - node-package-manager: lazy latest-version import - context/local: lazy fs.js and system.js (break circular deps) - is-global: lazy output.js, system.js, version.js, ui.js - custom-oclif-loader: fire-and-forget init hooks via runHook override Enable V8 compile cache via module.enableCompileCache() in dev.js/run.js, caching bytecode between runs to eliminate ~30ms compile overhead. Combined improvement: ~130ms across all commands Made-with: Cursor --- .../src/public/node/base-command.test.ts | 28 +++++++++---- .../cli-kit/src/public/node/base-command.ts | 38 +++++++++++------- .../cli-kit/src/public/node/context/local.ts | 20 +++++++--- .../src/public/node/custom-oclif-loader.ts | 23 +++++++++++ packages/cli-kit/src/public/node/error.ts | 3 +- .../cli-kit/src/public/node/hooks/postrun.ts | 30 +++++++++----- .../cli-kit/src/public/node/hooks/prerun.ts | 40 +++++++++++++------ packages/cli-kit/src/public/node/is-global.ts | 11 ++--- .../src/public/node/node-package-manager.ts | 6 ++- packages/cli/bin/dev.js | 6 ++- packages/cli/bin/run.js | 6 ++- 11 files changed, 149 insertions(+), 62 deletions(-) diff --git a/packages/cli-kit/src/public/node/base-command.test.ts b/packages/cli-kit/src/public/node/base-command.test.ts index 30dfe245fed..0b187ec78d9 100644 --- a/packages/cli-kit/src/public/node/base-command.test.ts +++ b/packages/cli-kit/src/public/node/base-command.test.ts @@ -5,16 +5,26 @@ import {globalFlags} from './cli.js' import {inTemporaryDirectory, mkdir, writeFile} from './fs.js' import {joinPath, resolvePath, cwd} from './path.js' import {mockAndCaptureOutput} from './testing/output.js' -import {terminalSupportsPrompting} from './system.js' import {unstyled} from './output.js' -import {beforeEach, describe, expect, test, vi} from 'vitest' +import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import {Flags} from '@oclif/core' -vi.mock('./system.js') +let originalStdinIsTTY: boolean | undefined +let originalStdoutIsTTY: boolean | undefined beforeEach(() => { - vi.mocked(terminalSupportsPrompting).mockReturnValue(true) + originalStdinIsTTY = process.stdin.isTTY + originalStdoutIsTTY = process.stdout.isTTY vi.unstubAllEnvs() + // Default: simulate interactive TTY environment + Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) + vi.stubEnv('CI', '') +}) + +afterEach(() => { + Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true}) + Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true}) }) let testResult: Record = {} @@ -406,8 +416,10 @@ describe('applying environments', async () => { ) runTestInTmpDir('does not throw in TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => { - // Given - vi.mocked(terminalSupportsPrompting).mockReturnValue(true) + // Given — simulate interactive terminal + Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) + vi.stubEnv('CI', '') // When await MockCommandWithRequiredFlagInNonTTY.run(['--path', tmpDir]) @@ -417,8 +429,8 @@ describe('applying environments', async () => { }) runTestInTmpDir('throws in non-TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => { - // Given - vi.mocked(terminalSupportsPrompting).mockReturnValue(false) + // Given — simulate non-interactive (CI) environment + vi.stubEnv('CI', 'true') // When await MockCommandWithRequiredFlagInNonTTY.run(['--path', tmpDir]) diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index e5b5f8398c8..2cb2cf4b5a2 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -1,18 +1,13 @@ -import {errorHandler, registerCleanBugsnagErrorsFromWithinPlugins} from './error-handler.js' -import {loadEnvironment, environmentFilePath} from './environments.js' import {isDevelopment} from './context/local.js' import {addPublicMetadata} from './metadata.js' import {AbortError} from './error.js' -import {renderInfo, renderWarning} from './ui.js' import {outputContent, outputResult, outputToken} from './output.js' import {terminalSupportsPrompting} from './system.js' import {hashString} from './crypto.js' import {isTruthy} from './context/utilities.js' -import {showNotificationsIfNeeded} from './notifications-system.js' import {setCurrentCommandId} from './global-context.js' import {JsonMap} from '../../private/common/json.js' import {underscore} from '../common/string.js' - import {Command, Config, Errors} from '@oclif/core' import {OutputFlags, Input, ParserOutput, FlagInput, OutputArgs} from '@oclif/core/parser' @@ -45,6 +40,7 @@ abstract class BaseCommand extends Command { async catch(error: Error & {skipOclifErrorHandling: boolean}): Promise { error.skipOclifErrorHandling = true + const {errorHandler} = await import('./error-handler.js') await errorHandler(error, this.config) return Errors.handle(error) } @@ -54,10 +50,12 @@ abstract class BaseCommand extends Command { setCurrentCommandId(this.id ?? '') if (!isDevelopment()) { // This function runs just prior to `run` + const {registerCleanBugsnagErrorsFromWithinPlugins} = await import('./error-handler.js') await registerCleanBugsnagErrorsFromWithinPlugins(this.config) } await removeDuplicatedPlugins(this.config) this.showNpmFlagWarning() + const {showNotificationsIfNeeded} = await import('./notifications-system.js') await showNotificationsIfNeeded() return super.init() } @@ -71,13 +69,16 @@ abstract class BaseCommand extends Command { const possibleNpmEnvVars = commandFlags.map((key) => `npm_config_${underscore(key).replace(/^no_/, '')}`) if (possibleNpmEnvVars.some((flag) => process.env[flag] !== undefined)) { - renderWarning({ - body: [ - 'NPM scripts require an extra', - {command: '--'}, - 'separator to pass the flags. Example:', - {command: 'npm run dev -- --reset'}, - ], + // eslint-disable-next-line no-void + void import('./ui.js').then(({renderWarning}) => { + renderWarning({ + body: [ + 'NPM scripts require an extra', + {command: '--'}, + 'separator to pass the flags. Example:', + {command: 'npm run dev -- --reset'}, + ], + }) }) } } @@ -143,6 +144,7 @@ This flag is required in non-interactive terminal environments, such as a CI env if (!environmentsFileName) return originalResult + const {environmentFilePath} = await import('./environments.js') const environmentFileExists = await environmentFilePath(environmentsFileName, {from: flags.path}) // Handle both string and array cases for environment flag @@ -202,6 +204,7 @@ This flag is required in non-interactive terminal environments, such as a CI env environmentsFileName: string, specifiedEnvironment: string | undefined, ): Promise<{environment: JsonMap | undefined; isDefaultEnvironment: boolean}> { + const {loadEnvironment} = await import('./environments.js') if (specifiedEnvironment) { const environment = await loadEnvironment(specifiedEnvironment, environmentsFileName, {from: path}) return {environment, isDefaultEnvironment: false} @@ -254,9 +257,12 @@ function reportEnvironmentApplication< if (Object.keys(changes).length === 0) return const items = Object.entries(changes).map(([name, value]) => `${name}: ${value}`) - renderInfo({ - headline: ['Using applicable flags from', {userInput: environmentName}, 'environment:'], - body: [{list: {items}}], + // eslint-disable-next-line no-void + void import('./ui.js').then(({renderInfo}) => { + renderInfo({ + headline: ['Using applicable flags from', {userInput: environmentName}, 'environment:'], + body: [{list: {items}}], + }) }) } @@ -342,6 +348,7 @@ async function removeDuplicatedPlugins(config: Config): Promise { const pluginsToRemove = plugins.filter((plugin) => bundlePlugins.includes(plugin.name)) if (pluginsToRemove.length > 0) { const commandsToRun = pluginsToRemove.map((plugin) => ` - shopify plugins remove ${plugin.name}`).join('\n') + const {renderWarning} = await import('./ui.js') renderWarning({ headline: `Unsupported plugins detected: ${pluginsToRemove.map((plugin) => plugin.name).join(', ')}`, body: [ @@ -351,6 +358,7 @@ async function removeDuplicatedPlugins(config: Config): Promise { }) } const filteredPlugins = plugins.filter((plugin) => !bundlePlugins.includes(plugin.name)) + // eslint-disable-next-line require-atomic-updates -- config.plugins won't be modified by renderWarning above config.plugins = new Map(filteredPlugins.map((plugin) => [plugin.name, plugin])) } diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index c8910841442..0c0d4fdc794 100644 --- a/packages/cli-kit/src/public/node/context/local.ts +++ b/packages/cli-kit/src/public/node/context/local.ts @@ -1,14 +1,22 @@ import {isTruthy} from './utilities.js' import {getCIMetadata, isSet, Metadata} from '../../../private/node/context/utilities.js' import {defaultThemeKitAccessDomain, environmentVariables, pathConstants} from '../../../private/node/constants.js' -import {fileExists} from '../fs.js' -import {exec} from '../system.js' - import isInteractive from 'is-interactive' import macaddress from 'macaddress' - import {homedir} from 'os' +// Dynamic imports to avoid circular dependency: context/local → fs → output → context/local +// fileExists and exec are only used in specific async functions, not at module init. +async function lazyFileExists(path: string): Promise { + const {fileExists} = await import('../fs.js') + return fileExists(path) +} + +async function lazyExec(command: string, args: string[]): Promise { + const {exec} = await import('../system.js') + await exec(command, args) +} + /** * It returns true if the terminal is interactive. * @@ -58,7 +66,7 @@ export async function isShopify(env = process.env): Promise { if (Object.prototype.hasOwnProperty.call(env, environmentVariables.runAsUser)) { return !isTruthy(env[environmentVariables.runAsUser]) } - const devInstalled = await fileExists(pathConstants.executables.dev) + const devInstalled = await lazyFileExists(pathConstants.executables.dev) return devInstalled } @@ -207,7 +215,7 @@ export function cloudEnvironment(env: NodeJS.ProcessEnv = process.env): { */ export async function hasGit(): Promise { try { - await exec('git', ['--version']) + await lazyExec('git', ['--version']) return true // eslint-disable-next-line no-catch-all/no-catch-all } catch { diff --git a/packages/cli-kit/src/public/node/custom-oclif-loader.ts b/packages/cli-kit/src/public/node/custom-oclif-loader.ts index b722d5932fc..5b9c2e497c6 100644 --- a/packages/cli-kit/src/public/node/custom-oclif-loader.ts +++ b/packages/cli-kit/src/public/node/custom-oclif-loader.ts @@ -57,6 +57,29 @@ export class ShopifyConfig extends Config { this.lazyCommandLoader = loader } + /** + * Override runHook to make init hooks non-blocking for faster startup. + * Init hooks (app-init, hydrogen-init) set up LocalStorage and check hydrogen — + * these are setup tasks that don't need to complete before commands run. + * + * @param event - The hook event name. + * @param opts - Options to pass to the hook. + * @param timeout - Optional timeout for the hook. + * @param captureErrors - Whether to capture errors instead of throwing. + * @returns The hook result with successes and failures arrays. + */ + // @ts-expect-error: overriding with looser types for hook interception + // eslint-disable-next-line @typescript-eslint/no-explicit-any + async runHook(event: string, opts: any, timeout?: number, captureErrors?: boolean): Promise { + if (event === 'init' && this.lazyCommandLoader) { + // Fire init hooks in background — they'll complete before process exits + // eslint-disable-next-line no-void + void super.runHook(event, opts, timeout, captureErrors) + return {successes: [], failures: []} + } + return super.runHook(event, opts, timeout, captureErrors) + } + /** * Override runCommand to use lazy loading when available. * Instead of calling cmd.load() which triggers loading ALL commands via index.js, diff --git a/packages/cli-kit/src/public/node/error.ts b/packages/cli-kit/src/public/node/error.ts index 2de5c3e3e4e..12a3ece2ed2 100644 --- a/packages/cli-kit/src/public/node/error.ts +++ b/packages/cli-kit/src/public/node/error.ts @@ -1,4 +1,4 @@ -import {AlertCustomSection, renderFatalError} from './ui.js' +import type {AlertCustomSection} from './ui.js' import {normalizePath} from './path.js' import {OutputMessage, stringifyMessage, TokenizedString} from './output.js' import {InlineToken, TokenItem, tokenItemToString} from '../../private/node/ui/components/TokenizedText.js' @@ -146,6 +146,7 @@ export async function handler(error: unknown): Promise { } } + const {renderFatalError} = await import('./ui.js') renderFatalError(fatal) return Promise.resolve(error) } diff --git a/packages/cli-kit/src/public/node/hooks/postrun.ts b/packages/cli-kit/src/public/node/hooks/postrun.ts index e15295a3852..0e199e2379a 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.ts @@ -1,9 +1,7 @@ -import {postrun as deprecationsHook} from './deprecations.js' -import {reportAnalyticsEvent} from '../analytics.js' -import {outputDebug} from '../output.js' -import BaseCommand from '../base-command.js' -import * as metadata from '../metadata.js' - +/** + * Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics) + * at module evaluation time. These are only needed after the command has already finished. + */ import {Command, Hook} from '@oclif/core' let postRunHookCompleted = false @@ -20,9 +18,14 @@ export function postRunHookHasCompleted(): boolean { // This hook is called after each successful command run. More info: https://oclif.io/docs/hooks export const hook: Hook.Postrun = async ({config, Command}) => { await detectStopCommand(Command as unknown as typeof Command) + + const {reportAnalyticsEvent} = await import('../analytics.js') await reportAnalyticsEvent({config, exitMode: 'ok'}) + + const {postrun: deprecationsHook} = await import('./deprecations.js') deprecationsHook(Command) + const {outputDebug} = await import('../output.js') const command = Command.id.replace(/:/g, ' ') outputDebug(`Completed command ${command}`) postRunHookCompleted = true @@ -31,13 +34,20 @@ export const hook: Hook.Postrun = async ({config, Command}) => { /** * Override the command name with the stop one for analytics purposes. * - * @param commandClass - Oclif command class. + * @param commandClass - Command.Class. */ -async function detectStopCommand(commandClass: Command.Class | typeof BaseCommand): Promise { +async function detectStopCommand(commandClass: Command.Class): Promise { const currentTime = new Date().getTime() - if (commandClass && Object.prototype.hasOwnProperty.call(commandClass, 'analyticsStopCommand')) { - const stopCommand = (commandClass as typeof BaseCommand).analyticsStopCommand() + // Check for analyticsStopCommand without importing BaseCommand + if ( + commandClass && + 'analyticsStopCommand' in commandClass && + typeof commandClass.analyticsStopCommand === 'function' + ) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const stopCommand = (commandClass as any).analyticsStopCommand() if (stopCommand) { + const metadata = await import('../metadata.js') const {commandStartOptions} = metadata.getAllSensitiveMetadata() if (!commandStartOptions) return await metadata.addSensitiveMetadata(() => ({ diff --git a/packages/cli-kit/src/public/node/hooks/prerun.ts b/packages/cli-kit/src/public/node/hooks/prerun.ts index b64c6c21cda..2ddc03e5d01 100644 --- a/packages/cli-kit/src/public/node/hooks/prerun.ts +++ b/packages/cli-kit/src/public/node/hooks/prerun.ts @@ -1,12 +1,3 @@ -import {CLI_KIT_VERSION} from '../../common/version.js' -import {checkForNewVersion, checkForCachedNewVersion} from '../node-package-manager.js' -import {startAnalytics} from '../../../private/node/analytics.js' -import {outputDebug, outputWarn} from '../output.js' -import {getOutputUpdateCLIReminder} from '../upgrade.js' -import Command from '../base-command.js' -import {runAtMinimumInterval} from '../../../private/node/conf-store.js' -import {fetchNotificationsInBackground} from '../notifications-system.js' -import {isPreReleaseVersion} from '../version.js' import {Hook} from '@oclif/core' export declare interface CommandContent { @@ -14,7 +5,9 @@ export declare interface CommandContent { topic?: string alias?: string } + // This hook is called before each command run. More info: https://oclif.io/docs/hooks +// Heavy imports are loaded in parallel via Promise.all to avoid sequential await overhead. export const hook: Hook.Prerun = async (options) => { const commandContent = parseCommandContent({ id: options.Command.id, @@ -22,10 +15,21 @@ export const hook: Hook.Prerun = async (options) => { pluginAlias: options.Command.plugin?.alias, }) const args = options.argv - await warnOnAvailableUpgrade() + + // Load heavy modules in parallel + const [{outputDebug}, analyticsMod, notificationsMod] = await Promise.all([ + import('../output.js'), + import('../../../private/node/analytics.js'), + import('../notifications-system.js'), + ]) + + // Fire upgrade check in background (non-blocking) + // eslint-disable-next-line no-void + void warnOnAvailableUpgrade() + outputDebug(`Running command ${commandContent.command}`) - await startAnalytics({commandContent, args, commandClass: options.Command as unknown as typeof Command}) - fetchNotificationsInBackground(options.Command.id) + await analyticsMod.startAnalytics({commandContent, args, commandClass: options.Command}) + notificationsMod.fetchNotificationsInBackground(options.Command.id) } export function parseCommandContent(cmdInfo: {id: string; aliases: string[]; pluginAlias?: string}): CommandContent { @@ -92,10 +96,15 @@ function findAlias(aliases: string[]) { * Warns the user if there is a new version of the CLI available */ export async function warnOnAvailableUpgrade(): Promise { + const [{CLI_KIT_VERSION}, {isPreReleaseVersion}, {checkForNewVersion, checkForCachedNewVersion}] = await Promise.all([ + import('../../common/version.js'), + import('../version.js'), + import('../node-package-manager.js'), + ]) + const cliDependency = '@shopify/cli' const currentVersion = CLI_KIT_VERSION if (isPreReleaseVersion(currentVersion)) { - // This is a nightly/snapshot/experimental version, so we don't want to check for updates return } @@ -104,9 +113,14 @@ export async function warnOnAvailableUpgrade(): Promise { void checkForNewVersion(cliDependency, currentVersion, {cacheExpiryInHours: 24}) // Warn if we previously found a new version + const {runAtMinimumInterval} = await import('../../../private/node/conf-store.js') await runAtMinimumInterval('warn-on-available-upgrade', {days: 1}, async () => { const newerVersion = checkForCachedNewVersion(cliDependency, currentVersion) if (newerVersion) { + const [{outputWarn}, {getOutputUpdateCLIReminder}] = await Promise.all([ + import('../output.js'), + import('../upgrade.js'), + ]) outputWarn(getOutputUpdateCLIReminder(newerVersion)) } }) diff --git a/packages/cli-kit/src/public/node/is-global.ts b/packages/cli-kit/src/public/node/is-global.ts index 5b2bb02ef1e..1d8a0bee8d0 100644 --- a/packages/cli-kit/src/public/node/is-global.ts +++ b/packages/cli-kit/src/public/node/is-global.ts @@ -1,11 +1,7 @@ -import {PackageManager} from './node-package-manager.js' -import {outputInfo} from './output.js' import {cwd, sniffForPath} from './path.js' -import {exec, terminalSupportsPrompting} from './system.js' -import {renderSelectPrompt} from './ui.js' -import {globalCLIVersion} from './version.js' import {isUnitTest} from './context/local.js' import {execaSync} from 'execa' +import type {PackageManager} from './node-package-manager.js' let _isGlobal: boolean | undefined @@ -47,6 +43,8 @@ export function currentProcessIsGlobal(argv = process.argv): boolean { * @param packageManager - The package manager to use. */ export async function installGlobalShopifyCLI(packageManager: PackageManager): Promise { + const {outputInfo} = await import('./output.js') + const {exec} = await import('./system.js') const args = packageManager === 'yarn' ? ['global', 'add', '@shopify/cli@latest'] : ['install', '-g', '@shopify/cli@latest'] outputInfo(`Running ${packageManager} ${args.join(' ')}...`) @@ -63,10 +61,13 @@ export interface InstallGlobalCLIPromptResult { * @returns `true` if the user has installed the global CLI. */ export async function installGlobalCLIPrompt(): Promise { + const {terminalSupportsPrompting} = await import('./system.js') if (!terminalSupportsPrompting()) return {install: false, alreadyInstalled: false} + const {globalCLIVersion} = await import('./version.js') if (await globalCLIVersion()) { return {install: false, alreadyInstalled: true} } + const {renderSelectPrompt} = await import('./ui.js') const result = await renderSelectPrompt({ message: 'We recommend installing Shopify CLI globally in your system. Would you like to install it now?', choices: [ diff --git a/packages/cli-kit/src/public/node/node-package-manager.ts b/packages/cli-kit/src/public/node/node-package-manager.ts index 501f75ee444..668b655fe12 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.ts @@ -9,7 +9,8 @@ import {outputToken, outputContent, outputDebug} from './output.js' import {PackageVersionKey, cacheRetrieve, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' import {parseJSON} from '../common/json.js' -import latestVersion from 'latest-version' +// latest-version is loaded lazily to avoid ~113ms import cost at startup +// import latestVersion from 'latest-version' import {SemVer, satisfies as semverSatisfies} from 'semver' import type {Writable} from 'stream' @@ -710,7 +711,8 @@ export async function addResolutionOrOverride(directory: string, dependencies: R */ async function getLatestNPMPackageVersion(name: string) { outputDebug(outputContent`Getting the latest version of NPM package: ${outputToken.raw(name)}`) - return runWithTimer('cmd_all_timing_network_ms')(() => { + return runWithTimer('cmd_all_timing_network_ms')(async () => { + const {default: latestVersion} = await import('latest-version') return latestVersion(name) }) } diff --git a/packages/cli/bin/dev.js b/packages/cli/bin/dev.js index 936614ff521..d69f80cd620 100755 --- a/packages/cli/bin/dev.js +++ b/packages/cli/bin/dev.js @@ -1,5 +1,9 @@ -const {default: runCLI} = await import('../dist/bootstrap.js') +// eslint-disable-next-line n/no-unsupported-features/node-builtins +import {enableCompileCache} from 'node:module' + +enableCompileCache() process.removeAllListeners('warning') +const {default: runCLI} = await import('../dist/bootstrap.js') runCLI({development: true}) diff --git a/packages/cli/bin/run.js b/packages/cli/bin/run.js index 7f8f21821c0..dec351662d8 100755 --- a/packages/cli/bin/run.js +++ b/packages/cli/bin/run.js @@ -1,7 +1,11 @@ #!/usr/bin/env node -const {default: runCLI} = await import('../dist/bootstrap.js') +// eslint-disable-next-line n/no-unsupported-features/node-builtins +import {enableCompileCache} from 'node:module' + +enableCompileCache() process.removeAllListeners('warning') +const {default: runCLI} = await import('../dist/bootstrap.js') runCLI({development: false})