From 1f27f574720fd48701f91a0745904776ae09c314 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 20 Feb 2023 18:19:14 +0100 Subject: [PATCH 1/2] refactor all webpack5 related migrations --- .../src/automigrate/fixes/angular12.test.ts | 48 ++++++----- .../cli/src/automigrate/fixes/angular12.ts | 12 ++- .../cli/src/automigrate/fixes/cra5.test.ts | 35 ++++---- code/lib/cli/src/automigrate/fixes/cra5.ts | 16 ++-- .../cli/src/automigrate/fixes/vue3.test.ts | 40 ++++++---- code/lib/cli/src/automigrate/fixes/vue3.ts | 12 ++- .../src/automigrate/fixes/webpack5.test.ts | 39 +++++---- .../lib/cli/src/automigrate/fixes/webpack5.ts | 80 ++++--------------- .../helpers/checkWebpack5Builder.ts | 46 +++++++++++ 9 files changed, 159 insertions(+), 169 deletions(-) create mode 100644 code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts diff --git a/code/lib/cli/src/automigrate/fixes/angular12.test.ts b/code/lib/cli/src/automigrate/fixes/angular12.test.ts index 02da0ae88506..f0aaaf774b0a 100644 --- a/code/lib/cli/src/automigrate/fixes/angular12.test.ts +++ b/code/lib/cli/src/automigrate/fixes/angular12.test.ts @@ -1,46 +1,44 @@ -/* eslint-disable no-underscore-dangle */ -import * as path from 'path'; import type { StorybookConfig } from '@storybook/types'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { angular12 } from './angular12'; -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); - const checkAngular12 = async ({ packageJson, - main, + main: mainConfig = {}, + storybookVersion = '7.0.0', }: { packageJson: PackageJson; - main: Partial; + main?: Partial & Record; + storybookVersion?: string; }) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + mockStorybookData({ mainConfig, storybookVersion }); + + return angular12.check({ + packageManager: makePackageManager(packageJson), + configDir: '', }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return angular12.check({ packageManager }); }; describe('angular12 fix', () => { + afterEach(jest.restoreAllMocks); + describe('sb < 6.3', () => { describe('angular12 dependency', () => { const packageJson = { - dependencies: { '@storybook/react': '^6.2.0', '@angular/core': '^12.0.0' }, + dependencies: { '@storybook/angular': '^6.2.0', '@angular/core': '^12.0.0' }, }; it('should fail', async () => { await expect( checkAngular12({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); }); describe('no angular dependency', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + const packageJson = { dependencies: { '@storybook/angular': '^6.2.0' } }; it('should no-op', async () => { await expect( checkAngular12({ @@ -54,7 +52,7 @@ describe('angular12 fix', () => { describe('sb 6.3 - 7.0', () => { describe('angular12 dependency', () => { const packageJson = { - dependencies: { '@storybook/react': '^6.3.0', '@angular/core': '^12.0.0' }, + dependencies: { '@storybook/angular': '^6.3.0', '@angular/core': '^12.0.0' }, }; describe('webpack5 builder', () => { it('should no-op', async () => { @@ -62,6 +60,7 @@ describe('angular12 fix', () => { checkAngular12({ packageJson, main: { core: { builder: 'webpack5' } }, + storybookVersion: '6.3.0', }) ).resolves.toBeFalsy(); }); @@ -82,10 +81,11 @@ describe('angular12 fix', () => { checkAngular12({ packageJson, main: { core: { builder: 'webpack4' } }, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ angularVersion: '^12.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -94,11 +94,11 @@ describe('angular12 fix', () => { await expect( checkAngular12({ packageJson, - main: {}, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ angularVersion: '^12.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -108,7 +108,6 @@ describe('angular12 fix', () => { await expect( checkAngular12({ packageJson: {}, - main: {}, }) ).resolves.toBeFalsy(); }); @@ -117,13 +116,12 @@ describe('angular12 fix', () => { describe('sb 7.0+', () => { describe('angular12 dependency', () => { const packageJson = { - dependencies: { '@storybook/react': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, + dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, }; it('should no-op', async () => { await expect( checkAngular12({ packageJson, - main: {}, }) ).resolves.toBeFalsy(); }); diff --git a/code/lib/cli/src/automigrate/fixes/angular12.ts b/code/lib/cli/src/automigrate/fixes/angular12.ts index b22a3e131a56..a778556e9207 100644 --- a/code/lib/cli/src/automigrate/fixes/angular12.ts +++ b/code/lib/cli/src/automigrate/fixes/angular12.ts @@ -1,15 +1,14 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; import semver from 'semver'; -import type { ConfigFile } from '@storybook/csf-tools'; import type { Fix } from '../types'; import { webpack5 } from './webpack5'; +import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder'; interface Angular12RunOptions { angularVersion: string; // FIXME angularPresetVersion: string; storybookVersion: string; - main: ConfigFile; } /** @@ -21,17 +20,16 @@ interface Angular12RunOptions { export const angular12: Fix = { id: 'angular12', - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { dependencies, devDependencies } = packageJson; - const angularVersion = dependencies['@angular/core'] || devDependencies['@angular/core']; + async check({ packageManager, configDir }) { + const allDependencies = packageManager.getAllDependencies(); + const angularVersion = allDependencies['@angular/core']; const angularCoerced = semver.coerce(angularVersion)?.version; if (!angularCoerced || semver.lt(angularCoerced, '12.0.0')) { return null; } - const builderInfo = await webpack5.checkWebpack5Builder(packageJson); + const builderInfo = await checkWebpack5Builder({ packageManager, configDir }); return builderInfo ? { angularVersion, ...builderInfo } : null; }, diff --git a/code/lib/cli/src/automigrate/fixes/cra5.test.ts b/code/lib/cli/src/automigrate/fixes/cra5.test.ts index db3066f6f5a8..58caadb19065 100644 --- a/code/lib/cli/src/automigrate/fixes/cra5.test.ts +++ b/code/lib/cli/src/automigrate/fixes/cra5.test.ts @@ -1,30 +1,27 @@ -/* eslint-disable no-underscore-dangle */ -import * as path from 'path'; import type { StorybookConfig } from '@storybook/types'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; import { cra5 } from './cra5'; - -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; const checkCra5 = async ({ packageJson, - main, + main: mainConfig, + storybookVersion = '7.0.0', }: { packageJson: PackageJson; - main: Partial; + main?: Partial & Record; + storybookVersion?: string; }) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + mockStorybookData({ mainConfig, storybookVersion }); + + return cra5.check({ + packageManager: makePackageManager(packageJson), }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return cra5.check({ packageManager }); }; describe('cra5 fix', () => { + afterEach(jest.restoreAllMocks); + describe('sb < 6.3', () => { describe('cra5 dependency', () => { const packageJson = { @@ -34,7 +31,7 @@ describe('cra5 fix', () => { await expect( checkCra5({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); @@ -82,10 +79,11 @@ describe('cra5 fix', () => { checkCra5({ packageJson, main: { core: { builder: 'webpack4' } }, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ craVersion: '^5.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -95,10 +93,11 @@ describe('cra5 fix', () => { checkCra5({ packageJson, main: {}, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ craVersion: '^5.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/cra5.ts b/code/lib/cli/src/automigrate/fixes/cra5.ts index b4d9e156e83a..04ca458d6871 100644 --- a/code/lib/cli/src/automigrate/fixes/cra5.ts +++ b/code/lib/cli/src/automigrate/fixes/cra5.ts @@ -1,15 +1,14 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; import semver from 'semver'; -import type { ConfigFile } from '@storybook/csf-tools'; import type { Fix } from '../types'; import { webpack5 } from './webpack5'; +import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder'; interface CRA5RunOptions { craVersion: string; // FIXME craPresetVersion: string; storybookVersion: string; - main: ConfigFile; } /** @@ -21,25 +20,22 @@ interface CRA5RunOptions { export const cra5: Fix = { id: 'cra5', - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { dependencies, devDependencies } = packageJson; - const craVersion = dependencies['react-scripts'] || devDependencies['react-scripts']; + async check({ packageManager, configDir }) { + const allDependencies = packageManager.getAllDependencies(); + const craVersion = allDependencies['react-scripts']; const craCoerced = semver.coerce(craVersion)?.version; if (!craCoerced || semver.lt(craCoerced, '5.0.0')) { return null; } - const builderInfo = await webpack5.checkWebpack5Builder(packageJson); + const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); return builderInfo ? { craVersion, ...builderInfo } : null; }, - prompt({ craVersion, ...rest }) { + prompt({ craVersion }) { const craFormatted = chalk.cyan(`Create React App (CRA) ${craVersion}`); - console.log({ ...rest }); - return dedent` We've detected you are running ${craFormatted} which is powered by webpack5. Your Storybook's main.js files specifies webpack4, which is incompatible. diff --git a/code/lib/cli/src/automigrate/fixes/vue3.test.ts b/code/lib/cli/src/automigrate/fixes/vue3.test.ts index 6ca7a23eef61..a4effab1ea23 100644 --- a/code/lib/cli/src/automigrate/fixes/vue3.test.ts +++ b/code/lib/cli/src/automigrate/fixes/vue3.test.ts @@ -1,23 +1,27 @@ -/* eslint-disable no-underscore-dangle */ -import * as path from 'path'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { StorybookConfig } from '@storybook/types'; +import type { PackageJson } from '../../js-package-manager'; import { vue3 } from './vue3'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); +const checkVue3 = async ({ + packageJson, + main: mainConfig = {}, + storybookVersion = '7.0.0', +}: { + packageJson: PackageJson; + main?: Partial & Record; + storybookVersion?: string; +}) => { + mockStorybookData({ mainConfig, storybookVersion }); -const checkVue3 = async ({ packageJson, main }: { packageJson: PackageJson; main: unknown }) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + return vue3.check({ + packageManager: makePackageManager(packageJson), }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return vue3.check({ packageManager }); }; describe('vue3 fix', () => { + afterEach(jest.restoreAllMocks); + describe('sb < 6.3', () => { describe('vue3 dependency', () => { const packageJson = { @@ -27,7 +31,7 @@ describe('vue3 fix', () => { await expect( checkVue3({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); @@ -38,7 +42,7 @@ describe('vue3 fix', () => { await expect( checkVue3({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); }); @@ -75,10 +79,11 @@ describe('vue3 fix', () => { checkVue3({ packageJson, main: { core: { builder: 'webpack4' } }, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ vueVersion: '^3.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -88,10 +93,11 @@ describe('vue3 fix', () => { checkVue3({ packageJson, main: {}, + storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ vueVersion: '^3.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/vue3.ts b/code/lib/cli/src/automigrate/fixes/vue3.ts index de38ed472123..f1c5041e1885 100644 --- a/code/lib/cli/src/automigrate/fixes/vue3.ts +++ b/code/lib/cli/src/automigrate/fixes/vue3.ts @@ -1,14 +1,13 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; import semver from 'semver'; -import type { ConfigFile } from '@storybook/csf-tools'; import type { Fix } from '../types'; import { webpack5 } from './webpack5'; +import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder'; interface Vue3RunOptions { vueVersion: string; storybookVersion: string; - main: ConfigFile; } /** @@ -20,17 +19,16 @@ interface Vue3RunOptions { export const vue3: Fix = { id: 'vue3', - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { dependencies, devDependencies } = packageJson; - const vueVersion = dependencies.vue || devDependencies.vue; + async check({ configDir, packageManager }) { + const allDependencies = packageManager.getAllDependencies(); + const vueVersion = allDependencies.vue; const vueCoerced = semver.coerce(vueVersion)?.version; if (!vueCoerced || semver.lt(vueCoerced, '3.0.0')) { return null; } - const builderInfo = await webpack5.checkWebpack5Builder(packageJson); + const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); return builderInfo ? { vueVersion, ...builderInfo } : null; }, diff --git a/code/lib/cli/src/automigrate/fixes/webpack5.test.ts b/code/lib/cli/src/automigrate/fixes/webpack5.test.ts index c231fa477e4c..2da36108d95d 100644 --- a/code/lib/cli/src/automigrate/fixes/webpack5.test.ts +++ b/code/lib/cli/src/automigrate/fixes/webpack5.test.ts @@ -1,30 +1,28 @@ -/* eslint-disable no-underscore-dangle */ -import * as path from 'path'; import type { StorybookConfig } from '@storybook/types'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; import { webpack5 } from './webpack5'; - -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; const checkWebpack5 = async ({ packageJson, - main, + main: mainConfig, + storybookVersion = '6.3.0', }: { packageJson: PackageJson; - main: Partial; + main?: Partial & Record; + storybookVersion?: string; }) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + mockStorybookData({ mainConfig, storybookVersion }); + + return webpack5.check({ + packageManager: makePackageManager(packageJson), + configDir: '', }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return webpack5.check({ packageManager }); }; describe('webpack5 fix', () => { + afterEach(jest.restoreAllMocks); + describe('sb < 6.3', () => { describe('webpack5 dependency', () => { const packageJson = { dependencies: { '@storybook/react': '^6.2.0', webpack: '^5.0.0' } }; @@ -32,7 +30,7 @@ describe('webpack5 fix', () => { await expect( checkWebpack5({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); @@ -43,7 +41,7 @@ describe('webpack5 fix', () => { await expect( checkWebpack5({ packageJson, - main: {}, + storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); }); @@ -81,7 +79,7 @@ describe('webpack5 fix', () => { }) ).resolves.toMatchObject({ webpackVersion: '^5.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -94,7 +92,7 @@ describe('webpack5 fix', () => { }) ).resolves.toMatchObject({ webpackVersion: '^5.0.0', - storybookVersion: '^6.3.0', + storybookVersion: '6.3.0', }); }); }); @@ -104,7 +102,6 @@ describe('webpack5 fix', () => { await expect( checkWebpack5({ packageJson: {}, - main: {}, }) ).resolves.toBeFalsy(); }); @@ -118,7 +115,6 @@ describe('webpack5 fix', () => { webpack: '4', }, }, - main: {}, }) ).resolves.toBeFalsy(); }); @@ -134,6 +130,7 @@ describe('webpack5 fix', () => { checkWebpack5({ packageJson, main: {}, + storybookVersion: '7.0.0', }) ).resolves.toBeFalsy(); }); diff --git a/code/lib/cli/src/automigrate/fixes/webpack5.ts b/code/lib/cli/src/automigrate/fixes/webpack5.ts index 68bf103fdad0..edac5e468696 100644 --- a/code/lib/cli/src/automigrate/fixes/webpack5.ts +++ b/code/lib/cli/src/automigrate/fixes/webpack5.ts @@ -1,24 +1,15 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; import semver from 'semver'; -import type { ConfigFile } from '@storybook/csf-tools'; -import { readConfig, writeConfig } from '@storybook/csf-tools'; -import { getStorybookInfo } from '@storybook/core-common'; import type { Fix } from '../types'; -import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; +import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder'; +import { updateMainConfig } from '../helpers/mainConfigFile'; const logger = console; interface Webpack5RunOptions { webpackVersion: string; storybookVersion: string; - main: ConfigFile; -} - -interface CheckBuilder { - checkWebpack5Builder: ( - packageJson: PackageJsonWithDepsAndDevDeps - ) => Promise<{ storybookVersion: string; main: ConfigFile }>; } /** @@ -31,58 +22,13 @@ interface CheckBuilder { * - Add core.builder = 'webpack5' to main.js * - Add 'webpack5' as a project dependency */ -export const webpack5: Fix & CheckBuilder = { +export const webpack5: Fix = { id: 'webpack5', - async checkWebpack5Builder(packageJson: PackageJsonWithDepsAndDevDeps) { - const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); - - const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!storybookCoerced) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } - - if (semver.lt(storybookCoerced, '6.3.0')) { - logger.warn( - dedent` - Detected SB 6.3 or below, please upgrade storybook to use webpack5. - - To upgrade to the latest stable release, run this from your project directory: - - ${chalk.cyan('npx storybook upgrade')} - - Add the ${chalk.cyan('--prerelease')} flag to get the latest prerelease. - `.trim() - ); - return null; - } - - if (semver.gte(storybookCoerced, '7.0.0')) { - return null; - } - - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config'); - return null; - } - const main = await readConfig(mainConfig); - const builder = main.getFieldValue(['core', 'builder']); - if (builder && builder !== 'webpack4') { - logger.info(`Found builder ${builder}, skipping`); - return null; - } - - return { storybookVersion, main }; - }, - - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { dependencies, devDependencies } = packageJson; + async check({ configDir, packageManager }) { + const allDependencies = packageManager.retrievePackageJson().dependencies; - const webpackVersion = dependencies.webpack || devDependencies.webpack; + const webpackVersion = allDependencies.webpack; const webpackCoerced = semver.coerce(webpackVersion)?.version; if ( @@ -92,7 +38,7 @@ export const webpack5: Fix & CheckBuilder = { ) return null; - const builderInfo = await this.checkWebpack5Builder(packageJson); + const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); return builderInfo ? { webpackVersion, ...builderInfo } : null; }, @@ -113,7 +59,12 @@ export const webpack5: Fix & CheckBuilder = { `; }, - async run({ result: { main, storybookVersion, webpackVersion }, packageManager, dryRun }) { + async run({ + result: { storybookVersion, webpackVersion }, + packageManager, + dryRun, + mainConfigPath, + }) { const deps = [`@storybook/builder-webpack5@${storybookVersion}`]; // this also gets called by 'cra5' fix so we need to add // webpack5 at the project root so that it gets hoisted @@ -125,8 +76,9 @@ export const webpack5: Fix & CheckBuilder = { logger.info('✅ Setting `core.builder` to `@storybook/builder-webpack5` in main.js'); if (!dryRun) { - main.setFieldValue(['core', 'builder'], '@storybook/builder-webpack5'); - await writeConfig(main); + await updateMainConfig({ mainConfigPath, dryRun }, async (main) => { + main.setFieldValue(['core', 'builder'], '@storybook/builder-webpack5'); + }); } }, }; diff --git a/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts new file mode 100644 index 000000000000..b900d40b89bd --- /dev/null +++ b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts @@ -0,0 +1,46 @@ +import chalk from 'chalk'; +import semver from 'semver'; +import dedent from 'ts-dedent'; +import type { GetStorybookData } from './mainConfigFile'; +import { getStorybookData } from './mainConfigFile'; + +const logger = console; + +export const checkWebpack5Builder = async ({ + configDir, + packageManager, +}: Parameters[0]) => { + const { mainConfig, storybookVersion } = await getStorybookData({ configDir, packageManager }); + + if (semver.lt(storybookVersion, '6.3.0')) { + logger.warn( + dedent` + Detected SB 6.3 or below, please upgrade storybook to use webpack5. + + To upgrade to the latest stable release, run this from your project directory: + + ${chalk.cyan('npx storybook upgrade')} + + Add the ${chalk.cyan('--prerelease')} flag to get the latest prerelease. + `.trim() + ); + return null; + } + + if (semver.gte(storybookVersion, '7.0.0')) { + return null; + } + + if (!mainConfig) { + logger.warn('Unable to find storybook main.js config'); + return null; + } + + const builder = mainConfig.core?.builder; + if (builder && builder !== 'webpack4') { + logger.info(`Found builder ${builder}, skipping`); + return null; + } + + return { storybookVersion }; +}; From 5ef1d6c6a55c0aebe34d1bc70041e80d851ddd61 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 20 Feb 2023 19:18:00 +0100 Subject: [PATCH 2/2] refactor and cleanup automigrations --- .../automigrate/fixes/autodocs-true.test.ts | 46 +++++++++++++++ .../src/automigrate/fixes/autodocs-true.ts | 38 ++++++------- .../fixes/bare-mdx-stories-glob.test.ts | 55 +++++------------- .../fixes/bare-mdx-stories-glob.ts | 57 ++++++------------- .../automigrate/fixes/builder-vite.test.ts | 27 ++++----- .../cli/src/automigrate/fixes/builder-vite.ts | 41 ++++++------- .../automigrate/fixes/eslint-plugin.test.ts | 30 ++-------- .../src/automigrate/fixes/eslint-plugin.ts | 28 +++------ .../automigrate/fixes/missing-babelrc.test.ts | 39 +++++++++---- .../src/automigrate/fixes/missing-babelrc.ts | 28 ++------- .../fixes/new-frameworks/new-frameworks.ts | 6 +- .../automigrate/fixes/new-frameworks/utils.ts | 4 +- .../fixes/nodejs-requirement.test.ts | 24 ++++---- .../automigrate/fixes/nodejs-requirement.ts | 17 ++---- .../fixes/remove-global-client-apis.ts | 4 +- .../src/automigrate/fixes/sb-binary.test.ts | 17 ++++-- .../cli/src/automigrate/fixes/sb-binary.ts | 24 +++----- .../src/automigrate/fixes/sb-scripts.test.ts | 21 ++++--- .../cli/src/automigrate/fixes/sb-scripts.ts | 16 ++---- .../src/automigrate/helpers/mainConfigFile.ts | 5 +- 20 files changed, 231 insertions(+), 296 deletions(-) create mode 100644 code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts diff --git a/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts b/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts new file mode 100644 index 000000000000..ae8a472f28a0 --- /dev/null +++ b/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts @@ -0,0 +1,46 @@ +import type { StorybookConfig } from '@storybook/types'; +import type { PackageJson } from '../../js-package-manager'; +import { autodocsTrue } from './autodocs-true'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; + +const checkAutodocs = async ({ + packageJson = {}, + main: mainConfig, + storybookVersion = '7.0.0', +}: { + packageJson?: PackageJson; + main: Partial & Record; + storybookVersion?: string; +}) => { + mockStorybookData({ mainConfig, storybookVersion }); + + return autodocsTrue.check({ + packageManager: makePackageManager(packageJson), + }); +}; + +describe('autodocs-true fix', () => { + afterEach(jest.restoreAllMocks); + + it('should skip when docs.autodocs is already defined', async () => { + await expect(checkAutodocs({ main: { docs: { autodocs: 'tag' } } })).resolves.toBeFalsy(); + }); + + it('should throw when docs.docsPage contains invalid value', async () => { + const main = { docs: { docsPage: 123 } } as any; + await expect(checkAutodocs({ main })).rejects.toThrow(); + }); + + it('should prompt when using docs.docsPage legacy property', async () => { + const main = { docs: { docsPage: true } } as any; + await expect(checkAutodocs({ main })).resolves.toEqual({ + value: 'tag', + }); + }); + + it('should prompt when not using docs.autodocs', async () => { + await expect(checkAutodocs({ main: {} })).resolves.toEqual({ + value: true, + }); + }); +}); diff --git a/code/lib/cli/src/automigrate/fixes/autodocs-true.ts b/code/lib/cli/src/automigrate/fixes/autodocs-true.ts index 00bc5faed2ba..973f4cafbdc0 100644 --- a/code/lib/cli/src/automigrate/fixes/autodocs-true.ts +++ b/code/lib/cli/src/automigrate/fixes/autodocs-true.ts @@ -1,17 +1,14 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; -import type { ConfigFile } from '@storybook/csf-tools'; -import { readConfig, writeConfig } from '@storybook/csf-tools'; -import { getStorybookInfo } from '@storybook/core-common'; import type { StorybookConfig } from '@storybook/types'; import type { Fix } from '../types'; +import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; const logger = console; interface AutodocsTrueFrameworkRunOptions { - main: ConfigFile; value?: StorybookConfig['docs']['autodocs']; } @@ -21,35 +18,31 @@ interface AutodocsTrueFrameworkRunOptions { export const autodocsTrue: Fix = { id: 'autodocsTrue', - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); + async check({ packageManager, configDir }) { + const { mainConfig } = await getStorybookData({ packageManager, configDir }); - const { mainConfig } = getStorybookInfo(packageJson); - - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config, skipping'); - return null; - } - - const main = await readConfig(mainConfig); - const docs = main.getFieldValue(['docs']); + const { docs } = mainConfig; const docsPageToAutodocsMapping = { true: 'tag' as const, automatic: true, false: false, }; + + // @ts-expect-error docsPage does not exist anymore but we need to account for legacy code if (docs?.docsPage) { + // @ts-expect-error same as above const oldValue = docs?.docsPage.toString(); - if (!(oldValue in docsPageToAutodocsMapping)) + if (!(oldValue in docsPageToAutodocsMapping)) { throw new Error(`Unexpected value for docs.docsPage: ${oldValue}`); + } + return { - main, value: docsPageToAutodocsMapping[oldValue as keyof typeof docsPageToAutodocsMapping], }; } - return docs?.autodocs === undefined ? { main } : null; + return docs?.autodocs === undefined ? { value: true } : null; }, prompt({ value }) { @@ -90,12 +83,13 @@ export const autodocsTrue: Fix = { `; }, - async run({ result: { main, value }, dryRun }) { + async run({ result: { value }, dryRun, mainConfigPath }) { logger.info(`✅ Setting 'docs.autodocs' to true in main.js`); if (!dryRun) { - main.removeField(['docs', 'docsPage']); - main.setFieldValue(['docs', 'autodocs'], value ?? true); - await writeConfig(main); + await updateMainConfig({ mainConfigPath, dryRun }, async (main) => { + main.removeField(['docs', 'docsPage']); + main.setFieldValue(['docs', 'autodocs'], value ?? true); + }); } }, }; diff --git a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts index 02b048b18a9b..38025e609e32 100644 --- a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts +++ b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts @@ -1,47 +1,40 @@ -/* eslint-disable no-underscore-dangle */ /// ; import type { StorybookConfig } from '@storybook/types'; -import path from 'path'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; import { ansiRegex } from '../helpers/cleanLog'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import type { BareMdxStoriesGlobRunOptions } from './bare-mdx-stories-glob'; import { bareMdxStoriesGlob } from './bare-mdx-stories-glob'; -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); - const checkBareMdxStoriesGlob = async ({ packageJson, - main, + main: mainConfig, + storybookVersion = '7.0.0', }: { packageJson: PackageJson; - main?: Partial; + main?: Partial & Record; + storybookVersion?: string; }) => { - if (main) { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, - }); - } else { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({}); - } - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; + mockStorybookData({ mainConfig, storybookVersion }); - return bareMdxStoriesGlob.check({ packageManager }); + return bareMdxStoriesGlob.check({ + packageManager: makePackageManager(packageJson), + }); }; describe('bare-mdx fix', () => { + afterEach(jest.restoreAllMocks); + describe('should no-op', () => { it('in SB < v7.0.0', async () => { const packageJson = { dependencies: { '@storybook/react': '^6.2.0' }, }; const main = { stories: ['../**/*.stories.mdx'] }; - await expect(checkBareMdxStoriesGlob({ packageJson, main })).resolves.toBeFalsy(); + await expect( + checkBareMdxStoriesGlob({ packageJson, main, storybookVersion: '6.5.0' }) + ).resolves.toBeFalsy(); }); describe('in SB >= v7.0.0', () => { @@ -60,24 +53,6 @@ describe('bare-mdx fix', () => { await expect(checkBareMdxStoriesGlob({ packageJson, main })).rejects.toThrow(); }); - it('with variable references in stories field', async () => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: ` - const globVar = '../**/*.stories.mdx'; - module.exports = { stories: [globVar] };`, - }); - - const packageManager = { - retrievePackageJson: () => ({ - dependencies: { '@storybook/react': '^7.0.0' }, - devDependencies: {}, - }), - } as unknown as JsPackageManager; - - await expect(bareMdxStoriesGlob.check({ packageManager })).rejects.toThrow(); - }); - it('without .stories.mdx in globs', async () => { const packageJson = { dependencies: { '@storybook/react': '^7.0.0' }, diff --git a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts index 5ad960f10b86..4091ee9f7ed9 100644 --- a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts +++ b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts @@ -1,18 +1,13 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import semver from 'semver'; -import type { ConfigFile } from '@storybook/csf-tools'; -import { readConfig, writeConfig } from '@storybook/csf-tools'; -import { getStorybookInfo } from '@storybook/core-common'; -import type { StoriesEntry } from 'lib/types/src'; +import type { StoriesEntry } from '@storybook/types'; +import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; import type { Fix } from '../types'; const logger = console; -const fixId = 'bare-mdx-stories-glob'; - export interface BareMdxStoriesGlobRunOptions { - main: ConfigFile; existingStoriesEntries: StoriesEntry[]; nextStoriesEntries: StoriesEntry[]; } @@ -35,41 +30,24 @@ const getNextGlob = (glob: string) => { }; export const bareMdxStoriesGlob: Fix = { - id: fixId, - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - - const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config, skipping'); - return null; - } - - const sbVersionCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!sbVersionCoerced) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } + id: 'bare-mdx-stories-glob', + async check({ packageManager, configDir }) { + const { storybookVersion, mainConfig } = await getStorybookData({ + configDir, + packageManager, + }); - if (!semver.gte(sbVersionCoerced, '7.0.0')) { + if (!semver.gte(storybookVersion, '7.0.0')) { return null; } - const main = await readConfig(mainConfig); - - let existingStoriesEntries; - - try { - existingStoriesEntries = main.getFieldValue(['stories']) as StoriesEntry[]; - } catch (e) { - // throws in next null check below - } + const existingStoriesEntries = mainConfig.stories as StoriesEntry[]; if (!existingStoriesEntries) { throw new Error(dedent` - ❌ Unable to determine Storybook stories globs, skipping ${chalk.cyan(fixId)} fix. + ❌ Unable to determine Storybook stories globs in ${chalk.blue( + mainConfig + )}, skipping ${chalk.cyan(this.id)} fix. In Storybook 7, we have deprecated defining stories in MDX files, and consequently have changed the suffix to simply .mdx. @@ -112,7 +90,7 @@ export const bareMdxStoriesGlob: Fix = { return null; } - return { existingStoriesEntries, nextStoriesEntries, main }; + return { existingStoriesEntries, nextStoriesEntries }; }, prompt({ existingStoriesEntries, nextStoriesEntries }) { @@ -138,13 +116,14 @@ export const bareMdxStoriesGlob: Fix = { `; }, - async run({ dryRun, result: { nextStoriesEntries, main } }) { + async run({ dryRun, mainConfigPath, result: { nextStoriesEntries } }) { logger.info(dedent`✅ Setting 'stories' config: ${JSON.stringify(nextStoriesEntries, null, 2)}`); - main.setFieldValue(['stories'], nextStoriesEntries); if (!dryRun) { - await writeConfig(main); + await updateMainConfig({ mainConfigPath, dryRun }, async (main) => { + main.setFieldValue(['stories'], nextStoriesEntries); + }); } }, }; diff --git a/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts b/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts index 6981d288c265..0a3285684c3b 100644 --- a/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts +++ b/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts @@ -1,30 +1,27 @@ -/* eslint-disable no-underscore-dangle */ -import * as path from 'path'; import type { StorybookConfig } from '@storybook/types'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; import { builderVite } from './builder-vite'; - -// eslint-disable-next-line global-require, jest/no-mocks-import -jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; const checkBuilderVite = async ({ packageJson = {}, - main, + main: mainConfig, + storybookVersion = '7.0.0', }: { packageJson?: PackageJson; - main: Partial; + main: Partial & Record; + storybookVersion?: string; }) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + mockStorybookData({ mainConfig, storybookVersion }); + + return builderVite.check({ + packageManager: makePackageManager(packageJson), }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return builderVite.check({ packageManager }); }; describe('builder-vite fix', () => { + afterEach(jest.restoreAllMocks); + describe('storybook-builder-vite', () => { it('using storybook-builder-vite', async () => { const main = { core: { builder: 'storybook-builder-vite' } }; diff --git a/code/lib/cli/src/automigrate/fixes/builder-vite.ts b/code/lib/cli/src/automigrate/fixes/builder-vite.ts index 16ff7e5c8cdc..c107cc6d7d17 100644 --- a/code/lib/cli/src/automigrate/fixes/builder-vite.ts +++ b/code/lib/cli/src/automigrate/fixes/builder-vite.ts @@ -1,18 +1,16 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; -import type { ConfigFile } from '@storybook/csf-tools'; -import { readConfig, writeConfig } from '@storybook/csf-tools'; -import { getStorybookInfo } from '@storybook/core-common'; +import { writeConfig } from '@storybook/csf-tools'; import type { Fix } from '../types'; import type { PackageJson } from '../../js-package-manager'; +import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; const logger = console; interface BuilderViteOptions { builder: any; - main: ConfigFile; packageJson: PackageJson; } @@ -28,22 +26,17 @@ interface BuilderViteOptions { export const builderVite: Fix = { id: 'builder-vite', - async check({ packageManager }) { + async check({ configDir, packageManager }) { const packageJson = packageManager.retrievePackageJson(); - const { mainConfig } = getStorybookInfo(packageJson); - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config'); - return null; - } - const main = await readConfig(mainConfig); - const builder = main.getFieldValue(['core', 'builder']); + const { mainConfig } = await getStorybookData({ configDir, packageManager }); + const builder = mainConfig.core?.builder; const builderName = typeof builder === 'string' ? builder : builder?.name; if (builderName !== 'storybook-builder-vite') { return null; } - return { builder, main, packageJson }; + return { builder, packageJson }; }, prompt({ builder }) { @@ -64,31 +57,33 @@ export const builderVite: Fix = { `; }, - async run({ result: { builder, main, packageJson }, packageManager, dryRun }) { + async run({ result: { builder, packageJson }, packageManager, dryRun, mainConfigPath }) { const { dependencies = {}, devDependencies = {} } = packageJson; - logger.info(`Removing existing 'storybook-builder-vite' dependency`); + logger.info(`✅ Removing existing 'storybook-builder-vite' dependency`); if (!dryRun) { delete dependencies['storybook-builder-vite']; delete devDependencies['storybook-builder-vite']; packageManager.writePackageJson(packageJson); } - logger.info(`Adding '@storybook/builder-vite' as dev dependency`); + logger.info(`✅ Adding '@storybook/builder-vite' as dev dependency`); if (!dryRun) { packageManager.addDependencies({ installAsDevDependencies: true }, [ '@storybook/builder-vite', ]); } - logger.info(`Updating main.js to use vite builder`); + logger.info(`✅ Updating main.js to use vite builder`); if (!dryRun) { - const updatedBuilder = - typeof builder === 'string' - ? '@storybook/builder-vite' - : { name: '@storybook/builder-vite', options: builder.options }; - main.setFieldValue(['core', 'builder'], updatedBuilder); - await writeConfig(main); + await updateMainConfig({ dryRun, mainConfigPath }, async (main) => { + const updatedBuilder = + typeof builder === 'string' + ? '@storybook/builder-vite' + : { name: '@storybook/builder-vite', options: builder.options }; + main.setFieldValue(['core', 'builder'], updatedBuilder); + await writeConfig(main); + }); } }, }; diff --git a/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts b/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts index a82ee63fed52..78fdedec90b8 100644 --- a/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts +++ b/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts @@ -1,29 +1,23 @@ /* eslint-disable no-underscore-dangle */ -import * as path from 'path'; import { dedent } from 'ts-dedent'; -import type { StorybookConfig } from '@storybook/types'; -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; import { eslintPlugin } from './eslint-plugin'; +import { makePackageManager } from '../helpers/testing-helpers'; // eslint-disable-next-line global-require, jest/no-mocks-import jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); const checkEslint = async ({ packageJson, - main = {}, hasEslint = true, eslintExtension = 'js', }: { packageJson: PackageJson; - main?: Partial & Record; hasEslint?: boolean; eslintExtension?: string; }) => { // eslint-disable-next-line global-require require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: !main - ? null - : `module.exports = ${JSON.stringify(main)};`, [`.eslintrc.${eslintExtension}`]: !hasEslint ? null : dedent(` @@ -45,10 +39,9 @@ const checkEslint = async ({ } `), }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return eslintPlugin.check({ packageManager }); + return eslintPlugin.check({ + packageManager: makePackageManager(packageJson), + }); }; describe('eslint-plugin fix', () => { @@ -78,19 +71,6 @@ describe('eslint-plugin fix', () => { const packageJson = { dependencies: { '@storybook/react': '^6.2.0', eslint: '^7.0.0' } }; describe('should no-op and warn when', () => { - it('main.js is not found', async () => { - const loggerSpy = jest.spyOn(console, 'warn').mockImplementationOnce(jest.fn); - const result = await checkEslint({ - packageJson, - main: null, - hasEslint: false, - }); - - expect(loggerSpy).toHaveBeenCalledWith('Unable to find storybook main.js config, skipping'); - - await expect(result).toBeFalsy(); - }); - it('.eslintrc is not found', async () => { const loggerSpy = jest.spyOn(console, 'warn').mockImplementationOnce(jest.fn); const result = await checkEslint({ diff --git a/code/lib/cli/src/automigrate/fixes/eslint-plugin.ts b/code/lib/cli/src/automigrate/fixes/eslint-plugin.ts index ba3b481b080d..c5b155ff5366 100644 --- a/code/lib/cli/src/automigrate/fixes/eslint-plugin.ts +++ b/code/lib/cli/src/automigrate/fixes/eslint-plugin.ts @@ -1,8 +1,6 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; -import type { ConfigFile } from '@storybook/csf-tools'; import { readConfig, writeConfig } from '@storybook/csf-tools'; -import { getStorybookInfo } from '@storybook/core-common'; import { readFile, readJson, writeJson } from 'fs-extra'; import detectIndent from 'detect-indent'; @@ -13,7 +11,6 @@ import type { Fix } from '../types'; const logger = console; interface EslintPluginRunOptions { - main: ConfigFile; eslintFile: string; unsupportedExtension?: string; } @@ -28,24 +25,15 @@ export const eslintPlugin: Fix = { id: 'eslintPlugin', async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { dependencies, devDependencies } = packageJson; + const allDependencies = packageManager.getAllDependencies(); - const eslintPluginStorybook = - dependencies['eslint-plugin-storybook'] || devDependencies['eslint-plugin-storybook']; - const eslintDependency = dependencies.eslint || devDependencies.eslint; + const eslintPluginStorybook = allDependencies['eslint-plugin-storybook']; + const eslintDependency = allDependencies.eslint; if (eslintPluginStorybook || !eslintDependency) { return null; } - const { mainConfig } = getStorybookInfo(packageJson); - - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config, skipping'); - return null; - } - let eslintFile; let unsupportedExtension; try { @@ -59,10 +47,7 @@ export const eslintPlugin: Fix = { return null; } - // If in the future the eslint plugin has a framework option, using main to extract the framework field will be very useful - const main = await readConfig(mainConfig); - - return { eslintFile, main, unsupportedExtension }; + return { eslintFile, unsupportedExtension }; }, prompt() { @@ -75,11 +60,12 @@ export const eslintPlugin: Fix = { `; }, - async run({ result: { eslintFile, unsupportedExtension }, packageManager, dryRun }) { + async run({ result: { eslintFile, unsupportedExtension }, packageManager, dryRun, skipInstall }) { const deps = [`eslint-plugin-storybook`]; logger.info(`✅ Adding dependencies: ${deps}`); - if (!dryRun) packageManager.addDependencies({ installAsDevDependencies: true }, deps); + if (!dryRun) + packageManager.addDependencies({ installAsDevDependencies: true, skipInstall }, deps); if (!dryRun && unsupportedExtension) { logger.info(dedent` diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts index 4f706b130fb2..6ce9aa71cab2 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts @@ -1,8 +1,9 @@ /* eslint-disable no-underscore-dangle */ /// ; -import path from 'path'; -import type { JsPackageManager } from '../../js-package-manager'; +import type { StorybookConfig } from '@storybook/types'; +import type { PackageJson } from '../../js-package-manager'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { missingBabelRc } from './missing-babelrc'; // eslint-disable-next-line global-require, jest/no-mocks-import @@ -25,20 +26,34 @@ const babelContent = JSON.stringify({ plugins: [], }); -const check = async ({ packageJson = {}, main = {}, extraFiles }: any) => { - // eslint-disable-next-line global-require - require('fs-extra').__setMockFiles({ - [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, - ...(extraFiles || {}), - }); +const check = async ({ + packageJson = {}, + main: mainConfig, + storybookVersion = '7.0.0', + extraFiles, +}: { + packageJson?: PackageJson; + main?: Partial & Record; + storybookVersion?: string; + extraFiles?: Record; +}) => { + if (extraFiles) { + // eslint-disable-next-line global-require + require('fs-extra').__setMockFiles(extraFiles); + } + + mockStorybookData({ mainConfig, storybookVersion }); - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return missingBabelRc.check({ packageManager }); + return missingBabelRc.check({ packageManager: makePackageManager(packageJson) }); }; describe('missing-babelrc fix', () => { + afterEach(jest.restoreAllMocks); + + it('skips when storybook version < 7.0.0', async () => { + await expect(check({ storybookVersion: '6.3.2' })).resolves.toBeNull(); + }); + it('skips when babelrc config is present', async () => { const packageJson = { devDependencies: { diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts index 57f1411d6800..3f165201693f 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts @@ -1,11 +1,10 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import semver from 'semver'; -import { getStorybookInfo } from '@storybook/core-common'; import { loadPartialConfigAsync } from '@babel/core'; -import { readConfig } from '@storybook/csf-tools'; import type { Fix } from '../types'; import { generateStorybookBabelConfigInCWD } from '../../babel-config'; +import { getStorybookData } from '../helpers/mainConfigFile'; interface MissingBabelRcOptions { needsBabelRc: boolean; @@ -23,32 +22,17 @@ const frameworksThatNeedBabelConfig = [ export const missingBabelRc: Fix = { id: 'missing-babelrc', - async check({ packageManager }) { + async check({ configDir, packageManager }) { const packageJson = packageManager.retrievePackageJson(); - const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); - - const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!storybookCoerced) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } + const { mainConfig, storybookVersion } = await getStorybookData({ configDir, packageManager }); - if (!semver.gte(storybookCoerced, '7.0.0')) { + if (!semver.gte(storybookVersion, '7.0.0')) { return null; } - if (!mainConfig) { - logger.warn('Unable to find storybook main.js config, skipping'); - return null; - } - - const main = await readConfig(mainConfig); - - const frameworkPackage = main.getNameFromPath(['framework']); + const { framework, addons } = mainConfig; - const addons = main.getNamesFromPath(['addons']); + const frameworkPackage = typeof framework === 'string' ? framework : framework.name; const hasCraPreset = addons && addons.find((addon) => addon === '@storybook/preset-create-react-app'); diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks/new-frameworks.ts index 42d1ecb0d882..242279abebd5 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks/new-frameworks.ts @@ -4,11 +4,11 @@ import findUp from 'find-up'; import semver from 'semver'; import { frameworkPackages, rendererPackages } from '@storybook/core-common'; +import type { Preset } from '@storybook/types'; import type { Fix } from '../../types'; import type { PackageJsonWithDepsAndDevDeps } from '../../../js-package-manager'; import { getStorybookVersionSpecifier } from '../../../helpers'; import { detectRenderer } from '../../helpers/detectRenderer'; -import type { Addon } from './utils'; import { getNextjsAddonOptions, detectBuilderInfo, packagesMap } from './utils'; import { getStorybookData, updateMainConfig } from '../../helpers/mainConfigFile'; @@ -165,7 +165,7 @@ export const newFrameworks: Fix = { addonsToRemove = ['storybook-addon-next', 'storybook-addon-next-router'].filter( (dep) => allDependencies[dep] || - mainConfig.addons?.find((addon) => + mainConfig.addons?.find((addon: Preset) => typeof addon === 'string' ? dep === addon : dep === addon.name ) ); @@ -504,7 +504,7 @@ export const newFrameworks: Fix = { } if (addonsToRemove.length > 0) { - const existingAddons = main.getFieldValue(['addons']) as Addon[]; + const existingAddons = main.getFieldValue(['addons']) as Preset[]; const updatedAddons = existingAddons.filter((addon) => { if (typeof addon === 'string') { return !addonsToRemove.includes(addon); diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks/utils.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks/utils.ts index b481fc2556b4..df6698daa629 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks/utils.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks/utils.ts @@ -149,9 +149,7 @@ export const detectBuilderInfo = async ({ }; }; -export type Addon = string | { name: string; options?: Record }; - -export const getNextjsAddonOptions = (addons: Addon[]) => { +export const getNextjsAddonOptions = (addons: Preset[]) => { const nextjsAddon = addons?.find((addon) => typeof addon === 'string' ? addon === 'storybook-addon-next' diff --git a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts index 0dc62e43ecd6..5a4f6327e657 100644 --- a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts +++ b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts @@ -1,22 +1,16 @@ /// ; -import type { JsPackageManager } from '../../js-package-manager'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { nodeJsRequirement } from './nodejs-requirement'; // eslint-disable-next-line global-require, jest/no-mocks-import jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); -const check = async ({ packageJson = {} }: any) => { - const packageManager = { - retrievePackageJson: () => ({ - dependencies: { - '@storybook/react': '^7.0.0', - }, - devDependencies: {}, - ...packageJson, - }), - } as JsPackageManager; - return nodeJsRequirement.check({ packageManager }); +const check = async ({ storybookVersion = '7.0.0' }) => { + mockStorybookData({ mainConfig: {}, storybookVersion }); + return nodeJsRequirement.check({ + packageManager: makePackageManager({}), + }); }; const originalNodeVersion = process.version; @@ -31,6 +25,12 @@ const mockNodeVersion = (version: string) => { describe('nodejs-requirement fix', () => { afterAll(() => { mockNodeVersion(originalNodeVersion); + jest.restoreAllMocks(); + }); + + it('skips when sb <= 7.0.0', async () => { + mockNodeVersion('14.0.0'); + await expect(check({ storybookVersion: '6.3.2' })).resolves.toBeNull(); }); it('skips when node >= 16.0.0', async () => { diff --git a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts index c9351d6939a3..276d4b9e1ead 100644 --- a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts +++ b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts @@ -1,8 +1,8 @@ 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 { getStorybookData } from '../helpers/mainConfigFile'; interface NodeJsRequirementOptions { nodeVersion: string; @@ -12,19 +12,10 @@ export const nodeJsRequirement: Fix = { id: 'nodejs-requirement', promptOnly: true, - async check({ packageManager }) { - const packageJson = packageManager.retrievePackageJson(); - const { version: storybookVersion } = getStorybookInfo(packageJson); + async check({ packageManager, configDir }) { + const { storybookVersion } = await getStorybookData({ packageManager, configDir }); - const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!storybookCoerced) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } - - if (!semver.gte(storybookCoerced, '7.0.0')) { + if (!semver.gte(storybookVersion, '7.0.0')) { return null; } diff --git a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts index 793d0c9aa80d..9e725b810d73 100644 --- a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts +++ b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts @@ -22,10 +22,10 @@ export const removedGlobalClientAPIs: Fix = { id: 'removedglobalclientapis', promptOnly: true, - async check({ packageManager }) { + async check({ packageManager, configDir }) { const packageJson = packageManager.retrievePackageJson(); - const { previewConfig } = getStorybookInfo(packageJson); + const { previewConfig } = getStorybookInfo(packageJson, configDir); if (previewConfig) { const contents = await readFile(previewConfig, 'utf8'); diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts index 70b019c737d8..b67fbd12a496 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts @@ -1,11 +1,15 @@ -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; +import { makePackageManager } from '../helpers/testing-helpers'; import { sbBinary } from './sb-binary'; -const checkStorybookBinary = async ({ packageJson }: { packageJson: PackageJson }) => { - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return sbBinary.check({ packageManager }); +const checkStorybookBinary = async ({ + packageJson, + storybookVersion = '7.0.0', +}: { + packageJson: PackageJson; + storybookVersion?: string; +}) => { + return sbBinary.check({ packageManager: makePackageManager(packageJson) }); }; describe('storybook-binary fix', () => { @@ -16,6 +20,7 @@ describe('storybook-binary fix', () => { await expect( checkStorybookBinary({ packageJson, + storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); }); diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.ts index b07de972822e..b97d1faa5536 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-binary.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.ts @@ -1,10 +1,10 @@ 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'; +import { getStorybookData } from '../helpers/mainConfigFile'; interface SbBinaryRunOptions { storybookVersion: string; @@ -25,28 +25,18 @@ const logger = console; export const sbBinary: Fix = { id: 'storybook-binary', - async check({ packageManager }) { + async check({ packageManager, configDir }) { 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) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } + const allDependencies = packageManager.getAllDependencies(); + const { storybookVersion } = await getStorybookData({ packageManager, configDir }); // Nx provides their own binary, so we don't need to do anything - if (allDeps['@nrwl/storybook'] || semver.lt(storybookCoerced, '7.0.0')) { + if (allDependencies['@nrwl/storybook'] || semver.lt(storybookVersion, '7.0.0')) { return null; } - const hasSbBinary = !!allDeps.sb; - const hasStorybookBinary = !!allDeps.storybook; + const hasSbBinary = !!allDependencies.sb; + const hasStorybookBinary = !!allDependencies.storybook; if (!hasSbBinary && hasStorybookBinary) { return null; 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 eb8b827a89a5..1e8d4edf71ca 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts @@ -1,14 +1,20 @@ -import type { JsPackageManager, PackageJson } from '../../js-package-manager'; +import type { PackageJson } from '../../js-package-manager'; +import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { getStorybookScripts, sbScripts } from './sb-scripts'; -const checkSbScripts = async ({ packageJson }: { packageJson: PackageJson }) => { - const packageManager = { - retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), - } as JsPackageManager; - return sbScripts.check({ packageManager }); +const checkSbScripts = async ({ + packageJson, + storybookVersion = '7.0.0', +}: { + packageJson: PackageJson; + storybookVersion?: string; +}) => { + mockStorybookData({ mainConfig: {}, storybookVersion }); + return sbScripts.check({ packageManager: makePackageManager(packageJson) }); }; describe('getStorybookScripts', () => { + afterEach(jest.restoreAllMocks); it('detects default storybook scripts', () => { expect( getStorybookScripts({ @@ -57,6 +63,7 @@ describe('sb-scripts fix', () => { await expect( checkSbScripts({ packageJson, + storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); }); @@ -91,7 +98,6 @@ describe('sb-scripts fix', () => { before: 'start-storybook -p 6006', }, }, - storybookVersion: '^7.0.0-alpha.0', }) ); }); @@ -130,7 +136,6 @@ describe('sb-scripts fix', () => { 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "CI=true storybook build --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', }, }, - 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 5e63885a6c4f..478a5b8c33ce 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts @@ -1,9 +1,9 @@ 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 type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; +import { getStorybookData } from '../helpers/mainConfigFile'; interface SbScriptsRunOptions { storybookScripts: Record; @@ -70,20 +70,12 @@ export const getStorybookScripts = (allScripts: Record) => { export const sbScripts: Fix = { id: 'sb-scripts', - async check({ packageManager }) { + async check({ packageManager, configDir }) { const packageJson = packageManager.retrievePackageJson(); const { scripts = {} } = packageJson; - const { version: storybookVersion } = getStorybookInfo(packageJson); - - const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!storybookCoerced) { - throw new Error(dedent` - ❌ Unable to determine storybook version. - 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. - `); - } + const { storybookVersion } = await getStorybookData({ packageManager, configDir }); - if (semver.lt(storybookCoerced, '7.0.0')) { + if (semver.lt(storybookVersion, '7.0.0')) { return null; } diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index d3b00467dd4d..5073e6f8cc84 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -1,4 +1,5 @@ import { getStorybookInfo, loadMainConfig } from '@storybook/core-common'; +import type { StorybookConfig } from '@storybook/types'; import type { ConfigFile } from '@storybook/csf-tools'; import { readConfig, writeConfig as writeConfigFile } from '@storybook/csf-tools'; import chalk from 'chalk'; @@ -20,13 +21,14 @@ export const getStorybookData = async ({ mainConfig: mainConfigPath, version: storybookVersionSpecifier, configDir: configDirFromScript, + previewConfig: previewConfigPath, } = getStorybookInfo(packageJson, userDefinedConfigDir); const storybookVersion = storybookVersionSpecifier && semver.coerce(storybookVersionSpecifier)?.version; const configDir = userDefinedConfigDir || configDirFromScript || '.storybook'; - let mainConfig; + let mainConfig: StorybookConfig; try { mainConfig = await loadMainConfig({ configDir }); } catch (err) { @@ -41,6 +43,7 @@ export const getStorybookData = async ({ storybookVersionSpecifier, storybookVersion, mainConfigPath, + previewConfigPath, }; }; export type GetStorybookData = typeof getStorybookData;