Merge pull request #25752 from storybookjs/jeppe/fix-upgrade-version-…

…detection

CLI: Fix `upgrade` detecting the wrong version of existing Storybooks

code/lib/cli/src/upgrade.test.ts

@@ -1,5 +1,4 @@
 import { describe, it, expect, vi } from 'vitest';
-import { getStorybookCoreVersion } from '@storybook/telemetry';
 import {
   UpgradeStorybookToLowerVersionError,
   UpgradeStorybookToSameVersionError,
@@ -8,11 +7,18 @@ import { doUpgrade, getStorybookVersion } from './upgrade';
 
 import type * as sbcc from '@storybook/core-common';
 
+const findInstallationsMock = vi.fn<string[], Promise<sbcc.InstallationMetadata | undefined>>();
+
 vi.mock('@storybook/telemetry');
 vi.mock('@storybook/core-common', async (importOriginal) => {
   const originalModule = (await importOriginal()) as typeof sbcc;
   return {
     ...originalModule,
+    JsPackageManagerFactory: {
+      getPackageManager: () => ({
+        findInstallations: findInstallationsMock,
+      }),
+    },
     versions: Object.keys(originalModule.versions).reduce(
       (acc, key) => {
         acc[key] = '8.0.0';
@@ -46,13 +52,37 @@ describe.each([
 
 describe('Upgrade errors', () => {
   it('should throw an error when upgrading to a lower version number', async () => {
-    vi.mocked(getStorybookCoreVersion).mockResolvedValue('8.1.0');
+    findInstallationsMock.mockResolvedValue({
+      dependencies: {
+        '@storybook/cli': [
+          {
+            version: '8.1.0',
+          },
+        ],
+      },
+      duplicatedDependencies: {},
+      infoCommand: '',
+      dedupeCommand: '',
+    });
 
     await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToLowerVersionError);
+    expect(findInstallationsMock).toHaveBeenCalledWith(['storybook', '@storybook/cli']);
   });
   it('should throw an error when upgrading to the same version number', async () => {
-    vi.mocked(getStorybookCoreVersion).mockResolvedValue('8.0.0');
+    findInstallationsMock.mockResolvedValue({
+      dependencies: {
+        '@storybook/cli': [
+          {
+            version: '8.0.0',
+          },
+        ],
+      },
+      duplicatedDependencies: {},
+      infoCommand: '',
+      dedupeCommand: '',
+    });
 
     await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToSameVersionError);
+    expect(findInstallationsMock).toHaveBeenCalledWith(['storybook', '@storybook/cli']);
   });
 });

code/lib/cli/src/upgrade.ts

@@ -1,5 +1,5 @@
 import { sync as spawnSync } from 'cross-spawn';
-import { telemetry, getStorybookCoreVersion } from '@storybook/telemetry';
+import { telemetry } from '@storybook/telemetry';
 import semver, { eq, lt, prerelease } from 'semver';
 import { logger } from '@storybook/node-logger';
 import { withTelemetry } from '@storybook/core-server';
@@ -11,7 +11,7 @@ import {
 import chalk from 'chalk';
 import dedent from 'ts-dedent';
 import boxen from 'boxen';
-import type { PackageManagerName } from '@storybook/core-common';
+import type { JsPackageManager, PackageManagerName } from '@storybook/core-common';
 import {
   JsPackageManagerFactory,
   isCorePackage,
@@ -37,6 +37,18 @@ export const getStorybookVersion = (line: string) => {
   };
 };
 
+const getInstalledStorybookVersion = async (packageManager: JsPackageManager) => {
+  const installations = await packageManager.findInstallations(['storybook', '@storybook/cli']);
+  if (!installations) {
+    return;
+  }
+  const cliVersion = installations.dependencies['@storybook/cli']?.[0].version;
+  if (cliVersion) {
+    return cliVersion;
+  }
+  return installations.dependencies['storybook']?.[0].version;
+};
+
 const deprecatedPackages = [
   {
     minVersion: '6.0.0-alpha.0',
@@ -113,8 +125,9 @@ export const doUpgrade = async ({
 }: UpgradeOptions) => {
   const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr });
 
-  // If we can't determine the existing version (Yarn PnP), fallback to v0.0.0 to not block the upgrade
-  const beforeVersion = (await getStorybookCoreVersion()) ?? '0.0.0';
+  // If we can't determine the existing version fallback to v0.0.0 to not block the upgrade
+  const beforeVersion = (await getInstalledStorybookVersion(packageManager)) ?? '0.0.0';
+
   const currentVersion = versions['@storybook/cli'];
   const isCanary = currentVersion.startsWith('0.0.0');
 
@@ -206,7 +219,7 @@ export const doUpgrade = async ({
     automigrationResults = await automigrate({ dryRun, yes, packageManager: pkgMgr, configDir });
   }
   if (!options.disableTelemetry) {
-    const afterVersion = await getStorybookCoreVersion();
+    const afterVersion = await getInstalledStorybookVersion(packageManager);
     const { preCheckFailure, fixResults } = automigrationResults || {};
     const automigrationTelemetry = {
       automigrationResults: preCheckFailure ? null : fixResults,

code/lib/telemetry/src/index.ts

@@ -11,8 +11,6 @@ export * from './storybook-metadata';
 
 export * from './types';
 
-export { getStorybookCoreVersion } from './package-json';
-
 export { getPrecedingUpgrade } from './event-cache';
 
 export { addToGlobalContext } from './telemetry';

code/lib/telemetry/src/package-json.ts

@@ -27,9 +27,3 @@ export const getActualPackageJson = async (packageName: string) => {
   const packageJson = await fs.readJson(resolvedPackageJson);
   return packageJson;
 };
-
-// Note that this probably doesn't work in Yarn PNP mode because @storybook/telemetry doesn't depend on @storybook/cli
-export const getStorybookCoreVersion = async () => {
-  const { version } = await getActualPackageVersion('@storybook/cli');
-  return version;
-};