From 1efcba5106ff560410aae964deed2ae9c8ab05bb Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Thu, 14 Mar 2024 14:51:03 +0100 Subject: [PATCH 1/4] check the real installed version to compare --- .../upgrade-storybook-related-dependencies.ts | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts index 5614b7e35ad..acbd0367cc5 100644 --- a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts +++ b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts @@ -1,6 +1,6 @@ import { dedent } from 'ts-dedent'; import { cyan, yellow } from 'chalk'; -import { valid, coerce } from 'semver'; +import { gt } from 'semver'; import type { JsPackageManager } from '@storybook/core-common'; import { isCorePackage } from '@storybook/core-common'; import type { Fix } from '../types'; @@ -16,29 +16,28 @@ interface Options { upgradable: PackageMetadata[]; } +const getInstalledVersion = async (packageName: string, packageManager: JsPackageManager) => { + const installations = await packageManager.findInstallations([packageName]); + if (!installations) { + return null; + } + + return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; +}; + async function getLatestVersions( packageManager: JsPackageManager, packages: [string, string][] ): Promise { return Promise.all( - packages.map(async ([packageName, beforeVersion]) => ({ + packages.map(async ([packageName]) => ({ packageName, - beforeVersion: coerce(beforeVersion)?.toString() || null, + beforeVersion: await getInstalledVersion(packageName, packageManager), afterVersion: await packageManager.latestVersion(packageName).catch(() => null), })) ); } -function isPackageUpgradable( - afterVersion: string, - packageName: string, - allDependencies: Record -) { - const installedVersion = coerce(allDependencies[packageName])?.toString(); - - return valid(afterVersion) && afterVersion !== installedVersion; -} - /** * Is the user upgrading to the `latest` version of Storybook? * Let's try to pull along some of the storybook related dependencies to `latest` as well! @@ -75,15 +74,13 @@ export const upgradeStorybookRelatedDependencies = { const packageVersions = await getLatestVersions(packageManager, uniquePackages); - const upgradablePackages = packageVersions.filter( - ({ packageName, afterVersion, beforeVersion }) => { - if (beforeVersion === null || afterVersion === null) { - return false; - } - - return isPackageUpgradable(afterVersion, packageName, allDependencies); + const upgradablePackages = packageVersions.filter(({ afterVersion, beforeVersion }) => { + if (beforeVersion === null || afterVersion === null) { + return false; } - ); + + return gt(afterVersion, beforeVersion); + }); return upgradablePackages.length > 0 ? { upgradable: upgradablePackages } : null; }, From 979ca6922159f90d9862ace79eed2da8a6cf02b6 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 14 Mar 2024 16:37:59 +0100 Subject: [PATCH 2/4] fix tests --- ...ade-storybook-related-dependencies.test.ts | 20 ++++++++++++------- .../upgrade-storybook-related-dependencies.ts | 16 +++++---------- .../getIncompatibleStorybookPackages.ts | 12 +++++++++++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts index 0c16309647b..df1e78ff77d 100644 --- a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts +++ b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts @@ -52,20 +52,26 @@ describe('upgrade-storybook-related-dependencies fix', () => { { packageName: 'storybook', packageVersion: '8.0.0', - availableUpgrade: undefined, + availableUpgrade: '8.0.0', hasIncompatibleDependencies: true, }, ]; vi.mocked(docsUtils.getIncompatibleStorybookPackages).mockResolvedValue(analyzedPackages); + vi.mocked(docsUtils.getInstalledVersion).mockImplementation( + async (pkgName) => + analyzedPackages.find((pkg) => pkg.packageName === pkgName)?.packageVersion || null + ); await expect( check({ packageManager: { - getAllDependencies: async () => ({ - '@chromatic-com/storybook': '1.2.9', - '@storybook/jest': '0.2.3', - '@storybook/preset-create-react-app': '3.2.0', - storybook: '8.0.0', - }), + getAllDependencies: async () => + analyzedPackages.reduce( + (acc, { packageName, packageVersion }) => { + acc[packageName] = packageVersion; + return acc; + }, + {} as Record + ), latestVersion: async (pkgName) => analyzedPackages.find((pkg) => pkg.packageName === pkgName)?.availableUpgrade || '', }, diff --git a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts index acbd0367cc5..1a96785e602 100644 --- a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts +++ b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts @@ -4,7 +4,10 @@ import { gt } from 'semver'; import type { JsPackageManager } from '@storybook/core-common'; import { isCorePackage } from '@storybook/core-common'; import type { Fix } from '../types'; -import { getIncompatibleStorybookPackages } from '../../doctor/getIncompatibleStorybookPackages'; +import { + getIncompatibleStorybookPackages, + getInstalledVersion, +} from '../../doctor/getIncompatibleStorybookPackages'; type PackageMetadata = { packageName: string; @@ -16,15 +19,6 @@ interface Options { upgradable: PackageMetadata[]; } -const getInstalledVersion = async (packageName: string, packageManager: JsPackageManager) => { - const installations = await packageManager.findInstallations([packageName]); - if (!installations) { - return null; - } - - return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; -}; - async function getLatestVersions( packageManager: JsPackageManager, packages: [string, string][] @@ -32,7 +26,7 @@ async function getLatestVersions( return Promise.all( packages.map(async ([packageName]) => ({ packageName, - beforeVersion: await getInstalledVersion(packageName, packageManager), + beforeVersion: await getInstalledVersion(packageName, packageManager).catch(() => null), afterVersion: await packageManager.latestVersion(packageName).catch(() => null), })) ); diff --git a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts index 09f0c40d5bb..09f6d5e8d31 100644 --- a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts +++ b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts @@ -19,6 +19,18 @@ type Context = { skipErrors?: boolean; }; +export const getInstalledVersion = async ( + packageName: string, + packageManager: JsPackageManager +) => { + const installations = await packageManager.findInstallations([packageName]); + if (!installations) { + return null; + } + + return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; +}; + export const checkPackageCompatibility = async (dependency: string, context: Context) => { const { currentStorybookVersion, skipErrors, packageManager } = context; try { From dcc314cb90646c62548781591d8506df6021206f Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 14 Mar 2024 16:53:05 +0100 Subject: [PATCH 3/4] move getInstalledVersion utility to package manager abstraction --- ...grade-storybook-related-dependencies.test.ts | 6 ++---- .../upgrade-storybook-related-dependencies.ts | 7 ++----- .../doctor/getIncompatibleStorybookPackages.ts | 12 ------------ .../src/js-package-manager/JsPackageManager.ts | 17 +++++++++++++++++ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts index df1e78ff77d..7a8beeba0f7 100644 --- a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts +++ b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.test.ts @@ -57,10 +57,6 @@ describe('upgrade-storybook-related-dependencies fix', () => { }, ]; vi.mocked(docsUtils.getIncompatibleStorybookPackages).mockResolvedValue(analyzedPackages); - vi.mocked(docsUtils.getInstalledVersion).mockImplementation( - async (pkgName) => - analyzedPackages.find((pkg) => pkg.packageName === pkgName)?.packageVersion || null - ); await expect( check({ packageManager: { @@ -74,6 +70,8 @@ describe('upgrade-storybook-related-dependencies fix', () => { ), latestVersion: async (pkgName) => analyzedPackages.find((pkg) => pkg.packageName === pkgName)?.availableUpgrade || '', + getInstalledVersion: async (pkgName) => + analyzedPackages.find((pkg) => pkg.packageName === pkgName)?.packageVersion || null, }, }) ).resolves.toMatchInlineSnapshot(` diff --git a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts index 1a96785e602..c5a9d4218bb 100644 --- a/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts +++ b/code/lib/cli/src/automigrate/fixes/upgrade-storybook-related-dependencies.ts @@ -4,10 +4,7 @@ import { gt } from 'semver'; import type { JsPackageManager } from '@storybook/core-common'; import { isCorePackage } from '@storybook/core-common'; import type { Fix } from '../types'; -import { - getIncompatibleStorybookPackages, - getInstalledVersion, -} from '../../doctor/getIncompatibleStorybookPackages'; +import { getIncompatibleStorybookPackages } from '../../doctor/getIncompatibleStorybookPackages'; type PackageMetadata = { packageName: string; @@ -26,7 +23,7 @@ async function getLatestVersions( return Promise.all( packages.map(async ([packageName]) => ({ packageName, - beforeVersion: await getInstalledVersion(packageName, packageManager).catch(() => null), + beforeVersion: await packageManager.getInstalledVersion(packageName).catch(() => null), afterVersion: await packageManager.latestVersion(packageName).catch(() => null), })) ); diff --git a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts index 09f6d5e8d31..09f0c40d5bb 100644 --- a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts +++ b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts @@ -19,18 +19,6 @@ type Context = { skipErrors?: boolean; }; -export const getInstalledVersion = async ( - packageName: string, - packageManager: JsPackageManager -) => { - const installations = await packageManager.findInstallations([packageName]); - if (!installations) { - return null; - } - - return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; -}; - export const checkPackageCompatibility = async (dependency: string, context: Context) => { const { currentStorybookVersion, skipErrors, packageManager } = context; try { 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 8523d7224ed..3ef0d7b7508 100644 --- a/code/lib/core-common/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/core-common/src/js-package-manager/JsPackageManager.ts @@ -535,6 +535,23 @@ export abstract class JsPackageManager { } } + /** + * Returns the installed (within node_modules or pnp zip) version of a specified package + */ + public async getInstalledVersion(packageName: string): Promise { + const installations = await this.findInstallations([packageName]); + if (!installations) { + return null; + } + + console.log({ + installations, + version: Object.entries(installations.dependencies)[0]?.[1]?.[0].version, + }); + + return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; + } + public async executeCommand({ command, args = [], From 164ea49f02b30f748ce36d590102f2daf90db52b Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 15 Mar 2024 09:20:59 +0100 Subject: [PATCH 4/4] Update code/lib/core-common/src/js-package-manager/JsPackageManager.ts --- .../core-common/src/js-package-manager/JsPackageManager.ts | 5 ----- 1 file changed, 5 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 3ef0d7b7508..42a31985047 100644 --- a/code/lib/core-common/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/core-common/src/js-package-manager/JsPackageManager.ts @@ -544,11 +544,6 @@ export abstract class JsPackageManager { return null; } - console.log({ - installations, - version: Object.entries(installations.dependencies)[0]?.[1]?.[0].version, - }); - return Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null; }