Skip to content

Commit 1f4fd78

Browse files
committed
Allow multi-env theme commands to accept CLI flags
Previously required multi-environment command flags were only taken from the `shopify.theme.toml` file. This commit updates ThemeCommand to accept flags from the CLI as well, with CLI flags taking precedence.
1 parent 1d63429 commit 1f4fd78

File tree

3 files changed

+199
-42
lines changed

3 files changed

+199
-42
lines changed

.changeset/breezy-papayas-watch.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/theme': minor
3+
'@shopify/cli': minor
4+
---
5+
6+
Allow multi-environment theme commands to accept flags from CLI

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

Lines changed: 101 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {describe, vi, expect, test, beforeEach} from 'vitest'
33
import {Config, Flags} from '@oclif/core'
44
import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
55
import {loadEnvironment} from '@shopify/cli-kit/node/environments'
6-
import {renderConcurrent} from '@shopify/cli-kit/node/ui'
6+
import {renderConcurrent, renderConfirmationPrompt, renderError} from '@shopify/cli-kit/node/ui'
77
import type {Writable} from 'stream'
88

99
vi.mock('@shopify/cli-kit/node/session')
@@ -29,10 +29,19 @@ class TestThemeCommand extends ThemeCommand {
2929

3030
static multiEnvironmentsFlags = ['store']
3131

32-
commandCalls: {flags: any; session: AdminSession; context?: any}[] = []
32+
commandCalls: {flags: any; session: AdminSession; multiEnvironment?: boolean; context?: any}[] = []
3333

34-
async command(flags: any, session: AdminSession, context?: {stdout?: Writable; stderr?: Writable}): Promise<void> {
35-
this.commandCalls.push({flags, session, context})
34+
async command(
35+
flags: any,
36+
session: AdminSession,
37+
multiEnvironment?: boolean,
38+
context?: {stdout?: Writable; stderr?: Writable},
39+
): Promise<void> {
40+
this.commandCalls.push({flags, session, multiEnvironment, context})
41+
42+
if (flags.environment && flags.environment[0] === 'command-error') {
43+
throw new Error('Mocking a command error')
44+
}
3645
}
3746
}
3847

@@ -125,4 +134,92 @@ describe('ThemeCommand', () => {
125134
)
126135
})
127136
})
137+
138+
describe('multi environment', () => {
139+
test('should not execute commands in environments that are missing required flags', async () => {
140+
// Given
141+
vi.mocked(loadEnvironment)
142+
.mockResolvedValueOnce({store: 'store1.myshopify.com'})
143+
.mockResolvedValueOnce({})
144+
.mockResolvedValueOnce({store: 'store3.myshopify.com'})
145+
146+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
147+
vi.mocked(renderConcurrent).mockResolvedValue(undefined)
148+
149+
await CommandConfig.load()
150+
const command = new TestThemeCommand(
151+
['--environment', 'development', '--environment', 'env-missing-store', '--environment', 'production'],
152+
CommandConfig,
153+
)
154+
155+
// When
156+
await command.run()
157+
158+
// Then
159+
const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
160+
expect(renderConcurrentProcesses).toHaveLength(2)
161+
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development', 'production'])
162+
})
163+
164+
test('commands error gracefully and continue with other environments', async () => {
165+
// Given
166+
vi.mocked(loadEnvironment).mockResolvedValue({store: 'store.myshopify.com'})
167+
168+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
169+
vi.mocked(renderConcurrent).mockImplementation(async ({processes}) => {
170+
for (const process of processes) {
171+
// eslint-disable-next-line no-await-in-loop
172+
await process.action({} as Writable, {} as Writable, {} as any)
173+
}
174+
})
175+
176+
await CommandConfig.load()
177+
const command = new TestThemeCommand(
178+
['--environment', 'command-error', '--environment', 'development', '--environment', 'production'],
179+
CommandConfig,
180+
)
181+
182+
// When
183+
await command.run()
184+
185+
// Then
186+
const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
187+
expect(renderConcurrentProcesses).toHaveLength(3)
188+
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual([
189+
'command-error',
190+
'development',
191+
'production',
192+
])
193+
})
194+
195+
test('error messages contain the environment name', async () => {
196+
// Given
197+
const environmentConfig = {store: 'store.myshopify.com'}
198+
vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig)
199+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
200+
201+
vi.mocked(renderConcurrent).mockImplementation(async ({processes}) => {
202+
for (const process of processes) {
203+
// eslint-disable-next-line no-await-in-loop
204+
await process.action({} as Writable, {} as Writable, {} as any)
205+
}
206+
})
207+
208+
await CommandConfig.load()
209+
const command = new TestThemeCommand(
210+
['--environment', 'command-error', '--environment', 'development'],
211+
CommandConfig,
212+
)
213+
214+
// When
215+
await command.run()
216+
217+
// Then
218+
expect(renderError).toHaveBeenCalledWith(
219+
expect.objectContaining({
220+
body: ['Environment command-error failed: \n\nMocking a command error'],
221+
}),
222+
)
223+
})
224+
})
128225
})

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

