Skip to content

Commit 620cb80

Browse files
committed
Merge pull request #25752 from storybookjs/jeppe/fix-upgrade-version-detection
CLI: Fix `upgrade` detecting the wrong version of existing Storybooks (cherry picked from commit 7c21c34)
1 parent 81561cd commit 620cb80

4 files changed

Lines changed: 67 additions & 24 deletions

File tree

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

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { getStorybookCoreVersion } from '@storybook/telemetry';
21
import {
32
UpgradeStorybookToLowerVersionError,
43
UpgradeStorybookToSameVersionError,
@@ -16,6 +15,20 @@ jest.mock('./versions', () => {
1615
};
1716
});
1817

18+
const findInstallationsMock = jest.fn();
19+
20+
jest.mock('./js-package-manager', () => {
21+
const originalModule = jest.requireActual('./js-package-manager');
22+
return {
23+
...originalModule,
24+
JsPackageManagerFactory: {
25+
getPackageManager: () => ({
26+
findInstallations: findInstallationsMock,
27+
}),
28+
},
29+
};
30+
});
31+
1932
describe.each([
2033
['│ │ │ ├── @babel/code-frame@7.10.3 deduped', null],
2134
[
@@ -39,13 +52,37 @@ describe.each([
3952

4053
describe('Upgrade errors', () => {
4154
it('should throw an error when upgrading to a lower version number', async () => {
42-
jest.mocked(getStorybookCoreVersion).mockResolvedValue('8.1.0');
55+
findInstallationsMock.mockResolvedValue({
56+
dependencies: {
57+
'@storybook/cli': [
58+
{
59+
version: '8.1.0',
60+
},
61+
],
62+
},
63+
duplicatedDependencies: {},
64+
infoCommand: '',
65+
dedupeCommand: '',
66+
});
4367

4468
await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToLowerVersionError);
69+
expect(findInstallationsMock).toHaveBeenCalledWith(['storybook', '@storybook/cli']);
4570
});
4671
it('should throw an error when upgrading to the same version number', async () => {
47-
jest.mocked(getStorybookCoreVersion).mockResolvedValue('8.0.0');
72+
findInstallationsMock.mockResolvedValue({
73+
dependencies: {
74+
'@storybook/cli': [
75+
{
76+
version: '8.0.0',
77+
},
78+
],
79+
},
80+
duplicatedDependencies: {},
81+
infoCommand: '',
82+
dedupeCommand: '',
83+
});
4884

4985
await expect(doUpgrade({} as any)).rejects.toThrowError(UpgradeStorybookToSameVersionError);
86+
expect(findInstallationsMock).toHaveBeenCalledWith(['storybook', '@storybook/cli']);
5087
});
5188
});

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

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { sync as spawnSync } from 'cross-spawn';
2-
import { telemetry, getStorybookCoreVersion } from '@storybook/telemetry';
2+
import { telemetry } from '@storybook/telemetry';
33
import semver, { coerce, eq, lt } from 'semver';
44
import { deprecate, logger } from '@storybook/node-logger';
55
import { withTelemetry } from '@storybook/core-server';
@@ -11,7 +11,11 @@ import {
1111
import chalk from 'chalk';
1212
import dedent from 'ts-dedent';
1313
import boxen from 'boxen';
14-
import type { PackageJsonWithMaybeDeps, PackageManagerName } from './js-package-manager';
14+
import type {
15+
JsPackageManager,
16+
PackageJsonWithMaybeDeps,
17+
PackageManagerName,
18+
} from './js-package-manager';
1519
import { JsPackageManagerFactory, getPackageDetails, useNpmWarning } from './js-package-manager';
1620
import { commandLog } from './helpers';
1721
import { automigrate } from './automigrate';
@@ -34,6 +38,18 @@ export const getStorybookVersion = (line: string) => {
3438
};
3539
};
3640

41+
const getInstalledStorybookVersion = async (packageManager: JsPackageManager) => {
42+
const installations = await packageManager.findInstallations(['storybook', '@storybook/cli']);
43+
if (!installations) {
44+
return undefined;
45+
}
46+
const cliVersion = installations.dependencies['@storybook/cli']?.[0].version;
47+
if (cliVersion) {
48+
return cliVersion;
49+
}
50+
return installations.dependencies['storybook']?.[0].version;
51+
};
52+
3753
const deprecatedPackages = [
3854
{
3955
minVersion: '6.0.0-alpha.0',
@@ -91,9 +107,7 @@ export const checkVersionConsistency = () => {
91107
});
92108
};
93109

94-
/**
95-
* DEPRECATED BEHAVIOR SECTION
96-
*/
110+
// #region DEPRECATED BEHAVIOR SECTION
97111

98112
type ExtraFlags = Record<string, string[]>;
99113
const EXTRA_FLAGS: ExtraFlags = {
@@ -170,7 +184,8 @@ export const deprecatedUpgrade = async ({
170184
}
171185
const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr });
172186

173-
const beforeVersion = await getStorybookCoreVersion();
187+
// If we can't determine the existing version fallback to v0.0.0 to not block the upgrade
188+
const beforeVersion = (await getInstalledStorybookVersion(packageManager)) ?? '0.0.0';
174189

175190
commandLog(`Checking for latest versions of '@storybook/*' packages`);
176191

@@ -221,7 +236,7 @@ export const deprecatedUpgrade = async ({
221236
automigrationResults = await automigrate({ dryRun, yes, packageManager: pkgMgr, configDir });
222237
}
223238
if (!options.disableTelemetry) {
224-
const afterVersion = await getStorybookCoreVersion();
239+
const afterVersion = await getInstalledStorybookVersion(packageManager);
225240
const { preCheckFailure, fixResults } = automigrationResults || {};
226241
const automigrationTelemetry = {
227242
automigrationResults: preCheckFailure ? null : fixResults,
@@ -237,9 +252,7 @@ export const deprecatedUpgrade = async ({
237252
}
238253
};
239254

240-
/**
241-
* DEPRECATED BEHAVIOR SECTION END
242-
*/
255+
// #endregion DEPRECATED BEHAVIOR SECTION
243256

244257
export interface UpgradeOptions {
245258
/**
@@ -292,8 +305,9 @@ export const doUpgrade = async ({
292305

293306
const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr });
294307

295-
// If we can't determine the existing version (Yarn PnP), fallback to v0.0.0 to not block the upgrade
296-
const beforeVersion = (await getStorybookCoreVersion()) ?? '0.0.0';
308+
// If we can't determine the existing version fallback to v0.0.0 to not block the upgrade
309+
const beforeVersion = (await getInstalledStorybookVersion(packageManager)) ?? '0.0.0';
310+
297311
const currentVersion = versions['@storybook/cli'];
298312
const isCanary = currentVersion.startsWith('0.0.0');
299313

@@ -385,7 +399,7 @@ export const doUpgrade = async ({
385399
automigrationResults = await automigrate({ dryRun, yes, packageManager: pkgMgr, configDir });
386400
}
387401
if (!options.disableTelemetry) {
388-
const afterVersion = await getStorybookCoreVersion();
402+
const afterVersion = await getInstalledStorybookVersion(packageManager);
389403
const { preCheckFailure, fixResults } = automigrationResults || {};
390404
const automigrationTelemetry = {
391405
automigrationResults: preCheckFailure ? null : fixResults,

‎code/lib/telemetry/src/index.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ export * from './storybook-metadata';
1111

1212
export * from './types';
1313

14-
export { getStorybookCoreVersion } from './package-json';
15-
1614
export { getPrecedingUpgrade } from './event-cache';
1715

1816
export { addToGlobalContext } from './telemetry';

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,3 @@ export const getActualPackageJson = async (packageName: string) => {
2727
const packageJson = await fs.readJson(resolvedPackageJson);
2828
return packageJson;
2929
};
30-
31-
// Note that this probably doesn't work in Yarn PNP mode because @storybook/telemetry doesn't depend on @storybook/cli
32-
export const getStorybookCoreVersion = async () => {
33-
const { version } = await getActualPackageVersion('@storybook/cli');
34-
return version;
35-
};

0 commit comments

Comments
 (0)