Skip to content
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

Update CLI init command to create or link storefront #1681

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

aswamy
Copy link
Contributor

@aswamy aswamy commented Jan 24, 2024

WHY are these changes introduced?

  • Quality of life improvement for CLI users
    • When they create a storefront on Admin, they can just npm create @shopify/hydrogen or shopify hydrogen init command to connect to their existing storefront instead of creating another one

WHAT is this pull request doing?

During the creation of the storefront using the init command, we want to give the option to link an existing storefront on Admin.

  • The old flow always assumed that you'd be creating a brand new storefront on Admin.
  • The current flow follows an identical path to the link command
    • It will ask to either create a new storefront or link an existing storefront on Admin

Old process

image

New process

image

HOW to test your changes?

  1. checkout the branch
  2. npm ci on the parent folder
  3. cd packages/cli && npm run build
  4. run ../../node_modules/.bin/shopify hydrogen init
  5. Create a new storefront
  6. run ../../node_modules/.bin/shopify hydrogen link --path=<path-to-your-storefront>
    • ensure you get a message saying what storefront on Admin it is connected to
  7. run ../../node_modules/.bin/shopify hydrogen init
  8. Link an existing storefront
  9. run ../../node_modules/.bin/shopify hydrogen link --path=<path-to-your-storefront>
    • ensure you get a message saying what storefront on Admin it is connected to
    • for regression, link it to another storefront or create another storefront on Admin

Post-merge steps

n/a

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 updated existing tests. commons.ts and local.ts doesn't have a test file.
  • I've added or updated the documentation

Copy link
Contributor

@frandiox frandiox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! 🚀

packages/cli/src/lib/onboarding/local.ts Outdated Show resolved Hide resolved
packages/cli/src/lib/onboarding/common.ts Outdated Show resolved Hide resolved
.changeset/witty-clouds-sing.md Outdated Show resolved Hide resolved
@aswamy
Copy link
Contributor Author

aswamy commented Jan 25, 2024

@frandiox I noticed that src/commands/hydrogen/init.test.ts has been failing on my local system before this PR. It seems to work in our CI. Is there a specific way to run that test file so it works on my local system?

I use:

cd packages/cli && npm run test -t "init"

Copy link
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩 went great!

@aswamy aswamy merged commit 1bc053c into main Jan 25, 2024
10 checks passed
@aswamy aswamy deleted the create-or-link-storefront-on-init branch January 25, 2024 20:43
@frandiox
Copy link
Contributor

I noticed that src/commands/hydrogen/init.test.ts has been failing on my local system before this PR. It seems to work in our CI. Is there a specific way to run that test file so it works on my local system?

I normally do cd packages/cli && npm run test -- init.test. Then, modify the file adding it.only` to the one that fails and repeat to get clear errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants