Skip to content

Commit cb170bd

Browse files
committed
Warn if path is set via CLI during multi env command runs
The path flag should point to where a command will be run. This defaults to the current working directory While running a command with multiple environment flags, providing a path via the CLI will take precedence over toml and default values and try to run every environment at the same path. This PR prevents this by displaying a warning message
1 parent 1416fe1 commit cb170bd

File tree

2 files changed

+89
-5
lines changed

2 files changed

+89
-5
lines changed

packages/theme/src/cli/utilities/theme-command.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import {Config, Flags} from '@oclif/core'
55
import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
66
import {loadEnvironment} from '@shopify/cli-kit/node/environments'
77
import {renderConcurrent, renderConfirmationPrompt, renderError} from '@shopify/cli-kit/node/ui'
8+
import {fileExistsSync} from '@shopify/cli-kit/node/fs'
89
import type {Writable} from 'stream'
910

1011
vi.mock('@shopify/cli-kit/node/session')
1112
vi.mock('@shopify/cli-kit/node/environments')
1213
vi.mock('@shopify/cli-kit/node/ui')
1314
vi.mock('./theme-store.js')
15+
vi.mock('@shopify/cli-kit/node/fs')
1416

1517
const CommandConfig = new Config({root: __dirname})
1618

@@ -450,6 +452,61 @@ describe('ThemeCommand', () => {
450452
)
451453
})
452454

455+
test('commands should display an error if the --path flag is used', async () => {
456+
// Given
457+
const environmentConfig = {store: 'store.myshopify.com'}
458+
vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig)
459+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
460+
vi.mocked(fileExistsSync).mockReturnValue(true)
461+
462+
await CommandConfig.load()
463+
const command = new TestThemeCommand(
464+
['--environment', 'command-error', '--environment', 'development', '--path', 'path'],
465+
CommandConfig,
466+
)
467+
468+
// When
469+
await command.run()
470+
471+
// Then
472+
expect(renderError).toHaveBeenCalledWith(
473+
expect.objectContaining({
474+
body: [
475+
"Can't use `--path` flag with multiple environments.",
476+
"Configure each environment's theme path in your shopify.theme.toml file instead.",
477+
],
478+
}),
479+
)
480+
})
481+
482+
test('commands should display an error if the --path flag is used and no shopify.theme.toml is found', async () => {
483+
// Given
484+
const environmentConfig = {store: 'store.myshopify.com'}
485+
vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig)
486+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
487+
vi.mocked(fileExistsSync).mockReturnValue(false)
488+
489+
await CommandConfig.load()
490+
const command = new TestThemeCommand(
491+
['--environment', 'command-error', '--environment', 'development', '--path', 'path'],
492+
CommandConfig,
493+
)
494+
495+
// When
496+
await command.run()
497+
498+
// Then
499+
expect(renderError).toHaveBeenCalledWith(
500+
expect.objectContaining({
501+
body: [
502+
"Can't use `--path` flag with multiple environments.",
503+
'Run this command from the directory containing shopify.theme.toml.',
504+
'No shopify.theme.toml found in current directory.',
505+
],
506+
}),
507+
)
508+
})
509+
453510
test('CLI and shopify.theme.toml flag values take precedence over defaults', async () => {
454511
// Given
455512
vi.mocked(loadEnvironment)

packages/theme/src/cli/utilities/theme-command.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
import {AbortController} from '@shopify/cli-kit/node/abort'
1717
import {recordEvent, compileData} from '@shopify/cli-kit/node/analytics'
1818
import {addPublicMetadata, addSensitiveMetadata} from '@shopify/cli-kit/node/metadata'
19+
import {cwd, joinPath} from '@shopify/cli-kit/node/path'
20+
import {fileExistsSync} from '@shopify/cli-kit/node/fs'
1921
import type {Writable} from 'stream'
2022

2123
export interface FlagValues {
@@ -103,7 +105,13 @@ export default abstract class ThemeCommand extends Command {
103105
return
104106
}
105107

106-
const environmentsMap = await this.loadEnvironments(environments, flags, klass)
108+
const {flags: flagsWithoutDefaults} = await this.parse(noDefaultsOptions(klass), this.argv)
109+
if ('path' in flagsWithoutDefaults) {
110+
this.errorOnGlobalPath()
111+
return
112+
}
113+
114+
const environmentsMap = await this.loadEnvironments(environments, flags, flagsWithoutDefaults)
107115
const validationResults = await this.validateEnvironments(environmentsMap, requiredFlags)
108116

109117
const commandAllowsForceFlag = 'force' in klass.flags
@@ -120,16 +128,14 @@ export default abstract class ThemeCommand extends Command {
120128
* Create a map of environments from the shopify.theme.toml file
121129
* @param environments - Names of environments to load
122130
* @param flags - Flags provided via the CLI or by default
123-
* @param klass - The command class
131+
* @param flagsWithoutDefaults - Flags provided via the CLI
124132
* @returns The map of environments
125133
*/
126134
private async loadEnvironments(
127135
environments: EnvironmentName[],
128136
flags: FlagValues,
129-
klass: Input<FlagOutput, FlagOutput, ArgOutput>,
137+
flagsWithoutDefaults: FlagValues,
130138
): Promise<Map<EnvironmentName, FlagValues>> {
131-
const {flags: flagsWithoutDefaults} = await this.parse(noDefaultsOptions(klass))
132-
133139
const environmentMap = new Map<EnvironmentName, FlagValues>()
134140

135141
for (const environmentName of environments) {
@@ -328,6 +334,27 @@ export default abstract class ThemeCommand extends Command {
328334
return true
329335
}
330336

337+
/**
338+
* Error if the --path flag is provided via CLI when running a multi environment command
339+
* Commands that act on local files require each environment to specify its own path in the shopify.theme.toml
340+
*/
341+
private errorOnGlobalPath() {
342+
const tomlPath = joinPath(cwd(), 'shopify.theme.toml')
343+
const tomlInCwd = fileExistsSync(tomlPath)
344+
345+
renderError({
346+
body: [
347+
"Can't use `--path` flag with multiple environments.",
348+
...(tomlInCwd
349+
? ["Configure each environment's theme path in your shopify.theme.toml file instead."]
350+
: [
351+
'Run this command from the directory containing shopify.theme.toml.',
352+
'No shopify.theme.toml found in current directory.',
353+
]),
354+
],
355+
})
356+
}
357+
331358
private async logAnalyticsData(session: AdminSession): Promise<void> {
332359
const data = compileData()
333360

0 commit comments

Comments
 (0)