-
-
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: Support version specifiers in init
, upgrade
and sandbox
#25526
Conversation
…ned-main CLI: Sandbox script should use current version to init (main)
CLI: Versioned upgrade of monorepo packages (main)
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: @storybook/client-logger@7.5.0, @storybook/theming@7.5.0 |
…dbox CLI: Support upgrading to canary versions (cherry picked from commit f091347)
|
||
on: | ||
workflow_dispatch: | ||
inputs: | ||
pr: | ||
description: 'Pull request number to create a canary release for' |
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.
why the formatting change? is this due to the prettier upgrade, or a configuration issue on your machine?
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.
Caused by the prettier upgrade removing the root config, Kasper brought it back again here: 4997513
(#25601)
@@ -86,6 +86,7 @@ | |||
"esbuild": "^0.18.0", | |||
"eslint": "^8.28.0", | |||
"playwright": "1.36.0", | |||
"@storybook/theming": "workspace:*", |
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.
why? and why here?
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.
This change will not affect users.
This ensures we'll use the workspace version, even if other versions are referenced, somehow.
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.
As you can see in the lockfile changes, the result is we don't install the 7.5.0 version anymore, due to this change.
Which:
- saves time during install
- reduced the risk of bundling in the wrong version
- reduced the risk of type conflicts / failures to compile
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.
why now? I had merge conflicts, and this is how I was able to resolve + I noticed during the merge conflicts that we were installing the 7.5.0 version of some packages, which seemed very wrong, so I fixed it.
Again: AFAICS, this change cannot affect users.
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 I think this is the right thing to do, because we should not install 2 version of @storybook/theming
in the monorepo.
It IS though if I remove this line.
And this is because of:
├─ @storybook/design-system@npm:7.15.15
│ └─ @storybook/theming@workspace:lib/theming (via npm:^7.0.2)
init
, upgrade
and sandbox
Closes #25521
Closes #25571
Closes #25572
Tracking: #25293
What I did
Init
versions.ts
is used, and added, unless that version happens to be the latest version, in which case, no modifier is added.sb init
has been changed to be more noticeable, as it can now contain a warning to the user, that they are running an old version of the CLI. (The CLI will still proceed to run the actual init!)upgrade
This PR changes the
upgrade
command from always upgrading tolatest
ornext
versions, to upgrading to the version that the CLI is started as. This adds support for the scenario where users want to upgrade to a specific version that might not be the latest, by doing eg.npx storybook@8.0.1 upgrade
.It also throws errors if users attempt to "upgrade" to a lower or equal version than the one already installed. A warning is also shown if users are upgrading to a non-latest version. This is a common mistake when running
npx storybook upgrade
without the version specifier, but it is a valid use-case, hence the warning.Another major change here is that previously we would upgrade (almost) all packages containing "storybook" in their name, while now we only upgrade the packages in this monorepo, because they are the only ones we truly know how to upgrade.
Sandbox
after-storybook
dirbefore-storybook
dir, and run init (if requested)Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
nr task compile -s compile --no-link
run-registry.ts
, comment out lines130-137
nr task publish --no-link -s publish
nr task run-registry --no-link -s run-registry
(this process will stay running)http://localhost:6001
, check if verdaccio is running and has all packagesnpm config edit
and addregistry=http://localhost:6001
, remove all other registries (you can comment them out).~/projects/storybook/repros/cli-versioning
).npx storybook@latest sandbox
, answer yes to the "should download" questionreact-vite/default-ts
after-storybook
dir should have been used.run-registry
processpackage.json
in the monorepo to the next patch version, e.g.1.0.0
->1.0.1
versions.ts
file manually to reflect the new versions.nr task compile -s compile --no-link
nr task publish --no-link -s publish
nr task run-registry --no-link -s run-registry
(this process will stay running)http://localhost:6001
, check if verdaccio is running and has all packages (should now have 2 versions, where the latest is the one you just published)~/projects/storybook/repros/cli-versioning/react-vite/default-ts
)npx storybook@latest upgrade
~/projects/storybook/repros/cli-versioning
)npx storybook@{{version before the patch}} sandbox
, answer yes to the "should download" questionat the time of writing
main
was on"version": "7.6.8"
, so after bumping it was on"version": "7.6.9"
.so the command would be
npx storybook@7.6.8 sandbox
react-vite/default-ts
, it will complain the folder name is taken, give it another name, I usedreact-vite/default-ts2
.before-storybook
dir should have been used, and theinit
step should be executed by the sandbox script.The
package.json
of the sandbox should have the exact version of the CLI that was used to create it, no^
-modifier.You're done, we've tested
We did not test:
When you're done:
npm config edit
and removeregistry=http://localhost:6001
(restore it to how it was before)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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>