From 7bff9d13a0a5986f97f47e083a3adeae7e03895c Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Tue, 21 May 2024 13:16:34 +0200 Subject: [PATCH 1/3] The UpgradeStorybookToSameVersionError is too flaky, so only logging it for now and continue the upgrade as normal --- code/lib/cli/src/upgrade.ts | 4 +++- code/lib/core-events/src/errors/server-errors.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index 4483bdb0f2e3..a23d5ed313ed 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -142,8 +142,10 @@ export const doUpgrade = async ({ if (!isCanary && lt(currentVersion, beforeVersion)) { throw new UpgradeStorybookToLowerVersionError({ beforeVersion, currentVersion }); } + if (!isCanary && eq(currentVersion, beforeVersion)) { - throw new UpgradeStorybookToSameVersionError({ beforeVersion }); + // Not throwing, as the beforeVersion calculation doesn't always work in monorepos. + logger.error(new UpgradeStorybookToSameVersionError({ beforeVersion })); } const [latestVersion, packageJson] = await Promise.all([ diff --git a/code/lib/core-events/src/errors/server-errors.ts b/code/lib/core-events/src/errors/server-errors.ts index efd3c929de0c..e6719945d3f6 100644 --- a/code/lib/core-events/src/errors/server-errors.ts +++ b/code/lib/core-events/src/errors/server-errors.ts @@ -545,7 +545,7 @@ export class UpgradeStorybookToSameVersionError extends StorybookError { template() { return dedent` - You are trying to upgrade Storybook to the same version that is currently installed in the project, version ${this.data.beforeVersion}. This is not supported. + You are upgrading Storybook to the same version that is currently installed in the project, version ${this.data.beforeVersion}. This usually happens when running the upgrade command without a version specifier, e.g. "npx storybook upgrade". This will cause npm to run the globally cached storybook binary, which might be the same version that you already have. From 51fcb1bd65397f50f34cebe4e76aeb1ed2ff1781 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 31 May 2024 14:45:42 +0200 Subject: [PATCH 2/3] Update logger to warn instead of error for same version upgrade error --- code/lib/cli/src/upgrade.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index a23d5ed313ed..892c04148001 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -145,7 +145,7 @@ export const doUpgrade = async ({ if (!isCanary && eq(currentVersion, beforeVersion)) { // Not throwing, as the beforeVersion calculation doesn't always work in monorepos. - logger.error(new UpgradeStorybookToSameVersionError({ beforeVersion })); + logger.warn(new UpgradeStorybookToSameVersionError({ beforeVersion }).message); } const [latestVersion, packageJson] = await Promise.all([ From 394b98453cb8ea70249b131900495e8a3cc61fd4 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Tue, 4 Jun 2024 15:13:24 +0200 Subject: [PATCH 3/3] Adjust spec --- code/lib/cli/src/upgrade.test.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/code/lib/cli/src/upgrade.test.ts b/code/lib/cli/src/upgrade.test.ts index c64bf35a9399..87440b78cb5a 100644 --- a/code/lib/cli/src/upgrade.test.ts +++ b/code/lib/cli/src/upgrade.test.ts @@ -1,10 +1,8 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import * as sbcc from '@storybook/core-common'; -import { - UpgradeStorybookToLowerVersionError, - UpgradeStorybookToSameVersionError, -} from '@storybook/core-events/server-errors'; +import { UpgradeStorybookToLowerVersionError } from '@storybook/core-events/server-errors'; import { doUpgrade, getStorybookVersion } from './upgrade'; +import { logger } from '@storybook/node-logger'; const findInstallationsMock = vi.fn>(); @@ -16,6 +14,8 @@ vi.mock('@storybook/core-common', async (importOriginal) => { JsPackageManagerFactory: { getPackageManager: () => ({ findInstallations: findInstallationsMock, + latestVersion: async () => '8.0.0', + retrievePackageJson: async () => {}, getAllDependencies: async () => ({ storybook: '8.0.0' }), }), }, @@ -68,7 +68,7 @@ describe('Upgrade errors', () => { await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToLowerVersionError); expect(findInstallationsMock).toHaveBeenCalledWith(Object.keys(sbcc.versions)); }); - it('should throw an error when upgrading to the same version number', async () => { + it('should show a warning when upgrading to the same version number', async () => { findInstallationsMock.mockResolvedValue({ dependencies: { '@storybook/cli': [ @@ -82,7 +82,15 @@ describe('Upgrade errors', () => { dedupeCommand: '', }); - await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToSameVersionError); + // Mock as a throw, so that we don't have to mock the content of the doUpgrade fn that comes after it + vi.spyOn(logger, 'warn').mockImplementation((error) => { + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw error; + }); + + await expect(doUpgrade({ packageManager: 'npm' } as any)).rejects.toContain( + 'You are upgrading Storybook to the same version that is currently installed in the project' + ); expect(findInstallationsMock).toHaveBeenCalledWith(Object.keys(sbcc.versions)); }); });