Lines changed: 92 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import {Input} from '@oclif/core/interfaces'
44
import Command, {ArgOutput, FlagOutput} from '@shopify/cli-kit/node/base-command'
55
import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
66
import {loadEnvironment} from '@shopify/cli-kit/node/environments'
7-
import {renderWarning, renderConcurrent} from '@shopify/cli-kit/node/ui'
8-
import {AbortError} from '@shopify/cli-kit/node/error'
7+
import {renderWarning, renderConcurrent, renderError} from '@shopify/cli-kit/node/ui'
98
import {AbortController} from '@shopify/cli-kit/node/abort'
109
import type {Writable} from 'stream'
1110

@@ -16,6 +15,7 @@ interface PassThroughFlagsOptions {
1615
// Only pass on flags that are relevant to CLI2
1716
allowedFlags?: string[]
1817
}
18+
type EnvironmentName = string
1919

2020
export default abstract class ThemeCommand extends Command {
2121
passThroughFlags(flags: FlagValues, {allowedFlags}: PassThroughFlagsOptions): string[] {
@@ -41,6 +41,7 @@ export default abstract class ThemeCommand extends Command {
4141
async command(
4242
_flags: FlagValues,
4343
_session: AdminSession,
44+
_multiEnvironment = false,
4445
_context?: {stdout?: Writable; stderr?: Writable},
4546
): Promise<void> {}
4647

@@ -60,72 +61,125 @@ export default abstract class ThemeCommand extends Command {
6061

6162
// Single environment or no environment
6263
if (environments.length <= 1) {
63-
const session = await this.ensureAuthenticated(flags)
64+
const session = await this.createSession(flags)
65+
6466
await this.command(flags, session)
6567
return
6668
}
6769

6870
// Multiple environments
69-
const sessions: {[storeFqdn: string]: AdminSession} = {}
71+
const environmentsMap = await this.loadEnvironments(environments, flags)
72+
const validationResults = await this.validateEnvironments(environmentsMap, requiredFlags)
73+
74+
await this.runConcurrent(validationResults.valid)
75+
}
76+
77+
/**
78+
* Create a map of environments from the shopify.theme.toml file
79+
* @param environments - Names of environments to load
80+
* @param flags - Flags provided via the CLI
81+
* @returns The map of environments
82+
*/
83+
private async loadEnvironments(
84+
environments: EnvironmentName[],
85+
flags: FlagValues,
86+
): Promise<Map<EnvironmentName, FlagValues>> {
87+
const environmentMap = new Map<EnvironmentName, FlagValues>()
7088

71-
// Authenticate on all environments sequentially to avoid race conditions,
72-
// with authentication happening in parallel.
7389
for (const environmentName of environments) {
7490
// eslint-disable-next-line no-await-in-loop
75-
const environmentConfig = await loadEnvironment(environmentName, 'shopify.theme.toml', {
76-
from: flags.path,
91+
const environmentFlags = await loadEnvironment(environmentName, 'shopify.theme.toml', {
92+
from: flags.path as string,
7793
silent: true,
7894
})
95+
96+
environmentMap.set(environmentName, {
97+
...environmentFlags,
98+
...flags,
99+
environment: [environmentName],
100+
})
101+
}
102+
103+
return environmentMap
104+
}
105+
106+
/**
107+
* Split environments into valid and invalid based on flags
108+
* @param environmentMap - The map of environments to validate
109+
* @param requiredFlags - The required flags to check for
110+
* @returns An object containing valid and invalid environment arrays
111+
*/
112+
private async validateEnvironments(environmentMap: Map<EnvironmentName, FlagValues>, requiredFlags: string[]) {
113+
const valid: {environment: EnvironmentName; flags: FlagValues; session: AdminSession}[] = []
114+
const invalid: {environment: EnvironmentName; reason: string}[] = []
115+
116+
for (const [environmentName, environmentFlags] of environmentMap) {
79117
// eslint-disable-next-line no-await-in-loop
80-
sessions[environmentName] = await this.ensureAuthenticated(environmentConfig as FlagValues)
118+
const session = await this.createSession(environmentFlags)
119+
120+
const validationResult = this.validConfig(environmentFlags, requiredFlags, environmentName)
121+
if (validationResult !== true) {
122+
const missingFlagsText = validationResult.join(', ')
123+
invalid.push({environment: environmentName, reason: `Missing flags: ${missingFlagsText}`})
124+
continue
125+
}
126+
127+
valid.push({environment: environmentName, flags: environmentFlags, session})
81128
}
82129

83-
// Use renderConcurrent for multi-environment execution
130+
return {valid, invalid}
131+
}
132+
133+
/**
134+
* Run the command in each valid environment concurrently
135+
* @param validEnvironments - The valid environments to run the command in
136+
*/
137+
private async runConcurrent(
138+
validEnvironments: {environment: EnvironmentName; flags: FlagValues; session: AdminSession}[],
139+
) {
84140
const abortController = new AbortController()
85141

86142
await renderConcurrent({
87-
processes: environments.map((environment: string) => ({
143+
processes: validEnvironments.map(({environment, flags, session}) => ({
88144
prefix: environment,
89145
action: async (stdout: Writable, stderr: Writable, _signal) => {
90-
const environmentConfig = await loadEnvironment(environment, 'shopify.theme.toml', {
91-
from: flags.path,
92-
silent: true,
93-
})
94-
const environmentFlags = {
95-
...flags,
96-
...environmentConfig,
97-
environment: [environment],
146+
try {
147+
await this.command(flags, session, true, {stdout, stderr})
148+
149+
// eslint-disable-next-line no-catch-all/no-catch-all
150+
} catch (error) {
151+
if (error instanceof Error) {
152+
error.message = `Environment ${environment} failed: \n\n${error.message}`
153+
renderError({body: [error.message]})
154+
}
98155
}
99-
100-
if (!this.validConfig(environmentConfig as FlagValues, requiredFlags, environment)) return
101-
102-
const session = sessions[environment]
103-
104-
if (!session) {
105-
throw new AbortError(`No session found for environment ${environment}`)
106-
}
107-
108-
await this.command(environmentFlags, session, {stdout, stderr})
109156
},
110157
})),
111158
abortSignal: abortController.signal,
112159
showTimestamps: true,
113160
})
114161
}
115162

116-
private async ensureAuthenticated(flags: FlagValues) {
163+
/**
164+
* Create an authenticated session object from the flags
165+
* @param flags - The environment flags containing store and password
166+
* @returns The unauthenticated session object
167+
*/
168+
private async createSession(flags: FlagValues) {
117169
const store = flags.store as string
118170
const password = flags.password as string
119171
return ensureAuthenticatedThemes(ensureThemeStore({store}), password)
120172
}
121173

122-
private validConfig(environmentConfig: FlagValues, requiredFlags: string[], environmentName: string): boolean {
123-
if (!environmentConfig) {
124-
renderWarning({body: 'Environment configuration is empty.'})
125-
return false
126-
}
127-
const required = [...requiredFlags]
128-
const missingFlags = required.filter((flag) => !environmentConfig[flag])
174+
/**
175+
* Ensure that all required flags are present
176+
* @param environmentFlags - The environment flags
177+
* @param requiredFlags - The flags required by the command
178+
* @param environmentName - The name of the environment
179+
* @returns The missing flags or true if the environment has all required flags
180+
*/
181+
private validConfig(environmentFlags: FlagValues, requiredFlags: string[], environmentName: string): string[] | true {
182+
const missingFlags = requiredFlags.filter((flag) => !environmentFlags[flag])
129183

130184
if (missingFlags.length > 0) {
131185
renderWarning({
@@ -134,7 +188,7 @@ export default abstract class ThemeCommand extends Command {
134188
{list: {items: missingFlags}},
135189
],
136190
})
137-
return false
191+
return missingFlags
138192
}
139193

140194
return true

0 commit comments

Comments
 (0)