Add `--env` & deprecate `--env-branch` flag for Hydrogen CLI commands #1841

aswamy · 2024-03-12T19:05:42Z

WHY are these changes introduced?

Adding --env flag to standardize how we refer to environments in the CLI
- pass in an environment's handle you can view from running env list
- remove existing --env flag which was used to identify environment by name (used only by env push__unstable)
- deprecate --env-branch from env pull, env push__unstable, deploy, preview, dev-vite, dev

WHAT is this pull request doing?

HOW to test your changes?

checkout branch
npm ci && cd packages/cli & npm run build
If you don't have a hydrogen storefront project, npx shopify hydrogen init and connect it a repo on Admin
In packages/cli test the following commands:
- view handles: npx shopify hydrogen env list --path=/path/to/repo
- deploy without specifying env handle: npx shopify hydrogen deploy --path=/path/to/repo
- deploy by specifying env handle: npx shopify hydrogen deploy --path=/path/to/repo --env-handle=production
- deploy by specifying a bad env handle: npx shopify hydrogen deploy --path=/path/to/repo --env-handle=fake
- pull environment variables for preview: npx shopify hydrogen env pull --path=/path/to/repo
- pull environment variables for production: npx shopify hydrogen env pull --path=/path/to/repo --env-handle=production
- pull environment variables using bad env handle: npx shopify hydrogen env pull --path=/path/to/repo --env-handle=fake
- Add an environment variable in the .env file in the storefront project
- push environment variables for preview: npx shopify hydrogen env push__unstable --path=/path/to/repo
- push environment variables for production: npx shopify hydrogen env push__unstable --path=/path/to/repo --env-handle=production
- push environment variables using bad env handle: npx shopify hydrogen env push__unstable --path=/path/to/repo --env-handle=fake

Screenshots

env list

env pull

env push__unstable

deploy

Post-merge steps

Checklist

I've read the Contributing Guidelines
I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've added a changeset if this PR contains user-facing or noteworthy changes
I've added tests to cover my changes
I've added or updated the documentation
- This will be a follow-up PR

shopify · 2024-03-12T19:15:09Z

Oxygen deployed a preview of your add-env-handle-flag branch. Details:

Storefront	Status	Preview link	Deployment details	Last update (UTC)
third-party-queries-caching	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM
custom-cart-method	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM
vite	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM
subscriptions	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM
optimistic-cart-ui	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM
skeleton	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 22, 2024 7:25 PM

Learn more about Hydrogen's GitHub integration.

graygilmore

Initial thoughts

packages/cli/src/commands/hydrogen/deploy.ts

packages/cli/src/lib/flags.ts

packages/cli/src/commands/hydrogen/env/push__unstable.ts

packages/cli/src/lib/common.ts

packages/cli/src/commands/hydrogen/env/list.ts

aswamy · 2024-03-14T20:31:26Z

Force Push

Re-added --envBranch to Hydrogen CLI commands
Rename --envHandle to --env
Ensure user can't pass in --envBranch and --env at the same time
Remove -e shorthand for --envBranch flag
Changed description for --envBranch flag so it's more generic
The following commands will now allow you to pass in --envBranch or --env: env pull, env push__unstable, deploy, dev, dev-vite, preview

What happens when you pass in both flags?

An error is shown

What happens when you pass in the deprecated --envBranch flag?

You see a warning and get directed towards the newer flag

What happens when you pass in --env flag?

You see not warning

Where can i see the handles?

Run env list

graygilmore

Looking good! Did a whole bunch of 🎩ing and commented on a couple of things that popped up for me.

.changeset/lovely-donkeys-destroy.md

packages/cli/src/commands/hydrogen/env/list.ts

packages/cli/src/lib/common.ts

graygilmore · 2024-03-14T21:05:06Z

packages/cli/src/commands/hydrogen/deploy.ts

+    environmentTag = findEnvironmentByBranchOrThrow(
+      deploymentData?.environments || [],
+      envBranch,
+    ).branch;


This is a change in behavior that I think makes sense but could also be considered a breaking change. This change would mean that you cannot use the --env-branch flag in CI. Prefer to support the existing behavior and let envHandle be the new flow.

Could tackle this a few different ways so you've got options!

(deploymentData?.environments is always undefined in CI so this will always throw)

oh dang... this means that --env-handle wouldn't work in CI as well... OxygenCLI only understands branch names (not handles).

Prefer to support the existing behavior and let envHandle be the new flow.

Does this mean that --env-handle should exclusively be for non-CI?

Does this mean that --env-handle should exclusively be for non-CI?

exactly!

graygilmore · 2024-03-14T21:16:08Z

packages/cli/src/lib/flags.ts

+      env: 'SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH',
+      deprecated: {
+        to: 'env',
+        message: 'Flag is deprecated. Use --env instead.',


This renders as:

› Warning: Flag is deprecated. Use --env instead.

with no indication as to which flag is deprecated. Maybe we could do:

Suggested change

message: 'Flag is deprecated. Use --env instead.',

message: '--env-branch is deprecated. Use --env instead.',

Not sure if we want to mention that it'll be removed in a future version or of that's implied.

I think that's implied. I also don't know if we will remember to remove it in the following major version. We need a TODO list of "these are items we need to update / remove before the next major version" before we promise what will be changed/removed in the following version of Hydrogen CLI.

packages/cli/src/lib/flags.ts

packages/cli/src/lib/common.ts

benwolfram

Nothing stands out to me, but still some ongoing things from Gray's feedback. I'll re-review when the dust settles! Great stuff here and improvements though.

aswamy · 2024-03-15T20:29:55Z

Force push

Update changeset message
Update flag deprecation message
deploy's envHandle flag will only work in non-CI environments (added a test for that as well)
correct spacing for env list
update "next steps" when environment not found

frandiox · 2024-03-18T13:32:22Z

packages/cli/src/commands/hydrogen/env/pull.ts

+      (await getStorefrontEnvironments(session, config.storefront.id))
+        ?.environments || [];
+    if (envHandle) {
+      findEnvironmentOrThrow(environments, envHandle);


Shouldn't this write to envHandle or envBranch variable? Or is just a code check?

It's just to check and throw if the user provided bad envHandle

frandiox · 2024-03-18T13:37:40Z

packages/cli/src/lib/flags.ts

    }),
  },
