-
Notifications
You must be signed in to change notification settings - Fork 210
Allow theme commands to use glob patterns for the --environment
flag
#6493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2ece2c8
to
78ad480
Compare
Coverage report
Show files with reduced coverage 🔻
Test suite run success3265 tests passing in 1347 suites. Report generated by 🧪jest coverage report action from e4bb2d9 |
49f89d1
to
322d2ef
Compare
const matchedEnvironments = new Set<string>() | ||
|
||
for (const pattern of patterns) { | ||
const isGlob = ENVIRONMENT_ALLOWED_GLOB_PATTERN.test(pattern) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of matching against this we could always parse with minimatch
but this would result in us ignoring the flag if it doesn't find any matches.
While this is open for review I'm going to play around with the consequences of that approach.
322d2ef
to
e6d5cf4
Compare
/snapit |
🫰✨ Thanks @graygilmore! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20251008192534 Caution After installing, validate the version by running just |
Without the quotes we're unable to use globbing patterns like `--only "templates/*"` because the pattern will get expanded as arguments instead of passing `"templates/*"` to the command which may have specific logic to handle those cases.
We'll be adding another function that will need to do this same behavior so let's separate it out now to make the next diff easier to parse.
e6d5cf4
to
e4bb2d9
Compare
/snapit |
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/environments.d.ts@@ -8,9 +8,26 @@ interface LoadEnvironmentOptions {
}
/**
* Loads environments from a file.
- * @param dir - The file path to load environments from.
- * @returns The loaded environments.
+ * @param environmentName - The name of the environment.
+ * @param fileName - The file name to load environments from.
+ * @param options - Optional configuration for loading.
+ * @returns The loaded environment.
*/
export declare function loadEnvironment(environmentName: string, fileName: string, options?: LoadEnvironmentOptions): Promise<JsonMap | undefined>;
export declare function environmentFilePath(fileName: string, options?: LoadEnvironmentOptions): Promise<string | undefined>;
+/**
+ * Gets all available environment names from a file.
+ * @param fileName - The file name to load environments from.
+ * @param options - Optional configuration for loading.
+ * @returns Array of environment names, or empty array if none found.
+ */
+export declare function getEnvironmentNames(fileName: string, options?: LoadEnvironmentOptions): Promise<string[]>;
+/**
+ * Expands environment patterns (including globs) to actual environment names.
+ * @param patterns - Array of environment names or glob patterns.
+ * @param fileName - The file name to load environments from.
+ * @param options - Optional configuration for loading.
+ * @returns Array of matched environment names.
+ */
+export declare function expandEnvironmentPatterns(patterns: string[], fileName: string, options?: LoadEnvironmentOptions): Promise<string[]>;
export {};
\ No newline at end of file
|
🫰✨ Thanks @graygilmore! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20251009161632 Caution After installing, validate the version by running just |
WHY are these changes introduced?
Some theme commands accept multiple environments to be specified but it can be annoying to write them all out if you have many environments that follow a certain pattern.
This PR allows you to specify environment names using glob patterns:
The above example will try to match all environments defined in your
shopify.theme.toml
file that end with-production
. We're support all pattern matching fromminimatch
including these common patterns:*
: Matches zero or more characters?
: Matches exactly one character[]
: Matches any single character within the brackets{}
: Matches any of the comma-separated patternsHow to test your changes?
Install the snapped version:
Try running commands with glob patterns:
With a
shopify.theme.toml
that looks like this:Measuring impact
How do we know this change was effective? Please choose one:
Checklist