Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions bin/bundling/esbuild-plugin-graphiql-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@ const GraphiQLImportsPlugin = {
setup(build) {
// GraphiQL uses require.resolve with paths that won't work with esbuild
// We need to replace them with valid paths
build.onLoad({filter: /.*server\.js/}, async (args) => {
// graphiql/server.ts uses require.resolve('@shopify/app/assets/...'). The bundled CLI does not ship
// @shopify/app as a dependency; assets are copied to dist/assets. Rewrite to paths relative to bundled
// command files under dist/cli/commands/** (e.g. app/dev.js -> ../../../assets/graphiql/...).
build.onLoad({filter: /[/\\]graphiql[/\\]server\.[cm]?[jt]s$/}, async (args) => {
const contents = await readFile(args.path, 'utf8')
// When `contents` is returned, esbuild defaults the loader to `js` unless set — TypeScript would then
// fail to parse (e.g. "Expected ')' but found ':'" on parameter type annotations).
const loader = args.path.endsWith('.tsx') ? 'tsx' : 'ts'
return {
loader,
contents: contents
.replace('@shopify/app/assets/graphiql/favicon.ico', './assets/graphiql/favicon.ico')
.replace('@shopify/app/assets/graphiql/style.css', './assets/graphiql/style.css'),
.replace('@shopify/app/assets/graphiql/favicon.ico', '../../../assets/graphiql/favicon.ico')
.replace('@shopify/app/assets/graphiql/style.css', '../../../assets/graphiql/style.css'),
}
})
},
Expand Down
28 changes: 20 additions & 8 deletions packages/cli-kit/src/public/node/base-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> = {}
Expand Down Expand Up @@ -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])
Expand All @@ -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])
Expand Down
38 changes: 23 additions & 15 deletions packages/cli-kit/src/public/node/base-command.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -45,6 +40,7 @@ abstract class BaseCommand extends Command {

async catch(error: Error & {skipOclifErrorHandling: boolean}): Promise<void> {
error.skipOclifErrorHandling = true
const {errorHandler} = await import('./error-handler.js')
await errorHandler(error, this.config)
return Errors.handle(error)
}
Expand All @@ -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()
}
Expand All @@ -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'},
],
})
})
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}}],
})
})
}

Expand Down Expand Up @@ -342,6 +348,7 @@ async function removeDuplicatedPlugins(config: Config): Promise<void> {
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: [
Expand All @@ -351,6 +358,7 @@ async function removeDuplicatedPlugins(config: Config): Promise<void> {
})
}
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]))
}

Expand Down
20 changes: 14 additions & 6 deletions packages/cli-kit/src/public/node/context/local.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
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 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<boolean> {
const {fileExists} = await import('../fs.js')
return fileExists(path)
}

async function lazyExec(command: string, args: string[]): Promise<void> {
const {exec} = await import('../system.js')
await exec(command, args)
}

/**
* It returns true if the terminal is interactive.
*
Expand Down Expand Up @@ -67,7 +75,7 @@ export async function isShopify(env = process.env): Promise<boolean> {
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
}

Expand Down Expand Up @@ -216,7 +224,7 @@ export function cloudEnvironment(env: NodeJS.ProcessEnv = process.env): {
*/
export async function hasGit(): Promise<boolean> {
try {
await exec('git', ['--version'])
await lazyExec('git', ['--version'])
return true
// eslint-disable-next-line no-catch-all/no-catch-all
} catch {
Expand Down
4 changes: 3 additions & 1 deletion packages/cli-kit/src/public/node/error.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {AlertCustomSection, renderFatalError} 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'

import {Errors} from '@oclif/core'

import type {AlertCustomSection} from './ui.js'

export {ExtendableError} from 'ts-error'

export enum FatalErrorType {
Expand Down Expand Up @@ -146,6 +147,7 @@ export async function handler(error: unknown): Promise<unknown> {
}
}

const {renderFatalError} = await import('./ui.js')
renderFatalError(fatal)
return Promise.resolve(error)
}
Expand Down
55 changes: 40 additions & 15 deletions packages/cli-kit/src/public/node/hooks/postrun.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
import {postrun as deprecationsHook} from './deprecations.js'
import {reportAnalyticsEvent} from '../analytics.js'
import {outputDebug, outputWarn} from '../output.js'
import {getOutputUpdateCLIReminder, runCLIUpgrade, versionToAutoUpgrade, warnIfUpgradeAvailable} from '../upgrade.js'
import {inferPackageManagerForGlobalCLI} from '../is-global.js'
import BaseCommand from '../base-command.js'
import * as metadata from '../metadata.js'
import {runAtMinimumInterval} from '../../../private/node/conf-store.js'
import {CLI_KIT_VERSION} from '../../common/version.js'
import {isMajorVersionChange} from '../version.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
Expand All @@ -24,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
Expand All @@ -41,6 +40,7 @@ export const hook: Hook.Postrun = async ({config, Command}) => {
* @returns Resolves when the upgrade attempt (or fallback warning) is complete.
*/
export async function autoUpgradeIfNeeded(): Promise<void> {
const {versionToAutoUpgrade, warnIfUpgradeAvailable} = await import('../upgrade.js')
const newerVersion = versionToAutoUpgrade()
if (!newerVersion) {
await warnIfUpgradeAvailable()
Expand All @@ -53,6 +53,7 @@ export async function autoUpgradeIfNeeded(): Promise<void> {
if (forced) {
await performAutoUpgrade(newerVersion)
} else {
const {runAtMinimumInterval} = await import('../../../private/node/conf-store.js')
// Rate-limit the entire upgrade flow to once per day to avoid repeated attempts and major-version warnings.
await runAtMinimumInterval('auto-upgrade', {days: 1}, async () => {
await performAutoUpgrade(newerVersion)
Expand All @@ -61,6 +62,20 @@ export async function autoUpgradeIfNeeded(): Promise<void> {
}

async function performAutoUpgrade(newerVersion: string): Promise<void> {
const [
{CLI_KIT_VERSION},
{isMajorVersionChange},
{outputWarn, outputDebug},
{getOutputUpdateCLIReminder, runCLIUpgrade},
metadata,
] = await Promise.all([
import('../../common/version.js'),
import('../version.js'),
import('../output.js'),
import('../upgrade.js'),
import('../metadata.js'),
])

if (isMajorVersionChange(CLI_KIT_VERSION, newerVersion)) {
outputWarn(getOutputUpdateCLIReminder(newerVersion, true))
await metadata.addPublicMetadata(() => ({
Expand All @@ -79,7 +94,10 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
outputWarn(getOutputUpdateCLIReminder(newerVersion))
await metadata.addPublicMetadata(() => ({env_auto_upgrade_success: false}))
// Report to Observe as a handled error without showing anything extra to the user
const {sendErrorToBugsnag} = await import('../error-handler.js')
const [{sendErrorToBugsnag}, {inferPackageManagerForGlobalCLI}] = await Promise.all([
import('../error-handler.js'),
import('../is-global.js'),
])
const enrichedError = Object.assign(new Error(errorMessage), {
packageManager: inferPackageManagerForGlobalCLI(),
platform: process.platform,
Expand All @@ -92,13 +110,20 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
/**
* 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<void> {
async function detectStopCommand(commandClass: Command.Class): Promise<void> {
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(() => ({
Expand Down
Loading
Loading