From 7987e76169d2340aa49330ac878a62a0c689fc99 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 29 Jan 2024 15:57:16 +0100 Subject: [PATCH 1/4] Refactor isCorePackage function to use versions map --- code/lib/core-common/src/utils/cli.ts | 32 +-------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/code/lib/core-common/src/utils/cli.ts b/code/lib/core-common/src/utils/cli.ts index be022eac4460..71081381be99 100644 --- a/code/lib/core-common/src/utils/cli.ts +++ b/code/lib/core-common/src/utils/cli.ts @@ -97,34 +97,4 @@ export const createLogStream = async ( }); }; -const PACKAGES_EXCLUDED_FROM_CORE = [ - '@storybook/addon-bench', - '@storybook/addon-console', - '@storybook/addon-onboarding', - '@storybook/addon-postcss', - '@storybook/addon-designs', - '@storybook/addon-styling', - '@storybook/addon-styling-webpack', - '@storybook/bench', - '@storybook/builder-vite', - '@storybook/csf', - '@storybook/design-system', - '@storybook/ember-cli-storybook', - '@storybook/eslint-config-storybook', - '@storybook/expect', - '@storybook/jest', - '@storybook/linter-config', - '@storybook/mdx1-csf', - '@storybook/mdx2-csf', - '@storybook/react-docgen-typescript-plugin', - '@storybook/storybook-deployer', - '@storybook/test-runner', - '@storybook/testing-library', - '@storybook/testing-react', - '@nrwl/storybook', - '@nx/storybook', -]; -export const isCorePackage = (pkg: string) => - pkg.startsWith('@storybook/') && - !pkg.startsWith('@storybook/preset-') && - !PACKAGES_EXCLUDED_FROM_CORE.includes(pkg); +export const isCorePackage = (pkg: string) => Object.keys(versions).includes(pkg); From 731531ebcfe32e9a5a348a7b1ed633ad97a95055 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 29 Jan 2024 15:57:26 +0100 Subject: [PATCH 2/4] Update name of getStorybookVersion to getCoercedStorybookVersion --- code/lib/cli/src/add.ts | 5 +++-- code/lib/cli/src/automigrate/helpers/mainConfigFile.ts | 4 ++-- code/lib/cli/src/automigrate/index.ts | 4 ++-- code/lib/core-common/src/utils/cli.test.ts | 5 ++--- code/lib/core-common/src/utils/cli.ts | 3 ++- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/code/lib/cli/src/add.ts b/code/lib/cli/src/add.ts index acfd7e4e6eb7..0321ec966fd7 100644 --- a/code/lib/cli/src/add.ts +++ b/code/lib/cli/src/add.ts @@ -1,7 +1,7 @@ import { getStorybookInfo, serverRequire, - getStorybookVersion, + getCoercedStorybookVersion, isCorePackage, JsPackageManagerFactory, type PackageManagerName, @@ -107,8 +107,9 @@ export async function add( // add to package.json const isStorybookAddon = addonName.startsWith('@storybook/'); const isAddonFromCore = isCorePackage(addonName); - const storybookVersion = await getStorybookVersion(packageManager); + const storybookVersion = await getCoercedStorybookVersion(packageManager); const version = versionSpecifier || (isAddonFromCore ? storybookVersion : latestVersion); + const addonWithVersion = SemVer.valid(version) ? `${addonName}@^${version}` : `${addonName}@${version}`; diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index c816ee90c5ec..c721dae39a31 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -12,7 +12,7 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import path from 'path'; import type { JsPackageManager } from '@storybook/core-common'; -import { getStorybookVersion } from '@storybook/core-common'; +import { getCoercedStorybookVersion } from '@storybook/core-common'; const logger = console; @@ -93,7 +93,7 @@ export const getStorybookData = async ({ configDir: configDirFromScript, previewConfig: previewConfigPath, } = getStorybookInfo(packageJson, userDefinedConfigDir); - const storybookVersion = await getStorybookVersion(packageManager); + const storybookVersion = await getCoercedStorybookVersion(packageManager); const configDir = userDefinedConfigDir || configDirFromScript || '.storybook'; diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index 604393de492c..3adeff5e0ead 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -10,7 +10,7 @@ import invariant from 'tiny-invariant'; import { getStorybookInfo, loadMainConfig, - getStorybookVersion, + getCoercedStorybookVersion, JsPackageManagerFactory, } from '@storybook/core-common'; import type { PackageManagerName } from '@storybook/core-common'; @@ -156,7 +156,7 @@ export async function runFixes({ userSpecifiedConfigDir ); - const storybookVersion = await getStorybookVersion(packageManager); + const storybookVersion = await getCoercedStorybookVersion(packageManager); if (!storybookVersion) { logger.info(dedent` diff --git a/code/lib/core-common/src/utils/cli.test.ts b/code/lib/core-common/src/utils/cli.test.ts index f2444debb22a..708a11ebbba8 100644 --- a/code/lib/core-common/src/utils/cli.test.ts +++ b/code/lib/core-common/src/utils/cli.test.ts @@ -5,13 +5,12 @@ describe('UTILS', () => { describe.each([ ['@storybook/react', true], ['@storybook/node-logger', true], - ['@storybook/addon-info', true], - ['@storybook/something-random', true], - ['@storybook/preset-create-react-app', false], ['@storybook/linter-config', false], ['@storybook/design-system', false], ['@storybook/addon-styling', false], ['@storybook/addon-styling-webpack', false], + ['@storybook/addon-webpack5-compiler-swc', false], + ['@storybook/addon-webpack5-compiler-babel', false], ['@nx/storybook', false], ['@nrwl/storybook', false], ])('isCorePackage', (input, output) => { diff --git a/code/lib/core-common/src/utils/cli.ts b/code/lib/core-common/src/utils/cli.ts index 71081381be99..0dd8eae5277d 100644 --- a/code/lib/core-common/src/utils/cli.ts +++ b/code/lib/core-common/src/utils/cli.ts @@ -4,6 +4,7 @@ import { join } from 'path'; import tempy from 'tempy'; import { rendererPackages } from './get-storybook-info'; import type { JsPackageManager } from '../js-package-manager'; +import versions from '../versions'; export function parseList(str: string): string[] { return str @@ -12,7 +13,7 @@ export function parseList(str: string): string[] { .filter((item) => item.length > 0); } -export async function getStorybookVersion(packageManager: JsPackageManager) { +export async function getCoercedStorybookVersion(packageManager: JsPackageManager) { const packages = ( await Promise.all( Object.keys(rendererPackages).map(async (pkg) => ({ From 66010a21046588edaf11a4e47de496aa5385aac5 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 Jan 2024 14:24:05 +0100 Subject: [PATCH 3/4] Add documentation to getCoercedStorybookVersion func --- code/lib/core-common/src/utils/cli.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/code/lib/core-common/src/utils/cli.ts b/code/lib/core-common/src/utils/cli.ts index 0dd8eae5277d..c4103bd59137 100644 --- a/code/lib/core-common/src/utils/cli.ts +++ b/code/lib/core-common/src/utils/cli.ts @@ -13,6 +13,12 @@ export function parseList(str: string): string[] { .filter((item) => item.length > 0); } +/** + * Given a package manager, returns the coerced version of Storybook. + * It tries to find renderer packages in the project and returns the coerced version of the first one found. + * Example: + * If @storybook/react version 8.0.0-alpha.14 is installed, it returns the coerced version 8.0.0 + */ export async function getCoercedStorybookVersion(packageManager: JsPackageManager) { const packages = ( await Promise.all( From 5c368f2e8341909971da4108399035c0e6a16065 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 Jan 2024 14:59:55 +0100 Subject: [PATCH 4/4] Refactor package version retrieval in JsPackageManager --- .../core-common/src/js-package-manager/JsPackageManager.ts | 5 ++++- code/lib/core-common/src/js-package-manager/NPMProxy.ts | 6 ------ code/lib/core-common/src/js-package-manager/PNPMProxy.ts | 7 ------- code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts | 6 ------ code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts | 6 ------ 5 files changed, 4 insertions(+), 26 deletions(-) diff --git a/code/lib/core-common/src/js-package-manager/JsPackageManager.ts b/code/lib/core-common/src/js-package-manager/JsPackageManager.ts index 79dfe4a5fb29..923a06409968 100644 --- a/code/lib/core-common/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/core-common/src/js-package-manager/JsPackageManager.ts @@ -55,7 +55,10 @@ export abstract class JsPackageManager { basePath?: string ): Promise; - public abstract getPackageVersion(packageName: string, basePath?: string): Promise; + async getPackageVersion(packageName: string, basePath = this.cwd): Promise { + const packageJSON = await this.getPackageJSON(packageName, basePath); + return packageJSON ? packageJSON.version ?? null : null; + } // NOTE: for some reason yarn prefers the npm registry in // local development, so always use npm diff --git a/code/lib/core-common/src/js-package-manager/NPMProxy.ts b/code/lib/core-common/src/js-package-manager/NPMProxy.ts index 6e4e2939fca1..62d8be2fec5c 100644 --- a/code/lib/core-common/src/js-package-manager/NPMProxy.ts +++ b/code/lib/core-common/src/js-package-manager/NPMProxy.ts @@ -4,7 +4,6 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import { existsSync, readFileSync } from 'fs'; import path from 'path'; -import semver from 'semver'; import { logger } from '@storybook/node-logger'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -102,11 +101,6 @@ export class NPMProxy extends JsPackageManager { return packageJson; } - public async getPackageVersion(packageName: string, basePath = this.cwd): Promise { - const packageJson = await this.getPackageJSON(packageName, basePath); - return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; - } - getInstallArgs(): string[] { if (!this.installArgs) { this.installArgs = []; diff --git a/code/lib/core-common/src/js-package-manager/PNPMProxy.ts b/code/lib/core-common/src/js-package-manager/PNPMProxy.ts index d6cb6c99c175..7e7f0279e226 100644 --- a/code/lib/core-common/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/core-common/src/js-package-manager/PNPMProxy.ts @@ -3,7 +3,6 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import path from 'path'; import fs from 'fs'; -import semver from 'semver'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; import type { InstallationMetadata, PackageMetadata } from './types'; @@ -159,12 +158,6 @@ export class PNPMProxy extends JsPackageManager { return JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); } - async getPackageVersion(packageName: string, basePath = this.cwd): Promise { - const packageJSON = await this.getPackageJSON(packageName, basePath); - - return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; - } - protected getResolutions(packageJson: PackageJson, versions: Record) { return { overrides: { diff --git a/code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts b/code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts index 039a06f956cf..2018e8f8ddf4 100644 --- a/code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts +++ b/code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts @@ -2,7 +2,6 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import { existsSync, readFileSync } from 'fs'; import path from 'path'; -import semver from 'semver'; import { createLogStream } from '../utils/cli'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -82,11 +81,6 @@ export class Yarn1Proxy extends JsPackageManager { return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; } - public async getPackageVersion(packageName: string, basePath = this.cwd): Promise { - const packageJson = await this.getPackageJSON(packageName, basePath); - return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; - } - public async findInstallations(pattern: string[]) { const commandResult = await this.executeCommand({ command: 'yarn', diff --git a/code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts b/code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts index 3dde844cba15..4f76868d395c 100644 --- a/code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts +++ b/code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts @@ -4,7 +4,6 @@ import { existsSync, readFileSync } from 'fs'; import path from 'path'; import { PosixFS, VirtualFS, ZipOpenFS } from '@yarnpkg/fslib'; import { getLibzipSync } from '@yarnpkg/libzip'; -import semver from 'semver'; import { createLogStream } from '../utils/cli'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -174,11 +173,6 @@ export class Yarn2Proxy extends JsPackageManager { return packageJson; } - async getPackageVersion(packageName: string, basePath = this.cwd): Promise { - const packageJSON = await this.getPackageJSON(packageName, basePath); - return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; - } - protected getResolutions(packageJson: PackageJson, versions: Record) { return { resolutions: {