From 84d31143f090094c60cf998960d5d07dea72ed9d Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 27 Sep 2021 15:01:40 +0800 Subject: [PATCH 01/18] Fix CSFFile for arbitrary components --- lib/csf-tools/src/CsfFile.test.ts | 23 ++++++++++++++++++++++- lib/csf-tools/src/CsfFile.ts | 7 ++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/lib/csf-tools/src/CsfFile.test.ts b/lib/csf-tools/src/CsfFile.test.ts index 389126ae7df..fc53678179e 100644 --- a/lib/csf-tools/src/CsfFile.test.ts +++ b/lib/csf-tools/src/CsfFile.test.ts @@ -151,7 +151,7 @@ describe('CsfFile', () => { ) ).toMatchInlineSnapshot(` meta: - component: foo + component: '''foo''' title: Default Title stories: - id: default-title--a @@ -183,6 +183,27 @@ describe('CsfFile', () => { `); }); + it('component object', async () => { + expect( + await parse( + dedent` + export default { component: {} } + export const A = () => {}; + export const B = () => {}; + ` + ) + ).toMatchInlineSnapshot(` + meta: + component: '{}' + title: Default Title + stories: + - id: default-title--a + name: A + - id: default-title--b + name: B + `); + }); + it('template bind', async () => { expect( await parse( diff --git a/lib/csf-tools/src/CsfFile.ts b/lib/csf-tools/src/CsfFile.ts index 5d3dfecdac0..e0e46aa27b4 100644 --- a/lib/csf-tools/src/CsfFile.ts +++ b/lib/csf-tools/src/CsfFile.ts @@ -137,11 +137,8 @@ export class CsfFile { // @ts-ignore meta[p.key.name] = parseIncludeExclude(p.value); } else if (p.key.name === 'component') { - if (t.isIdentifier(p.value)) { - meta.component = p.value.name; - } else if (t.isStringLiteral(p.value)) { - meta.component = p.value.value; - } + const { code } = generate(p.value, {}); + meta.component = code; } } }); From df6d9ffea0aae76e465e368d997103182abc9d4e Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:05:04 +0800 Subject: [PATCH 02/18] CSF tools: Add utility for reading/writing config files --- lib/csf-tools/src/ConfigFile.test.ts | 272 +++++++++++++++++++++++++++ lib/csf-tools/src/ConfigFile.ts | 228 ++++++++++++++++++++++ lib/csf-tools/src/index.ts | 1 + 3 files changed, 501 insertions(+) create mode 100644 lib/csf-tools/src/ConfigFile.test.ts create mode 100644 lib/csf-tools/src/ConfigFile.ts diff --git a/lib/csf-tools/src/ConfigFile.test.ts b/lib/csf-tools/src/ConfigFile.test.ts new file mode 100644 index 00000000000..19d3b0793c5 --- /dev/null +++ b/lib/csf-tools/src/ConfigFile.test.ts @@ -0,0 +1,272 @@ +import dedent from 'ts-dedent'; +import { formatConfig, loadConfig } from './ConfigFile'; + +expect.addSnapshotSerializer({ + print: (val: any) => val, + test: (val) => true, +}); + +const getField = (path: string[], source: string) => { + const config = loadConfig(source).parse(); + return config.getFieldValue(path); +}; + +const setField = (path: string[], value: any, source: string) => { + const config = loadConfig(source).parse(); + config.setFieldValue(path, value); + return formatConfig(config); +}; + +describe('ConfigFile', () => { + describe('getField', () => { + describe('named exports', () => { + it('missing export', () => { + expect( + getField( + ['core', 'builder'], + dedent` + export const foo = { builder: 'webpack5' } + ` + ) + ).toBeUndefined(); + }); + it('missing field', () => { + expect( + getField( + ['core', 'builder'], + dedent` + export const core = { foo: 'webpack5' } + ` + ) + ).toBeUndefined(); + }); + it('found scalar', () => { + expect( + getField( + ['core', 'builder'], + dedent` + export const core = { builder: 'webpack5' } + ` + ) + ).toEqual('webpack5'); + }); + it('found object', () => { + expect( + getField( + ['core', 'builder'], + dedent` + export const core = { builder: { name: 'webpack5' } } + ` + ) + ).toEqual({ name: 'webpack5' }); + }); + it('variable ref export', () => { + expect( + getField( + ['core', 'builder'], + dedent` + const coreVar = { builder: 'webpack5' }; + export const core = coreVar; + ` + ) + ).toEqual('webpack5'); + }); + it('variable export', () => { + expect( + getField( + ['core', 'builder'], + dedent` + const coreVar = { builder: 'webpack5' }; + export const core = coreVar; + ` + ) + ).toEqual('webpack5'); + }); + }); + describe('module exports', () => { + it('missing export', () => { + expect( + getField( + ['core', 'builder'], + dedent` + module.exports = { foo: { builder: 'webpack5' } } + ` + ) + ).toBeUndefined(); + }); + it('found scalar', () => { + expect( + getField( + ['core', 'builder'], + dedent` + module.exports = { core: { builder: 'webpack5' } } + ` + ) + ).toEqual('webpack5'); + }); + it('variable ref export', () => { + expect( + getField( + ['core', 'builder'], + dedent` + const core = { builder: 'webpack5' }; + module.exports = { core }; + ` + ) + ).toEqual('webpack5'); + }); + it('variable rename', () => { + expect( + getField( + ['core', 'builder'], + dedent` + const coreVar = { builder: 'webpack5' }; + module.exports = { core: coreVar }; + ` + ) + ).toEqual('webpack5'); + }); + }); + }); + + describe('setField', () => { + describe('named exports', () => { + it('missing export', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + export const addons = []; + ` + ) + ).toMatchInlineSnapshot(` + export const addons = []; + export const core = { + builder: "webpack5" + }; + `); + }); + it('missing field', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + export const core = { foo: 'bar' }; + ` + ) + ).toMatchInlineSnapshot(` + export const core = { + foo: 'bar', + builder: "webpack5" + }; + `); + }); + it('found scalar', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + export const core = { builder: 'webpack4' }; + ` + ) + ).toMatchInlineSnapshot(` + export const core = { + builder: "webpack5" + }; + `); + }); + it('found object', () => { + expect( + setField( + ['core', 'builder'], + { name: 'webpack5' }, + dedent` + export const core = { builder: { name: 'webpack4' } }; + ` + ) + ).toMatchInlineSnapshot(` + export const core = { + builder: { + "name": "webpack5" + } + }; + `); + }); + it('variable export', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + const coreVar = { builder: 'webpack4' }; + export const core = coreVar; + ` + ) + ).toMatchInlineSnapshot(` + const coreVar = { + builder: "webpack5" + }; + export const core = coreVar; + `); + }); + }); + describe('module exports', () => { + it('missing export', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + module.exports = { addons: [] }; + ` + ) + ).toMatchInlineSnapshot(` + module.exports = { + addons: [], + core: { + builder: "webpack5" + } + }; + `); + }); + it('missing field', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + module.exports = { core: { foo: 'bar' }}; + ` + ) + ).toMatchInlineSnapshot(` + module.exports = { + core: { + foo: 'bar', + builder: "webpack5" + } + }; + `); + }); + it('found scalar', () => { + expect( + setField( + ['core', 'builder'], + 'webpack5', + dedent` + module.exports = { core: { builder: 'webpack4' } }; + ` + ) + ).toMatchInlineSnapshot(` + module.exports = { + core: { + builder: "webpack5" + } + }; + `); + }); + }); + }); +}); diff --git a/lib/csf-tools/src/ConfigFile.ts b/lib/csf-tools/src/ConfigFile.ts new file mode 100644 index 00000000000..fa7b84a2dd9 --- /dev/null +++ b/lib/csf-tools/src/ConfigFile.ts @@ -0,0 +1,228 @@ +/* eslint-disable no-underscore-dangle */ +import fs from 'fs-extra'; +import * as t from '@babel/types'; +import generate from '@babel/generator'; +import traverse from '@babel/traverse'; +import { babelParse } from './babelParse'; + +const logger = console; + +const _getPath = (path: string[], node: t.Node): t.Node | undefined => { + if (path.length === 0) { + return node; + } + if (t.isObjectExpression(node)) { + const [first, ...rest] = path; + const field = node.properties.find((p: t.ObjectProperty) => { + return t.isIdentifier(p.key) && p.key.name === first; + }); + if (field) { + return _getPath(rest, (field as t.ObjectProperty).value); + } + } + return undefined; +}; + +const _findVarInitialization = (identifier: string, program: t.Program) => { + let init: t.Expression = null; + let declarations: t.VariableDeclarator[] = null; + program.body.find((node: t.Node) => { + if (t.isVariableDeclaration(node)) { + declarations = node.declarations; + } else if (t.isExportNamedDeclaration(node) && t.isVariableDeclaration(node.declaration)) { + declarations = node.declaration.declarations; + } + + return ( + declarations && + declarations.find((decl: t.Node) => { + if ( + t.isVariableDeclarator(decl) && + t.isIdentifier(decl.id) && + decl.id.name === identifier + ) { + init = decl.init; + return true; // stop looking + } + return false; + }) + ); + }); + return init; +}; + +const _makeObjectExpression = (path: string[], value: t.Expression): t.Expression => { + if (path.length === 0) return value; + const [first, ...rest] = path; + const innerExpression = _makeObjectExpression(rest, value); + return t.objectExpression([t.objectProperty(t.identifier(first), innerExpression)]); +}; + +const _updateExportNode = (path: string[], expr: t.Expression, existing: t.ObjectExpression) => { + const [first, ...rest] = path; + const existingField = existing.properties.find((p: t.ObjectProperty) => { + return t.isIdentifier(p.key) && p.key.name === first; + }) as t.ObjectProperty; + if (!existingField) { + existing.properties.push( + t.objectProperty(t.identifier(first), _makeObjectExpression(rest, expr)) + ); + } else if (t.isObjectExpression(existingField.value) && rest.length > 0) { + _updateExportNode(rest, expr, existingField.value); + } else { + existingField.value = _makeObjectExpression(rest, expr); + } +}; + +export class ConfigFile { + _ast: t.File; + + _exports: Record = {}; + + _exportsObject: t.ObjectExpression; + + fileName?: string; + + constructor(ast: t.File, fileName?: string) { + this._ast = ast; + this.fileName = fileName; + } + + parse() { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const self = this; + traverse(this._ast, { + ExportNamedDeclaration: { + enter({ node, parent }) { + if (t.isVariableDeclaration(node.declaration)) { + // export const X = ...; + node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl) && t.isIdentifier(decl.id)) { + const { name: exportName } = decl.id; + let exportVal = decl.init; + if (t.isIdentifier(exportVal)) { + exportVal = _findVarInitialization(exportVal.name, parent as t.Program); + } + self._exports[exportName] = exportVal; + } + }); + } else { + logger.warn(`Unexpected ${node}`); + } + }, + }, + ExpressionStatement: { + enter({ node, parent }) { + if (t.isAssignmentExpression(node.expression) && node.expression.operator === '=') { + const { left, right } = node.expression; + if ( + t.isMemberExpression(left) && + t.isIdentifier(left.object) && + left.object.name === 'module' && + t.isIdentifier(left.property) && + left.property.name === 'exports' + ) { + if (t.isObjectExpression(right)) { + self._exportsObject = right; + right.properties.forEach((p: t.ObjectProperty) => { + if (t.isIdentifier(p.key)) { + let exportVal = p.value; + if (t.isIdentifier(exportVal)) { + exportVal = _findVarInitialization(exportVal.name, parent as t.Program); + } + self._exports[p.key.name] = exportVal as t.Expression; + } + }); + } else { + logger.warn(`Unexpected ${node}`); + } + } + } + }, + }, + }); + return self; + } + + getFieldNode(path: string[]) { + const [root, ...rest] = path; + const exported = this._exports[root]; + if (!exported) return undefined; + return _getPath(rest, exported); + } + + getFieldValue(path: string[]) { + const node = this.getFieldNode(path); + if (node) { + const { code } = generate(node, {}); + // eslint-disable-next-line no-eval + const value = eval(`(() => (${code}))()`); + return value; + } + return undefined; + } + + setFieldNode(path: string[], expr: t.Expression) { + const [first, ...rest] = path; + const exportNode = this._exports[first]; + if (this._exportsObject) { + _updateExportNode(path, expr, this._exportsObject); + this._exports[path[0]] = expr; + } else if (exportNode && t.isObjectExpression(exportNode) && rest.length > 0) { + _updateExportNode(rest, expr, exportNode); + } else { + // create a new named export and add it to the top level + const exportObj = _makeObjectExpression(rest, expr); + const newExport = t.exportNamedDeclaration( + t.variableDeclaration('const', [t.variableDeclarator(t.identifier(first), exportObj)]) + ); + this._exports[first] = exportObj; + this._ast.program.body.push(newExport); + } + } + + setFieldValue(path: string[], value: any) { + const stringified = JSON.stringify(value); + const program = babelParse(`const __x = ${stringified}`); + let valueNode; + traverse(program, { + VariableDeclaration: { + enter({ node }) { + if ( + node.declarations.length === 1 && + t.isVariableDeclarator(node.declarations[0]) && + t.isIdentifier(node.declarations[0].id) && + node.declarations[0].id.name === '__x' + ) { + valueNode = node.declarations[0].init; + } + }, + }, + }); + if (!valueNode) { + throw new Error(`Unexpected value ${value}`); + } + this.setFieldNode(path, valueNode); + } +} + +export const loadConfig = (code: string, fileName?: string) => { + const ast = babelParse(code); + return new ConfigFile(ast, fileName); +}; + +export const formatConfig = (config: ConfigFile) => { + const { code } = generate(config._ast, {}); + return code; +}; + +export const readConfig = async (fileName: string) => { + const code = (await fs.readFile(fileName, 'utf-8')).toString(); + return loadConfig(code, fileName).parse(); +}; + +export const writeConfig = async (config: ConfigFile, fileName?: string) => { + const fname = fileName || config.fileName; + if (!fname) throw new Error('Please specify a fileName for writeConfig'); + await fs.writeFile(fname, await formatConfig(config)); +}; diff --git a/lib/csf-tools/src/index.ts b/lib/csf-tools/src/index.ts index 010eab70efb..d7eeba07560 100644 --- a/lib/csf-tools/src/index.ts +++ b/lib/csf-tools/src/index.ts @@ -13,4 +13,5 @@ export const readCsfOrMdx = async (fileName: string, options: CsfOptions) => { }; export * from './CsfFile'; +export * from './ConfigFile'; export * from './getStorySortParameter'; From 0b59b9952caccfa2a9ca512b6f8992bded3f461b Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:07:12 +0800 Subject: [PATCH 03/18] CLI: Add fix command --- lib/cli/src/fix/fixes/index.ts | 4 +++ lib/cli/src/fix/index.ts | 52 ++++++++++++++++++++++++++++++++++ lib/cli/src/fix/types.ts | 18 ++++++++++++ lib/cli/src/generate.ts | 12 ++++++++ 4 files changed, 86 insertions(+) create mode 100644 lib/cli/src/fix/fixes/index.ts create mode 100644 lib/cli/src/fix/index.ts create mode 100644 lib/cli/src/fix/types.ts diff --git a/lib/cli/src/fix/fixes/index.ts b/lib/cli/src/fix/fixes/index.ts new file mode 100644 index 00000000000..9b316d6b74f --- /dev/null +++ b/lib/cli/src/fix/fixes/index.ts @@ -0,0 +1,4 @@ +import { Fix } from '../types'; + +export * from '../types'; +export const fixes: Fix[] = []; diff --git a/lib/cli/src/fix/index.ts b/lib/cli/src/fix/index.ts new file mode 100644 index 00000000000..41cb6af1fae --- /dev/null +++ b/lib/cli/src/fix/index.ts @@ -0,0 +1,52 @@ +/* eslint-disable no-await-in-loop */ +import prompts from 'prompts'; +import chalk from 'chalk'; +import boxen from 'boxen'; +import { JsPackageManagerFactory } from '../js-package-manager'; + +import { fixes, Fix } from './fixes'; + +const logger = console; + +interface FixOptions { + fixId?: string; + yes?: boolean; + dryRun?: boolean; +} + +export const fix = async ({ fixId, dryRun, yes }: FixOptions) => { + const packageManager = JsPackageManagerFactory.getPackageManager(); + const filtered = fixId ? fixes.filter((f) => f.id === fixId) : fixes; + + for (let i = 0; i < filtered.length; i += 1) { + const f = fixes[i] as Fix; + logger.info(`🔎 checking '${chalk.cyan(f.id)}'`); + const result = await f.check({ packageManager }); + if (result) { + const message = f.prompt(result); + + logger.info( + boxen(message, { borderStyle: 'round', padding: 1, borderColor: '#F1618C' } as any) + ); + + const runAnswer = + yes || dryRun + ? { fix: false } + : await prompts([ + { + type: 'confirm', + name: 'fix', + message: `Do you want to run fix '${chalk.cyan(f.id)}' on your project?`, + }, + ]); + + if (runAnswer.fix) { + await f.run({ result, packageManager, dryRun }); + } else { + logger.info(`Skipping the ${chalk.cyan(f.id)} fix.`); + logger.info(); + logger.info(`If you change your mind, just run '${chalk.cyan('npx sb@next fix')}'`); + } + } + } +}; diff --git a/lib/cli/src/fix/types.ts b/lib/cli/src/fix/types.ts new file mode 100644 index 00000000000..ff812f7064d --- /dev/null +++ b/lib/cli/src/fix/types.ts @@ -0,0 +1,18 @@ +import { JsPackageManager } from '../js-package-manager'; + +export interface CheckOptions { + packageManager: JsPackageManager; +} + +export interface RunOptions { + packageManager: JsPackageManager; + result: ResultType; + dryRun?: boolean; +} + +export interface Fix { + id: string; + check: (options: CheckOptions) => Promise; + prompt: (result: ResultType) => string; + run: (options: RunOptions) => Promise; +} diff --git a/lib/cli/src/generate.ts b/lib/cli/src/generate.ts index b514be300d9..7a91e53fcc3 100644 --- a/lib/cli/src/generate.ts +++ b/lib/cli/src/generate.ts @@ -11,6 +11,7 @@ import { extract } from './extract'; import { upgrade } from './upgrade'; import { repro } from './repro'; import { link } from './link'; +import { fix } from './fix'; import { generateStorybookBabelConfigInCWD } from './babel-config'; const pkg = sync({ cwd: __dirname }).packageJson; @@ -126,6 +127,17 @@ program }) ); +program + .command('fix [fixId]') + .description('Check storybook for known problems or migrations and apply fixes') + .option('-y --yes', 'Skip prompting the user') + .action((fixId, options) => + fix({ fixId, ...options }).catch((e) => { + logger.error(e); + process.exit(1); + }) + ); + program.on('command:*', ([invalidCmd]) => { logger.error(' Invalid command: %s.\n See --help for a list of available commands.', invalidCmd); // eslint-disable-next-line From ef27483dd7428c8abf72ebf37833d71875f043a1 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:08:40 +0800 Subject: [PATCH 04/18] CLI: Add webpack5 fix --- lib/cli/package.json | 1 + lib/cli/src/fix/fixes/index.ts | 3 +- lib/cli/src/fix/fixes/webpack5.ts | 118 ++++++++++++++++++ .../helpers/getStorybookConfiguration.test.ts | 28 +++++ .../fix/helpers/getStorybookConfiguration.ts | 21 ++++ lib/cli/src/fix/helpers/getStorybookInfo.ts | 97 ++++++++++++++ 6 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 lib/cli/src/fix/fixes/webpack5.ts create mode 100644 lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts create mode 100644 lib/cli/src/fix/helpers/getStorybookConfiguration.ts create mode 100644 lib/cli/src/fix/helpers/getStorybookInfo.ts diff --git a/lib/cli/package.json b/lib/cli/package.json index b83c63b9f88..f2b7e429ef5 100644 --- a/lib/cli/package.json +++ b/lib/cli/package.json @@ -50,6 +50,7 @@ "@babel/preset-env": "^7.12.11", "@storybook/codemod": "6.4.0-beta.1", "@storybook/core-common": "6.4.0-beta.1", + "@storybook/csf-tools": "6.4.0-beta.1", "@storybook/node-logger": "6.4.0-beta.1", "@storybook/semver": "^7.3.2", "boxen": "^4.2.0", diff --git a/lib/cli/src/fix/fixes/index.ts b/lib/cli/src/fix/fixes/index.ts index 9b316d6b74f..c522f534b3a 100644 --- a/lib/cli/src/fix/fixes/index.ts +++ b/lib/cli/src/fix/fixes/index.ts @@ -1,4 +1,5 @@ +import { webpack5 } from './webpack5'; import { Fix } from '../types'; export * from '../types'; -export const fixes: Fix[] = []; +export const fixes: Fix[] = [webpack5]; diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/fix/fixes/webpack5.ts new file mode 100644 index 00000000000..612e857d207 --- /dev/null +++ b/lib/cli/src/fix/fixes/webpack5.ts @@ -0,0 +1,118 @@ +import chalk from 'chalk'; +import dedent from 'ts-dedent'; +import semver from '@storybook/semver'; +import { ConfigFile, readConfig, writeConfig } from '@storybook/csf-tools'; +import { Fix } from '../types'; +import { getStorybookInfo } from '../helpers/getStorybookInfo'; +import { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; + +const logger = console; + +/** + * Is the user using webpack5 in their project? + * + * If the user is using a version of SB >= 6.3, + * prompt them to upgrade to webpack5. + * + * - Add manager-webpack5 builder-webpack5 as dev dependencies + * - Add core.builder = 'webpack5' to main.js + * - Add 'webpack5' as a project dependency + */ + +interface Webpack5RunOptions { + webpackVersion: string; + storybookVersion: string; + main: ConfigFile; +} + +interface CheckBuilder { + checkWebpack5Builder: ( + packageJson: PackageJsonWithDepsAndDevDeps + ) => Promise<{ storybookVersion: string; main: ConfigFile }>; +} + +export const webpack5: Fix & CheckBuilder = { + id: 'webpack5', + + async checkWebpack5Builder(packageJson: PackageJsonWithDepsAndDevDeps) { + const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); + const storybookCoerced = semver.coerce(storybookVersion).version; + if (!storybookCoerced) { + logger.warn(`Unable to determine storybook version, skipping webpack5 fix.`); + return null; + } + + if (semver.lte(storybookCoerced, '6.3.0')) { + logger.warn( + 'Detected SB 6.3 or below, please upgrade storybook if you want to use webpack5.' + ); + 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; + + const webpackVersion = dependencies.webpack || devDependencies.webpack; + const webpackCoerced = semver.coerce(webpackVersion)?.version; + + if ( + !webpackCoerced || + semver.gte(webpackCoerced, '5.0.0') || + semver.lt(webpackCoerced, '6.0.0') + ) + return null; + + const builderInfo = await this.checkWebpack5Builder(packageJson); + return builderInfo ? { webpackVersion, ...builderInfo } : null; + }, + + prompt({ webpackVersion, storybookVersion }) { + const webpackFormatted = chalk.cyan(`webpack ${webpackVersion}`); + const sbFormatted = chalk.cyan(`Storybook ${storybookVersion}`); + + return dedent` + We've detected you're running ${webpackFormatted}. + ${sbFormatted} runs webpack4 by default, which may not be compatible. + + To run Storybook in webpack5-mode, we can install Storybook's ${chalk.cyan( + 'webpack5 builder' + )} for you. + `; + }, + + async run({ result: { main, storybookVersion, webpackVersion }, packageManager, dryRun }) { + if (dryRun) return; + const deps = [ + `@storybook/manager-webpack5@${storybookVersion}`, + `@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 + if (!webpackVersion) { + deps.push('webpack@5'); + } + packageManager.addDependencies({ installAsDevDependencies: true }, deps); + main.setFieldValue(['core', 'builder'], 'webpack5'); + await writeConfig(main); + }, +}; diff --git a/lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts b/lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts new file mode 100644 index 00000000000..47c57eeba42 --- /dev/null +++ b/lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts @@ -0,0 +1,28 @@ +import { getStorybookConfiguration } from './getStorybookConfiguration'; + +describe('getStorybookConfiguration', () => { + it('handles short names', () => { + const port = getStorybookConfiguration('start-storybook -p 9001', '-p', '--port'); + expect(port).toBe('9001'); + }); + it('handles long names', () => { + const port = getStorybookConfiguration('start-storybook --port 9001', '-p', '--port'); + expect(port).toBe('9001'); + }); + it('handles equals', () => { + const port = getStorybookConfiguration('start-storybook --port=9001', '-p', '--port'); + expect(port).toBe('9001'); + }); + it('handles double space', () => { + const port = getStorybookConfiguration('start-storybook --port 9001', '-p', '--port'); + expect(port).toBe('9001'); + }); + it('handles complex scripts', () => { + const port = getStorybookConfiguration( + "node verify-node-version.js && concurrently --raw --kill-others 'yarn relay --watch' 'start-storybook -s ./public -p 9001'", + '-p', + '--port' + ); + expect(port).toBe('9001'); + }); +}); diff --git a/lib/cli/src/fix/helpers/getStorybookConfiguration.ts b/lib/cli/src/fix/helpers/getStorybookConfiguration.ts new file mode 100644 index 00000000000..98529af472b --- /dev/null +++ b/lib/cli/src/fix/helpers/getStorybookConfiguration.ts @@ -0,0 +1,21 @@ +/* + * Lifted from chromatic-cli + * + * This is not exactly clever but it works most of the time + * we receive the full text of the npm script, and we look if we can find the cli flag + */ +export function getStorybookConfiguration( + storybookScript: string, + shortName: string, + longName: string +) { + const parts = storybookScript.split(/[\s='"]+/); + let index = parts.indexOf(longName); + if (index === -1) { + index = parts.indexOf(shortName); + } + if (index === -1) { + return null; + } + return parts[index + 1]; +} diff --git a/lib/cli/src/fix/helpers/getStorybookInfo.ts b/lib/cli/src/fix/helpers/getStorybookInfo.ts new file mode 100644 index 00000000000..ef6b3efd9d0 --- /dev/null +++ b/lib/cli/src/fix/helpers/getStorybookInfo.ts @@ -0,0 +1,97 @@ +import path from 'path'; +import fse from 'fs-extra'; +import { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; +import { getStorybookConfiguration } from './getStorybookConfiguration'; + +interface StorybookInfo { + framework: string; + version: string; + configDir?: string; + mainConfig?: string; + previewConfig?: string; + managerConfig?: string; +} + +const viewLayers: Record = { + '@storybook/react': 'react', + '@storybook/vue': 'vue', + '@storybook/vue3': 'vue3', + '@storybook/angular': 'angular', + '@storybook/html': 'html', + '@storybook/web-components': 'web-components', + '@storybook/polymer': 'polymer', + '@storybook/ember': 'ember', + '@storybook/marko': 'marko', + '@storybook/mithril': 'mithril', + '@storybook/riot': 'riot', + '@storybook/svelte': 'svelte', + '@storybook/preact': 'preact', + '@storybook/rax': 'rax', +}; + +const logger = console; + +const findDependency = ( + { dependencies, devDependencies, peerDependencies }: PackageJsonWithDepsAndDevDeps, + predicate: (entry: [string, string]) => string +) => [ + Object.entries(dependencies || {}).find(predicate), + Object.entries(devDependencies || {}).find(predicate), + Object.entries(peerDependencies || {}).find(predicate), +]; + +const getFrameworkInfo = (packageJson: PackageJsonWithDepsAndDevDeps) => { + // Pull the viewlayer from dependencies in package.json + const [dep, devDep, peerDep] = findDependency(packageJson, ([key]) => viewLayers[key]); + const [pkg, version] = dep || devDep || peerDep || []; + const framework = viewLayers[pkg]; + + if (dep && devDep && dep[0] === devDep[0]) { + logger.warn( + `Found "${dep[0]}" in both "dependencies" and "devDependencies". This is probably a mistake.` + ); + } + if (dep && peerDep && dep[0] === peerDep[0]) { + logger.warn( + `Found "${dep[0]}" in both "dependencies" and "peerDependencies". This is probably a mistake.` + ); + } + + return { framework, version }; +}; + +const validConfigExtensions = ['ts', 'js', 'tsx', 'jsx', 'mjs', 'cjs']; + +const findConfigFile = (prefix: string, configDir: string) => { + const filePrefix = path.join(configDir, prefix); + const extension = validConfigExtensions.find((ext: string) => + fse.existsSync(`${filePrefix}.${ext}`) + ); + return extension ? `${filePrefix}.${extension}` : null; +}; + +const getConfigInfo = (packageJson: PackageJsonWithDepsAndDevDeps) => { + let configDir = '.storybook'; + const storybookScript = packageJson.scripts?.storybook; + if (storybookScript) { + const configParam = getStorybookConfiguration(storybookScript, '-c', '--config-dir'); + if (configParam) configDir = configParam; + } + + return { + configDir, + mainConfig: findConfigFile('main', configDir), + previewConfig: findConfigFile('preview', configDir), + managerConfig: findConfigFile('manager', configDir), + }; +}; + +export const getStorybookInfo = (packageJson: PackageJsonWithDepsAndDevDeps) => { + const frameworkInfo = getFrameworkInfo(packageJson); + const configInfo = getConfigInfo(packageJson); + + return { + ...frameworkInfo, + ...configInfo, + } as StorybookInfo; +}; From 42080cd825d0776c88389e8b27c6e5209c781cb2 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:09:30 +0800 Subject: [PATCH 05/18] CLI: Add cra5 fix --- lib/cli/src/fix/fixes/cra5.ts | 57 ++++++++++++++++++++++++++++++++++ lib/cli/src/fix/fixes/index.ts | 3 +- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 lib/cli/src/fix/fixes/cra5.ts diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/fix/fixes/cra5.ts new file mode 100644 index 00000000000..592325772a2 --- /dev/null +++ b/lib/cli/src/fix/fixes/cra5.ts @@ -0,0 +1,57 @@ +import chalk from 'chalk'; +import dedent from 'ts-dedent'; +import semver from '@storybook/semver'; +import { ConfigFile } from '@storybook/csf-tools'; +import { Fix } from '../types'; +import { webpack5 } from './webpack5'; + +/** + * Is the user upgrading from CRA4 to CRA5? + * + * If so: + * - Run upgrades/webpack5 + */ +interface CRA5RunOptions { + craVersion: string; + // FIXME craPresetVersion: string; + storybookVersion: string; + main: ConfigFile; +} + +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']; + const craCoerced = semver.coerce(craVersion)?.version; + + if (semver.lt(craCoerced, '5.0.0')) { + return null; + } + + const builderInfo = await webpack5.checkWebpack5Builder(packageJson); + return builderInfo ? { craVersion, ...builderInfo } : null; + }, + + prompt({ craVersion, storybookVersion }) { + const craFormatted = chalk.cyan(`Create React App (CRA) ${craVersion}`); + const sbFormatted = chalk.cyan(`Storybook ${storybookVersion}`); + return dedent` + We've detected you are running ${craFormatted} which is powered by webpack5. + ${sbFormatted} runs webpack4 by default, which is incompatible. + + In order to work with your version of CRA, we need to install Storybook's ${chalk.cyan( + 'webpack5 builder' + )}. + `; + }, + + async run(options) { + return webpack5.run({ + ...options, + result: { webpackVersion: null, ...options.result }, + }); + }, +}; diff --git a/lib/cli/src/fix/fixes/index.ts b/lib/cli/src/fix/fixes/index.ts index c522f534b3a..e3095d2f172 100644 --- a/lib/cli/src/fix/fixes/index.ts +++ b/lib/cli/src/fix/fixes/index.ts @@ -1,5 +1,6 @@ +import { cra5 } from './cra5'; import { webpack5 } from './webpack5'; import { Fix } from '../types'; export * from '../types'; -export const fixes: Fix[] = [webpack5]; +export const fixes: Fix[] = [cra5, webpack5]; From c3a69ec14354320f299ba21ff662e6b8f5f3967c Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:30:31 +0800 Subject: [PATCH 06/18] Add links --- MIGRATION.md | 32 +++++++++++++++++++++++++++++-- lib/cli/src/fix/fixes/cra5.ts | 4 ++++ lib/cli/src/fix/fixes/webpack5.ts | 4 ++++ lib/cli/src/fix/index.ts | 1 + 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index b036b68f14c..01bc6192bc1 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,6 +1,7 @@

Migration

- [From version 6.3.x to 6.4.0](#from-version-63x-to-640) + - [CRA5 upgrade](#cra5-upgrade) - [CSF3 enabled](#csf3-enabled) - [Story Store v7](#story-store-v7) - [Behavioral differences](#behavioral-differences) @@ -173,6 +174,33 @@ ## From version 6.3.x to 6.4.0 +### CRA5 upgrade + +Storybook 6.3 supports CRA5 out of the box when you install it fresh. However, if you're upgrading your project from a previous version, you'll need to +upgrade the configuration. You can do this automatically by running: + +``` +npx sb@next fix +``` + +Or you can do the following steps manually to force Storybook to use webpack 5 for building your project: + +```shell +yarn add @storybook/builder-webpack5@next @storybook/manager-webpack5 --dev +# Or +npm install @storybook/builder-webpack5@next @storybook/manager-webpack5 --save-dev +``` + +Then edit your `.storybook/main.js` config: + +```js +module.exports = { + core: { + builder: 'webpack5', + }, +}; +``` + ### CSF3 enabled SB6.3 introduced a feature flag, `features.previewCsfV3`, to opt-in to experimental [CSF3 syntax support](https://storybook.js.org/blog/component-story-format-3-0/). In SB6.4, CSF3 is supported regardless of `previewCsfV3`'s value. This should be a fully backwards-compatible change. The `previewCsfV3` flag has been deprecated and will be removed in SB7.0. @@ -330,9 +358,9 @@ npm install webpack@5 --save-dev Storybook 6.3 supports Angular 12 out of the box when you install it fresh. However, if you're upgrading your project from a previous version, you'll need to do the following steps to force Storybook to use webpack 5 for building your project: ```shell -yarn add @storybook/builder-webpack5@next @storybook/manager-webpack5@next --dev +yarn add @storybook/builder-webpack5@next @storybook/manager-webpack5 --dev # Or -npm install @storybook/builder-webpack5@next @storybook/manager-webpack5@next --save-dev +npm install @storybook/builder-webpack5@next @storybook/manager-webpack5 --save-dev ``` Then edit your `.storybook/main.js` config: diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/fix/fixes/cra5.ts index 592325772a2..6b478003d15 100644 --- a/lib/cli/src/fix/fixes/cra5.ts +++ b/lib/cli/src/fix/fixes/cra5.ts @@ -45,6 +45,10 @@ export const cra5: Fix = { In order to work with your version of CRA, we need to install Storybook's ${chalk.cyan( 'webpack5 builder' )}. + + See also ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#cra5-upgrade' + )} `; }, diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/fix/fixes/webpack5.ts index 612e857d207..a226f1f9f64 100644 --- a/lib/cli/src/fix/fixes/webpack5.ts +++ b/lib/cli/src/fix/fixes/webpack5.ts @@ -97,6 +97,10 @@ export const webpack5: Fix & CheckBuilder = { To run Storybook in webpack5-mode, we can install Storybook's ${chalk.cyan( 'webpack5 builder' )} for you. + + See also ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#webpack-5-manager-build' + )} `; }, diff --git a/lib/cli/src/fix/index.ts b/lib/cli/src/fix/index.ts index 41cb6af1fae..0090f1cca30 100644 --- a/lib/cli/src/fix/index.ts +++ b/lib/cli/src/fix/index.ts @@ -42,6 +42,7 @@ export const fix = async ({ fixId, dryRun, yes }: FixOptions) => { if (runAnswer.fix) { await f.run({ result, packageManager, dryRun }); + logger.info(`✅ fixed ${chalk.cyan(f.id)}`); } else { logger.info(`Skipping the ${chalk.cyan(f.id)} fix.`); logger.info(); From ffb254bd9dccbfa2e1eec812735baa9af3f760ed Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 02:45:07 +0800 Subject: [PATCH 07/18] Update yarn.lock --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index de8f906f5f9..740dd1d071d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7804,6 +7804,7 @@ __metadata: "@storybook/client-api": 6.4.0-beta.1 "@storybook/codemod": 6.4.0-beta.1 "@storybook/core-common": 6.4.0-beta.1 + "@storybook/csf-tools": 6.4.0-beta.1 "@storybook/node-logger": 6.4.0-beta.1 "@storybook/semver": ^7.3.2 "@types/cross-spawn": ^6.0.2 From 05cd8c3a303789e05a53e4818c577464622abd40 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 16:25:33 +0800 Subject: [PATCH 08/18] CLI: Tests and bugfixes for webpack5 fix --- __mocks__/fs-extra.js | 29 ++++++ lib/cli/src/fix/fixes/webpack5.test.ts | 121 +++++++++++++++++++++++++ lib/cli/src/fix/fixes/webpack5.ts | 8 +- lib/csf-tools/src/ConfigFile.ts | 21 +++-- 4 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 __mocks__/fs-extra.js create mode 100644 lib/cli/src/fix/fixes/webpack5.test.ts diff --git a/__mocks__/fs-extra.js b/__mocks__/fs-extra.js new file mode 100644 index 00000000000..349ffb2b4c7 --- /dev/null +++ b/__mocks__/fs-extra.js @@ -0,0 +1,29 @@ +const fs = jest.createMockFromModule('fs-extra'); + +// This is a custom function that our tests can use during setup to specify +// what the files on the "mock" filesystem should look like when any of the +// `fs` APIs are used. +let mockFiles = Object.create(null); + +// eslint-disable-next-line no-underscore-dangle +function __setMockFiles(newMockFiles) { + mockFiles = newMockFiles; +} + +// A custom version of `readdirSync` that reads from the special mocked out +// file list set via __setMockFiles +const readFile = async (filePath) => mockFiles[filePath]; +const readFileSync = (filePath = '') => mockFiles[filePath]; +const existsSync = (filePath) => !!mockFiles[filePath]; +const lstatSync = (filePath) => ({ + isFile: () => !!mockFiles[filePath], +}); + +// eslint-disable-next-line no-underscore-dangle +fs.__setMockFiles = __setMockFiles; +fs.readFile = readFile; +fs.readFileSync = readFileSync; +fs.existsSync = existsSync; +fs.lstatSync = lstatSync; + +module.exports = fs; diff --git a/lib/cli/src/fix/fixes/webpack5.test.ts b/lib/cli/src/fix/fixes/webpack5.test.ts new file mode 100644 index 00000000000..c2677c427b6 --- /dev/null +++ b/lib/cli/src/fix/fixes/webpack5.test.ts @@ -0,0 +1,121 @@ +/* eslint-disable no-underscore-dangle */ +import { JsPackageManager } 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')); + +const checkWebpack5 = async ({ packageJson, main }) => { + // eslint-disable-next-line global-require + require('fs-extra').__setMockFiles({ + '.storybook/main.js': `module.exports = ${JSON.stringify(main)};`, + }); + const packageManager = { + retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + } as JsPackageManager; + return webpack5.check({ packageManager }); +}; + +describe('webpack5 fix', () => { + describe('sb < 6.3', () => { + describe('webpack5 dependency', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.2.0', webpack: '^5.0.0' } }; + it('should fail', async () => { + await expect( + checkWebpack5({ + packageJson, + main: {}, + }) + ).rejects.toThrow(); + }); + }); + describe('no webpack5 dependency', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 6.3 - 7.0', () => { + describe('webpack5 dependency', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.3.0', webpack: '^5.0.0' } }; + describe('webpack5 builder', () => { + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson, + main: { core: { builder: 'webpack5' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('custom builder', () => { + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson, + main: { core: { builder: 'storybook-builder-vite' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('no webpack5 builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkWebpack5({ + packageJson, + main: {}, + }) + ).resolves.toMatchObject({ + webpackVersion: '^5.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + }); + describe('no webpack dependency', () => { + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson: {}, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('webpack4 dependency', () => { + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson: { + dependencies: { + webpack: '4', + }, + }, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 7.0+', () => { + describe('webpack5 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^7.0.0-alpha.0', webpack: '^5.0.0' }, + }; + it('should no-op', async () => { + await expect( + checkWebpack5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); +}); diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/fix/fixes/webpack5.ts index a226f1f9f64..3d379f3e77a 100644 --- a/lib/cli/src/fix/fixes/webpack5.ts +++ b/lib/cli/src/fix/fixes/webpack5.ts @@ -36,13 +36,14 @@ export const webpack5: Fix & CheckBuilder = { async checkWebpack5Builder(packageJson: PackageJsonWithDepsAndDevDeps) { const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); + const storybookCoerced = semver.coerce(storybookVersion).version; if (!storybookCoerced) { logger.warn(`Unable to determine storybook version, skipping webpack5 fix.`); return null; } - if (semver.lte(storybookCoerced, '6.3.0')) { + if (semver.lt(storybookCoerced, '6.3.0')) { logger.warn( 'Detected SB 6.3 or below, please upgrade storybook if you want to use webpack5.' ); @@ -57,7 +58,6 @@ export const webpack5: Fix & CheckBuilder = { 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') { @@ -77,8 +77,8 @@ export const webpack5: Fix & CheckBuilder = { if ( !webpackCoerced || - semver.gte(webpackCoerced, '5.0.0') || - semver.lt(webpackCoerced, '6.0.0') + semver.lt(webpackCoerced, '5.0.0') || + semver.gte(webpackCoerced, '6.0.0') ) return null; diff --git a/lib/csf-tools/src/ConfigFile.ts b/lib/csf-tools/src/ConfigFile.ts index fa7b84a2dd9..9d45cc887c0 100644 --- a/lib/csf-tools/src/ConfigFile.ts +++ b/lib/csf-tools/src/ConfigFile.ts @@ -7,15 +7,19 @@ import { babelParse } from './babelParse'; const logger = console; +const propKey = (p: t.ObjectProperty) => { + if (t.isIdentifier(p.key)) return p.key.name; + if (t.isStringLiteral(p.key)) return p.key.value; + return null; +}; + const _getPath = (path: string[], node: t.Node): t.Node | undefined => { if (path.length === 0) { return node; } if (t.isObjectExpression(node)) { const [first, ...rest] = path; - const field = node.properties.find((p: t.ObjectProperty) => { - return t.isIdentifier(p.key) && p.key.name === first; - }); + const field = node.properties.find((p: t.ObjectProperty) => propKey(p) === first); if (field) { return _getPath(rest, (field as t.ObjectProperty).value); } @@ -60,9 +64,9 @@ const _makeObjectExpression = (path: string[], value: t.Expression): t.Expressio const _updateExportNode = (path: string[], expr: t.Expression, existing: t.ObjectExpression) => { const [first, ...rest] = path; - const existingField = existing.properties.find((p: t.ObjectProperty) => { - return t.isIdentifier(p.key) && p.key.name === first; - }) as t.ObjectProperty; + const existingField = existing.properties.find( + (p: t.ObjectProperty) => propKey(p) === first + ) as t.ObjectProperty; if (!existingField) { existing.properties.push( t.objectProperty(t.identifier(first), _makeObjectExpression(rest, expr)) @@ -125,12 +129,13 @@ export class ConfigFile { if (t.isObjectExpression(right)) { self._exportsObject = right; right.properties.forEach((p: t.ObjectProperty) => { - if (t.isIdentifier(p.key)) { + const exportName = propKey(p); + if (exportName) { let exportVal = p.value; if (t.isIdentifier(exportVal)) { exportVal = _findVarInitialization(exportVal.name, parent as t.Program); } - self._exports[p.key.name] = exportVal as t.Expression; + self._exports[exportName] = exportVal as t.Expression; } }); } else { From bfb16b7795c0b993f7f8fde05ffa8fbce24f7b5e Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 16:49:17 +0800 Subject: [PATCH 09/18] CLI: Tests and bugfix for cra5 fix --- lib/cli/src/fix/fixes/cra5.test.ts | 138 +++++++++++++++++++++++++ lib/cli/src/fix/fixes/cra5.ts | 2 +- lib/cli/src/fix/fixes/webpack5.test.ts | 15 ++- 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 lib/cli/src/fix/fixes/cra5.test.ts diff --git a/lib/cli/src/fix/fixes/cra5.test.ts b/lib/cli/src/fix/fixes/cra5.test.ts new file mode 100644 index 00000000000..15717948ff6 --- /dev/null +++ b/lib/cli/src/fix/fixes/cra5.test.ts @@ -0,0 +1,138 @@ +/* eslint-disable no-underscore-dangle */ +import { JsPackageManager } 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')); + +const checkCra5 = async ({ packageJson, main }) => { + // eslint-disable-next-line global-require + require('fs-extra').__setMockFiles({ + '.storybook/main.js': `module.exports = ${JSON.stringify(main)};`, + }); + const packageManager = { + retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + } as JsPackageManager; + return cra5.check({ packageManager }); +}; + +describe('cra5 fix', () => { + describe('sb < 6.3', () => { + describe('cra5 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^6.2.0', 'react-scripts': '^5.0.0' }, + }; + it('should fail', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).rejects.toThrow(); + }); + }); + describe('no cra5 dependency', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 6.3 - 7.0', () => { + describe('cra5 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^6.3.0', 'react-scripts': '^5.0.0' }, + }; + describe('webpack5 builder', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'webpack5' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('custom builder', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'storybook-builder-vite' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('webpack4 builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'webpack4' } }, + }) + ).resolves.toMatchObject({ + craVersion: '^5.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + describe('no builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toMatchObject({ + craVersion: '^5.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + }); + describe('no cra dependency', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson: {}, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('cra4 dependency', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson: { + dependencies: { + 'react-scripts': '4', + }, + }, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 7.0+', () => { + describe('cra5 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^7.0.0-alpha.0', 'react-scripts': '^5.0.0' }, + }; + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); +}); diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/fix/fixes/cra5.ts index 6b478003d15..65da0cd4078 100644 --- a/lib/cli/src/fix/fixes/cra5.ts +++ b/lib/cli/src/fix/fixes/cra5.ts @@ -27,7 +27,7 @@ export const cra5: Fix = { const craVersion = dependencies['react-scripts'] || devDependencies['react-scripts']; const craCoerced = semver.coerce(craVersion)?.version; - if (semver.lt(craCoerced, '5.0.0')) { + if (!craCoerced || semver.lt(craCoerced, '5.0.0')) { return null; } diff --git a/lib/cli/src/fix/fixes/webpack5.test.ts b/lib/cli/src/fix/fixes/webpack5.test.ts index c2677c427b6..7d5f519cfe6 100644 --- a/lib/cli/src/fix/fixes/webpack5.test.ts +++ b/lib/cli/src/fix/fixes/webpack5.test.ts @@ -64,7 +64,20 @@ describe('webpack5 fix', () => { ).resolves.toBeFalsy(); }); }); - describe('no webpack5 builder', () => { + describe('webpack4 builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkWebpack5({ + packageJson, + main: { core: { builder: 'webpack4' } }, + }) + ).resolves.toMatchObject({ + webpackVersion: '^5.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + describe('no builder', () => { it('should add webpack5 builder', async () => { await expect( checkWebpack5({ From 3e477078b2c607763f2f557a22ec0333c9985127 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 17:18:20 +0800 Subject: [PATCH 10/18] Fix review comments --- lib/cli/src/fix/fixes/cra5.ts | 4 ++-- lib/cli/src/fix/fixes/webpack5.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/fix/fixes/cra5.ts index 65da0cd4078..e4c5fe85a70 100644 --- a/lib/cli/src/fix/fixes/cra5.ts +++ b/lib/cli/src/fix/fixes/cra5.ts @@ -9,7 +9,7 @@ import { webpack5 } from './webpack5'; * Is the user upgrading from CRA4 to CRA5? * * If so: - * - Run upgrades/webpack5 + * - Run webpack5 fix */ interface CRA5RunOptions { craVersion: string; @@ -46,7 +46,7 @@ export const cra5: Fix = { 'webpack5 builder' )}. - See also ${chalk.yellow( + More info: ${chalk.yellow( 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#cra5-upgrade' )} `; diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/fix/fixes/webpack5.ts index 3d379f3e77a..3fe2e5f41de 100644 --- a/lib/cli/src/fix/fixes/webpack5.ts +++ b/lib/cli/src/fix/fixes/webpack5.ts @@ -45,7 +45,15 @@ export const webpack5: Fix & CheckBuilder = { if (semver.lt(storybookCoerced, '6.3.0')) { logger.warn( - 'Detected SB 6.3 or below, please upgrade storybook if you want to use webpack5.' + 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 sb upgrade')} + + Add the ${chalk.cyan('--prerelease')} flag to get the latest prerelease. + `.trim() ); return null; } @@ -98,7 +106,7 @@ export const webpack5: Fix & CheckBuilder = { 'webpack5 builder' )} for you. - See also ${chalk.yellow( + More info: ${chalk.yellow( 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#webpack-5-manager-build' )} `; From a619d5bd8b0596f8ec7ab4e2da09331ee9ad4461 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 17:24:51 +0800 Subject: [PATCH 11/18] More review fixes --- lib/cli/src/fix/fixes/cra5.ts | 12 ++++++------ lib/cli/src/fix/fixes/webpack5.ts | 21 ++++++++++----------- lib/cli/src/fix/index.ts | 4 ++-- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/fix/fixes/cra5.ts index e4c5fe85a70..74d7b567d4e 100644 --- a/lib/cli/src/fix/fixes/cra5.ts +++ b/lib/cli/src/fix/fixes/cra5.ts @@ -5,12 +5,6 @@ import { ConfigFile } from '@storybook/csf-tools'; import { Fix } from '../types'; import { webpack5 } from './webpack5'; -/** - * Is the user upgrading from CRA4 to CRA5? - * - * If so: - * - Run webpack5 fix - */ interface CRA5RunOptions { craVersion: string; // FIXME craPresetVersion: string; @@ -18,6 +12,12 @@ interface CRA5RunOptions { main: ConfigFile; } +/** + * Is the user upgrading from CRA4 to CRA5? + * + * If so: + * - Run webpack5 fix + */ export const cra5: Fix = { id: 'cra5', diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/fix/fixes/webpack5.ts index 3fe2e5f41de..c5a70307d3d 100644 --- a/lib/cli/src/fix/fixes/webpack5.ts +++ b/lib/cli/src/fix/fixes/webpack5.ts @@ -8,17 +8,6 @@ import { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; const logger = console; -/** - * Is the user using webpack5 in their project? - * - * If the user is using a version of SB >= 6.3, - * prompt them to upgrade to webpack5. - * - * - Add manager-webpack5 builder-webpack5 as dev dependencies - * - Add core.builder = 'webpack5' to main.js - * - Add 'webpack5' as a project dependency - */ - interface Webpack5RunOptions { webpackVersion: string; storybookVersion: string; @@ -31,6 +20,16 @@ interface CheckBuilder { ) => Promise<{ storybookVersion: string; main: ConfigFile }>; } +/** + * Is the user using webpack5 in their project? + * + * If the user is using a version of SB >= 6.3, + * prompt them to upgrade to webpack5. + * + * - Add manager-webpack5 builder-webpack5 as dev dependencies + * - Add core.builder = 'webpack5' to main.js + * - Add 'webpack5' as a project dependency + */ export const webpack5: Fix & CheckBuilder = { id: 'webpack5', diff --git a/lib/cli/src/fix/index.ts b/lib/cli/src/fix/index.ts index 0090f1cca30..81fdd1759d2 100644 --- a/lib/cli/src/fix/index.ts +++ b/lib/cli/src/fix/index.ts @@ -36,7 +36,7 @@ export const fix = async ({ fixId, dryRun, yes }: FixOptions) => { { type: 'confirm', name: 'fix', - message: `Do you want to run fix '${chalk.cyan(f.id)}' on your project?`, + message: `Do you want to run the '${chalk.cyan(f.id)}' fix on your project?`, }, ]); @@ -46,7 +46,7 @@ export const fix = async ({ fixId, dryRun, yes }: FixOptions) => { } else { logger.info(`Skipping the ${chalk.cyan(f.id)} fix.`); logger.info(); - logger.info(`If you change your mind, just run '${chalk.cyan('npx sb@next fix')}'`); + logger.info(`If you change your mind, run '${chalk.cyan('npx sb@next fix')}'`); } } } From 6410d97b9d1d38565145ad865c02089045cafbec Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 30 Sep 2021 17:25:17 +0800 Subject: [PATCH 12/18] Add angular12 fix --- lib/cli/src/fix/fixes/angular12.test.ts | 138 ++++++++++++++++++++++++ lib/cli/src/fix/fixes/angular12.ts | 61 +++++++++++ 2 files changed, 199 insertions(+) create mode 100644 lib/cli/src/fix/fixes/angular12.test.ts create mode 100644 lib/cli/src/fix/fixes/angular12.ts diff --git a/lib/cli/src/fix/fixes/angular12.test.ts b/lib/cli/src/fix/fixes/angular12.test.ts new file mode 100644 index 00000000000..6a962667493 --- /dev/null +++ b/lib/cli/src/fix/fixes/angular12.test.ts @@ -0,0 +1,138 @@ +/* eslint-disable no-underscore-dangle */ +import { JsPackageManager } from '../../js-package-manager'; +import { angular12 } from './angular12'; + +// eslint-disable-next-line global-require, jest/no-mocks-import +jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); + +const checkCra5 = async ({ packageJson, main }) => { + // eslint-disable-next-line global-require + require('fs-extra').__setMockFiles({ + '.storybook/main.js': `module.exports = ${JSON.stringify(main)};`, + }); + const packageManager = { + retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + } as JsPackageManager; + return angular12.check({ packageManager }); +}; + +describe('angular12 fix', () => { + describe('sb < 6.3', () => { + describe('angular12 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^6.2.0', '@angular/core': '^12.0.0' }, + }; + it('should fail', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).rejects.toThrow(); + }); + }); + describe('no angular dependency', () => { + const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 6.3 - 7.0', () => { + describe('angular12 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^6.3.0', '@angular/core': '^12.0.0' }, + }; + describe('webpack5 builder', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'webpack5' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('custom builder', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'storybook-builder-vite' } }, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('webpack4 builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkCra5({ + packageJson, + main: { core: { builder: 'webpack4' } }, + }) + ).resolves.toMatchObject({ + angularVersion: '^12.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + describe('no builder', () => { + it('should add webpack5 builder', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toMatchObject({ + angularVersion: '^12.0.0', + storybookVersion: '^6.3.0', + }); + }); + }); + }); + describe('no angular dependency', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson: {}, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + describe('angular11 dependency', () => { + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson: { + dependencies: { + '@angular/core': '11', + }, + }, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); + describe('sb 7.0+', () => { + describe('angular12 dependency', () => { + const packageJson = { + dependencies: { '@storybook/react': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, + }; + it('should no-op', async () => { + await expect( + checkCra5({ + packageJson, + main: {}, + }) + ).resolves.toBeFalsy(); + }); + }); + }); +}); diff --git a/lib/cli/src/fix/fixes/angular12.ts b/lib/cli/src/fix/fixes/angular12.ts new file mode 100644 index 00000000000..b64289f5e1c --- /dev/null +++ b/lib/cli/src/fix/fixes/angular12.ts @@ -0,0 +1,61 @@ +import chalk from 'chalk'; +import dedent from 'ts-dedent'; +import semver from '@storybook/semver'; +import { ConfigFile } from '@storybook/csf-tools'; +import { Fix } from '../types'; +import { webpack5 } from './webpack5'; + +interface Angular12RunOptions { + angularVersion: string; + // FIXME angularPresetVersion: string; + storybookVersion: string; + main: ConfigFile; +} + +/** + * Is the user upgrading to Angular12? + * + * If so: + * - Run webpack5 fix + */ +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']; + const angularCoerced = semver.coerce(angularVersion)?.version; + + if (!angularCoerced || semver.lt(angularCoerced, '12.0.0')) { + return null; + } + + const builderInfo = await webpack5.checkWebpack5Builder(packageJson); + return builderInfo ? { angularVersion, ...builderInfo } : null; + }, + + prompt({ angularVersion, storybookVersion }) { + const angularFormatted = chalk.cyan(`Angular ${angularVersion}`); + const sbFormatted = chalk.cyan(`Storybook ${storybookVersion}`); + return dedent` + We've detected you are running ${angularFormatted} which is powered by webpack5. + ${sbFormatted} runs webpack4 by default, which is incompatible. + + In order to work with your version of Angular, we need to install Storybook's ${chalk.cyan( + 'webpack5 builder' + )}. + + More info: ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#angular12-upgrade' + )} + `; + }, + + async run(options) { + return webpack5.run({ + ...options, + result: { webpackVersion: null, ...options.result }, + }); + }, +}; From cf0a7a657a18509246960f6f9330e7872dcd6303 Mon Sep 17 00:00:00 2001 From: Kyle Gach Date: Fri, 1 Oct 2021 08:06:53 -0600 Subject: [PATCH 13/18] Update MIGRATION.md, re: webpack 5 - This area was a point of confusion - Attempted to reduce confusion while maintaining existing headings (so that anchor links already shared don't break) --- MIGRATION.md | 57 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 01bc6192bc1..6eab6f4e5a9 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -11,7 +11,10 @@ - [Loader behavior with args changes](#loader-behavior-with-args-changes) - [Angular component parameter removed](#angular-component-parameter-removed) - [From version 6.2.x to 6.3.0](#from-version-62x-to-630) - - [Webpack 5 manager build](#webpack-5-manager-build) + - [Webpack 5](#webpack-5-manager-build) + - [Fixing hoisting issues](#fixing-hoisting-issues) + - [Webpack 5 manager build](#webpack-5-manager-build) + - [Wrong webpack version](#wrong-webpack-version) - [Angular 12 upgrade](#angular-12-upgrade) - [Lit support](#lit-support) - [No longer inferring default values of args](#no-longer-inferring-default-values-of-args) @@ -333,29 +336,9 @@ export const MyStory = () => ({ component: MyComponent, ... }) ## From version 6.2.x to 6.3.0 -### Webpack 5 manager build +### Webpack 5 -Storybook 6.2 introduced **experimental** webpack5 support for building user components. Storybook 6.3 also supports building the manager UI in webpack 5 to avoid strange hoisting issues. - -If you're upgrading from 6.2 and already using the experimental webpack5 feature, this might be a breaking change (hence the 'experimental' label) and you should try adding the manager builder: - -```shell -yarn add @storybook/manager-webpack5 --dev -# Or -npm install @storybook/manager-webpack5 --save-dev -``` - -Because Storybook uses `webpack@4` as the default, it's possible for the wrong version of webpack to get hoisted by your package manager. If you receive an error that looks like you might be using the wrong version of webpack, install `webpack@5` explicitly as a dev dependency to force it to be hoisted: - -```shell -yarn add webpack@5 --dev -# Or -npm install webpack@5 --save-dev -``` - -### Angular 12 upgrade - -Storybook 6.3 supports Angular 12 out of the box when you install it fresh. However, if you're upgrading your project from a previous version, you'll need to do the following steps to force Storybook to use webpack 5 for building your project: +Storybook 6.3 brings opt-in support for building both your project and the manager UI with webpack 5. To do so: ```shell yarn add @storybook/builder-webpack5@next @storybook/manager-webpack5 --dev @@ -373,6 +356,34 @@ module.exports = { }; ``` +#### Fixing hoisting issues + +##### Webpack 5 manager build + +Storybook 6.2 introduced **experimental** webpack5 support for building user components. Storybook 6.3 also supports building the manager UI in webpack 5 to avoid strange hoisting issues. + +If you're upgrading from 6.2 and already using the experimental webpack5 feature, this might be a breaking change (hence the 'experimental' label) and you should try adding the manager builder: + +```shell +yarn add @storybook/manager-webpack5 --dev +# Or +npm install @storybook/manager-webpack5 --save-dev +``` + +##### Wrong webpack version + +Because Storybook uses `webpack@4` as the default, it's possible for the wrong version of webpack to get hoisted by your package manager. If you receive an error that looks like you might be using the wrong version of webpack, install `webpack@5` explicitly as a dev dependency to force it to be hoisted: + +```shell +yarn add webpack@5 --dev +# Or +npm install webpack@5 --save-dev +``` + +### Angular 12 upgrade + +Storybook 6.3 supports Angular 12 out of the box when you install it fresh. However, if you're upgrading your project from a previous version, you'll need to [follow the steps for opting-in to webpack 5](#webpack-5). + ### Lit support Storybook 6.3 introduces Lit 2 support in a non-breaking way to ease migration from `lit-html`/`lit-element` to `lit`. From d6a35c5673b6e31889fab71c7f7b794869cab833 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Tue, 5 Oct 2021 18:19:31 +0800 Subject: [PATCH 14/18] Rename `sb fix` to `sb automigrate` --- .../src/{fix => automigrate}/fixes/angular12.test.ts | 0 lib/cli/src/{fix => automigrate}/fixes/angular12.ts | 0 lib/cli/src/{fix => automigrate}/fixes/cra5.test.ts | 0 lib/cli/src/{fix => automigrate}/fixes/cra5.ts | 0 lib/cli/src/{fix => automigrate}/fixes/index.ts | 3 ++- .../src/{fix => automigrate}/fixes/webpack5.test.ts | 0 lib/cli/src/{fix => automigrate}/fixes/webpack5.ts | 12 ++++++++---- .../helpers/getStorybookConfiguration.test.ts | 0 .../helpers/getStorybookConfiguration.ts | 0 .../{fix => automigrate}/helpers/getStorybookInfo.ts | 0 lib/cli/src/{fix => automigrate}/index.ts | 2 +- lib/cli/src/{fix => automigrate}/types.ts | 0 lib/cli/src/generate.ts | 7 ++++--- 13 files changed, 15 insertions(+), 9 deletions(-) rename lib/cli/src/{fix => automigrate}/fixes/angular12.test.ts (100%) rename lib/cli/src/{fix => automigrate}/fixes/angular12.ts (100%) rename lib/cli/src/{fix => automigrate}/fixes/cra5.test.ts (100%) rename lib/cli/src/{fix => automigrate}/fixes/cra5.ts (100%) rename lib/cli/src/{fix => automigrate}/fixes/index.ts (56%) rename lib/cli/src/{fix => automigrate}/fixes/webpack5.test.ts (100%) rename lib/cli/src/{fix => automigrate}/fixes/webpack5.ts (92%) rename lib/cli/src/{fix => automigrate}/helpers/getStorybookConfiguration.test.ts (100%) rename lib/cli/src/{fix => automigrate}/helpers/getStorybookConfiguration.ts (100%) rename lib/cli/src/{fix => automigrate}/helpers/getStorybookInfo.ts (100%) rename lib/cli/src/{fix => automigrate}/index.ts (95%) rename lib/cli/src/{fix => automigrate}/types.ts (100%) diff --git a/lib/cli/src/fix/fixes/angular12.test.ts b/lib/cli/src/automigrate/fixes/angular12.test.ts similarity index 100% rename from lib/cli/src/fix/fixes/angular12.test.ts rename to lib/cli/src/automigrate/fixes/angular12.test.ts diff --git a/lib/cli/src/fix/fixes/angular12.ts b/lib/cli/src/automigrate/fixes/angular12.ts similarity index 100% rename from lib/cli/src/fix/fixes/angular12.ts rename to lib/cli/src/automigrate/fixes/angular12.ts diff --git a/lib/cli/src/fix/fixes/cra5.test.ts b/lib/cli/src/automigrate/fixes/cra5.test.ts similarity index 100% rename from lib/cli/src/fix/fixes/cra5.test.ts rename to lib/cli/src/automigrate/fixes/cra5.test.ts diff --git a/lib/cli/src/fix/fixes/cra5.ts b/lib/cli/src/automigrate/fixes/cra5.ts similarity index 100% rename from lib/cli/src/fix/fixes/cra5.ts rename to lib/cli/src/automigrate/fixes/cra5.ts diff --git a/lib/cli/src/fix/fixes/index.ts b/lib/cli/src/automigrate/fixes/index.ts similarity index 56% rename from lib/cli/src/fix/fixes/index.ts rename to lib/cli/src/automigrate/fixes/index.ts index e3095d2f172..31aa247f563 100644 --- a/lib/cli/src/fix/fixes/index.ts +++ b/lib/cli/src/automigrate/fixes/index.ts @@ -1,6 +1,7 @@ import { cra5 } from './cra5'; import { webpack5 } from './webpack5'; +import { angular12 } from './angular12'; import { Fix } from '../types'; export * from '../types'; -export const fixes: Fix[] = [cra5, webpack5]; +export const fixes: Fix[] = [cra5, webpack5, angular12]; diff --git a/lib/cli/src/fix/fixes/webpack5.test.ts b/lib/cli/src/automigrate/fixes/webpack5.test.ts similarity index 100% rename from lib/cli/src/fix/fixes/webpack5.test.ts rename to lib/cli/src/automigrate/fixes/webpack5.test.ts diff --git a/lib/cli/src/fix/fixes/webpack5.ts b/lib/cli/src/automigrate/fixes/webpack5.ts similarity index 92% rename from lib/cli/src/fix/fixes/webpack5.ts rename to lib/cli/src/automigrate/fixes/webpack5.ts index c5a70307d3d..0ed2debebf3 100644 --- a/lib/cli/src/fix/fixes/webpack5.ts +++ b/lib/cli/src/automigrate/fixes/webpack5.ts @@ -112,7 +112,6 @@ export const webpack5: Fix & CheckBuilder = { }, async run({ result: { main, storybookVersion, webpackVersion }, packageManager, dryRun }) { - if (dryRun) return; const deps = [ `@storybook/manager-webpack5@${storybookVersion}`, `@storybook/builder-webpack5@${storybookVersion}`, @@ -122,8 +121,13 @@ export const webpack5: Fix & CheckBuilder = { if (!webpackVersion) { deps.push('webpack@5'); } - packageManager.addDependencies({ installAsDevDependencies: true }, deps); - main.setFieldValue(['core', 'builder'], 'webpack5'); - await writeConfig(main); + logger.info(`✅ Adding dependencies: ${deps}`); + if (!dryRun) packageManager.addDependencies({ installAsDevDependencies: true }, deps); + + logger.info('✅ Setting `core.builder` to `webpack5` in main.js'); + if (!dryRun) { + main.setFieldValue(['core', 'builder'], 'webpack5'); + await writeConfig(main); + } }, }; diff --git a/lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts b/lib/cli/src/automigrate/helpers/getStorybookConfiguration.test.ts similarity index 100% rename from lib/cli/src/fix/helpers/getStorybookConfiguration.test.ts rename to lib/cli/src/automigrate/helpers/getStorybookConfiguration.test.ts diff --git a/lib/cli/src/fix/helpers/getStorybookConfiguration.ts b/lib/cli/src/automigrate/helpers/getStorybookConfiguration.ts similarity index 100% rename from lib/cli/src/fix/helpers/getStorybookConfiguration.ts rename to lib/cli/src/automigrate/helpers/getStorybookConfiguration.ts diff --git a/lib/cli/src/fix/helpers/getStorybookInfo.ts b/lib/cli/src/automigrate/helpers/getStorybookInfo.ts similarity index 100% rename from lib/cli/src/fix/helpers/getStorybookInfo.ts rename to lib/cli/src/automigrate/helpers/getStorybookInfo.ts diff --git a/lib/cli/src/fix/index.ts b/lib/cli/src/automigrate/index.ts similarity index 95% rename from lib/cli/src/fix/index.ts rename to lib/cli/src/automigrate/index.ts index 81fdd1759d2..9b8e5577f7e 100644 --- a/lib/cli/src/fix/index.ts +++ b/lib/cli/src/automigrate/index.ts @@ -14,7 +14,7 @@ interface FixOptions { dryRun?: boolean; } -export const fix = async ({ fixId, dryRun, yes }: FixOptions) => { +export const automigrate = async ({ fixId, dryRun, yes }: FixOptions) => { const packageManager = JsPackageManagerFactory.getPackageManager(); const filtered = fixId ? fixes.filter((f) => f.id === fixId) : fixes; diff --git a/lib/cli/src/fix/types.ts b/lib/cli/src/automigrate/types.ts similarity index 100% rename from lib/cli/src/fix/types.ts rename to lib/cli/src/automigrate/types.ts diff --git a/lib/cli/src/generate.ts b/lib/cli/src/generate.ts index 7a91e53fcc3..faf61d554c6 100644 --- a/lib/cli/src/generate.ts +++ b/lib/cli/src/generate.ts @@ -11,7 +11,7 @@ import { extract } from './extract'; import { upgrade } from './upgrade'; import { repro } from './repro'; import { link } from './link'; -import { fix } from './fix'; +import { automigrate } from './automigrate'; import { generateStorybookBabelConfigInCWD } from './babel-config'; const pkg = sync({ cwd: __dirname }).packageJson; @@ -128,11 +128,12 @@ program ); program - .command('fix [fixId]') + .command('automigrate [fixId]') .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') .action((fixId, options) => - fix({ fixId, ...options }).catch((e) => { + automigrate({ fixId, ...options }).catch((e) => { logger.error(e); process.exit(1); }) From fdaa2209d6016b465e25704f734f6a16d70d77d4 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Tue, 5 Oct 2021 18:29:26 +0800 Subject: [PATCH 15/18] CLI: Add automigrate to `sb upgrade` --- lib/cli/src/generate.ts | 3 ++- lib/cli/src/upgrade.ts | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/cli/src/generate.ts b/lib/cli/src/generate.ts index faf61d554c6..f7f0f3698bb 100644 --- a/lib/cli/src/generate.ts +++ b/lib/cli/src/generate.ts @@ -48,9 +48,10 @@ program .command('upgrade') .description('Upgrade your Storybook packages to the latest') .option('-N --use-npm', 'Use NPM to build the Storybook server') + .option('-y --yes', 'Skip prompting the user') .option('-n --dry-run', 'Only check for upgrades, do not install') .option('-p --prerelease', 'Upgrade to the pre-release packages') - .option('-s --skip-check', 'Skip postinstall version consistency checks') + .option('-s --skip-check', 'Skip postinstall version and automigration checks') .action((options) => upgrade(options)); program diff --git a/lib/cli/src/upgrade.ts b/lib/cli/src/upgrade.ts index b82c90e102e..cc554951ffa 100644 --- a/lib/cli/src/upgrade.ts +++ b/lib/cli/src/upgrade.ts @@ -3,6 +3,7 @@ import semver from '@storybook/semver'; import { logger } from '@storybook/node-logger'; import { JsPackageManagerFactory } from './js-package-manager'; import { commandLog } from './helpers'; +import { automigrate } from './automigrate'; type Package = { package: string; @@ -91,8 +92,15 @@ export const checkVersionConsistency = () => { }); }; -type Options = { prerelease: boolean; skipCheck: boolean; useNpm: boolean; dryRun: boolean }; -export const upgrade = async ({ prerelease, skipCheck, useNpm, dryRun }: Options) => { +interface UpgradeOptions { + prerelease: boolean; + skipCheck: boolean; + useNpm: boolean; + dryRun: boolean; + yes: boolean; +} + +export const upgrade = async ({ prerelease, skipCheck, useNpm, dryRun, yes }: UpgradeOptions) => { const packageManager = JsPackageManagerFactory.getPackageManager(useNpm); commandLog(`Checking for latest versions of '@storybook/*' packages`); @@ -111,5 +119,8 @@ export const upgrade = async ({ prerelease, skipCheck, useNpm, dryRun }: Options packageManager.installDependencies(); } - if (!skipCheck) checkVersionConsistency(); + if (!skipCheck) { + checkVersionConsistency(); + await automigrate({ dryRun, yes }); + } }; From 31045fb32d53918e71f6af115d0264b7625c49b8 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Tue, 5 Oct 2021 21:29:15 +0800 Subject: [PATCH 16/18] Added upgrading documentation --- docs/configure/upgrading.md | 45 +++++++++++++++++++++++++++++++++++++ docs/toc.js | 5 +++++ 2 files changed, 50 insertions(+) create mode 100644 docs/configure/upgrading.md diff --git a/docs/configure/upgrading.md b/docs/configure/upgrading.md new file mode 100644 index 00000000000..3a9c6f6a8ba --- /dev/null +++ b/docs/configure/upgrading.md @@ -0,0 +1,45 @@ +--- +title: 'Upgrading Storybook' +--- + +The frontend ecosystem is a fast-moving place. Regular dependency upgrades are way of life, whether it's upgrading a framework, library, tooling, or all of the above! Storybook provides a few resources to help ease the pain of upgrading. + +## Upgrade script + +The most common upgrade is Storybook itself. Storybook follows [Semantic Versioning](https://semver.org/). We release patch releases with bugfixes continuously, minor versions of Storybook with new features every few months, and major versions of Storybook with breaking changes roughly once per year. + +To help ease the pain of keeping Storybook up-to-date, we provide a command-line script: + +```sh +npx sb@next upgrade +``` + +This upgrades all your Storybook packages to the latest stable version, perform sanity checks of your package versions, and also check for [automigrations](#automigrate) to automatically update your configuration. + +
+ +In addition to running the command, we also recommend skimming [MIGRATION.md](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md), an exhaustive log of changes relevant changes and deprecations that might affect your upgrade. + +
+ +## Automigrate script + +Storybook upgrades are not the only thing that can break your Storybook: changes in the ecosystem are also present challenges. For example, lots of frameworks (Angular 12, Create React App v5, NextJS) have recently migrated from webpack4 to webpack5, so even if you don't upgrade your Storybook version, you might need to update your configuration accordingly. That's what Automigrate is for: + +``` +npx sb@next automigrate +``` + +This runs a set of common configuration checks, explains what's potentially out of date, and offers to fix it for you automatically. It also points to the relevant documentation so you can learn more. This gets run automatically as part of `sb upgrade`, but it's also available on its own in case you don't want to upgrade Storybook. + +## Prereleases + +In addition to the above, Storybook is under constant development, and we publish prerelease versions almost daily. Pre-releases are the best way to try out new features before they are generally available, and we do our best to keep them as stable as possible, although this is not always possible. + +To upgrade to the latest prerelease: + +```sh +npx sb@next upgrade --prerelease +``` + +If you'd like to downgrade to a stable version, manually edit the package version numbers in your `package.json` and reinstall. diff --git a/docs/toc.js b/docs/toc.js index ed535a4083a..fa46813e256 100644 --- a/docs/toc.js +++ b/docs/toc.js @@ -169,6 +169,11 @@ module.exports = { title: 'Overview', type: 'link', }, + { + pathSegment: 'upgrading', + title: 'Upgrading', + type: 'link', + }, { pathSegment: '', title: 'Integration', From 077f18c8ecee391d086816a11bd79b5cd5b58871 Mon Sep 17 00:00:00 2001 From: Kyle Gach Date: Tue, 5 Oct 2021 09:17:59 -0600 Subject: [PATCH 17/18] Edit upgrading doc page --- docs/configure/upgrading.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/configure/upgrading.md b/docs/configure/upgrading.md index 3a9c6f6a8ba..340e8a1dd99 100644 --- a/docs/configure/upgrading.md +++ b/docs/configure/upgrading.md @@ -2,11 +2,11 @@ title: 'Upgrading Storybook' --- -The frontend ecosystem is a fast-moving place. Regular dependency upgrades are way of life, whether it's upgrading a framework, library, tooling, or all of the above! Storybook provides a few resources to help ease the pain of upgrading. +The frontend ecosystem is a fast-moving place. Regular dependency upgrades are a way of life, whether it's upgrading a framework, library, tooling, or all of the above! Storybook provides a few resources to help ease the pain of upgrading. ## Upgrade script -The most common upgrade is Storybook itself. Storybook follows [Semantic Versioning](https://semver.org/). We release patch releases with bugfixes continuously, minor versions of Storybook with new features every few months, and major versions of Storybook with breaking changes roughly once per year. +The most common upgrade is Storybook itself. [Storybook releases](/releases) follow [Semantic Versioning](https://semver.org/). We publish patch releases with bugfixes continuously, minor versions of Storybook with new features every few months, and major versions of Storybook with breaking changes roughly once per year. To help ease the pain of keeping Storybook up-to-date, we provide a command-line script: @@ -14,32 +14,32 @@ To help ease the pain of keeping Storybook up-to-date, we provide a command-line npx sb@next upgrade ``` -This upgrades all your Storybook packages to the latest stable version, perform sanity checks of your package versions, and also check for [automigrations](#automigrate) to automatically update your configuration. +This upgrades all of the Storybook packages in your project to the latest stable version, performs confidence checks of your package versions, and checks for opportunities to run [automigrations](#automigrate) to automatically update your configuration.
-In addition to running the command, we also recommend skimming [MIGRATION.md](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md), an exhaustive log of changes relevant changes and deprecations that might affect your upgrade. +In addition to running the command, we also recommend skimming [MIGRATION.md](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md), an exhaustive log of relevant changes and deprecations that might affect your upgrade.
## Automigrate script -Storybook upgrades are not the only thing that can break your Storybook: changes in the ecosystem are also present challenges. For example, lots of frameworks (Angular 12, Create React App v5, NextJS) have recently migrated from webpack4 to webpack5, so even if you don't upgrade your Storybook version, you might need to update your configuration accordingly. That's what Automigrate is for: +Storybook upgrades are not the only thing to consider: changes in the ecosystem also present challenges. For example, lots of frameworks ([Angular 12](https://angular.io/guide/updating-to-version-12#breaking-changes-in-angular-version-12), [Create React App v5](https://github.com/facebook/create-react-app/pull/11201), [NextJS](https://nextjs.org/docs/upgrading#webpack-5)) have recently migrated from [webpack 4 to webpack 5](https://webpack.js.org/migrate/5/), so even if you don't upgrade your Storybook version, you might need to update your configuration accordingly. That's what Automigrate is for: ``` npx sb@next automigrate ``` -This runs a set of common configuration checks, explains what's potentially out of date, and offers to fix it for you automatically. It also points to the relevant documentation so you can learn more. This gets run automatically as part of `sb upgrade`, but it's also available on its own in case you don't want to upgrade Storybook. +This runs a set of common configuration checks, explains what is potentially out-of-date, and offers to fix it for you automatically. It also points to the relevant documentation so you can learn more. This gets run automatically as part of [`sb upgrade`](#upgrade-script), but it's also available on its own in case you don't want to upgrade Storybook. ## Prereleases -In addition to the above, Storybook is under constant development, and we publish prerelease versions almost daily. Pre-releases are the best way to try out new features before they are generally available, and we do our best to keep them as stable as possible, although this is not always possible. +In addition to the above, Storybook is under constant development, and we publish pre-release versions almost daily. Pre-releases are the best way to try out new features before they are generally available, and we do our best to keep them as stable as possible, although this is not always possible. -To upgrade to the latest prerelease: +To upgrade to the latest pre-release: ```sh npx sb@next upgrade --prerelease ``` -If you'd like to downgrade to a stable version, manually edit the package version numbers in your `package.json` and reinstall. +If you'd like to downgrade to a stable version, manually edit the package version numbers in your `package.json` and re-install. From 64a6ef7477f0d8cebe35d1d118af9082d8884827 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 6 Oct 2021 18:55:51 +0800 Subject: [PATCH 18/18] Fix typo --- docs/configure/upgrading.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configure/upgrading.md b/docs/configure/upgrading.md index 340e8a1dd99..2bbecc54ac1 100644 --- a/docs/configure/upgrading.md +++ b/docs/configure/upgrading.md @@ -11,7 +11,7 @@ The most common upgrade is Storybook itself. [Storybook releases](/releases) fol To help ease the pain of keeping Storybook up-to-date, we provide a command-line script: ```sh -npx sb@next upgrade +npx sb upgrade ``` This upgrades all of the Storybook packages in your project to the latest stable version, performs confidence checks of your package versions, and checks for opportunities to run [automigrations](#automigrate) to automatically update your configuration.