Ensure user provided flag values override defaults

Previously, default flag values were treated by
multi environment command mapping as CLI values,
which take precedence over .toml values. This commit
ensures that flag values are selected in order:

1. User provided CLI args
2. User provided shopify.theme.toml values
3. Default flag values

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

@@ -27,6 +27,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 +127,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 +159,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 +449,51 @@ describe('ThemeCommand', () => {
         }),
       )
     })
+
+    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 {
@@ -103,7 +103,7 @@ export default abstract class ThemeCommand extends Command {
       return
     }
 
-    const environmentsMap = await this.loadEnvironments(environments, flags)
+    const environmentsMap = await this.loadEnvironments(environments, flags, klass)
     const validationResults = await this.validateEnvironments(environmentsMap, requiredFlags)
 
     const commandAllowsForceFlag = 'force' in klass.flags
@@ -119,13 +119,17 @@ 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 klass - The command class
    * @returns The map of environments
    */
   private async loadEnvironments(
     environments: EnvironmentName[],
     flags: FlagValues,
+    klass: Input<FlagOutput, FlagOutput, ArgOutput>,
   ): Promise<Map<EnvironmentName, FlagValues>> {
+    const {flags: flagsWithoutDefaults} = await this.parse(noDefaultsOptions(klass))
+
     const environmentMap = new Map<EnvironmentName, FlagValues>()
 
     for (const environmentName of environments) {
@@ -136,8 +140,9 @@ export default abstract class ThemeCommand extends Command {
       })
 
       environmentMap.set(environmentName, {
-        ...environmentFlags,
         ...flags,
+        ...environmentFlags,
+        ...flagsWithoutDefaults,
         environment: [environmentName],
       })
     }