From 27d58ad70145cf40503541b2666274540d248337 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 10 Aug 2022 07:44:32 +0200 Subject: [PATCH 01/11] CLI: add new frameworks format automigration --- code/lib/cli/src/automigrate/fixes/index.ts | 2 + .../automigrate/fixes/new-frameworks.test.ts | 180 ++++++++++++++ .../src/automigrate/fixes/new-frameworks.ts | 221 ++++++++++++++++++ 3 files changed, 403 insertions(+) create mode 100644 code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts create mode 100644 code/lib/cli/src/automigrate/fixes/new-frameworks.ts diff --git a/code/lib/cli/src/automigrate/fixes/index.ts b/code/lib/cli/src/automigrate/fixes/index.ts index 766d7db30352..e43ad3f2025d 100644 --- a/code/lib/cli/src/automigrate/fixes/index.ts +++ b/code/lib/cli/src/automigrate/fixes/index.ts @@ -7,6 +7,7 @@ import { eslintPlugin } from './eslint-plugin'; import { builderVite } from './builder-vite'; import { npm7 } from './npm7'; import { sbScripts } from './sb-scripts'; +import { newFrameworks } from './new-frameworks'; import { Fix } from '../types'; export * from '../types'; @@ -20,4 +21,5 @@ export const fixes: Fix[] = [ builderVite, npm7, sbScripts, + newFrameworks, ]; diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts new file mode 100644 index 000000000000..8cce78e2b5ec --- /dev/null +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts @@ -0,0 +1,180 @@ +/* eslint-disable no-underscore-dangle */ +import path from 'path'; +import { JsPackageManager } from '../../js-package-manager'; +import { newFrameworks } from './new-frameworks'; + +// eslint-disable-next-line global-require, jest/no-mocks-import +jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); + +const checkNewFrameworks = async ({ packageJson, main }) => { + if (main) { + // eslint-disable-next-line global-require + require('fs-extra').__setMockFiles({ + [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`, + }); + } + const packageManager = { + retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + } as JsPackageManager; + return newFrameworks.check({ packageManager }); +}; + +describe('new-frameworks fix', () => { + describe('should no-op', () => { + it('in sb < 7', async () => { + const packageJson = { dependencies: { '@storybook/vue': '^6.2.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + + it('in sb 7 with no main', async () => { + const packageJson = { dependencies: { '@storybook/vue': '^7.0.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: undefined, + }) + ).resolves.toBeFalsy(); + }); + + it('in sb 7 with no framework field in main', async () => { + const packageJson = { dependencies: { '@storybook/vue': '^7.0.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + + it('in sb 7 with no builder', async () => { + const packageJson = { dependencies: { '@storybook/vue': '^7.0.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/vue', + }, + }) + ).resolves.toBeFalsy(); + }); + + it('in sb 7 with unsupported package', async () => { + const packageJson = { dependencies: { '@storybook/riot': '^7.0.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/riot', + core: { + builder: 'webpack5', + }, + }, + }) + ).resolves.toBeFalsy(); + }); + + // TODO: once we have vite frameworks e.g. @storybook/react-vite, then we should remove this test + it('in sb 7 with vite', async () => { + const packageJson = { dependencies: { '@storybook/react': '^7.0.0' } }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/react', + core: { + builder: '@storybook/builder-vite', + }, + }, + }) + ).resolves.toBeFalsy(); + }); + }); + + describe('sb >= 7', () => { + describe('new-frameworks dependency', () => { + it('should update to @storybook/react-webpack5', async () => { + const packageJson = { + dependencies: { + '@storybook/react': '^7.0.0-alpha.0', + '@storybook/builder-webpack5': '^6.5.9', + '@storybook/manager-webpack5': '^6.5.9', + }, + }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/react', + core: { + builder: { + name: 'webpack5', + options: { + lazyCompilation: true, + }, + }, + }, + reactOptions: { + fastRefresh: true, + }, + }, + }) + ).resolves.toEqual( + expect.objectContaining({ + frameworkPackage: '@storybook/react', + dependenciesToAdd: ['@storybook/react-webpack5'], + dependenciesToRemove: [ + '@storybook/react', + '@storybook/builder-webpack5', + '@storybook/manager-webpack5', + ], + frameworkOptions: { + fastRefresh: true, + }, + builderInfo: { + name: 'webpack5', + options: { + lazyCompilation: true, + }, + }, + }) + ); + }); + }); + + // TODO: enable this once Vite is supported + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should update to @storybook/react-vite', async () => { + const packageJson = { + dependencies: { + '@storybook/react': '^7.0.0-alpha.0', + '@storybook/builder-vite': '^0.0.2', + }, + }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/react', + core: { + builder: '@storybook/builder-vite', + }, + }, + }) + ).resolves.toEqual( + expect.objectContaining({ + dependenciesToAdd: '@storybook/react-vite', + dependenciesToRemove: [ + '@storybook/react', + 'storybook-builder-vite', + '@storybook/builder-vite', + ], + }) + ); + }); + }); +}); diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts new file mode 100644 index 000000000000..75c4c16d7fcd --- /dev/null +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -0,0 +1,221 @@ +import chalk from 'chalk'; +import dedent from 'ts-dedent'; +import semver from '@storybook/semver'; +import { ConfigFile, readConfig, writeConfig } from '@storybook/csf-tools'; +import { getStorybookInfo } from '@storybook/core-common'; + +import type { Fix } from '../types'; +import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; + +const logger = console; + +const packagesMap = { + '@storybook/react': { + webpack5: '@storybook/react-webpack5', + vite: '@storybook/react-vite', + }, + '@storybook/preact': { + webpack5: '@storybook/preact-webpack5', + }, + '@storybook/server': { + webpack5: '@storybook/server-webpack5', + }, + '@storybook/angular': { + webpack5: '@storybook/angular-webpack5', + }, + '@storybook/vue': { + webpack5: '@storybook/vue-webpack5', + vite: '@storybook/vue-vite', + }, + '@storybook/vue3': { + webpack5: '@storybook/vue3-webpack5', + vite: '@storybook/vue3-vite', + }, + '@storybook/svelte': { + webpack5: '@storybook/svelte-webpack5', + vite: '@storybook/svelte-vite', + }, + '@storybook/web-components': { + webpack5: '@storybook/web-components-webpack5', + }, + '@storybook/html': { + webpack5: '@storybook/html-webpack5', + }, +}; + +interface NewFrameworkRunOptions { + main: ConfigFile; + packageJson: PackageJsonWithDepsAndDevDeps; + dependenciesToAdd: string[]; + dependenciesToRemove: string[]; + frameworkPackage: keyof typeof packagesMap; + frameworkOptions: Record; + builderInfo: { + name: string; + options: Record; + }; +} + +export const getBuilder = (builder: string | { name: string }) => { + if (typeof builder === 'string') { + return builder.includes('vite') ? 'vite' : 'webpack5'; + } + + return builder.name.includes('vite') ? 'vite' : 'webpack5'; +}; + +export const getFrameworkOptions = (framework: string, main: ConfigFile) => { + const frameworkOptions = main.getFieldValue([`${framework}Options`]); + return frameworkOptions || {}; +}; + +/** + * Does the user have separate framework and builders (e.g. @storybook/react + core.builder -> webpack5? + * + * If so: + * - Remove the dependencies (@storybook/react + @storybook/builder-webpack5 + @storybook/manager-webpack5) + * - Install the correct new package e.g. (@storybook/react-webpack5) + * - Update the main config to use the new framework + * -- moving core.builder into framework.options.builder + * -- moving frameworkOptions (e.g. reactOptions) into framework.options + */ +export const newFrameworks: Fix = { + id: 'newFrameworks', + + async check({ packageManager }) { + const packageJson = packageManager.retrievePackageJson(); + const allDeps = { ...packageJson.dependencies, ...packageJson.devDependencies }; + + const config = getStorybookInfo(packageJson); + const { mainConfig, version: storybookVersion, framework } = config; + if (!mainConfig) { + logger.warn('Unable to find storybook main.js config, skipping'); + return null; + } + + const storybookCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; + if (!storybookCoerced) { + logger.warn(dedent` + ❌ Unable to determine storybook version, skipping ${chalk.cyan('newFrameworks')} fix. + 🤔 Are you running automigrate from your project directory? + `); + return null; + } + + if (!semver.gte(storybookCoerced, '7.0.0')) { + console.log('lower than 7.0.0!'); + 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); + + if (!main) { + console.log('no main'); + return null; + } + + const frameworkPackage = main.getFieldValue(['framework']) as keyof typeof packagesMap; + const builder = main.getFieldValue(['core', 'builder']); + + if (!frameworkPackage || !builder) { + console.log('no framework or no builder, skipping'); + return null; + } + + const supportedPackages = Object.keys(packagesMap); + if (!supportedPackages.includes(frameworkPackage)) { + console.log('no supported package, skipping'); + return null; + } + + const builderInfo = { + name: getBuilder(builder), + options: main.getFieldValue(['core', 'builder', 'options']) || {}, + } as const; + + // TODO: once we have vite frameworks e.g. @storybook/react-vite, then we support it here + // and remove ['storybook-builder-vite', '@storybook/builder-vite'] from deps + if (builderInfo.name === 'vite') { + return null; + } + + const frameworkOptions = getFrameworkOptions(framework, main); + + const dependenciesToRemove = [ + frameworkPackage, + '@storybook/builder-webpack5', + '@storybook/manager-webpack5', + ].filter((dep) => allDeps[dep]); + + const newFrameworkPackage = packagesMap[frameworkPackage][builderInfo.name]; + const dependenciesToAdd = [newFrameworkPackage]; + + return { + main, + dependenciesToAdd, + dependenciesToRemove, + frameworkPackage: newFrameworkPackage, + frameworkOptions, + builderInfo, + packageJson, + }; + }, + + prompt() { + return dedent` + We've detected you are using an older format of Storybook frameworks and builders. + + In Storybook 7, frameworks also specify the builder to be used. + + We can remove the dependencies that are no longer needed and install the new framework that already includes the builder. + + More info: ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#framework-field-mandatory' + )} + `; + }, + + async run({ + result: { + dependenciesToAdd, + dependenciesToRemove, + main, + frameworkPackage, + frameworkOptions, + builderInfo, + packageJson, + }, + packageManager, + dryRun, + }) { + logger.info(`✅ Removing legacy dependencies: ${dependenciesToRemove.join(', ')}`); + if (!dryRun) { + packageManager.removeDependencies({ skipInstall: true, packageJson }, dependenciesToRemove); + } + + logger.info(`✅ Installing new dependencies: ${dependenciesToAdd.join(', ')}`); + if (!dryRun) { + packageManager.addDependencies({ installAsDevDependencies: true }, dependenciesToAdd); + } + + if (!dryRun) { + logger.info(`✅ Updating framework field in main.js`); + const currentCore = main.getFieldValue(['core']); + main.setFieldValue(['framework', 'name'], frameworkPackage); + main.setFieldValue(['framework', 'options'], frameworkOptions); + + main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options); + + delete currentCore.builder; + if (Object.keys(currentCore).length === 0) { + // TODO: this should delete the field instead + main.setFieldValue(['core'], {}); + } else { + main.setFieldValue(['core'], currentCore); + } + + await writeConfig(main); + } + }, +}; From a7f3904910e13877d60faabaca2796096c0539cd Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 10 Aug 2022 07:50:35 +0200 Subject: [PATCH 02/11] CLI: add pre-release flag to automigrate --- code/lib/cli/src/automigrate/index.ts | 5 +++-- code/lib/cli/src/automigrate/types.ts | 1 + code/lib/cli/src/generate.ts | 1 + code/lib/cli/src/upgrade.ts | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index a20325c86119..7e4b1e045376 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -12,9 +12,10 @@ interface FixOptions { fixId?: string; yes?: boolean; dryRun?: boolean; + prerelease?: boolean; } -export const automigrate = async ({ fixId, dryRun, yes }: FixOptions = {}) => { +export const automigrate = async ({ fixId, dryRun, yes, prerelease }: FixOptions = {}) => { const packageManager = JsPackageManagerFactory.getPackageManager(); const filtered = fixId ? fixes.filter((f) => f.id === fixId) : fixes; @@ -53,7 +54,7 @@ export const automigrate = async ({ fixId, dryRun, yes }: FixOptions = {}) => { if (runAnswer.fix) { try { - await f.run({ result, packageManager, dryRun }); + await f.run({ result, packageManager, dryRun, prerelease }); logger.info(`✅ ran ${chalk.cyan(f.id)} migration`); } catch (error) { logger.info(`❌ error when running ${chalk.cyan(f.id)} migration:`); diff --git a/code/lib/cli/src/automigrate/types.ts b/code/lib/cli/src/automigrate/types.ts index ff812f7064d6..605f2e4ea7a8 100644 --- a/code/lib/cli/src/automigrate/types.ts +++ b/code/lib/cli/src/automigrate/types.ts @@ -8,6 +8,7 @@ export interface RunOptions { packageManager: JsPackageManager; result: ResultType; dryRun?: boolean; + prerelease?: boolean; } export interface Fix { diff --git a/code/lib/cli/src/generate.ts b/code/lib/cli/src/generate.ts index acfcb7e79021..f72982ce812b 100644 --- a/code/lib/cli/src/generate.ts +++ b/code/lib/cli/src/generate.ts @@ -177,6 +177,7 @@ program .description('Check storybook for known problems or migrations and apply fixes') .option('-y --yes', 'Skip prompting the user') .option('-n --dry-run', 'Only check for fixes, do not actually run them') + .option('-p --prerelease', 'Use pre-release versions when migrating') .action((fixId, options) => automigrate({ fixId, ...options }).catch((e) => { logger.error(e); diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index 2df15576bb94..79301873740d 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -175,6 +175,6 @@ export const upgrade = async ({ if (!skipCheck) { checkVersionConsistency(); - await automigrate({ dryRun, yes }); + await automigrate({ dryRun, yes, prerelease }); } }; From a73c5b15774b3a0be7c65e76a8b5e89642939f87 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 14:48:06 +0200 Subject: [PATCH 03/11] Revert "CLI: add pre-release flag to automigrate" This reverts commit a7f3904910e13877d60faabaca2796096c0539cd. --- code/lib/cli/src/automigrate/index.ts | 5 ++--- code/lib/cli/src/automigrate/types.ts | 1 - code/lib/cli/src/generate.ts | 1 - code/lib/cli/src/upgrade.ts | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index 7e4b1e045376..a20325c86119 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -12,10 +12,9 @@ interface FixOptions { fixId?: string; yes?: boolean; dryRun?: boolean; - prerelease?: boolean; } -export const automigrate = async ({ fixId, dryRun, yes, prerelease }: FixOptions = {}) => { +export const automigrate = async ({ fixId, dryRun, yes }: FixOptions = {}) => { const packageManager = JsPackageManagerFactory.getPackageManager(); const filtered = fixId ? fixes.filter((f) => f.id === fixId) : fixes; @@ -54,7 +53,7 @@ export const automigrate = async ({ fixId, dryRun, yes, prerelease }: FixOptions if (runAnswer.fix) { try { - await f.run({ result, packageManager, dryRun, prerelease }); + await f.run({ result, packageManager, dryRun }); logger.info(`✅ ran ${chalk.cyan(f.id)} migration`); } catch (error) { logger.info(`❌ error when running ${chalk.cyan(f.id)} migration:`); diff --git a/code/lib/cli/src/automigrate/types.ts b/code/lib/cli/src/automigrate/types.ts index 605f2e4ea7a8..ff812f7064d6 100644 --- a/code/lib/cli/src/automigrate/types.ts +++ b/code/lib/cli/src/automigrate/types.ts @@ -8,7 +8,6 @@ export interface RunOptions { packageManager: JsPackageManager; result: ResultType; dryRun?: boolean; - prerelease?: boolean; } export interface Fix { diff --git a/code/lib/cli/src/generate.ts b/code/lib/cli/src/generate.ts index f72982ce812b..acfcb7e79021 100644 --- a/code/lib/cli/src/generate.ts +++ b/code/lib/cli/src/generate.ts @@ -177,7 +177,6 @@ program .description('Check storybook for known problems or migrations and apply fixes') .option('-y --yes', 'Skip prompting the user') .option('-n --dry-run', 'Only check for fixes, do not actually run them') - .option('-p --prerelease', 'Use pre-release versions when migrating') .action((fixId, options) => automigrate({ fixId, ...options }).catch((e) => { logger.error(e); diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index 79301873740d..2df15576bb94 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -175,6 +175,6 @@ export const upgrade = async ({ if (!skipCheck) { checkVersionConsistency(); - await automigrate({ dryRun, yes, prerelease }); + await automigrate({ dryRun, yes }); } }; From 3e73edadb0f68e96ef40cb1699852fffc8c177f3 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 14:48:42 +0200 Subject: [PATCH 04/11] CLI: fix dependency logic in new-frameworks automigration --- code/lib/cli/src/automigrate/fixes/new-frameworks.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index 75c4c16d7fcd..dc7b076f7cf7 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -6,6 +6,7 @@ import { getStorybookInfo } from '@storybook/core-common'; import type { Fix } from '../types'; import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; +import { getStorybookVersionSpecifier } from '../../helpers'; const logger = console; @@ -196,7 +197,9 @@ export const newFrameworks: Fix = { logger.info(`✅ Installing new dependencies: ${dependenciesToAdd.join(', ')}`); if (!dryRun) { - packageManager.addDependencies({ installAsDevDependencies: true }, dependenciesToAdd); + const versionToInstall = getStorybookVersionSpecifier(packageJson); + const depsToAdd = dependenciesToAdd.map((dep) => `${dep}@${versionToInstall}`); + packageManager.addDependencies({ installAsDevDependencies: true }, depsToAdd); } if (!dryRun) { From 54e5f436e206bccc5c3592e470f03d21788d5e23 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 15:28:24 +0200 Subject: [PATCH 05/11] CLI: account for frameworks that did not change format in automigrate --- .../src/automigrate/fixes/new-frameworks.ts | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index dc7b076f7cf7..34a5b8f59204 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -22,7 +22,7 @@ const packagesMap = { webpack5: '@storybook/server-webpack5', }, '@storybook/angular': { - webpack5: '@storybook/angular-webpack5', + webpack5: '@storybook/angular', }, '@storybook/vue': { webpack5: '@storybook/vue-webpack5', @@ -144,13 +144,18 @@ export const newFrameworks: Fix = { const frameworkOptions = getFrameworkOptions(framework, main); const dependenciesToRemove = [ - frameworkPackage, '@storybook/builder-webpack5', '@storybook/manager-webpack5', ].filter((dep) => allDeps[dep]); const newFrameworkPackage = packagesMap[frameworkPackage][builderInfo.name]; - const dependenciesToAdd = [newFrameworkPackage]; + const dependenciesToAdd = []; + + // some frameworks didn't change e.g. Angular, Ember + if (newFrameworkPackage !== frameworkPackage) { + dependenciesToRemove.push(frameworkPackage); + dependenciesToAdd.push(newFrameworkPackage); + } return { main, @@ -192,14 +197,18 @@ export const newFrameworks: Fix = { }) { logger.info(`✅ Removing legacy dependencies: ${dependenciesToRemove.join(', ')}`); if (!dryRun) { - packageManager.removeDependencies({ skipInstall: true, packageJson }, dependenciesToRemove); + packageManager.removeDependencies( + { skipInstall: dependenciesToAdd.length > 0, packageJson }, + dependenciesToRemove + ); } - - logger.info(`✅ Installing new dependencies: ${dependenciesToAdd.join(', ')}`); - if (!dryRun) { - const versionToInstall = getStorybookVersionSpecifier(packageJson); - const depsToAdd = dependenciesToAdd.map((dep) => `${dep}@${versionToInstall}`); - packageManager.addDependencies({ installAsDevDependencies: true }, depsToAdd); + if (dependenciesToAdd.length > 0) { + logger.info(`✅ Installing new dependencies: ${dependenciesToAdd.join(', ')}`); + if (!dryRun) { + const versionToInstall = getStorybookVersionSpecifier(packageJson); + const depsToAdd = dependenciesToAdd.map((dep) => `${dep}@${versionToInstall}`); + packageManager.addDependencies({ installAsDevDependencies: true }, depsToAdd); + } } if (!dryRun) { @@ -208,7 +217,9 @@ export const newFrameworks: Fix = { main.setFieldValue(['framework', 'name'], frameworkPackage); main.setFieldValue(['framework', 'options'], frameworkOptions); - main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options); + if (Object.keys(builderInfo.options).length > 0) { + main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options); + } delete currentCore.builder; if (Object.keys(currentCore).length === 0) { From fdcaf03a31a2ae3dfaf085c08e0e005516680405 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 11 Aug 2022 15:32:13 +0200 Subject: [PATCH 06/11] test: add case for angular in new-framerworks migration --- .../automigrate/fixes/new-frameworks.test.ts | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts index 8cce78e2b5ec..b1a37d627b19 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts @@ -125,12 +125,12 @@ describe('new-frameworks fix', () => { }) ).resolves.toEqual( expect.objectContaining({ - frameworkPackage: '@storybook/react', + frameworkPackage: '@storybook/react-webpack5', dependenciesToAdd: ['@storybook/react-webpack5'], dependenciesToRemove: [ - '@storybook/react', '@storybook/builder-webpack5', '@storybook/manager-webpack5', + '@storybook/react', ], frameworkOptions: { fastRefresh: true, @@ -144,6 +144,49 @@ describe('new-frameworks fix', () => { }) ); }); + it('should update only builders in @storybook/angular', async () => { + const packageJson = { + dependencies: { + '@storybook/angular': '^7.0.0-alpha.0', + '@storybook/builder-webpack5': '^6.5.9', + '@storybook/manager-webpack5': '^6.5.9', + }, + }; + await expect( + checkNewFrameworks({ + packageJson, + main: { + framework: '@storybook/angular', + core: { + builder: { + name: 'webpack5', + options: { + lazyCompilation: true, + }, + }, + }, + angularOptions: { + enableIvy: true, + }, + }, + }) + ).resolves.toEqual( + expect.objectContaining({ + frameworkPackage: '@storybook/angular', + dependenciesToAdd: [], + dependenciesToRemove: ['@storybook/builder-webpack5', '@storybook/manager-webpack5'], + frameworkOptions: { + enableIvy: true, + }, + builderInfo: { + name: 'webpack5', + options: { + lazyCompilation: true, + }, + }, + }) + ); + }); }); // TODO: enable this once Vite is supported From 9791f4e1f72bd19e1fda84ca18ba1c3a7c6727e7 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 12 Aug 2022 15:51:59 +0200 Subject: [PATCH 07/11] cleanup --- code/lib/cli/src/automigrate/fixes/new-frameworks.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index 34a5b8f59204..016e05a66145 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -104,29 +104,21 @@ export const newFrameworks: Fix = { } if (!semver.gte(storybookCoerced, '7.0.0')) { - console.log('lower than 7.0.0!'); 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); - if (!main) { - console.log('no main'); - return null; - } - const frameworkPackage = main.getFieldValue(['framework']) as keyof typeof packagesMap; const builder = main.getFieldValue(['core', 'builder']); if (!frameworkPackage || !builder) { - console.log('no framework or no builder, skipping'); return null; } const supportedPackages = Object.keys(packagesMap); if (!supportedPackages.includes(frameworkPackage)) { - console.log('no supported package, skipping'); return null; } From fcc4dca1e38051fa1ac251ee3924806a1d0d93d6 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 15 Aug 2022 18:42:12 +0800 Subject: [PATCH 08/11] Extend new frameworks automigration to include Webpack4 --- .../automigrate/fixes/new-frameworks.test.ts | 8 +++++++- .../src/automigrate/fixes/new-frameworks.ts | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts index b1a37d627b19..482389d3bf8e 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts @@ -60,7 +60,13 @@ describe('new-frameworks fix', () => { framework: '@storybook/vue', }, }) - ).resolves.toBeFalsy(); + ).resolves.toEqual( + expect.objectContaining({ + frameworkPackage: '@storybook/vue-webpack5', + dependenciesToAdd: ['@storybook/vue-webpack5'], + dependenciesToRemove: ['@storybook/vue'], + }) + ); }); it('in sb 7 with unsupported package', async () => { diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index 016e05a66145..9b96b69793ec 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -62,7 +62,7 @@ export const getBuilder = (builder: string | { name: string }) => { return builder.includes('vite') ? 'vite' : 'webpack5'; } - return builder.name.includes('vite') ? 'vite' : 'webpack5'; + return builder?.name.includes('vite') ? 'vite' : 'webpack5'; }; export const getFrameworkOptions = (framework: string, main: ConfigFile) => { @@ -113,7 +113,7 @@ export const newFrameworks: Fix = { const frameworkPackage = main.getFieldValue(['framework']) as keyof typeof packagesMap; const builder = main.getFieldValue(['core', 'builder']); - if (!frameworkPackage || !builder) { + if (!frameworkPackage) { return null; } @@ -138,6 +138,8 @@ export const newFrameworks: Fix = { const dependenciesToRemove = [ '@storybook/builder-webpack5', '@storybook/manager-webpack5', + '@storybook/builder-webpack4', + '@storybook/manager-webpack4', ].filter((dep) => allDeps[dep]); const newFrameworkPackage = packagesMap[frameworkPackage][builderInfo.name]; @@ -168,9 +170,20 @@ export const newFrameworks: Fix = { We can remove the dependencies that are no longer needed and install the new framework that already includes the builder. - More info: ${chalk.yellow( + To learn more about the framework field, see: ${chalk.yellow( 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#framework-field-mandatory' )} + + ${chalk.underline(chalk.bold(chalk.cyan('Webpack4 users')))} + + Unless you're using Storybook's Vite builder, this automigration will install a Webpack5-based framework. + + If you were using Storybook's Webpack4 builder (default in 6.x, discontinued in 7.0), this could be a breaking + change--especially if your project has a custom webpack configuration. + + To learn more about migrating from Webpack4, see: ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#webpack4-support-discontinued' + )} `; }, From e54e22a6f291482b960ee364174691415dd2733d Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 15 Aug 2022 13:02:02 +0200 Subject: [PATCH 09/11] log the error with stack trace in automigration --- code/lib/cli/src/automigrate/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index a20325c86119..0eab9bca3d9c 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -57,7 +57,7 @@ export const automigrate = async ({ fixId, dryRun, yes }: FixOptions = {}) => { logger.info(`✅ ran ${chalk.cyan(f.id)} migration`); } catch (error) { logger.info(`❌ error when running ${chalk.cyan(f.id)} migration:`); - logger.info(error.message); + logger.info(error); logger.info(); } } else { From 441d432882ce791582981db7893b26071cd3e402 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 15 Aug 2022 13:02:32 +0200 Subject: [PATCH 10/11] fix core builder logic in new-frameworks automigration --- .../cli/src/automigrate/fixes/new-frameworks.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index 9b96b69793ec..efa09e619ce6 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -224,14 +224,16 @@ export const newFrameworks: Fix = { if (Object.keys(builderInfo.options).length > 0) { main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options); + delete currentCore.builder; } - delete currentCore.builder; - if (Object.keys(currentCore).length === 0) { - // TODO: this should delete the field instead - main.setFieldValue(['core'], {}); - } else { - main.setFieldValue(['core'], currentCore); + if (currentCore) { + if (Object.keys(currentCore).length === 0) { + // TODO: this should delete the field instead + main.setFieldValue(['core'], {}); + } else { + main.setFieldValue(['core'], currentCore); + } } await writeConfig(main); From c0a47e16801a59fc6f6dfdaf4bdea7403a45edb0 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 15 Aug 2022 13:09:31 +0200 Subject: [PATCH 11/11] fix new-frameworks logic --- code/lib/cli/src/automigrate/fixes/new-frameworks.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index efa09e619ce6..f288787485bd 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -222,9 +222,12 @@ export const newFrameworks: Fix = { main.setFieldValue(['framework', 'name'], frameworkPackage); main.setFieldValue(['framework', 'options'], frameworkOptions); + if (currentCore?.builder) { + delete currentCore.builder; + } + if (Object.keys(builderInfo.options).length > 0) { main.setFieldValue(['framework', 'options', 'builder'], builderInfo.options); - delete currentCore.builder; } if (currentCore) {