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

CLI: Refactor to add autoblockers #25934

Merged
merged 31 commits into from
Feb 13, 2024
Merged

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Feb 6, 2024

Closes #25911
Closes #25453

What I did

  • I introduced the concept of autoblockers. These are similar to autmigrations, but with an important distinction:
    • they never change anything about the user's project
    • when they detect something they do not 'like' they block the user from upgrading
  • Autoblocker's intent is to prevent users from upgrading to a newer version of storybook that does not match the supported environment of the version being upgraded to.
  • I added an autoblocker preventing users using vue2 from upgrading
  • I added an autoblocker preventing users using *.stories.mdx` from upgrading
  • I added an autoblocker preventing users storyStoreV7:false from upgrading
  • I moved the node16 automigration to be an autoblocker
  • I added an automigration that will upgrade your dependency for vite to latest, if your version is lower than 4

A good example: vue2 support.
We stopped supporting this in sb8 and so we block those users from upgrading to sb8

A bad example: vite3 support
We stopped supporting vite3 in sb8, but making it an autoblocker would mean users have to upgrade vite first, which may not be possible, because the old version of storybook would prevent them. It's a chicken/egg problem really.
So instead we let the user upgrade, and then in an automigration tell them to upgrade vite.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  • Try to upgrade a project that still uses vue2, it should tell you to migrate to vue3

  • Try to upgrade a project that has storyStoreV7:false, it should tell you to remove that flag

  • Try to migrate whilst having *.stories.mdx somewhere in your project, it should tell you to run codemods

  • If you have none of the above, autoblockers should not block you from upgrading

  • upgrading a project that uses vite v3 should ask you if you want to upgrade to the latest version of vite, and if you answer yes, should do that for you.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/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-25934-sha-a4d40871. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-25934-sha-a4d40871
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch norbert/upgrade-auto-blockers
Commit a4d40871
Datetime Tue Feb 13 07:25:56 UTC 2024 (1707809156)
Workflow run 7883079504

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=25934

@ndelangen ndelangen changed the title CLI: refactor to add autoblockers CLI: Refactor to add autoblockers Feb 6, 2024
@ndelangen ndelangen requested review from yannbf and shilman February 6, 2024 23:15
@ndelangen ndelangen self-assigned this Feb 6, 2024
code/lib/cli/src/autoblock/block-storystorev6.ts Outdated Show resolved Hide resolved
code/lib/cli/src/autoblock/block-storystorev6.ts Outdated Show resolved Hide resolved
code/lib/cli/src/autoblock/block-storystorev6.ts Outdated Show resolved Hide resolved
code/lib/cli/src/autoblock/types.ts Outdated Show resolved Hide resolved
code/lib/cli/src/upgrade.ts Outdated Show resolved Hide resolved
@valentinpalkovic

This comment was marked as resolved.

Co-authored-by: Valentin Palkovic <[email protected]>
@valentinpalkovic valentinpalkovic merged commit ece84b9 into next Feb 13, 2024
57 of 58 checks passed
@valentinpalkovic valentinpalkovic deleted the norbert/upgrade-auto-blockers branch February 13, 2024 12:06
@github-actions github-actions bot mentioned this pull request Feb 13, 2024
13 tasks
code/lib/cli/src/automigrate/fixes/vite4.ts Show resolved Hide resolved
export const viteConfigFile = {
id: 'viteConfigFile',

async check({ mainConfig, packageManager }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any handling of custom vite config locations, was that a deliberate decision?

https://storybook.js.org/docs/builders/vite#override-the-default-configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point! I think this was just missed. @ndelangen, Can you put together a follow-up PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was missed indeed. I didn't know this was an option.

ndelangen added a commit that referenced this pull request Feb 19, 2024
usrrname pushed a commit to usrrname/storybook that referenced this pull request Mar 9, 2024
usrrname pushed a commit to usrrname/storybook that referenced this pull request Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants