Clean up messaging around unlinked storefronts when running CLI commands #1937

aswamy · 2024-04-02T22:29:38Z

WHY are these changes introduced?

Cleaning up Hydrogen CLI link command and messaging around linked storefront

WHAT is this pull request doing?

Changes:

We will show a warning (not error) when you run a Hydrogen CLI command on a storefront that isn't linked to a storefront on Admin.
If you don't have a storefront to link to on Admin, we will just ask you to create a storefront instead of displaying an option list of size 1.
If you deleted a storefront on Admin, we will try to relink your storefront when running a Hydrogen CLI command.

HOW to test your changes?

Checkout this repo and build it
- npm ci
- cd packages/cli && npm run build
- If it doesn't build, you might also need to run npm run build inside the other packages
Create a storefront using h2 init and link it
- You can run this command by navigating to the packages/cli directory of the project

Testing deleted storefront && unlinked storefront banner

Go to the directory of your newly created storefront
- Edit the .shopify/project.json file so storefront: {}
In hydrogen/packages/cli, run h2 deploy --path=<path-to-project>
- It should prompt you to link a storefront using the warning severity
- also try h2 env list, h2 env pull, h2 env push__unstable commands to see the banner

Testing no storefronts on Admin

Ensure you have Hydrogen App installed on your shop, but no storefronts
In hydrogen/packages/cli, run h2 init
- Try to link a storefront
- Instead of prompting you to select a storefront, it will assume you want to create the storefront and ask you its name

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

shopify · 2024-04-02T22:35:42Z

Oxygen deployed a preview of your cli-consistency-2 branch. Details:

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

Learn more about Hydrogen's GitHub integration.

packages/cli/src/lib/get-oxygen-deployment-data.ts

frandiox · 2024-04-03T10:32:13Z

.changeset/breezy-jokes-pump.md

+
+Clean up messaging around unlinked storefronts when running CLI commands
+
+- We will show a warning (not error) when you run a Hydrogen CLI command on a storefront that isn't linked to a storefront on Admin.


Let's clarify here that it's only for Admin related links somehow? I.e. this doesn't affect dev/build commands.

That's a good point, i'm going to update the code so it's for env push, env pull, env list, deploy. Do we not already show an error when we run h2 dev on unlinked storefronts?

aswamy · 2024-04-03T20:54:18Z

Force push

If you run h2 env push__unstable, h2 env pull, h2 env list, or h2 deploy, we will verify
- you have linked your storefront on Admin
- if the existing storefront you've connected still exists on Admin (or has been deleted)
moved verify-linked-storefront into a separate file for consistency and so we can easily test it
- other Hydrogen CLI commands will call this method to ensure we have linked a storefront
As Fran recommended, linkStorefront now takes an optional list of storefronts so we don't have to fetch it 2x

aswamy · 2024-04-04T21:03:12Z

Force push

Removing user-confirmation on h2 deploy when user selects a preview environment, user passes in preview env handle, or user provides --preview flag

graygilmore

🎩 went well, just a couple of questions

graygilmore · 2024-04-04T20:56:04Z

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


-  if (!configStorefront) return;
+  config.storefront = linkedStorefront;


I'm confused by the use of config.storefront = X in this PR. Should we be using setStorefront from shopify-config?

So setStorefront sets your shopify project.json file to have the linked storefront's data.

verifyLinkedStorefront just checks the following:

checks to make sure you have any storefront linked in your project.json file

talks to Admin and ensures that storefront still exists

this prevents the yucky GraphQL error you would get if you deleted your storefront on Admin

graygilmore · 2024-04-04T21:02:55Z

packages/cli/src/lib/onboarding/common.ts

@@ -284,6 +284,10 @@ export async function handleStorefrontSelection(
    })),
  ];

