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

Always install latest Hydrogen CLI when running create-hydrogen script #1873

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

gfscott
Copy link
Contributor

@gfscott gfscott commented Mar 19, 2024

WHY are these changes introduced?

People frequently get an annoying message telling them to update the package they're installing for the first time, which is wack:

image

WHAT is this pull request doing?

Updates the create-hydrogen package.json to always install the latest version of the Hydrogen CLI. Using a tag as a version specifier in package.json is perfectly valid and it stands to reason that if you're running the create script, then you want to install the latest dependencies.

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

@gfscott gfscott requested a review from a team March 19, 2024 15:48
@gfscott gfscott self-assigned this Mar 19, 2024
Copy link
Contributor

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

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.

TIL!

But unsure if we can make this work without installing a remote version in our monorepo with latest 🤔

@gfscott gfscott force-pushed the gfscott-create-with-latest-cli branch from b7241fc to ddc182a Compare March 19, 2024 17:04
@@ -13,7 +13,7 @@
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@shopify/cli-hydrogen": "^7.1.0"
"@shopify/cli-hydrogen": "latest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Moving here for threading)

But unsure if we can make this work without installing a remote version in our monorepo with latest 🤔

@frandiox Hm, yeah. I'm not sure what's causing the failure in CI here. But the latest tag is published by default and it's always available to install through npm:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

@gfscott it looks like the package lock file needs to get updated. Just run npm install from the root of the project, and push the changes to the package lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right. Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Went from a one line change to over 1k 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😂 😭
Alsooo... did it break a bunch of stuff? I see two failed actions, they seem to be related to the node version? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm going to revert this. Changesets can't deal with non-semver values so this isn't going to work in practice.

We'll have to just make sure that we always bump the create-hydrogen dependency whenever there's a CLI bump. Otherwise the create flow gets out of sync. Annoying!

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if something like link from chageset would help here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooo that's a good thought

Copy link
Contributor

shopify bot commented Mar 21, 2024

Oxygen deployed a preview of your gfscott-create-with-latest-cli branch. Details:

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

Learn more about Hydrogen's GitHub integration.

@gfscott gfscott force-pushed the gfscott-create-with-latest-cli branch from 0fea466 to 3214e6a Compare March 21, 2024 15:29
@gfscott gfscott merged commit 82b5ed5 into main Mar 21, 2024
13 checks passed
@gfscott gfscott deleted the gfscott-create-with-latest-cli branch March 21, 2024 16:00
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.

4 participants