-  env: {
-    env: Flags.string({
+  envBranch: {


Can you add some sort of /** @deprecated **/ comment around here so that we remember to deprecate this in the next major bump?

Done!

Do we have a standard for this? I would love to group all of our breaking changes into a major bump. Unless this comment is our standard 😅

We tend to do something like this for deprecated stuff, yeah.

frandiox · 2024-03-18T13:39:27Z

packages/cli/src/lib/flags.ts

+    'env-branch': Flags.string({
      description:
-        "Specifies an environment's name when using remote environment variables.",
-      env: 'SHOPIFY_HYDROGEN_ENVIRONMENT_NAME',
+        'Specifies the environment to perform the operation using its Git branch name.',
+      env: 'SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH',
+      deprecated: {
+        to: 'env',
+        message: '--env-branch is deprecated. Use --env instead.',
+      },
    }),


Maybe this one should be hidden: true to avoid new uses?

envBranch is the old flag we want to deprecate
env is the new flag we want users to start using right away. We will deprecate it in the following major bump.

for Hydrogen CLI commands

aswamy requested review from benwolfram, frandiox and graygilmore March 12, 2024 19:05

aswamy force-pushed the add-env-handle-flag branch from 956e0bb to 45a95e8 Compare March 12, 2024 19:13

aswamy force-pushed the add-env-handle-flag branch 2 times, most recently from 2ca814c to f15297f Compare March 12, 2024 19:32

aswamy self-assigned this Mar 12, 2024

graygilmore reviewed Mar 12, 2024

View reviewed changes

packages/cli/src/commands/hydrogen/deploy.ts Show resolved Hide resolved

packages/cli/src/lib/flags.ts Outdated Show resolved Hide resolved

packages/cli/src/lib/flags.ts Outdated Show resolved Hide resolved

benwolfram reviewed Mar 12, 2024

View reviewed changes

aswamy changed the title ~~Add --env-handle & remove --env-branch flag for deploy & env pull~~ Add --env-handle & deprecate --env-branch flag for deploy & env pull Mar 14, 2024

aswamy changed the title ~~Add --env-handle & deprecate --env-branch flag for deploy & env pull~~ Add --env & deprecate --env-branch flag for Hydrogen CLI commands Mar 14, 2024

aswamy force-pushed the add-env-handle-flag branch from f15297f to 5fef655 Compare March 14, 2024 20:23

aswamy requested review from graygilmore and benwolfram March 14, 2024 20:31

graygilmore reviewed Mar 14, 2024

View reviewed changes

benwolfram reviewed Mar 15, 2024

View reviewed changes

aswamy force-pushed the add-env-handle-flag branch from 5fef655 to 1dbd00e Compare March 15, 2024 20:29

frandiox reviewed Mar 18, 2024

View reviewed changes

aswamy force-pushed the add-env-handle-flag branch from 1dbd00e to 916caf4 Compare March 19, 2024 17:29

benwolfram approved these changes Mar 21, 2024

View reviewed changes

aswamy force-pushed the add-env-handle-flag branch from 916caf4 to b2d5dfc Compare March 22, 2024 19:21

Add --env & deprecate --env-branch flag

a8a7768

for Hydrogen CLI commands

aswamy force-pushed the add-env-handle-flag branch from b2d5dfc to a8a7768 Compare March 22, 2024 19:23

frandiox approved these changes Mar 25, 2024

View reviewed changes

aswamy merged commit 11879b1 into main Mar 25, 2024
13 checks passed

aswamy deleted the add-env-handle-flag branch March 25, 2024 14:06

shopify-github-actions-access bot mentioned this pull request Mar 25, 2024

[ci] release 2024-04 #1855

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `--env` & deprecate `--env-branch` flag for Hydrogen CLI commands #1841

Add `--env` & deprecate `--env-branch` flag for Hydrogen CLI commands #1841

aswamy commented Mar 12, 2024 •

edited

Loading

shopify bot commented Mar 12, 2024 •

edited

Loading

graygilmore left a comment

aswamy commented Mar 14, 2024

graygilmore left a comment

graygilmore Mar 14, 2024

aswamy Mar 15, 2024

graygilmore Mar 15, 2024

graygilmore Mar 14, 2024

aswamy Mar 15, 2024

benwolfram left a comment

aswamy commented Mar 15, 2024

frandiox Mar 18, 2024

aswamy Mar 19, 2024

frandiox Mar 18, 2024

aswamy Mar 19, 2024 •

edited

Loading

frandiox Mar 25, 2024

frandiox Mar 18, 2024

aswamy Mar 19, 2024

	message: 'Flag is deprecated. Use --env instead.',
	message: '--env-branch is deprecated. Use --env instead.',

Add --env & deprecate --env-branch flag for Hydrogen CLI commands #1841

Add --env & deprecate --env-branch flag for Hydrogen CLI commands #1841

Conversation

aswamy commented Mar 12, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

HOW to test your changes?

Screenshots

Post-merge steps

Checklist

shopify bot commented Mar 12, 2024 • edited Loading

graygilmore left a comment

Choose a reason for hiding this comment

aswamy commented Mar 14, 2024

Force Push

graygilmore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benwolfram left a comment

Choose a reason for hiding this comment

aswamy commented Mar 15, 2024

Force push

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aswamy Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `--env` & deprecate `--env-branch` flag for Hydrogen CLI commands #1841

Add `--env` & deprecate `--env-branch` flag for Hydrogen CLI commands #1841

aswamy commented Mar 12, 2024 •

edited

Loading

shopify bot commented Mar 12, 2024 •

edited

Loading

aswamy Mar 19, 2024 •

edited

Loading