Ensure user provided flag values override defaults in multi env commands

[dejmedus]

WHY are these changes introduced?

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

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

WHAT is this pull request doing?

• Creates each environment's flags obj by combining: CLI flags with defaults, shopify.theme.toml flags, and CLI flags without defaults in that order
• Displays a warning if --path flag is provided to the CLI during multi env runs (path defaults to cwd and should be set per environment in shopify.theme.toml)

shopify.theme.toml is respected over default values

defaults-demo.mov

path warning

How to test your changes?

• Add a shopify.theme.toml file with a value that has a default like json = true

Heads up: globals like no-color and verbose do not seem to be respected if they come from shopify.theme.toml single or multi env. I'll look into this in a follow up PR

Example toml

[environments.store1]
theme = "<theme>"
store = "<store>"
password  = "<shptka_password>"
json = true

[environments.store2]
theme = "<theme>"
store = "<store>"
password  = "<shptka_password>"

• Pull down the branch gh pr checkout 6301 or install the snapit version

npm i -g @shopify/[email protected] ---@shopify:registry=https://registry.npmjs.org

• Run list and you should see that the environments with json = true defined in the .toml print out json and other environments don't
• Using--path flag with multi env commands should display an error

shopify theme list -e store1 -e store2

Measuring impact

• Existing analytics will cater for this addition

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟡	Statements	78.61% (+0.04% 🔼)	13509/17185
🟡	Branches	72.6% (+0.08% 🔼)	6578/9061
🟡	Functions	78.83% (+0.01% 🔼)	3512/4455
🟡	Lines	78.97% (+0.04% 🔼)	12770/16170

Show files with reduced coverage 🔻

St.❔	File	Statements	Branches	Functions	Lines
🟢	... / ConcurrentOutput.tsx	98.36% (-1.64% 🔻)	92% (-4% 🔻)	100%	98.33% (-1.67% 🔻)

Test suite run success

3239 tests passing in 1354 suites.

Report generated by 🧪jest coverage report action from cb170bd

[dejmedus]

/snapit

[graygilmore]

Thinking out loud: what if I'm in ~ and I run shopify theme command -e 1 -e2 --path ~/path/to/shopify.theme.toml? I think that's probably the most common use case of including --path so we could potentially change the language of the warning maybe? Some pseudo code here:

if ('path' in flagsWithoutDefaults) {
  if (flagsWithoutDefaults.path === cwd()) {
    renderError({body: "Something about needing to run it in the same directory as the toml"});
    return;
  }

  renderError({body: "Cannot use the `--path` flag with multiple environments. Provide each environment's command path in your shopify.theme.toml file."});
  return
}

[graygilmore]

And maybe use renderError since we can't continue?

[dejmedus]

That makes sense, thanks! I updated the message

[dejmedus]

/snapit

[github-actions]

🫰✨ Thanks @dejmedus! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g @shopify/[email protected]

Tip

If you get an ETARGET error, install it with NPM and the flag --@shopify:registry=https://registry.npmjs.org

Caution

After installing, validate the version by running just shopify in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

[graygilmore]

🎩 went great! Just need a small amount of test coverage and we'll be good to go!

[graygilmore]

Let's make sure we add a couple of tests for the two error scenarios

[github-actions]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/base-command.d.ts

@@ -32,4 +32,17 @@ export declare function addFromParsedFlags(flags: {
     path?: string;
     verbose?: boolean;
 }): Promise<void>;
+/**
+ * Strips the defaults from configured flags. For example, if flags contains:
+ *
+ * 
+ *
+ * it becomes:
+ *
+ * 
+ *
+ * If we parse using this configuration, the only specified flags will be those
+ * the user actually passed on the command line.
+ */
+export declare function noDefaultsOptions<TFlags extends FlagOutput, TGlobalFlags extends FlagOutput, TArgs extends ArgOutput>(options: Input<TFlags, TGlobalFlags, TArgs> | undefined): Input<TFlags, TGlobalFlags, TArgs> | undefined;
 export default BaseCommand;
\ No newline at end of file

[EvilGenius13]

🎩 went well. Thanks for the changes!