From 468097e24a1a96c789885d82412f4048e81360b1 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 20 Feb 2023 19:18:00 +0100 Subject: [PATCH] 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;