From c7d5c02865d68fd9878e2e409173e58467c5122f Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 13 Dec 2022 11:23:23 +0100 Subject: [PATCH] CLI: split sb-scripts into two different migrations - One that takes care of sb binaries - Another that takes care of sb scripts --- code/lib/cli/src/automigrate/fixes/index.ts | 2 + .../src/automigrate/fixes/sb-binary.test.ts | 77 +++++++++++++ .../cli/src/automigrate/fixes/sb-binary.ts | 108 ++++++++++++++++++ .../src/automigrate/fixes/sb-scripts.test.ts | 22 +--- .../cli/src/automigrate/fixes/sb-scripts.ts | 44 +++---- 5 files changed, 207 insertions(+), 46 deletions(-) create mode 100644 code/lib/cli/src/automigrate/fixes/sb-binary.test.ts create mode 100644 code/lib/cli/src/automigrate/fixes/sb-binary.ts diff --git a/code/lib/cli/src/automigrate/fixes/index.ts b/code/lib/cli/src/automigrate/fixes/index.ts index fbe9f4007843..41a379d9c6ea 100644 --- a/code/lib/cli/src/automigrate/fixes/index.ts +++ b/code/lib/cli/src/automigrate/fixes/index.ts @@ -8,6 +8,7 @@ import { mainjsFramework } from './mainjsFramework'; import { eslintPlugin } from './eslint-plugin'; import { builderVite } from './builder-vite'; import { sbScripts } from './sb-scripts'; +import { sbBinary } from './sb-binary'; import { newFrameworks } from './new-frameworks'; import { removedGlobalClientAPIs } from './remove-global-client-apis'; import { mdx1to2 } from './mdx-1-to-2'; @@ -26,6 +27,7 @@ export const fixes: Fix[] = [ eslintPlugin, sveltekitFramework, builderVite, + sbBinary, sbScripts, newFrameworks, removedGlobalClientAPIs, diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts new file mode 100644 index 000000000000..a3d68cf6dfa8 --- /dev/null +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts @@ -0,0 +1,77 @@ +import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import { sbBinary } from './sb-binary'; + +const checkStorybookBinary = async ({ packageJson }: { packageJson: PackageJson }) => { + const packageManager = { + retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + } as JsPackageManager; + return sbBinary.check({ packageManager }); +}; + +describe('storybook-binary fix', () => { + describe('sb < 7.0', () => { + describe('does nothing', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + it('should no-op', async () => { + await expect( + checkStorybookBinary({ + packageJson, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + + describe('sb >= 7.0', () => { + it('should add storybook dependency if not present', async () => { + const packageJson = { + dependencies: { + '@storybook/react': '^7.0.0-alpha.0', + }, + }; + await expect( + checkStorybookBinary({ + packageJson, + }) + ).resolves.toEqual( + expect.objectContaining({ + hasSbBinary: false, + hasStorybookBinary: false, + }) + ); + }); + + it('should remove sb dependency if it is present', async () => { + const packageJson = { + dependencies: { + '@storybook/react': '^7.0.0-alpha.0', + sb: '6.5.0', + }, + }; + await expect( + checkStorybookBinary({ + packageJson, + }) + ).resolves.toEqual( + expect.objectContaining({ + hasSbBinary: true, + hasStorybookBinary: false, + }) + ); + }); + + it('should no op if storybook is present and sb is not present', async () => { + const packageJson = { + dependencies: { + '@storybook/react': '^7.0.0-alpha.0', + storybook: '^7.0.0-alpha.0', + }, + }; + await expect( + checkStorybookBinary({ + packageJson, + }) + ).resolves.toBeNull(); + }); + }); +}); diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.ts new file mode 100644 index 000000000000..075e841bb7cf --- /dev/null +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.ts @@ -0,0 +1,108 @@ +import chalk from 'chalk'; +import { dedent } from 'ts-dedent'; +import semver from 'semver'; +import { getStorybookInfo } from '@storybook/core-common'; +import type { Fix } from '../types'; +import { getStorybookVersionSpecifier } from '../../helpers'; +import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; + +interface SbBinaryRunOptions { + storybookVersion: string; + hasSbBinary: boolean; + hasStorybookBinary: boolean; + packageJson: PackageJsonWithDepsAndDevDeps; +} + +const logger = console; + +/** + * Does the user not have storybook dependency? + * + * If so: + * - Add storybook dependency + * - If they are using sb dependency, remove it + */ +export const sbBinary: Fix = { + id: 'storybook-binary', + + async check({ packageManager }) { + const packageJson = packageManager.retrievePackageJson(); + const { devDependencies, dependencies } = packageJson; + const { version: storybookVersion } = getStorybookInfo(packageJson); + + const allDeps = { ...dependencies, ...devDependencies }; + + const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; + if (!storybookCoerced) { + logger.warn(dedent` + ❌ Unable to determine storybook version, skipping ${chalk.cyan(this.id)} fix. + 🤔 Are you running automigrate from your project directory? + `); + return null; + } + + if (semver.lt(storybookCoerced, '7.0.0')) { + return null; + } + + const hasSbBinary = !!allDeps.sb; + const hasStorybookBinary = !!allDeps.storybook; + + if (!hasSbBinary && hasStorybookBinary) { + return null; + } + + return { + hasSbBinary, + hasStorybookBinary, + storybookVersion, + packageJson, + }; + }, + + prompt({ storybookVersion, hasSbBinary, hasStorybookBinary }) { + const sbFormatted = chalk.cyan(`Storybook ${storybookVersion}`); + + const storybookBinaryMessage = !hasStorybookBinary + ? `We've detected you are using ${sbFormatted} without Storybook's ${chalk.magenta( + 'storybook' + )} binary. Starting in Storybook 7.0, it has to be installed.` + : ''; + + const extraMessage = hasSbBinary + ? "You're using the 'sb' binary and it should be replaced, as 'storybook' is the recommended way to run Storybook.\n" + : ''; + + return dedent` + ${storybookBinaryMessage} + ${extraMessage} + + More info: ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#start-storybook--build-storybook-binaries-removed' + )} + `; + }, + + async run({ result: { packageJson, hasSbBinary, hasStorybookBinary }, packageManager, dryRun }) { + if (hasSbBinary) { + logger.info(`✅ Removing 'sb' dependency`); + if (!dryRun) { + packageManager.removeDependencies({ skipInstall: !hasStorybookBinary, packageJson }, [ + 'sb', + ]); + } + } + + if (!hasStorybookBinary) { + logger.log(); + logger.info(`✅ Adding 'storybook' as dev dependency`); + logger.log(); + if (!dryRun) { + const versionToInstall = getStorybookVersionSpecifier(packageJson); + packageManager.addDependencies({ installAsDevDependencies: true }, [ + `storybook@${versionToInstall}`, + ]); + } + } + }, +}; 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 a57e26953eb9..eb8b827a89a5 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts @@ -12,6 +12,7 @@ describe('getStorybookScripts', () => { it('detects default storybook scripts', () => { expect( getStorybookScripts({ + 'sb:upgrade': 'sb upgrade', storybook: 'start-storybook', 'build-storybook': 'build-storybook', }) @@ -24,6 +25,10 @@ describe('getStorybookScripts', () => { before: 'start-storybook', after: 'storybook dev', }, + 'sb:upgrade': { + before: 'sb upgrade', + after: 'storybook upgrade', + }, }); }); @@ -135,7 +140,6 @@ describe('sb-scripts fix', () => { const packageJson = { dependencies: { '@storybook/react': '^7.0.0-alpha.0', - storybook: '^7.0.0-alpha.0', }, scripts: { storybook: 'storybook dev -p 6006', @@ -150,21 +154,5 @@ describe('sb-scripts fix', () => { ).resolves.toBeFalsy(); }); }); - - describe('with storybook lib installed', () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - storybook: '^7.0.0-alpha.0', - }, - }; - it('should no-op', async () => { - await expect( - checkSbScripts({ - packageJson, - }) - ).resolves.toBeFalsy(); - }); - }); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts index 7752bb0c4202..e561cf2f7e01 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts @@ -3,7 +3,6 @@ import { dedent } from 'ts-dedent'; import semver from 'semver'; import { getStorybookInfo } from '@storybook/core-common'; import type { Fix } from '../types'; -import { getStorybookVersionSpecifier } from '../../helpers'; import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; interface SbScriptsRunOptions { @@ -29,7 +28,10 @@ export const getStorybookScripts = (allScripts: Record) => { const previousWord = allWordsFromScript[index - 1]; // full word check, rather than regex which could be faulty - const isSbBinary = currentWord === 'build-storybook' || currentWord === 'start-storybook'; + const isSbBinary = + currentWord === 'build-storybook' || + currentWord === 'start-storybook' || + currentWord === 'sb'; // in case people have scripts like `yarn start-storybook` const isPrependedByPkgManager = @@ -38,6 +40,7 @@ export const getStorybookScripts = (allScripts: Record) => { if (isSbBinary && !isPrependedByPkgManager) { isStorybookScript = true; return currentWord + .replace('sb', 'storybook') .replace('start-storybook', 'storybook dev') .replace('build-storybook', 'storybook build'); } @@ -58,32 +61,30 @@ export const getStorybookScripts = (allScripts: Record) => { }; /** - * Is the user using start-storybook + * Is the user using start-storybook or build-storybook in its scripts * * If so: - * - Add storybook dependency - * - Change start-storybook and build-storybook scripts + * - Change start-storybook and build-storybook scripts to storybook dev and storybook build + * - Change sb to storybook if they are using sb */ export const sbScripts: Fix = { id: 'sb-scripts', async check({ packageManager }) { const packageJson = packageManager.retrievePackageJson(); - const { scripts = {}, devDependencies, dependencies } = packageJson; + const { scripts = {} } = packageJson; const { version: storybookVersion } = getStorybookInfo(packageJson); - const allDeps = { ...dependencies, ...devDependencies }; - const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; if (!storybookCoerced) { logger.warn(dedent` - ❌ Unable to determine storybook version, skipping ${chalk.cyan('sb-scripts')} fix. + ❌ Unable to determine storybook version, skipping ${chalk.cyan(this.id)} fix. 🤔 Are you running automigrate from your project directory? `); return null; } - if (allDeps.sb || allDeps.storybook) { + if (semver.lt(storybookCoerced, '7.0.0')) { return null; } @@ -93,9 +94,7 @@ export const sbScripts: Fix = { return null; } - return semver.gte(storybookCoerced, '7.0.0') - ? { packageJson, storybookScripts, storybookVersion } - : null; + return { packageJson, storybookScripts, storybookVersion }; }, prompt({ storybookVersion, storybookScripts }) { @@ -121,9 +120,7 @@ export const sbScripts: Fix = { )} binaries have changed to ${chalk.magenta('storybook dev')} and ${chalk.magenta( 'storybook build' )} respectively. - In order to work with ${sbFormatted}, Storybook's ${chalk.magenta( - 'storybook' - )} binary has to be installed and your storybook scripts have to be adjusted to use the binary. We can install the storybook binary and adjust your scripts for you: + In order to work with ${sbFormatted}, your storybook scripts have to be adjusted to use the binary. We can adjust them for you: ${newScriptsMessage.join('\n\n')} @@ -133,19 +130,8 @@ export const sbScripts: Fix = { `; }, - async run({ result: { storybookScripts, packageJson }, packageManager, dryRun }) { - logger.log(); - logger.info(`Adding 'storybook' as dev dependency`); - logger.log(); - - if (!dryRun) { - const versionToInstall = getStorybookVersionSpecifier(packageJson); - packageManager.addDependencies({ installAsDevDependencies: true }, [ - `storybook@${versionToInstall}`, - ]); - } - - logger.info(`Updating scripts in package.json`); + async run({ result: { storybookScripts }, packageManager, dryRun }) { + logger.info(`✅ Updating scripts in package.json`); logger.log(); if (!dryRun) { const newScripts = Object.keys(storybookScripts).reduce((acc, scriptKey) => {