CLI: Fix version detection compatibility with pre SB7.5 #26055
Conversation
| /* eslint-disable @typescript-eslint/naming-convention */ | ||
| import { sync as spawnSync } from 'cross-spawn'; | ||
| import semver from 'semver'; | ||
| import { versionRegex } from '../upgrade'; | ||
|
|
||
| const legacy_excludeList = [ | ||
| '@storybook/linter-config', | ||
| '@storybook/design-system', | ||
| '@storybook/ember-cli-storybook', | ||
| '@storybook/semver', | ||
| '@storybook/eslint-config-storybook', | ||
| '@storybook/bench', | ||
| '@storybook/addon-bench', | ||
| '@storybook/addon-console', | ||
| '@storybook/csf', | ||
| '@storybook/storybook-deployer', | ||
| '@storybook/addon-postcss', | ||
| '@storybook/react-docgen-typescript-plugin', | ||
| '@storybook/babel-plugin-require-context-hook', | ||
| '@storybook/builder-vite', | ||
| '@storybook/mdx1-csf', | ||
| '@storybook/mdx2-csf', | ||
| '@storybook/expect', | ||
| '@storybook/jest', | ||
| '@storybook/test-runner', | ||
| '@storybook/testing-library', | ||
| ]; | ||
| const legacy_isCorePackage = (pkg: string) => | ||
| pkg.startsWith('@storybook/') && | ||
| !pkg.startsWith('@storybook/preset-') && | ||
| !legacy_excludeList.includes(pkg); | ||
|
|
||
| /** | ||
| * @deprecated This function has been deprecated. This is solely to support upgrading from SB6 (and possible SB7) | ||
| * I took this code from https://github.com/storybookjs/storybook/blob/v6.5.16/lib/cli/src/upgrade.ts#L29-L109 | ||
| */ | ||
| export const legacy_getStorybookVersion = (): string | undefined => { | ||
| const lines = spawnSync('npm', ['ls'], { stdio: 'pipe' }).output.toString().split('\n'); | ||
| const result = lines | ||
| .map((line) => { | ||
| if (line.startsWith('npm ')) return null; | ||
| const match = versionRegex.exec(line); | ||
| if (!match || !semver.clean(match[2])) return null; | ||
| return { | ||
| package: match[1], | ||
| version: match[2], | ||
| }; | ||
| }) | ||
| .filter(Boolean) | ||
| .filter((pkg) => legacy_isCorePackage(pkg!.package)) | ||
| .sort((a, b) => semver.rcompare(a!.version, b!.version)); | ||
|
|
||
| if (result.length === 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return result[0]?.version; | ||
| }; |
There was a problem hiding this comment.
All ripped out of SB6's code.
You can review this for problems, but please review with the context of:
Code we need to support SB6, it's not eloquent, because we'll remove it as soon as we stop support upgrading from SB6
ndelangen
added
the
patch:yes
…rcular references
| /** | ||
| * In older versions of Storybook, the user may not have a direct dependency on the CLI package. | ||
| * In this case, we need to use the legacy method to determine the version. | ||
| * @see legacy_getStorybookVersion | ||
| * @deprecated | ||
| * @remove in SB9 (or when we drop support for upgrading from SB6 & SB7) | ||
| */ | ||
| return legacy_getStorybookVersion(); |
There was a problem hiding this comment.
So where before we returned undefined which would cause an "no version detected"-error... we now have this fallback that should operate exactly how it was in SB6.
| * @deprecated This function has been deprecated. This is solely to support upgrading from SB6 (and possible SB7) | ||
| * I took this code from https://github.com/storybookjs/storybook/blob/v6.5.16/lib/cli/src/upgrade.ts#L29-L109 | ||
| */ | ||
| export const legacy_getStorybookVersion = (): string | undefined => { |
There was a problem hiding this comment.
Let's wait for the adjustments of findInstallations I did here: https://github.com/storybookjs/storybook/pull/26067/files#diff-afe49a7cb8e5ea8e3d02751da8f553064c9a6ab67a0a819ac8048a98bf43f506R133 and then reuse packageManager.findInstallations here.
Then the support for multiple package managers is guaranteed.
There was a problem hiding this comment.
I personally don't feel like we should spend a lot of time refining this as it's purely to temporarily support an upgrade from SB6 to SBX
|
Superseded by #26067 |
Closes #25975
What I did
The problem was that some users on older versions don't have the SB CLI as a (direct) dependency. Because back in the good old days, we shipped a bin with every renderer
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.tsMake 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-26055-sha-bcc55126. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-26055-sha-bcc55126norbert/fix-version-detection-cli-upgrading-from-pre-SB-7-5bcc551261708075428)To request a new release of this pull request, mention the
@storybookjs/coreteam.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=26055