Skip to content

Commit 44f4592

Browse files
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
1 parent 14a02dd commit 44f4592

11 files changed

Lines changed: 137 additions & 70 deletions

File tree

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,26 @@ import {globalFlags} from './cli.js'
55
import {inTemporaryDirectory, mkdir, writeFile} from './fs.js'
66
import {joinPath, resolvePath, cwd} from './path.js'
77
import {mockAndCaptureOutput} from './testing/output.js'
8-
import {terminalSupportsPrompting} from './system.js'
98
import {unstyled} from './output.js'
10-
import {beforeEach, describe, expect, test, vi} from 'vitest'
9+
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
1110
import {Flags} from '@oclif/core'
1211

13-
vi.mock('./system.js')
12+
let originalStdinIsTTY: boolean | undefined
13+
let originalStdoutIsTTY: boolean | undefined
1414

1515
beforeEach(() => {
16-
vi.mocked(terminalSupportsPrompting).mockReturnValue(true)
16+
originalStdinIsTTY = process.stdin.isTTY
17+
originalStdoutIsTTY = process.stdout.isTTY
1718
vi.unstubAllEnvs()
19+
// Default: simulate interactive TTY environment
20+
Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true})
21+
Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true})
22+
vi.stubEnv('CI', '')
23+
})
24+
25+
afterEach(() => {
26+
Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true})
27+
Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true})
1828
})
1929

2030
let testResult: Record<string, unknown> = {}
@@ -406,8 +416,10 @@ describe('applying environments', async () => {
406416
)
407417

408418
runTestInTmpDir('does not throw in TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => {
409-
// Given
410-
vi.mocked(terminalSupportsPrompting).mockReturnValue(true)
419+
// Given — simulate interactive terminal
420+
Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true})
421+
Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true})
422+
vi.stubEnv('CI', '')
411423

412424
// When
413425
await MockCommandWithRequiredFlagInNonTTY.run(['--path', tmpDir])
@@ -417,8 +429,8 @@ describe('applying environments', async () => {
417429
})
418430

419431
runTestInTmpDir('throws in non-TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => {
420-
// Given
421-
vi.mocked(terminalSupportsPrompting).mockReturnValue(false)
432+
// Given — simulate non-interactive (CI) environment
433+
vi.stubEnv('CI', 'true')
422434

423435
// When
424436
await MockCommandWithRequiredFlagInNonTTY.run(['--path', tmpDir])

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,13 @@
1-
import {errorHandler, registerCleanBugsnagErrorsFromWithinPlugins} from './error-handler.js'
2-
import {loadEnvironment, environmentFilePath} from './environments.js'
31
import {isDevelopment} from './context/local.js'
42
import {addPublicMetadata} from './metadata.js'
53
import {AbortError} from './error.js'
6-
import {renderInfo, renderWarning} from './ui.js'
74
import {outputContent, outputResult, outputToken} from './output.js'
85
import {terminalSupportsPrompting} from './system.js'
96
import {hashString} from './crypto.js'
107
import {isTruthy} from './context/utilities.js'
11-
import {showNotificationsIfNeeded} from './notifications-system.js'
128
import {setCurrentCommandId} from './global-context.js'
139
import {JsonMap} from '../../private/common/json.js'
1410
import {underscore} from '../common/string.js'
15-
1611
import {Command, Config, Errors} from '@oclif/core'
1712
import {OutputFlags, Input, ParserOutput, FlagInput, OutputArgs} from '@oclif/core/parser'
1813

@@ -45,6 +40,7 @@ abstract class BaseCommand extends Command {
4540

4641
async catch(error: Error & {skipOclifErrorHandling: boolean}): Promise<void> {
4742
error.skipOclifErrorHandling = true
43+
const {errorHandler} = await import('./error-handler.js')
4844
await errorHandler(error, this.config)
4945
return Errors.handle(error)
5046
}
@@ -54,10 +50,12 @@ abstract class BaseCommand extends Command {
5450
setCurrentCommandId(this.id ?? '')
5551
if (!isDevelopment()) {
5652
// This function runs just prior to `run`
53+
const {registerCleanBugsnagErrorsFromWithinPlugins} = await import('./error-handler.js')
5754
await registerCleanBugsnagErrorsFromWithinPlugins(this.config)
5855
}
5956
await removeDuplicatedPlugins(this.config)
6057
this.showNpmFlagWarning()
58+
const {showNotificationsIfNeeded} = await import('./notifications-system.js')
6159
await showNotificationsIfNeeded()
6260
return super.init()
6361
}
@@ -71,13 +69,16 @@ abstract class BaseCommand extends Command {
7169
const possibleNpmEnvVars = commandFlags.map((key) => `npm_config_${underscore(key).replace(/^no_/, '')}`)
7270

7371
if (possibleNpmEnvVars.some((flag) => process.env[flag] !== undefined)) {
74-
renderWarning({
75-
body: [
76-
'NPM scripts require an extra',
77-
{command: '--'},
78-
'separator to pass the flags. Example:',
79-
{command: 'npm run dev -- --reset'},
80-
],
72+
// eslint-disable-next-line no-void
73+
void import('./ui.js').then(({renderWarning}) => {
74+
renderWarning({
75+
body: [
76+
'NPM scripts require an extra',
77+
{command: '--'},
78+
'separator to pass the flags. Example:',
79+
{command: 'npm run dev -- --reset'},
80+
],
81+
})
8182
})
8283
}
8384
}
@@ -143,6 +144,7 @@ This flag is required in non-interactive terminal environments, such as a CI env
143144

144145
if (!environmentsFileName) return originalResult
145146

147+
const {environmentFilePath} = await import('./environments.js')
146148
const environmentFileExists = await environmentFilePath(environmentsFileName, {from: flags.path})
147149

148150
// 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
202204
environmentsFileName: string,
203205
specifiedEnvironment: string | undefined,
204206
): Promise<{environment: JsonMap | undefined; isDefaultEnvironment: boolean}> {
207+
const {loadEnvironment} = await import('./environments.js')
205208
if (specifiedEnvironment) {
206209
const environment = await loadEnvironment(specifiedEnvironment, environmentsFileName, {from: path})
207210
return {environment, isDefaultEnvironment: false}
@@ -254,9 +257,12 @@ function reportEnvironmentApplication<
254257
if (Object.keys(changes).length === 0) return
255258

256259
const items = Object.entries(changes).map(([name, value]) => `${name}: ${value}`)
257-
renderInfo({
258-
headline: ['Using applicable flags from', {userInput: environmentName}, 'environment:'],
259-
body: [{list: {items}}],
260+
// eslint-disable-next-line no-void
261+
void import('./ui.js').then(({renderInfo}) => {
262+
renderInfo({
263+
headline: ['Using applicable flags from', {userInput: environmentName}, 'environment:'],
264+
body: [{list: {items}}],
265+
})
260266
})
261267
}
262268

@@ -342,6 +348,7 @@ async function removeDuplicatedPlugins(config: Config): Promise<void> {
342348
const pluginsToRemove = plugins.filter((plugin) => bundlePlugins.includes(plugin.name))
343349
if (pluginsToRemove.length > 0) {
344350
const commandsToRun = pluginsToRemove.map((plugin) => ` - shopify plugins remove ${plugin.name}`).join('\n')
351+
const {renderWarning} = await import('./ui.js')
345352
renderWarning({
346353
headline: `Unsupported plugins detected: ${pluginsToRemove.map((plugin) => plugin.name).join(', ')}`,
347354
body: [
@@ -351,6 +358,7 @@ async function removeDuplicatedPlugins(config: Config): Promise<void> {
351358
})
352359
}
353360
const filteredPlugins = plugins.filter((plugin) => !bundlePlugins.includes(plugin.name))
361+
// eslint-disable-next-line require-atomic-updates -- config.plugins won't be modified by renderWarning above
354362
config.plugins = new Map(filteredPlugins.map((plugin) => [plugin.name, plugin]))
355363
}
356364

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
import {isTruthy} from './utilities.js'
22
import {getCIMetadata, isSet, Metadata} from '../../../private/node/context/utilities.js'
33
import {defaultThemeKitAccessDomain, environmentVariables, pathConstants} from '../../../private/node/constants.js'
4-
import {fileExists} from '../fs.js'
5-
import {exec} from '../system.js'
6-
74
import isInteractive from 'is-interactive'
85
import macaddress from 'macaddress'
9-
106
import {homedir} from 'os'
117

8+
// Dynamic imports to avoid circular dependency: context/local → fs → output → context/local
9+
// fileExists and exec are only used in specific async functions, not at module init.
10+
async function lazyFileExists(path: string): Promise<boolean> {
11+
const {fileExists} = await import('../fs.js')
12+
return fileExists(path)
13+
}
14+
15+
async function lazyExec(command: string, args: string[]): Promise<void> {
16+
const {exec} = await import('../system.js')
17+
await exec(command, args)
18+
}
19+
1220
/**
1321
* It returns true if the terminal is interactive.
1422
*
@@ -68,7 +76,7 @@ export async function isShopify(env = process.env): Promise<boolean> {
6876
if (Object.prototype.hasOwnProperty.call(env, environmentVariables.runAsUser)) {
6977
return !isTruthy(env[environmentVariables.runAsUser])
7078
}
71-
const devInstalled = await fileExists(pathConstants.executables.dev)
79+
const devInstalled = await lazyFileExists(pathConstants.executables.dev)
7280
return devInstalled
7381
}
7482

@@ -217,7 +225,7 @@ export function cloudEnvironment(env: NodeJS.ProcessEnv = process.env): {
217225
*/
218226
export async function hasGit(): Promise<boolean> {
219227
try {
220-
await exec('git', ['--version'])
228+
await lazyExec('git', ['--version'])
221229
return true
222230
// eslint-disable-next-line no-catch-all/no-catch-all
223231
} catch {

packages/cli-kit/src/public/node/custom-oclif-loader.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ export class ShopifyConfig extends Config {
1717

1818
constructor(options: Options) {
1919
if (isDevelopment()) {
20-
// eslint-disable-next-line @shopify/cli/no-process-cwd
2120
const currentPath = cwd()
2221

2322
let path = sniffForPath() ?? currentPath

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import {AlertCustomSection, renderFatalError} from './ui.js'
21
import {normalizePath} from './path.js'
32
import {OutputMessage, stringifyMessage, TokenizedString} from './output.js'
43
import {InlineToken, TokenItem, tokenItemToString} from '../../private/node/ui/components/TokenizedText.js'
54

65
import {Errors} from '@oclif/core'
76

7+
import type {AlertCustomSection} from './ui.js'
8+
89
export {ExtendableError} from 'ts-error'
910

1011
export enum FatalErrorType {
@@ -146,6 +147,7 @@ export async function handler(error: unknown): Promise<unknown> {
146147
}
147148
}
148149

150+
const {renderFatalError} = await import('./ui.js')
149151
renderFatalError(fatal)
150152
return Promise.resolve(error)
151153
}

packages/cli-kit/src/public/node/hooks/postrun.ts

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,7 @@
1-
import {postrun as deprecationsHook} from './deprecations.js'
2-
import {reportAnalyticsEvent} from '../analytics.js'
3-
import {outputDebug, outputWarn} from '../output.js'
4-
import {getOutputUpdateCLIReminder, runCLIUpgrade, versionToAutoUpgrade, warnIfUpgradeAvailable} from '../upgrade.js'
5-
import {inferPackageManagerForGlobalCLI} from '../is-global.js'
6-
import BaseCommand from '../base-command.js'
7-
import * as metadata from '../metadata.js'
8-
import {runAtMinimumInterval} from '../../../private/node/conf-store.js'
9-
import {CLI_KIT_VERSION} from '../../common/version.js'
10-
import {isMajorVersionChange} from '../version.js'
1+
/**
2+
* Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics)
3+
* at module evaluation time. These are only needed after the command has already finished.
4+
*/
115
import {Command, Hook} from '@oclif/core'
126

137
let postRunHookCompleted = false
@@ -24,9 +18,14 @@ export function postRunHookHasCompleted(): boolean {
2418
// This hook is called after each successful command run. More info: https://oclif.io/docs/hooks
2519
export const hook: Hook.Postrun = async ({config, Command}) => {
2620
await detectStopCommand(Command as unknown as typeof Command)
21+
22+
const {reportAnalyticsEvent} = await import('../analytics.js')
2723
await reportAnalyticsEvent({config, exitMode: 'ok'})
24+
25+
const {postrun: deprecationsHook} = await import('./deprecations.js')
2826
deprecationsHook(Command)
2927

28+
const {outputDebug} = await import('../output.js')
3029
const command = Command.id.replace(/:/g, ' ')
3130
outputDebug(`Completed command ${command}`)
3231
postRunHookCompleted = true
@@ -48,6 +47,7 @@ export const hook: Hook.Postrun = async ({config, Command}) => {
4847
* @returns Resolves when the upgrade attempt (or fallback warning) is complete.
4948
*/
5049
export async function autoUpgradeIfNeeded(): Promise<void> {
50+
const {versionToAutoUpgrade, warnIfUpgradeAvailable} = await import('../upgrade.js')
5151
const newerVersion = versionToAutoUpgrade()
5252
if (!newerVersion) {
5353
await warnIfUpgradeAvailable()
@@ -60,6 +60,7 @@ export async function autoUpgradeIfNeeded(): Promise<void> {
6060
if (forced) {
6161
await performAutoUpgrade(newerVersion)
6262
} else {
63+
const {runAtMinimumInterval} = await import('../../../private/node/conf-store.js')
6364
// Rate-limit the entire upgrade flow to once per day to avoid repeated attempts and major-version warnings.
6465
await runAtMinimumInterval('auto-upgrade', {days: 1}, async () => {
6566
await performAutoUpgrade(newerVersion)
@@ -68,6 +69,18 @@ export async function autoUpgradeIfNeeded(): Promise<void> {
6869
}
6970

7071
async function performAutoUpgrade(newerVersion: string): Promise<void> {
72+
const [
73+
{CLI_KIT_VERSION},
74+
{isMajorVersionChange},
75+
{outputWarn, outputDebug},
76+
{getOutputUpdateCLIReminder, runCLIUpgrade},
77+
] = await Promise.all([
78+
import('../../common/version.js'),
79+
import('../version.js'),
80+
import('../output.js'),
81+
import('../upgrade.js'),
82+
])
83+
7184
if (isMajorVersionChange(CLI_KIT_VERSION, newerVersion)) {
7285
return outputWarn(getOutputUpdateCLIReminder(newerVersion))
7386
}
@@ -79,8 +92,10 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
7992
const errorMessage = `Auto-upgrade failed: ${error}`
8093
outputDebug(errorMessage)
8194
outputWarn(getOutputUpdateCLIReminder(newerVersion))
82-
// Report to Observe as a handled error without showing anything extra to the user
83-
const {sendErrorToBugsnag} = await import('../error-handler.js')
95+
const [{sendErrorToBugsnag}, {inferPackageManagerForGlobalCLI}] = await Promise.all([
96+
import('../error-handler.js'),
97+
import('../is-global.js'),
98+
])
8499
const enrichedError = Object.assign(new Error(errorMessage), {
85100
packageManager: inferPackageManagerForGlobalCLI(),
86101
platform: process.platform,
@@ -93,13 +108,20 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
93108
/**
94109
* Override the command name with the stop one for analytics purposes.
95110
*
96-
* @param commandClass - Oclif command class.
111+
* @param commandClass - Command.Class.
97112
*/
98-
async function detectStopCommand(commandClass: Command.Class | typeof BaseCommand): Promise<void> {
113+
async function detectStopCommand(commandClass: Command.Class): Promise<void> {
99114
const currentTime = new Date().getTime()
100-
if (commandClass && Object.prototype.hasOwnProperty.call(commandClass, 'analyticsStopCommand')) {
101-
const stopCommand = (commandClass as typeof BaseCommand).analyticsStopCommand()
115+
// Check for analyticsStopCommand without importing BaseCommand
116+
if (
117+
commandClass &&
118+
'analyticsStopCommand' in commandClass &&
119+
typeof commandClass.analyticsStopCommand === 'function'
120+
) {
121+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
122+
const stopCommand = (commandClass as any).analyticsStopCommand()
102123
if (stopCommand) {
124+
const metadata = await import('../metadata.js')
103125
const {commandStartOptions} = metadata.getAllSensitiveMetadata()
104126
if (!commandStartOptions) return
105127
await metadata.addSensitiveMetadata(() => ({

0 commit comments

Comments
 (0)