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: Remove Storybook dependencies before adding re-adding them #30600

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Feb 20, 2025

Closes #30306

What I did

Users struggle to upgrade Storybook with npm because of conflicting peer-dependencies. This issue doesn't occur when Storybook is freshly installed. It seems that npm has issues to upgrade all dependencies at once.

Solution:

  • Remove all Storybook packages (except 'storybook') from the package.json before installing the new version again
  • We don't remove the storybook package, because other third-party Storybook integrations may depend on the storybook package.

Context:

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

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

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-storybook/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-30600-sha-4c65bea0. 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-30600-sha-4c65bea0
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/uninstall-dependencies-during-upgrade-for-npm-users
Commit 4c65bea0
Datetime Thu Feb 20 13:55:26 UTC 2025 (1740059726)
Workflow run 13436884342

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

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 80.5 MB 80.5 MB 0 B -0.62 0%
initSize 80.5 MB 80.5 MB 0 B -0.62 0%
diffSize 97 B 97 B 0 B - 0%
buildSize 7.31 MB 7.31 MB 0 B 1.4 0%
buildSbAddonsSize 1.9 MB 1.9 MB 0 B -0.78 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.88 MB 1.88 MB 0 B 1.11 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.97 MB 3.97 MB 0 B 1.3 0%
buildPreviewSize 3.34 MB 3.34 MB 0 B 1.11 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 7.3s 6.9s -386ms -0.82 -5.5%
generateTime 20.3s 17.4s -2s -820ms -1.53 🔰-16.1%
initTime 4.6s 4s -570ms -1.67 🔰-14%
buildTime 9.6s 7.7s -1s -876ms -1.96 🔰-24.1%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 9.2s 5.4s -3s -780ms -0.38 -69.7%
devManagerResponsive 8.6s 5.1s -3s -470ms 0.33 -66.8%
devManagerHeaderVisible 686ms 847ms 161ms 0.37 19%
devManagerIndexVisible 746ms 860ms 114ms 0.21 13.3%
devStoryVisibleUncached 1.6s 2s 407ms -1.03 20.1%
devStoryVisible 775ms 936ms 161ms 0.59 17.2%
devAutodocsVisible 761ms 814ms 53ms 0.04 6.5%
devMDXVisible 647ms 861ms 214ms 0.77 24.9%
buildManagerHeaderVisible 738ms 1s 292ms 1.84 🔺28.3%
buildManagerIndexVisible 741ms 1s 328ms 1.68 🔺30.7%
buildStoryVisible 725ms 1s 275ms 1.9 🔺27.5%
buildAutodocsVisible 672ms 802ms 130ms 1.08 16.2%
buildMDXVisible 584ms 846ms 262ms 2.62 🔺31%

Greptile Summary

This PR modifies the Storybook upgrade process to handle npm-specific dependency conflicts by removing and reinstalling Storybook packages during upgrades.

  • Modified code/lib/cli-storybook/src/upgrade.ts to remove all Storybook dependencies except the core 'storybook' package before reinstallation
  • Preserves the 'storybook' package to maintain third-party integration compatibility
  • Specifically targets npm package manager users to resolve peer dependency conflicts
  • Addresses upgrade failures reported in issue [Bug]: Upgrade fails with npx storybook@latest upgrade #30306 for npm users

Copy link

nx-cloud bot commented Feb 20, 2025

View your CI Pipeline Execution ↗ for commit 8457f7a.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 2m 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-21 11:40:54 UTC

@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Feb 20, 2025

Package Benchmarks

Commit: 8457f7a, ran on 21 February 2025 at 11:43:08 UTC

No significant changes detected, all good. 👏

@valentinpalkovic valentinpalkovic force-pushed the valentin/uninstall-dependencies-during-upgrade-for-npm-users branch 3 times, most recently from 4c65bea to 0b06a97 Compare February 20, 2025 14:15
@valentinpalkovic valentinpalkovic force-pushed the valentin/uninstall-dependencies-during-upgrade-for-npm-users branch from 0b06a97 to 7672b95 Compare February 20, 2025 14:29
@valentinpalkovic valentinpalkovic marked this pull request as ready for review February 20, 2025 14:32
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@storybook-bot
Copy link
Contributor

Failed to publish canary version of this pull request, triggered by @valentinpalkovic. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/13455898257

@valentinpalkovic valentinpalkovic force-pushed the valentin/uninstall-dependencies-during-upgrade-for-npm-users branch from fd3d3e6 to 8457f7a Compare February 21, 2025 11:34
@valentinpalkovic valentinpalkovic merged commit 65ee721 into next Feb 21, 2025
68 of 69 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/uninstall-dependencies-during-upgrade-for-npm-users branch February 21, 2025 11:55
@github-actions github-actions bot mentioned this pull request Feb 21, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Upgrade fails with npx storybook@latest upgrade
3 participants