Merge pull request #6301 from Shopify/jb-multi-env-default-flags

Ensure user provided flag values override defaults in multi env commands

Author: dejmedus

.changeset/shy-islands-guess.md

@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': minor
+'@shopify/theme': minor
+---
+
+Ensure user provided flag values override defaults in commands with multiple environments

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

@@ -281,7 +281,7 @@ function reportEnvironmentApplication<
  * If we parse using this configuration, the only specified flags will be those
  * the user actually passed on the command line.
  */
-function noDefaultsOptions<TFlags extends FlagOutput, TGlobalFlags extends FlagOutput, TArgs extends ArgOutput>(
+export function noDefaultsOptions<TFlags extends FlagOutput, TGlobalFlags extends FlagOutput, TArgs extends ArgOutput>(
   options: Input<TFlags, TGlobalFlags, TArgs> | undefined,
 ): Input<TFlags, TGlobalFlags, TArgs> | undefined {
   if (!options?.flags) return options

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

@@ -5,12 +5,14 @@ import {Config, Flags} from '@oclif/core'
 import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
 import {loadEnvironment} from '@shopify/cli-kit/node/environments'
 import {renderConcurrent, renderConfirmationPrompt, renderError} from '@shopify/cli-kit/node/ui'
+import {fileExistsSync} from '@shopify/cli-kit/node/fs'
 import type {Writable} from 'stream'
 
 vi.mock('@shopify/cli-kit/node/session')
 vi.mock('@shopify/cli-kit/node/environments')
 vi.mock('@shopify/cli-kit/node/ui')
 vi.mock('./theme-store.js')
+vi.mock('@shopify/cli-kit/node/fs')
 
 const CommandConfig = new Config({root: __dirname})
 
@@ -27,6 +29,14 @@ class TestThemeCommand extends ThemeCommand {
     password: Flags.string({
       env: 'SHOPIFY_FLAG_PASSWORD',
     }),
+    path: Flags.string({
+      env: 'SHOPIFY_FLAG_PATH',
+      default: 'current/working/directory',
+    }),
+    'no-color': Flags.boolean({
+      env: 'SHOPIFY_FLAG_NO_COLOR',
+      default: false,
+    }),
   }
 
   static multiEnvironmentsFlags: RequiredFlags = ['store']
@@ -119,7 +129,9 @@ describe('ThemeCommand', () => {
       await command.run()
 
       // Then
-      expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', {from: undefined})
+      expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', {
+        from: 'current/working/directory',
+      })
       expect(ensureAuthenticatedThemes).toHaveBeenCalledTimes(1)
       expect(renderConcurrent).not.toHaveBeenCalled()
       expect(command.commandCalls).toHaveLength(1)
@@ -149,8 +161,14 @@ describe('ThemeCommand', () => {
       await command.run()
 
       // Then
-      expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', {from: undefined, silent: true})
-      expect(loadEnvironment).toHaveBeenCalledWith('staging', 'shopify.theme.toml', {from: undefined, silent: true})
+      expect(loadEnvironment).toHaveBeenCalledWith('development', 'shopify.theme.toml', {
+        from: 'current/working/directory',
+        silent: true,
+      })
+      expect(loadEnvironment).toHaveBeenCalledWith('staging', 'shopify.theme.toml', {
+        from: 'current/working/directory',
+        silent: true,
+      })
 
       expect(renderConcurrent).toHaveBeenCalledOnce()
       expect(renderConcurrent).toHaveBeenCalledWith(
@@ -433,5 +451,106 @@ describe('ThemeCommand', () => {
         }),
       )
     })
+
+    test('commands should display an error if the --path flag is used', async () => {
+      // Given
+      const environmentConfig = {store: 'store.myshopify.com'}
+      vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig)
+      vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
+      vi.mocked(fileExistsSync).mockReturnValue(true)
+
+      await CommandConfig.load()
+      const command = new TestThemeCommand(
+        ['--environment', 'command-error', '--environment', 'development', '--path', 'path'],
+        CommandConfig,
+      )
+
+      // When
+      await command.run()
+
+      // Then
+      expect(renderError).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: [
+            "Can't use `--path` flag with multiple environments.",
+            "Configure each environment's theme path in your shopify.theme.toml file instead.",
+          ],
+        }),
+      )
+    })
+
+    test('commands should display an error if the --path flag is used and no shopify.theme.toml is found', async () => {
+      // Given
+      const environmentConfig = {store: 'store.myshopify.com'}
+      vi.mocked(loadEnvironment).mockResolvedValue(environmentConfig)
+      vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
+      vi.mocked(fileExistsSync).mockReturnValue(false)
+
+      await CommandConfig.load()
+      const command = new TestThemeCommand(
+        ['--environment', 'command-error', '--environment', 'development', '--path', 'path'],
+        CommandConfig,
+      )
+
+      // When
+      await command.run()
+
+      // Then
+      expect(renderError).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: [
+            "Can't use `--path` flag with multiple environments.",
+            'Run this command from the directory containing shopify.theme.toml.',
+            'No shopify.theme.toml found in current directory.',
+          ],
+        }),
+      )
+    })
+
+    test('CLI and shopify.theme.toml flag values take precedence over defaults', async () => {
+      // Given
+      vi.mocked(loadEnvironment)
+        .mockResolvedValueOnce({store: 'store1.myshopify.com', theme: 'theme1.myshopify.com', path: 'theme/path'})
+        .mockResolvedValueOnce({store: 'store2.myshopify.com', development: true, path: 'development/path'})
+        .mockResolvedValueOnce({store: 'store3.myshopify.com', live: true, 'no-color': false})
+
+      vi.mocked(renderConcurrent).mockImplementation(async ({processes}) => {
+        for (const process of processes) {
+          // eslint-disable-next-line no-await-in-loop
+          await process.action({} as Writable, {} as Writable, {} as any)
+        }
+      })
+
+      await CommandConfig.load()
+      const command = new TestThemeCommand(
+        ['--environment', 'theme', '--environment', 'development', '--environment', 'live', '--no-color'],
+        CommandConfig,
+      )
+
+      // When
+      await command.run()
+
+      // Then
+      const commandCalls = command.commandCalls
+      expect(commandCalls).toHaveLength(3)
+
+      const themeEnvFlags = commandCalls[0]?.flags
+      expect(themeEnvFlags?.path).toEqual('theme/path')
+      expect(themeEnvFlags?.store).toEqual('store1.myshopify.com')
+      expect(themeEnvFlags?.theme).toEqual('theme1.myshopify.com')
+      expect(themeEnvFlags?.['no-color']).toEqual(true)
+
+      const developmentEnvFlags = commandCalls[1]?.flags
+      expect(developmentEnvFlags?.path).toEqual('development/path')
+      expect(developmentEnvFlags?.store).toEqual('store2.myshopify.com')
+      expect(developmentEnvFlags?.development).toEqual(true)
+      expect(developmentEnvFlags?.['no-color']).toEqual(true)
+
+      const liveEnvFlags = commandCalls[2]?.flags
+      expect(liveEnvFlags?.path).toEqual('current/working/directory')
+      expect(liveEnvFlags?.store).toEqual('store3.myshopify.com')
+      expect(liveEnvFlags?.live).toEqual(true)
+      expect(liveEnvFlags?.['no-color']).toEqual(true)
+    })
   })
 })

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

@@ -3,7 +3,7 @@ import {configurationFileName} from '../constants.js'
 import {useThemeStoreContext} from '../services/local-storage.js'
 import {hashString} from '@shopify/cli-kit/node/crypto'
 import {Input} from '@oclif/core/interfaces'
-import Command, {ArgOutput, FlagOutput} from '@shopify/cli-kit/node/base-command'
+import Command, {ArgOutput, FlagOutput, noDefaultsOptions} from '@shopify/cli-kit/node/base-command'
 import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
 import {loadEnvironment} from '@shopify/cli-kit/node/environments'
 import {
@@ -16,6 +16,8 @@ import {
 import {AbortController} from '@shopify/cli-kit/node/abort'
 import {recordEvent, compileData} from '@shopify/cli-kit/node/analytics'
 import {addPublicMetadata, addSensitiveMetadata} from '@shopify/cli-kit/node/metadata'
+import {cwd, joinPath} from '@shopify/cli-kit/node/path'
+import {fileExistsSync} from '@shopify/cli-kit/node/fs'
 import type {Writable} from 'stream'
 
 export interface FlagValues {
@@ -103,7 +105,13 @@ export default abstract class ThemeCommand extends Command {
       return
     }
 
-    const environmentsMap = await this.loadEnvironments(environments, flags)
+    const {flags: flagsWithoutDefaults} = await this.parse(noDefaultsOptions(klass), this.argv)
+    if ('path' in flagsWithoutDefaults) {
+      this.errorOnGlobalPath()
+      return
+    }
+
+    const environmentsMap = await this.loadEnvironments(environments, flags, flagsWithoutDefaults)
     const validationResults = await this.validateEnvironments(environmentsMap, requiredFlags)
 
     const commandAllowsForceFlag = 'force' in klass.flags
@@ -119,12 +127,14 @@ export default abstract class ThemeCommand extends Command {
   /**
    * Create a map of environments from the shopify.theme.toml file
    * @param environments - Names of environments to load
-   * @param flags - Flags provided via the CLI
+   * @param flags - Flags provided via the CLI or by default
+   * @param flagsWithoutDefaults - Flags provided via the CLI
    * @returns The map of environments
    */
   private async loadEnvironments(
     environments: EnvironmentName[],
     flags: FlagValues,
+    flagsWithoutDefaults: FlagValues,
   ): Promise<Map<EnvironmentName, FlagValues>> {
     const environmentMap = new Map<EnvironmentName, FlagValues>()
 
@@ -136,8 +146,9 @@ export default abstract class ThemeCommand extends Command {
       })
 
       environmentMap.set(environmentName, {
-        ...environmentFlags,
         ...flags,
+        ...environmentFlags,
+        ...flagsWithoutDefaults,
         environment: [environmentName],
       })
     }
@@ -323,6 +334,27 @@ export default abstract class ThemeCommand extends Command {
     return true
   }
 
+  /**
+   * Error if the --path flag is provided via CLI when running a multi environment command
+   * Commands that act on local files require each environment to specify its own path in the shopify.theme.toml
+   */
+  private errorOnGlobalPath() {
+    const tomlPath = joinPath(cwd(), 'shopify.theme.toml')
+    const tomlInCwd = fileExistsSync(tomlPath)
+
+    renderError({
+      body: [
+        "Can't use `--path` flag with multiple environments.",
+        ...(tomlInCwd
+          ? ["Configure each environment's theme path in your shopify.theme.toml file instead."]
+          : [
+              'Run this command from the directory containing shopify.theme.toml.',
+              'No shopify.theme.toml found in current directory.',
+            ]),
+      ],
+    })
+  }
+
   private async logAnalyticsData(session: AdminSession): Promise<void> {
     const data = compileData()