+  if (choices.length === 1) {


Is this clear enough that we're launching them into the creation flow? I wonder if we should be printing some text like "No storefronts found on . Create a new one to continue"

Hmm yeah i think an extra message here saying that we're creating the storefront is fine.

Before:

After:

@mynameisadamf any issues with that? I know your original design merged those two banners. However, the first banner message must appear before you choose to run h2 link, where the latter message appear after.

@aswamy We decided on the implicit approach, which if they were to run h2 deploy we would put them in the link flow w/o prompting. Would be this:

user input: h2 deploy

Info banner:
You haven’t linked your project to a storefront yet.

Prompt: If existing storefronts

Select a Hydrogen storefront to link:

Create a new storefront

[storefront]

[storefront]

Prompt: If no existing storefronts
Create a Hydrogen storefront
[input]

relates to env pull https://github.com/Shopify/custom-storefronts/issues/352#issuecomment-2009983378

We decided on the implicit approach, which if they were to run h2 deploy we would put them in the link flow w/o prompting.

Should this follow this for all other admin commands like env push, env list, env pull?

Yes. This pattern will apply anywhere being linked to a store is required.

aswamy · 2024-04-05T20:59:43Z

Force push

Updated the "not linked" banner when running env push, env pull, env list, or deploy command without linking a storefront on Admin
Remove the user-prompt to link a storefront if it isn't linked

aswamy · 2024-04-10T15:34:43Z

Force Push

Rebased with main

martinamarien

🎩 went well and matched expected output.
I just have two questions around the use of !

martinamarien · 2024-04-17T13:57:11Z

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

+    let chosenEnvironment = findEnvironmentByBranchOrThrow(
+      deploymentData!.environments!,
+      config.environmentTag!,
+    );


In this case, do we know definitively that these arguments will not be undefined?

Yep, had to scroll up but confirmed deploymentData HAS to be defined or an early return would occur.
config.environmentTag has to be defined if one of the items userProvidedEnvironmentTag or userChosenEnvironmentTag is defined.

martinamarien · 2024-04-17T14:29:26Z

packages/cli/src/lib/verify-linked-storefront.ts

+  const storefronts = await getStorefronts(session);
+
+  let configuredStorefront = config.storefront?.id
+    ? storefronts.find(({id}) => id === config.storefront!.id)


Same here, can we guarantee that the config.storefront exists?

Yep, you can only get into this conditional statement if config.storefront AND config.storefront.id is truthy.

Thanks Alok. Frustrating that typescript can't infer that the config.storefront.id is defined inside of the find method.

aswamy requested review from graygilmore, benwolfram and martinamarien April 2, 2024 22:30

aswamy force-pushed the cli-consistency-2 branch from eadf80b to f43f43a Compare April 2, 2024 22:33

aswamy requested a review from a team April 2, 2024 22:34

frandiox reviewed Apr 3, 2024

View reviewed changes

aswamy marked this pull request as draft April 3, 2024 16:40

aswamy force-pushed the cli-consistency-2 branch from f43f43a to 7b8d46a Compare April 3, 2024 20:22

aswamy self-assigned this Apr 3, 2024

aswamy marked this pull request as ready for review April 3, 2024 20:40

aswamy force-pushed the cli-consistency-2 branch 2 times, most recently from 6171756 to 30acde7 Compare April 4, 2024 21:01

graygilmore reviewed Apr 4, 2024

View reviewed changes

aswamy force-pushed the cli-consistency-2 branch 2 times, most recently from 2d3d8cb to 4bd1e91 Compare April 5, 2024 20:57

aswamy force-pushed the cli-consistency-2 branch from 4bd1e91 to e0e1045 Compare April 10, 2024 15:34

martinamarien approved these changes Apr 17, 2024

View reviewed changes

Clean up messaging around unlinked storefronts when running CLI commands

0463082

aswamy force-pushed the cli-consistency-2 branch from e0e1045 to 0463082 Compare April 17, 2024 17:19

graygilmore approved these changes Apr 17, 2024

View reviewed changes

frandiox approved these changes Apr 18, 2024

View reviewed changes

aswamy merged commit b4dfda3 into main Apr 18, 2024
15 checks passed

aswamy deleted the cli-consistency-2 branch April 18, 2024 14:25

shopify-github-actions-access bot mentioned this pull request Apr 18, 2024

[ci] release 2024-04 #1994

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up messaging around unlinked storefronts when running CLI commands #1937

Clean up messaging around unlinked storefronts when running CLI commands #1937

aswamy commented Apr 2, 2024 •

edited

Loading

shopify bot commented Apr 2, 2024 •

edited

Loading

frandiox Apr 3, 2024

aswamy Apr 3, 2024

aswamy commented Apr 3, 2024

aswamy commented Apr 4, 2024

graygilmore left a comment

graygilmore Apr 4, 2024

aswamy Apr 4, 2024

graygilmore Apr 4, 2024

aswamy Apr 4, 2024

mynameisadamf Apr 5, 2024

aswamy Apr 5, 2024

mynameisadamf Apr 5, 2024

aswamy commented Apr 5, 2024

aswamy commented Apr 10, 2024

martinamarien left a comment

martinamarien Apr 17, 2024

aswamy Apr 17, 2024

martinamarien Apr 17, 2024

aswamy Apr 17, 2024

martinamarien Apr 17, 2024


		Clean up messaging around unlinked storefronts when running CLI commands

		- We will show a warning (not error) when you run a Hydrogen CLI command on a storefront that isn't linked to a storefront on Admin.


		if (!configStorefront) return;
		config.storefront = linkedStorefront;

Clean up messaging around unlinked storefronts when running CLI commands #1937

Clean up messaging around unlinked storefronts when running CLI commands #1937

Conversation

aswamy commented Apr 2, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

HOW to test your changes?

Testing deleted storefront && unlinked storefront banner

Testing no storefronts on Admin

Post-merge steps

Checklist

shopify bot commented Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aswamy commented Apr 3, 2024

Force push

aswamy commented Apr 4, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aswamy commented Apr 5, 2024

Force push

aswamy commented Apr 10, 2024

Force Push

martinamarien 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

aswamy commented Apr 2, 2024 •

edited

Loading

shopify bot commented Apr 2, 2024 •

edited

Loading