-
Notifications
You must be signed in to change notification settings - Fork 290
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
Check for new CLI versions when creating projects #518
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good ✅
expect(checkForNewVersion).toHaveBeenCalledWith( | ||
'@shopify/hydrogen', | ||
// Calver | ||
expect.stringMatching(/20\d{2}\.\d{1,2}\.\d{1,3}/), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this won't work if we have 1000 patches or this package lives till 2100 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah at this rate AI will conquer us before any of that happens 👍
@mynameisadamf Not really, the The idea is that this check basically reminds you to use |
if (process.env.LOCAL_DEV) return; | ||
const pkgName = PACKAGE_NAMES[pkgKey]; | ||
|
||
const require = createRequire(import.meta.url); | ||
const pkgJsonPath = locateDependency( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double checking, this function resolves to the package in their local node_modules rather what version is listed in the project package.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it resolves to the installed version, not the listed one. That's the correct behavior, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! Thanks for clarifying, this looks good to me 👍
This adds a check to the init command to show available upgrades for the CLI:
It's just an "info" log and you can continue generating the project with the current version (the
ELIFECYCLE Command failed
in the screenshot is just because Ictrl+c
'd).The check happens upfront so the whole process is slightly delayed, but I think it's better to do it at this time than later after choosing template etc.