-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
CLI: Fix upgrade
detecting the wrong version of existing Storybooks
#25752
Conversation
export const getStorybookCoreVersion = async () => { | ||
const { version } = await getActualPackageVersion('@storybook/cli'); | ||
return version; | ||
}; |
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.
@shilman this is a function in telemetry but it was only used by upgrade
which doesn't use it anymore, so I felt it was okay to remove it. Let me know if not.
if (!installations) { | ||
return; | ||
} | ||
const cliVersion = installations.dependencies['@storybook/cli']?.[0].version; |
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.
installations.dependencies['@storybook/cli']?.[0]
-> Can this be undefined? If so installations.dependencies['@storybook/cli']?.[0].version
will through.
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.
It can't, TS would complain, and the current code producing it doesn't do that either.
…ybookjs/storybook into jeppe/fix-upgrade-version-detection
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'm assuming you tested the scenarios thoroughly via the canary, in that case LGTM!
…detection CLI: Fix `upgrade` detecting the wrong version of existing Storybooks (cherry picked from commit 7c21c34)
Closes #25734
What I did
Changed the
upgrade
command to use package manager's CLI for detecting the existing version of bothstorybook
and@storybook/cli
instead of using a combination ofrequire.resolve
and readingpackage.json
, as that would run in the global context thus not detecting the correct version as it is in the project. ThepackageManager.findInstallations
function being used was originally develop for thedoctor
command but it works fine in this context too.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This can be (and has been) tested with the canary release and using the different package manager executors. Eg. in a 7.6.10 sandbox/project, running this should result in the message Upgrading Storybook from version 7.6.10 to version 0.0.0-pr-25752-sha-424630b7..
npx [email protected] upgrade
yarn dlx [email protected] upgrade
pnpm dlx [email protected] upgrade
pnpx [email protected] upgrade
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
bug
: Internal changes that fixes incorrect behavior.maintenance
: User-facing maintenance tasks.dependencies
: Upgrading (sometimes downgrading) dependencies.build
: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup
: Minor cleanup style change. Will not show up in release changelog.documentation
: Documentation only changes. Will not show up in release changelog.feature request
: Introducing a new feature.BREAKING CHANGE
: Changes that break compatibility in some way with current major version.other
: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-25752-sha-424630b7
. Try it out in a new sandbox by runningnpx [email protected] sandbox
or in an existing project withnpx [email protected] upgrade
.More information
0.0.0-pr-25752-sha-424630b7
jeppe/fix-upgrade-version-detection
424630b7
1706261623
)To request a new release of this pull request, mention the
@storybookjs/core
team.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=25752