From 38a6fcb58628c34db014351c7edbdd74e8f661ce Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 10:54:06 +0200 Subject: [PATCH 1/3] CLI: install the same version as the user in sb-scripts automigration Before it was installing latest, which could result in a wrong situation --- .../src/automigrate/fixes/sb-scripts.test.ts | 70 ++++++++++--------- .../cli/src/automigrate/fixes/sb-scripts.ts | 14 +++- code/lib/cli/src/helpers.test.ts | 24 +++++++ code/lib/cli/src/helpers.ts | 18 +++++ 4 files changed, 91 insertions(+), 35 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts index 443b3b2a2c0b..6d1ad7480c66 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts @@ -88,16 +88,18 @@ describe('sb scripts fix', () => { checkSbScripts({ packageJson, }) - ).resolves.toEqual({ - storybookScripts: { - official: { - storybook: 'storybook dev -p 6006', - 'build-storybook': 'storybook build -o build/storybook', + ).resolves.toEqual( + expect.objectContaining({ + storybookScripts: { + official: { + storybook: 'storybook dev -p 6006', + 'build-storybook': 'storybook build -o build/storybook', + }, + custom: {}, }, - custom: {}, - }, - storybookVersion: '^7.0.0-alpha.0', - }); + storybookVersion: '^7.0.0-alpha.0', + }) + ); }); }); @@ -122,18 +124,20 @@ describe('sb scripts fix', () => { checkSbScripts({ packageJson, }) - ).resolves.toEqual({ - storybookScripts: { - custom: { - 'sb:start': 'start-storybook -p 6006', - 'sb:build': 'build-storybook -o buid/storybook', - 'test-storybook:ci': - 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', + ).resolves.toEqual( + expect.objectContaining({ + storybookScripts: { + custom: { + 'sb:start': 'start-storybook -p 6006', + 'sb:build': 'build-storybook -o buid/storybook', + 'test-storybook:ci': + 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', + }, + official: {}, }, - official: {}, - }, - storybookVersion: '^7.0.0-alpha.0', - }); + storybookVersion: '^7.0.0-alpha.0', + }) + ); }); describe('with old official and custom scripts', () => { @@ -156,19 +160,21 @@ describe('sb scripts fix', () => { checkSbScripts({ packageJson, }) - ).resolves.toEqual({ - storybookScripts: { - custom: { - 'storybook:build': 'build-storybook -o buid/storybook', - 'test-storybook:ci': - 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', + ).resolves.toEqual( + expect.objectContaining({ + storybookScripts: { + custom: { + 'storybook:build': 'build-storybook -o buid/storybook', + 'test-storybook:ci': + 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "yarn build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', + }, + official: { + storybook: 'storybook dev -p 6006', + }, }, - official: { - storybook: 'storybook dev -p 6006', - }, - }, - storybookVersion: '^7.0.0-alpha.0', - }); + storybookVersion: '^7.0.0-alpha.0', + }) + ); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts index 575b61c17713..dad286a88ed7 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts @@ -3,6 +3,8 @@ import { dedent } from 'ts-dedent'; import semver from '@storybook/semver'; import { getStorybookInfo } from '@storybook/core-common'; import { Fix } from '../types'; +import { getStorybookVersionSpecifier } from '../../helpers'; +import { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; interface SbScriptsRunOptions { storybookScripts: { @@ -10,6 +12,7 @@ interface SbScriptsRunOptions { official: Record; }; storybookVersion: string; + packageJson: PackageJsonWithDepsAndDevDeps; } const logger = console; @@ -76,7 +79,9 @@ export const sbScripts: Fix = { .replace('build-storybook', 'storybook build'); }); - return semver.gte(storybookCoerced, '6.0.0') ? { storybookScripts, storybookVersion } : null; + return semver.gte(storybookCoerced, '6.0.0') + ? { packageJson, storybookScripts, storybookVersion } + : null; }, prompt({ storybookVersion }) { @@ -104,13 +109,16 @@ export const sbScripts: Fix = { .join('\n\n'); }, - async run({ result: { storybookScripts }, packageManager, dryRun }) { + async run({ result: { storybookScripts, packageJson }, packageManager, dryRun }) { logger.log(); logger.info(`Adding 'storybook' as dev dependency`); logger.log(); if (!dryRun) { - packageManager.addDependencies({ installAsDevDependencies: true }, ['storybook']); + const versionToInstall = getStorybookVersionSpecifier(packageJson); + packageManager.addDependencies({ installAsDevDependencies: true }, [ + `storybook@${versionToInstall}`, + ]); } logger.info(`Updating scripts in package.json`); diff --git a/code/lib/cli/src/helpers.test.ts b/code/lib/cli/src/helpers.test.ts index b2b5f90964d2..ce701e6fedee 100644 --- a/code/lib/cli/src/helpers.test.ts +++ b/code/lib/cli/src/helpers.test.ts @@ -108,4 +108,28 @@ describe('Helpers', () => { helpers.copyComponents(framework, SupportedLanguage.JAVASCRIPT) ).rejects.toThrowError(expectedMessage); }); + + describe.only('getStorybookVersionSpecifier', () => { + it(`should return the specifier if storybook lib exists in package.json`, () => { + expect( + helpers.getStorybookVersionSpecifier({ + dependencies: {}, + devDependencies: { + '@storybook/react': '^x.x.x', + }, + }) + ).toEqual('^x.x.x'); + }); + + it(`should throw an error if no package is found`, () => { + expect(() => { + helpers.getStorybookVersionSpecifier({ + dependencies: {}, + devDependencies: { + 'something-else': '^x.x.x', + }, + }); + }).toThrowError("Couldn't find any official storybook packages in package.json"); + }); + }); }); diff --git a/code/lib/cli/src/helpers.ts b/code/lib/cli/src/helpers.ts index ec5d9fae8267..1db460148191 100644 --- a/code/lib/cli/src/helpers.ts +++ b/code/lib/cli/src/helpers.ts @@ -9,6 +9,7 @@ import stripJsonComments from 'strip-json-comments'; import { SupportedRenderers, SupportedLanguage } from './project_types'; import { JsPackageManager, PackageJson, PackageJsonWithDepsAndDevDeps } from './js-package-manager'; import { getBaseDir } from './dirs'; +import storybookMonorepoPackages from './versions'; const logger = console; @@ -222,3 +223,20 @@ export async function copyComponents(framework: SupportedRenderers, language: Su overwrite: true, }); } + +// Given a package.json, finds any official storybook package within it +// and if it exists, returns the version of that package from the specified package.json +export function getStorybookVersionSpecifier(packageJson: PackageJsonWithDepsAndDevDeps) { + const allDeps = { ...packageJson.dependencies, ...packageJson.devDependencies }; + const storybookPackage = Object.keys(allDeps).find( + (name: keyof typeof storybookMonorepoPackages) => { + return storybookMonorepoPackages[name]; + } + ); + + if (!storybookPackage) { + throw new Error(`Couldn't find any official storybook packages in package.json`); + } + + return allDeps[storybookPackage]; +} From 0548b254c3465c75f8e36308beba7f5c315f82a0 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 10:57:19 +0200 Subject: [PATCH 2/3] CLI: fix version requirement in sb-script automigration --- code/lib/cli/src/automigrate/fixes/sb-scripts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts index dad286a88ed7..dcd2f329845d 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts @@ -79,7 +79,7 @@ export const sbScripts: Fix = { .replace('build-storybook', 'storybook build'); }); - return semver.gte(storybookCoerced, '6.0.0') + return semver.gte(storybookCoerced, '7.0.0') ? { packageJson, storybookScripts, storybookVersion } : null; }, From c95f7dce199a0e5e640ef4ae4a7dff9f0e5cc9c8 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 11:02:15 +0200 Subject: [PATCH 3/3] remove .only from test --- code/lib/cli/src/helpers.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/helpers.test.ts b/code/lib/cli/src/helpers.test.ts index ce701e6fedee..9c78e7873516 100644 --- a/code/lib/cli/src/helpers.test.ts +++ b/code/lib/cli/src/helpers.test.ts @@ -109,7 +109,7 @@ describe('Helpers', () => { ).rejects.toThrowError(expectedMessage); }); - describe.only('getStorybookVersionSpecifier', () => { + describe('getStorybookVersionSpecifier', () => { it(`should return the specifier if storybook lib exists in package.json`, () => { expect( helpers.getStorybookVersionSpecifier({