Add link shop and storefront creation flags#3834
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
2caddc8 to
f64cea7
Compare
b06380d to
8406940
Compare
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
f64cea7 to
56f192c
Compare
8406940 to
2250851
Compare
56f192c to
b337244
Compare
2250851 to
0147582
Compare
0147582 to
97b8c94
Compare
b337244 to
019d75b
Compare
97b8c94 to
29ebc6c
Compare
lucyxiang
left a comment
There was a problem hiding this comment.
Review assisted by pair-review
| }); | ||
|
|
||
| it('normalizes CLI flag names before creating a storefront', async () => { | ||
| await Link.run([ |
There was a problem hiding this comment.
Pair review pointed out this is the only time we use Link.run instead of runLink, which depends on "oclif resolving a Config in the vitest environment, which no other test in the suite relies on". Is this a valid issue- I don't have much context here
There was a problem hiding this comment.
Good catch. I replaced this with direct runLink coverage, so this test no longer depends on oclif resolving a Config in the Vitest environment.
| }), | ||
| 'storefront-name': Flags.string({ | ||
| description: 'The name to use when creating a new Hydrogen storefront.', | ||
| env: 'SHOPIFY_HYDROGEN_STOREFRONT_NAME', |
There was a problem hiding this comment.
Do we want to use SHOPIFY_HYDROGEN_FLAG_STOREFRONT_NAME like most of the other variables
There was a problem hiding this comment.
Updated. The creation name flag now uses the standard flag env var pattern as SHOPIFY_HYDROGEN_FLAG_NAME.
| }), | ||
| 'storefront-name': Flags.string({ | ||
| description: 'The name to use when creating a new Hydrogen storefront.', | ||
| env: 'SHOPIFY_HYDROGEN_STOREFRONT_NAME', |
There was a problem hiding this comment.
Thoughts on storefront-create-name/ create-storefront-name ? Since this is only used with create-storefront. I find it confusing there's --storefront and now --storefront-name
There was a problem hiding this comment.
Went with --name. In the hydrogen link command context it reads cleaner than another storefront-prefixed flag, and it avoids the --storefront / --storefront-name ambiguity.
| root, | ||
| session, | ||
| storefronts, | ||
| storefrontName, |
There was a problem hiding this comment.
This createNewStorefront call sits in the final else branch, which is only reached when flagStorefront is falsy AND (flagCreateStorefront || storefrontName) is also falsy. storefrontName here can only be falsy — passing it implies there is additional data here.
Suggestion: Consider dropping the always-falsy argument
There was a problem hiding this comment.
Fixed. The fallback createNewStorefront call no longer passes the name argument from the branch where it can only be absent.
| defaultValue: defaultProjectName, | ||
| }); | ||
| const projectName = | ||
| storefrontName ?? |
There was a problem hiding this comment.
The branch selector at line 180 uses a truthy test (flagCreateStorefront || storefrontName) while createNewStorefront uses a nullish fallback (storefrontName ?? renderTextPrompt(...)). An empty string is falsy but not nullish, creating a storefront with a blank name and no prompt (I assume blank name is invalid).
Suggestion: Consider treating a blank --storefront-name as absent (normalize '' to undefined) before the branch decision, or guard against an empty name explicitly, so the || and ?? checks share the same emptiness semantics.
There was a problem hiding this comment.
Fixed. The name is trimmed and blank values normalize to undefined before branch selection, with test coverage for the blank-name case.
29ebc6c to
ceded69
Compare
b941d59 to
e2e5d2e
Compare
ceded69 to
bec4169
Compare
1085989 to
fcc0333
Compare
a44fa79 to
d156023
Compare
5852064 to
a72a9b3
Compare
a72a9b3 to
286e006
Compare
Add --create-storefront and --storefront-name to hydrogen link so agents can create a new Hydrogen storefront without answering the storefront selection and name prompts.\n\nThe existing --storefront flag remains the explicit existing-storefront path and is validated against the creation flags. Passing --storefront-name implies creating a new storefront and skips the name prompt; passing --create-storefront skips storefront selection and keeps the existing name prompt.\n\nTests cover both creation flags and the conflict with --storefront. The Oclif manifest and changeset are updated for the new user-facing flags.\n\nCloses shop/issues-develop#22878
d156023 to
8612b1e
Compare

WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/22878
shopify hydrogen linkstill required prompts to choose the shop and name a new storefront, which blocked noninteractive agent runs after project scaffolding.WHAT is this pull request doing?
Adds
--shop,--create-storefront, and--storefront-nametohydrogen linkso agents can select a shop and create or link a storefront without prompts.HOW to test your changes?
Use an unlinked Hydrogen project and a development shop:
Both commands should complete without prompting for shop selection, storefront selection, or the new storefront name.
Post-merge steps
None.