From 4154066bcce97e178422c1b797f43f6d9aace96a Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 16 May 2023 15:28:23 +0200 Subject: [PATCH 01/24] Add support for "@yarnpkg/fslib" and "@yarnpkg/libzip", and implement "getPackageVersion" method in all package managers --- code/lib/cli/package.json | 2 + .../js-package-manager/JsPackageManager.ts | 5 ++ .../cli/src/js-package-manager/NPMProxy.ts | 20 +++++++ .../cli/src/js-package-manager/PNPMProxy.ts | 46 +++++++++++++++ .../cli/src/js-package-manager/Yarn1Proxy.ts | 20 +++++++ .../cli/src/js-package-manager/Yarn2Proxy.ts | 58 +++++++++++++++++++ code/yarn.lock | 31 +++++++++- 7 files changed, 181 insertions(+), 1 deletion(-) diff --git a/code/lib/cli/package.json b/code/lib/cli/package.json index 682a617a4d71..a741f3dc4780 100644 --- a/code/lib/cli/package.json +++ b/code/lib/cli/package.json @@ -65,6 +65,8 @@ "@storybook/telemetry": "7.1.0-alpha.28", "@storybook/types": "7.1.0-alpha.28", "@types/semver": "^7.3.4", + "@yarnpkg/fslib": "^2.10.3", + "@yarnpkg/libzip": "^2.3.0", "boxen": "^5.1.2", "chalk": "^4.1.0", "commander": "^6.2.1", diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index 7b6d0478f5c4..628b73f2166b 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -49,6 +49,11 @@ export abstract class JsPackageManager { public readonly cwd?: string; + public abstract getPackageVersion( + packageName: string, + basePath?: string + ): Promise; + // NOTE: for some reason yarn prefers the npm registry in // local development, so always use npm async setRegistryURL(url: string) { diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.ts b/code/lib/cli/src/js-package-manager/NPMProxy.ts index 47827adfd153..b626232988a0 100644 --- a/code/lib/cli/src/js-package-manager/NPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/NPMProxy.ts @@ -1,6 +1,9 @@ import sort from 'semver/functions/sort'; import { platform } from 'os'; import dedent from 'ts-dedent'; +import { sync as findUpSync } from 'find-up'; +import { existsSync, readFileSync } from 'fs'; +import path from 'path'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; import type { InstallationMetadata, PackageMetadata } from './types'; @@ -77,6 +80,23 @@ export class NPMProxy extends JsPackageManager { return this.executeCommand({ command: 'npm', args: ['--version'] }); } + public async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const packageJsonPath = await findUpSync( + (dir) => { + const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); + return existsSync(possiblePath) ? possiblePath : undefined; + }, + { cwd: basePath } + ); + + if (!packageJsonPath) { + return null; + } + + const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; + return packageJson.version; + } + getInstallArgs(): string[] { if (!this.installArgs) { this.installArgs = []; diff --git a/code/lib/cli/src/js-package-manager/PNPMProxy.ts b/code/lib/cli/src/js-package-manager/PNPMProxy.ts index 098bbfd7a4be..0af34ad9580d 100644 --- a/code/lib/cli/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/PNPMProxy.ts @@ -1,5 +1,8 @@ import { pathExistsSync } from 'fs-extra'; import dedent from 'ts-dedent'; +import { sync as findUpSync } from 'find-up'; +import path from 'path'; +import fs from 'fs'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; import type { InstallationMetadata, PackageMetadata } from './types'; @@ -107,6 +110,49 @@ export class PNPMProxy extends JsPackageManager { } } + async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); + + if (pnpapiPath) { + try { + // eslint-disable-next-line import/no-dynamic-require, global-require + const pnpApi = require(pnpapiPath); + + const resolvedPath = await pnpApi.resolveToUnqualified(packageName, basePath, { + considerBuiltins: false, + }); + + const pkgLocator = pnpApi.findPackageLocator(resolvedPath); + const pkg = pnpApi.getPackageInformation(pkgLocator); + + const packageJSON = fs.readFileSync( + path.join(pkg.packageLocation, 'package.json'), + 'utf-8' + ); + + return JSON.parse(packageJSON).version; + } catch (error) { + console.error('Error while fetching package version in Yarn PnP mode:', error); + } + } + + const packageJsonPath = await findUpSync( + (dir) => { + const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); + return fs.existsSync(possiblePath) ? possiblePath : undefined; + }, + { cwd: basePath } + ); + + if (!packageJsonPath) { + return null; + } + + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); + + return packageJson.version; + } + protected getResolutions(packageJson: PackageJson, versions: Record) { return { overrides: { diff --git a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts index be7103d47a36..3e8d23fc5e02 100644 --- a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts @@ -1,4 +1,7 @@ import dedent from 'ts-dedent'; +import { sync as findUpSync } from 'find-up'; +import { existsSync, readFileSync } from 'fs'; +import path from 'path'; import { createLogStream } from '../utils'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -59,6 +62,23 @@ export class Yarn1Proxy extends JsPackageManager { return this.executeCommand({ command: `yarn`, args: [command, ...args], cwd }); } + public async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const packageJsonPath = await findUpSync( + (dir) => { + const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); + return existsSync(possiblePath) ? possiblePath : undefined; + }, + { cwd: basePath } + ); + + if (!packageJsonPath) { + return null; + } + + const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; + return packageJson.version; + } + public async findInstallations(pattern: string[]) { const commandResult = await this.executeCommand({ command: 'yarn', diff --git a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts index f6675b57574c..d1fb0d048ef1 100644 --- a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts @@ -1,4 +1,9 @@ import dedent from 'ts-dedent'; +import { sync as findUpSync, sync as syncFindUp } from 'find-up'; +import fs, { existsSync, readFileSync } from 'fs'; +import path from 'path'; +import { NodeFS, VirtualFS, ZipOpenFS } from '@yarnpkg/fslib'; +import { getLibzipSync } from '@yarnpkg/libzip'; import { createLogStream } from '../utils'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -117,6 +122,59 @@ export class Yarn2Proxy extends JsPackageManager { } } + async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); + + if (pnpapiPath) { + try { + // eslint-disable-next-line import/no-dynamic-require, global-require + const pnpApi = require(pnpapiPath); + + const resolvedPath = await pnpApi.resolveToUnqualified(packageName, basePath, { + considerBuiltins: false, + }); + + const pkgLocator = pnpApi.findPackageLocator(resolvedPath); + const pkg = pnpApi.getPackageInformation(pkgLocator); + + const localFs: typeof fs = { ...fs }; + const nodeFs = new NodeFS(localFs); + + const zipOpenFs = new ZipOpenFS({ + libzip: getLibzipSync(), + baseFs: nodeFs, + readOnlyArchives: true, + }); + + const virtualFs = new VirtualFS({ + baseFs: zipOpenFs, + }); + + const virtualFile = virtualFs.readJsonSync( + path.join(pkg.packageLocation, 'package.json') as any + ); + return virtualFile.version; + } catch (error) { + console.error('Error while fetching package version in Yarn PnP mode:', error); + } + } + + const packageJsonPath = await syncFindUp( + (dir) => { + const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); + return existsSync(possiblePath) ? possiblePath : undefined; + }, + { cwd: basePath } + ); + + if (!packageJsonPath) { + return null; + } + + const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); + return packageJson.version; + } + protected getResolutions(packageJson: PackageJson, versions: Record) { return { resolutions: { diff --git a/code/yarn.lock b/code/yarn.lock index 3dacf21a1c43..d18353969a0f 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6164,6 +6164,8 @@ __metadata: "@types/semver": ^7.3.4 "@types/shelljs": ^0.8.7 "@types/util-deprecate": ^1.0.0 + "@yarnpkg/fslib": ^2.10.3 + "@yarnpkg/libzip": ^2.3.0 boxen: ^5.1.2 chalk: ^4.1.0 commander: ^6.2.1 @@ -8380,6 +8382,13 @@ __metadata: languageName: node linkType: hard +"@types/emscripten@npm:^1.39.6": + version: 1.39.6 + resolution: "@types/emscripten@npm:1.39.6" + checksum: cb1ea8ccddada1d304bdf11a54daa0d1e87f29cea947eceff54c1e0a752d2cc185eeffdcf52042f24420aa8e1fa9bbfdbab1231fb2531eefcfdc788199fee2de + languageName: node + linkType: hard + "@types/escodegen@npm:^0.0.6": version: 0.0.6 resolution: "@types/escodegen@npm:0.0.6" @@ -9934,6 +9943,26 @@ __metadata: languageName: node linkType: hard +"@yarnpkg/fslib@npm:^2.10.3": + version: 2.10.3 + resolution: "@yarnpkg/fslib@npm:2.10.3" + dependencies: + "@yarnpkg/libzip": ^2.3.0 + tslib: ^1.13.0 + checksum: c4fbbed99e801f17c381204e9699d9ea4fb51b14e99968985f477bdbc7b02b61e026860173f3f46bd60d9f46ae6a06f420a3edb3c02c3a45ae83779095928094 + languageName: node + linkType: hard + +"@yarnpkg/libzip@npm:^2.3.0": + version: 2.3.0 + resolution: "@yarnpkg/libzip@npm:2.3.0" + dependencies: + "@types/emscripten": ^1.39.6 + tslib: ^1.13.0 + checksum: 0c2361ccb002e28463ed98541f3bdaab54f52aad6a2080666c2a9ea605ebd9cdfb7b0340b1db6f105820d05bcb803cdfb3ce755a8f6034657298c291bf884f81 + languageName: node + linkType: hard + "@yarnpkg/lockfile@npm:1.1.0, @yarnpkg/lockfile@npm:^1.1.0": version: 1.1.0 resolution: "@yarnpkg/lockfile@npm:1.1.0" @@ -30062,7 +30091,7 @@ __metadata: languageName: node linkType: hard -"tslib@npm:^1.10.0, tslib@npm:^1.8.1, tslib@npm:^1.9.0, tslib@npm:^1.9.3": +"tslib@npm:^1.10.0, tslib@npm:^1.13.0, tslib@npm:^1.8.1, tslib@npm:^1.9.0, tslib@npm:^1.9.3": version: 1.14.1 resolution: "tslib@npm:1.14.1" checksum: 69ae09c49eea644bc5ebe1bca4fa4cc2c82b7b3e02f43b84bd891504edf66dbc6b2ec0eef31a957042de2269139e4acff911e6d186a258fb14069cd7f6febce2 From 9c57628d2ced6206c5be174a1c0d9ed245a8f5a5 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 16 May 2023 15:34:02 +0200 Subject: [PATCH 02/24] Remove Angular 12 automigration --- .../src/automigrate/fixes/angular12.test.ts | 130 ------------------ .../cli/src/automigrate/fixes/angular12.ts | 59 -------- code/lib/cli/src/automigrate/fixes/index.ts | 2 - 3 files changed, 191 deletions(-) delete mode 100644 code/lib/cli/src/automigrate/fixes/angular12.test.ts delete mode 100644 code/lib/cli/src/automigrate/fixes/angular12.ts diff --git a/code/lib/cli/src/automigrate/fixes/angular12.test.ts b/code/lib/cli/src/automigrate/fixes/angular12.test.ts deleted file mode 100644 index f0aaaf774b0a..000000000000 --- a/code/lib/cli/src/automigrate/fixes/angular12.test.ts +++ /dev/null @@ -1,130 +0,0 @@ -import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; -import { angular12 } from './angular12'; - -const checkAngular12 = async ({ - packageJson, - main: mainConfig = {}, - storybookVersion = '7.0.0', -}: { - packageJson: PackageJson; - main?: Partial & Record; - storybookVersion?: string; -}) => { - mockStorybookData({ mainConfig, storybookVersion }); - - return angular12.check({ - packageManager: makePackageManager(packageJson), - configDir: '', - }); -}; - -describe('angular12 fix', () => { - afterEach(jest.restoreAllMocks); - - describe('sb < 6.3', () => { - describe('angular12 dependency', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^6.2.0', '@angular/core': '^12.0.0' }, - }; - it('should fail', async () => { - await expect( - checkAngular12({ - packageJson, - storybookVersion: '6.2.0', - }) - ).rejects.toThrow(); - }); - }); - describe('no angular dependency', () => { - const packageJson = { dependencies: { '@storybook/angular': '^6.2.0' } }; - it('should no-op', async () => { - await expect( - checkAngular12({ - packageJson, - main: {}, - }) - ).resolves.toBeFalsy(); - }); - }); - }); - describe('sb 6.3 - 7.0', () => { - describe('angular12 dependency', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^6.3.0', '@angular/core': '^12.0.0' }, - }; - describe('webpack5 builder', () => { - it('should no-op', async () => { - await expect( - checkAngular12({ - packageJson, - main: { core: { builder: 'webpack5' } }, - storybookVersion: '6.3.0', - }) - ).resolves.toBeFalsy(); - }); - }); - describe('custom builder', () => { - it('should no-op', async () => { - await expect( - checkAngular12({ - packageJson, - main: { core: { builder: 'storybook-builder-vite' } }, - }) - ).resolves.toBeFalsy(); - }); - }); - describe('webpack4 builder', () => { - it('should add webpack5 builder', async () => { - await expect( - checkAngular12({ - packageJson, - main: { core: { builder: 'webpack4' } }, - storybookVersion: '6.3.0', - }) - ).resolves.toMatchObject({ - angularVersion: '^12.0.0', - storybookVersion: '6.3.0', - }); - }); - }); - describe('no builder', () => { - it('should add webpack5 builder', async () => { - await expect( - checkAngular12({ - packageJson, - storybookVersion: '6.3.0', - }) - ).resolves.toMatchObject({ - angularVersion: '^12.0.0', - storybookVersion: '6.3.0', - }); - }); - }); - }); - describe('no angular dependency', () => { - it('should no-op', async () => { - await expect( - checkAngular12({ - packageJson: {}, - }) - ).resolves.toBeFalsy(); - }); - }); - }); - describe('sb 7.0+', () => { - describe('angular12 dependency', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, - }; - it('should no-op', async () => { - await expect( - checkAngular12({ - packageJson, - }) - ).resolves.toBeFalsy(); - }); - }); - }); -}); diff --git a/code/lib/cli/src/automigrate/fixes/angular12.ts b/code/lib/cli/src/automigrate/fixes/angular12.ts deleted file mode 100644 index c8b98e6b0f00..000000000000 --- a/code/lib/cli/src/automigrate/fixes/angular12.ts +++ /dev/null @@ -1,59 +0,0 @@ -import chalk from 'chalk'; -import { dedent } from 'ts-dedent'; -import semver from 'semver'; -import type { Fix } from '../types'; -import { webpack5 } from './webpack5'; -import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder'; - -interface Angular12RunOptions { - angularVersion: string; - // FIXME angularPresetVersion: string; - storybookVersion: string; -} - -/** - * Is the user upgrading to Angular12? - * - * If so: - * - Run webpack5 fix - */ -export const angular12: Fix = { - id: 'angular12', - - async check({ packageManager, configDir }) { - const allDependencies = await packageManager.getAllDependencies(); - const angularVersion = allDependencies['@angular/core']; - const angularCoerced = semver.coerce(angularVersion)?.version; - - if (!angularCoerced || semver.lt(angularCoerced, '12.0.0')) { - return null; - } - - const builderInfo = await checkWebpack5Builder({ packageManager, configDir }); - return builderInfo ? { angularVersion, ...builderInfo } : null; - }, - - prompt({ angularVersion }) { - const angularFormatted = chalk.cyan(`Angular ${angularVersion}`); - - return dedent` - We've detected you are running ${angularFormatted} which is powered by webpack5. - Your Storybook's main.js files specifies webpack4, which is incompatible. - - In order to work with your version of Angular, we need to install Storybook's ${chalk.cyan( - '@storybook/builder-webpack5' - )}. - - 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 }, - }); - }, -}; diff --git a/code/lib/cli/src/automigrate/fixes/index.ts b/code/lib/cli/src/automigrate/fixes/index.ts index e1f02b510afc..b38f196bfe0e 100644 --- a/code/lib/cli/src/automigrate/fixes/index.ts +++ b/code/lib/cli/src/automigrate/fixes/index.ts @@ -2,7 +2,6 @@ import type { Fix } from '../types'; import { cra5 } from './cra5'; import { webpack5 } from './webpack5'; -import { angular12 } from './angular12'; import { vue3 } from './vue3'; import { mdxgfm } from './mdx-gfm'; import { eslintPlugin } from './eslint-plugin'; @@ -26,7 +25,6 @@ export const allFixes: Fix[] = [ nodeJsRequirement, cra5, webpack5, - angular12, vue3, eslintPlugin, builderVite, From ed7813dd76d59dc7dfc5b9d84b5643f563ac1375 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 19 May 2023 15:09:56 +0200 Subject: [PATCH 03/24] Read proper installed package version --- .../angular-builders-multiproject.test.ts | 96 ++-- .../fixes/angular-builders-multiproject.ts | 29 +- .../fixes/angular-builders.test.ts | 96 ++-- .../src/automigrate/fixes/angular-builders.ts | 28 +- .../automigrate/fixes/autodocs-true.test.ts | 8 +- .../src/automigrate/fixes/autodocs-true.ts | 6 +- .../fixes/bare-mdx-stories-glob.test.ts | 6 +- .../fixes/bare-mdx-stories-glob.ts | 9 +- .../automigrate/fixes/builder-vite.test.ts | 8 +- .../cli/src/automigrate/fixes/builder-vite.ts | 5 +- .../cli/src/automigrate/fixes/cra5.test.ts | 81 ++-- code/lib/cli/src/automigrate/fixes/cra5.ts | 10 +- .../fixes/incompatible-addons.test.ts | 29 +- .../automigrate/fixes/incompatible-addons.ts | 8 +- code/lib/cli/src/automigrate/fixes/index.ts | 2 +- .../cli/src/automigrate/fixes/mdx-gfm.test.ts | 28 +- code/lib/cli/src/automigrate/fixes/mdx-gfm.ts | 6 +- .../automigrate/fixes/missing-babelrc.test.ts | 82 ++-- .../src/automigrate/fixes/missing-babelrc.ts | 22 +- .../automigrate/fixes/new-frameworks.test.ts | 452 +++++++++--------- .../src/automigrate/fixes/new-frameworks.ts | 98 ++-- .../fixes/nodejs-requirement.test.ts | 7 +- .../automigrate/fixes/nodejs-requirement.ts | 5 +- .../fixes/remove-global-client-apis.test.ts | 24 +- .../fixes/remove-global-client-apis.ts | 13 +- .../src/automigrate/fixes/sb-binary.test.ts | 105 ++-- .../cli/src/automigrate/fixes/sb-binary.ts | 15 +- .../src/automigrate/fixes/sb-scripts.test.ts | 121 +++-- .../cli/src/automigrate/fixes/sb-scripts.ts | 4 +- .../cli/src/automigrate/fixes/vue3.test.ts | 114 +++-- code/lib/cli/src/automigrate/fixes/vue3.ts | 10 +- .../src/automigrate/fixes/webpack5.test.ts | 113 +++-- .../lib/cli/src/automigrate/fixes/webpack5.ts | 15 +- .../helpers/checkWebpack5Builder.test.ts | 78 +++ .../helpers/checkWebpack5Builder.ts | 21 +- .../helpers/mainConfigFile.test.ts | 133 ++++++ .../src/automigrate/helpers/mainConfigFile.ts | 62 ++- .../helpers/new-frameworks-utils.test.ts | 40 +- .../helpers/new-frameworks-utils.ts | 50 +- .../automigrate/helpers/testing-helpers.ts | 20 - code/lib/cli/src/automigrate/index.ts | 10 + code/lib/cli/src/automigrate/types.ts | 5 + code/lib/cli/src/detect.test.ts | 135 ++++-- code/lib/cli/src/detect.ts | 74 ++- code/lib/cli/src/generators/ANGULAR/index.ts | 12 +- code/lib/cli/src/generators/REACT/index.ts | 2 +- .../cli/src/generators/REACT_SCRIPTS/index.ts | 3 +- code/lib/cli/src/helpers.ts | 15 +- code/lib/cli/src/initiate.ts | 13 +- .../js-package-manager/JsPackageManager.ts | 5 +- .../cli/src/js-package-manager/NPMProxy.ts | 8 +- .../cli/src/js-package-manager/PNPMProxy.ts | 6 +- .../cli/src/js-package-manager/Yarn1Proxy.ts | 8 +- .../cli/src/js-package-manager/Yarn2Proxy.ts | 6 +- .../src/utils/get-storybook-info.ts | 2 + 55 files changed, 1465 insertions(+), 898 deletions(-) create mode 100644 code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.test.ts create mode 100644 code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts diff --git a/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.test.ts b/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.test.ts index b376cca15a30..b9066b9c48f2 100644 --- a/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.test.ts +++ b/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.test.ts @@ -1,25 +1,20 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { angularBuildersMultiproject } from './angular-builders-multiproject'; import * as helpers from '../../helpers'; import * as angularHelpers from '../../generators/ANGULAR/helpers'; const checkAngularBuilders = async ({ - packageJson, - main: mainConfig = {}, - storybookVersion = '7.0.0', + packageManager, + mainConfig = {}, }: { - packageJson: PackageJson; - main?: Partial & Record; - storybookVersion?: string; + packageManager: Partial; + mainConfig?: Partial; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - - // mock file system (look at eslint plugin test) - return angularBuildersMultiproject.check({ - packageManager: makePackageManager(packageJson), + packageManager: packageManager as any, + mainConfig: mainConfig as any, + storybookVersion: '7.0.0', }); }; @@ -34,51 +29,69 @@ jest.mock('../../generators/ANGULAR/helpers', () => ({ })); describe('is Nx project', () => { + const packageManager = { + getPackageVersion: () => { + return null; + }, + } as Partial; + beforeEach(() => { - (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(true); + (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(true); }); it('should return null', async () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0' }, - }; - - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect(checkAngularBuilders({ packageManager })).resolves.toBeNull(); }); }); describe('is not Nx project', () => { beforeEach(() => { - (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(false); + (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(false); }); describe('angular builders', () => { afterEach(jest.restoreAllMocks); describe('Angular not found', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0' }, - }; + const packageManager = { + getPackageVersion: jest.fn().mockResolvedValue(null), + } as Partial; it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } }) + ).resolves.toBeNull(); }); }); describe('Angular < 14.0.0', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + if (packageName === '@angular/core') { + return Promise.resolve('12.0.0'); + } + + return null; + }, + } as Partial; it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } }) + ).resolves.toBeNull(); }); }); describe('Angular >= 14.0.0', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^15.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + if (packageName === '@angular/core') { + return Promise.resolve('15.0.0'); + } + + return null; + }, + } as Partial; describe('has one Storybook builder defined', () => { beforeEach(() => { @@ -89,7 +102,12 @@ describe('is not Nx project', () => { }); it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toBeNull(); }); }); @@ -106,7 +124,12 @@ describe('is not Nx project', () => { }); it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toBeNull(); }); }); @@ -124,7 +147,12 @@ describe('is not Nx project', () => { }); it('should return an empty object', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toMatchObject({}); + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toMatchObject({}); }); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts b/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts index 2cd0a42fa987..1c543f1a5504 100644 --- a/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts +++ b/code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts @@ -4,6 +4,7 @@ import chalk from 'chalk'; import type { Fix } from '../types'; import { isNxProject } from '../../helpers'; import { AngularJSON } from '../../generators/ANGULAR/helpers'; +import { getFrameworkPackageName } from '../helpers/mainConfigFile'; // eslint-disable-next-line @typescript-eslint/no-empty-interface interface AngularBuildersMultiprojectRunOptions {} @@ -12,25 +13,17 @@ export const angularBuildersMultiproject: Fix throw an error (only supports ng 14) - if (semver.lt(angularCoerced, '14.0.0')) { + const angularVersion = await packageManager.getPackageVersion('@angular/core'); + const frameworkPackageName = getFrameworkPackageName(mainConfig); + + if ( + (await isNxProject(packageManager)) || + frameworkPackageName !== '@storybook/angular' || + !angularVersion || + semver.lt(angularVersion, '14.0.0') + ) { return null; } diff --git a/code/lib/cli/src/automigrate/fixes/angular-builders.test.ts b/code/lib/cli/src/automigrate/fixes/angular-builders.test.ts index d7505da9e567..8f1f0650deb6 100644 --- a/code/lib/cli/src/automigrate/fixes/angular-builders.test.ts +++ b/code/lib/cli/src/automigrate/fixes/angular-builders.test.ts @@ -1,25 +1,22 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { angularBuilders } from './angular-builders'; import * as helpers from '../../helpers'; import * as angularHelpers from '../../generators/ANGULAR/helpers'; +import type { JsPackageManager } from '../../js-package-manager'; const checkAngularBuilders = async ({ - packageJson, - main: mainConfig = {}, + packageManager, + mainConfig = {}, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; - main?: Partial & Record; + packageManager: Partial; + mainConfig?: Partial; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - - // mock file system (look at eslint plugin test) - return angularBuilders.check({ - packageManager: makePackageManager(packageJson), + packageManager: packageManager as any, + storybookVersion, + mainConfig: mainConfig as any, }); }; @@ -35,50 +32,70 @@ jest.mock('../../generators/ANGULAR/helpers', () => ({ describe('is Nx project', () => { beforeEach(() => { - (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(true); + (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(true); }); - it('should return null', async () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0' }, - }; + const packageManager = { + getPackageVersion: jest.fn().mockImplementation((packageName) => { + if (packageName === '@angular/core') { + return '12.0.0'; + } + + return null; + }), + } as Partial; - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + it('should return null', async () => { + await expect(checkAngularBuilders({ packageManager })).resolves.toBeNull(); }); }); describe('is not Nx project', () => { beforeEach(() => { - (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(false); + (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(false); }); describe('angular builders', () => { afterEach(jest.restoreAllMocks); describe('Angular not found', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0' }, - }; + const packageManager = { + getPackageVersion: jest.fn().mockReturnValue(null), + } as Partial; it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect(checkAngularBuilders({ packageManager })).resolves.toBeNull(); }); }); describe('Angular < 14.0.0', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName: string) => { + if (packageName === '@angular/core') { + return Promise.resolve('12.0.0'); + } + + return null; + }, + } as Partial; it('should throw an Error', async () => { - await expect(checkAngularBuilders({ packageJson })).rejects.toThrowErrorMatchingSnapshot(); + await expect( + checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } }) + ).rejects.toThrowErrorMatchingSnapshot(); }); }); describe('Angular >= 14.0.0', () => { - const packageJson = { - dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^15.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + if (packageName === '@angular/core') { + return Promise.resolve('15.0.0'); + } + + return null; + }, + } as Partial; describe('has one Storybook builder defined', () => { beforeEach(() => { @@ -89,7 +106,12 @@ describe('is not Nx project', () => { }); it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toBeNull(); }); }); @@ -107,7 +129,12 @@ describe('is not Nx project', () => { }); it('should return null', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull(); + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toBeNull(); }); }); @@ -124,7 +151,12 @@ describe('is not Nx project', () => { }); it('should proceed and return data', async () => { - await expect(checkAngularBuilders({ packageJson })).resolves.toMatchObject({ + await expect( + checkAngularBuilders({ + packageManager, + mainConfig: { framework: '@storybook/angular' }, + }) + ).resolves.toMatchObject({ mainConfig: {}, packageManager: {}, }); diff --git a/code/lib/cli/src/automigrate/fixes/angular-builders.ts b/code/lib/cli/src/automigrate/fixes/angular-builders.ts index 884db5e9f0b6..7063fcc8dcb3 100644 --- a/code/lib/cli/src/automigrate/fixes/angular-builders.ts +++ b/code/lib/cli/src/automigrate/fixes/angular-builders.ts @@ -4,10 +4,10 @@ import type { StorybookConfig } from '@storybook/types'; import chalk from 'chalk'; import prompts from 'prompts'; import type { Fix } from '../types'; -import { getStorybookData } from '../helpers/mainConfigFile'; import { isNxProject } from '../../helpers'; import { AngularJSON } from '../../generators/ANGULAR/helpers'; import type { JsPackageManager } from '../../js-package-manager'; +import { getFrameworkPackageName } from '../helpers/mainConfigFile'; interface AngularBuildersRunOptions { mainConfig: StorybookConfig; @@ -17,25 +17,21 @@ interface AngularBuildersRunOptions { export const angularBuilders: Fix = { id: 'angular-builders', - async check({ packageManager, configDir }) { - const packageJSON = await packageManager.retrievePackageJson(); + async check({ packageManager, mainConfig }) { + const angularVersion = await packageManager.getPackageVersion('@angular/core'); - // Skip in case of NX - if (isNxProject(packageJSON)) { - return null; - } - const allDependencies = await packageManager.getAllDependencies(); + const framewworkPackageName = getFrameworkPackageName(mainConfig); - const angularVersion = allDependencies['@angular/core']; - const angularCoerced = semver.coerce(angularVersion)?.version; - - // skip non-angular projects - if (!angularCoerced) { + // Skip in case of NX + if ( + !angularVersion || + (await isNxProject(packageManager)) || + framewworkPackageName !== '@storybook/angular' + ) { return null; } - // Is Angular version lower than 14? -> throw an error (only supports ng 14) - if (semver.lt(angularCoerced, '14.0.0')) { + if (semver.lt(angularVersion, '14.0.0')) { throw new Error(dedent` ❌ Your project uses Angular < 14.0.0. Storybook 7.0 for Angular requires Angular 14.0.0 or higher. Please upgrade your Angular version to at least version 14.0.0 to use Storybook 7.0 in your project. @@ -55,8 +51,6 @@ export const angularBuilders: Fix = { return null; } - const { mainConfig } = await getStorybookData({ configDir, packageManager }); - return { mainConfig, packageManager, diff --git a/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts b/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts index 1876ea7f1e0b..c21de1bd9727 100644 --- a/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts +++ b/code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts @@ -1,21 +1,19 @@ import type { StorybookConfig } from '@storybook/types'; import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import { makePackageManager } from '../helpers/testing-helpers'; import { autodocsTrue } from './autodocs-true'; const checkAutodocs = async ({ packageJson = {}, main: mainConfig, - storybookVersion = '7.0.0', }: { packageJson?: PackageJson; main: Partial & Record; - storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return autodocsTrue.check({ packageManager: makePackageManager(packageJson), + mainConfig: mainConfig as StorybookConfig, + storybookVersion: '7.0.0', }); }; diff --git a/code/lib/cli/src/automigrate/fixes/autodocs-true.ts b/code/lib/cli/src/automigrate/fixes/autodocs-true.ts index 238c44ebec61..336bd111d39a 100644 --- a/code/lib/cli/src/automigrate/fixes/autodocs-true.ts +++ b/code/lib/cli/src/automigrate/fixes/autodocs-true.ts @@ -4,7 +4,7 @@ import { dedent } from 'ts-dedent'; import type { StorybookConfig } from '@storybook/types'; import type { Fix } from '../types'; -import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; +import { updateMainConfig } from '../helpers/mainConfigFile'; const logger = console; @@ -18,9 +18,7 @@ interface AutodocsTrueFrameworkRunOptions { export const autodocsTrue: Fix = { id: 'autodocsTrue', - async check({ packageManager, configDir }) { - const { mainConfig } = await getStorybookData({ packageManager, configDir }); - + async check({ mainConfig }) { const { docs } = mainConfig; const docsPageToAutodocsMapping = { diff --git a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts index 38025e609e32..04f6b770f380 100644 --- a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts +++ b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts @@ -3,7 +3,7 @@ import type { StorybookConfig } from '@storybook/types'; import type { PackageJson } from '../../js-package-manager'; import { ansiRegex } from '../helpers/cleanLog'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import { makePackageManager } from '../helpers/testing-helpers'; import type { BareMdxStoriesGlobRunOptions } from './bare-mdx-stories-glob'; import { bareMdxStoriesGlob } from './bare-mdx-stories-glob'; @@ -16,10 +16,10 @@ const checkBareMdxStoriesGlob = async ({ main?: Partial & Record; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return bareMdxStoriesGlob.check({ + mainConfig: mainConfig as StorybookConfig, packageManager: makePackageManager(packageJson), + storybookVersion, }); }; diff --git a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts index 4091ee9f7ed9..b4f81f08a8fb 100644 --- a/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts +++ b/code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts @@ -2,7 +2,7 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import semver from 'semver'; import type { StoriesEntry } from '@storybook/types'; -import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; +import { updateMainConfig } from '../helpers/mainConfigFile'; import type { Fix } from '../types'; const logger = console; @@ -31,12 +31,7 @@ const getNextGlob = (glob: string) => { export const bareMdxStoriesGlob: Fix = { id: 'bare-mdx-stories-glob', - async check({ packageManager, configDir }) { - const { storybookVersion, mainConfig } = await getStorybookData({ - configDir, - packageManager, - }); - + async check({ storybookVersion, mainConfig }) { if (!semver.gte(storybookVersion, '7.0.0')) { return null; } diff --git a/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts b/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts index 2f54eb0dce7e..20b8f935c0e7 100644 --- a/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts +++ b/code/lib/cli/src/automigrate/fixes/builder-vite.test.ts @@ -1,21 +1,19 @@ import type { StorybookConfig } from '@storybook/types'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import { makePackageManager } from '../helpers/testing-helpers'; import type { PackageJson } from '../../js-package-manager'; import { builderVite } from './builder-vite'; const checkBuilderVite = async ({ packageJson = {}, main: mainConfig, - storybookVersion = '7.0.0', }: { packageJson?: PackageJson; main: Partial & Record; - storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return builderVite.check({ + mainConfig: mainConfig as StorybookConfig, packageManager: makePackageManager(packageJson), + storybookVersion: '7.0.0', }); }; diff --git a/code/lib/cli/src/automigrate/fixes/builder-vite.ts b/code/lib/cli/src/automigrate/fixes/builder-vite.ts index 9fef70b8b9d6..b7ee1317957c 100644 --- a/code/lib/cli/src/automigrate/fixes/builder-vite.ts +++ b/code/lib/cli/src/automigrate/fixes/builder-vite.ts @@ -5,7 +5,7 @@ import { writeConfig } from '@storybook/csf-tools'; import type { Fix } from '../types'; import type { PackageJson } from '../../js-package-manager'; -import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; +import { updateMainConfig } from '../helpers/mainConfigFile'; const logger = console; @@ -26,9 +26,8 @@ interface BuilderViteOptions { export const builderVite: Fix = { id: 'builder-vite', - async check({ configDir, packageManager }) { + async check({ packageManager, mainConfig }) { const packageJson = await packageManager.retrievePackageJson(); - const { mainConfig } = await getStorybookData({ configDir, packageManager }); const builder = mainConfig.core?.builder; const builderName = typeof builder === 'string' ? builder : builder?.name; diff --git a/code/lib/cli/src/automigrate/fixes/cra5.test.ts b/code/lib/cli/src/automigrate/fixes/cra5.test.ts index 24efd73fbc31..15ef6485851d 100644 --- a/code/lib/cli/src/automigrate/fixes/cra5.test.ts +++ b/code/lib/cli/src/automigrate/fixes/cra5.test.ts @@ -1,21 +1,20 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { cra5 } from './cra5'; const checkCra5 = async ({ - packageJson, + packageManager, main: mainConfig, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; + packageManager: any; main?: Partial & Record; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return cra5.check({ - packageManager: makePackageManager(packageJson), + packageManager, + mainConfig: mainConfig as StorybookConfig, + storybookVersion, }); }; @@ -24,24 +23,28 @@ describe('cra5 fix', () => { describe('sb < 6.3', () => { describe('cra5 dependency', () => { - const packageJson = { - dependencies: { '@storybook/react': '^6.2.0', 'react-scripts': '^5.0.0' }, - }; + const packageManager = { + getPackageVersion: jest.fn().mockResolvedValue('5.0.0'), + } as Partial; + it('should fail', async () => { await expect( checkCra5({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); }); describe('no cra5 dependency', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + const packageManager = { + getPackageVersion: jest.fn().mockResolvedValue(null), + } as Partial; + it('should no-op', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: {}, }) ).resolves.toBeFalsy(); @@ -50,14 +53,17 @@ describe('cra5 fix', () => { }); describe('sb 6.3 - 7.0', () => { describe('cra5 dependency', () => { - const packageJson = { - dependencies: { '@storybook/react': '^6.3.0', 'react-scripts': '^5.0.0' }, - }; + const packageManager = { + getPackageVersion: () => { + return Promise.resolve('5.0.0'); + }, + } as Partial; + describe('webpack5 builder', () => { it('should no-op', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: { core: { builder: 'webpack5' } }, }) ).resolves.toBeFalsy(); @@ -67,7 +73,7 @@ describe('cra5 fix', () => { it('should no-op', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: { core: { builder: 'storybook-builder-vite' } }, }) ).resolves.toBeFalsy(); @@ -77,12 +83,12 @@ describe('cra5 fix', () => { it('should add webpack5 builder', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: { core: { builder: 'webpack4' } }, storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ - craVersion: '^5.0.0', + craVersion: '5.0.0', storybookVersion: '6.3.0', }); }); @@ -91,36 +97,44 @@ describe('cra5 fix', () => { it('should add webpack5 builder', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: {}, storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ - craVersion: '^5.0.0', + craVersion: '5.0.0', storybookVersion: '6.3.0', }); }); }); }); describe('no cra dependency', () => { + const packageManager = { + getPackageVersion: () => { + return null; + }, + } as Partial; + it('should no-op', async () => { await expect( checkCra5({ - packageJson: {}, + packageManager, main: {}, }) ).resolves.toBeFalsy(); }); }); describe('cra4 dependency', () => { + const packageManager = { + getPackageVersion: () => { + return Promise.resolve('4.0.0'); + }, + } as Partial; + it('should no-op', async () => { await expect( checkCra5({ - packageJson: { - dependencies: { - 'react-scripts': '4', - }, - }, + packageManager, main: {}, }) ).resolves.toBeFalsy(); @@ -129,13 +143,16 @@ describe('cra5 fix', () => { }); describe('sb 7.0+', () => { describe('cra5 dependency', () => { - const packageJson = { - dependencies: { '@storybook/react': '^7.0.0-alpha.0', 'react-scripts': '^5.0.0' }, - }; + const packageManager = { + getPackageVersion: () => { + return Promise.resolve('5.0.0'); + }, + } as Partial; + it('should no-op', async () => { await expect( checkCra5({ - packageJson, + packageManager, main: {}, }) ).resolves.toBeFalsy(); diff --git a/code/lib/cli/src/automigrate/fixes/cra5.ts b/code/lib/cli/src/automigrate/fixes/cra5.ts index 1280a5de3155..d3786cd2d00f 100644 --- a/code/lib/cli/src/automigrate/fixes/cra5.ts +++ b/code/lib/cli/src/automigrate/fixes/cra5.ts @@ -20,16 +20,14 @@ interface CRA5RunOptions { export const cra5: Fix = { id: 'cra5', - async check({ packageManager, configDir }) { - const allDependencies = await packageManager.getAllDependencies(); - const craVersion = allDependencies['react-scripts']; - const craCoerced = semver.coerce(craVersion)?.version; + async check({ packageManager, mainConfig, storybookVersion }) { + const craVersion = await packageManager.getPackageVersion('react-scripts'); - if (!craCoerced || semver.lt(craCoerced, '5.0.0')) { + if (!craVersion || semver.lt(craVersion, '5.0.0')) { return null; } - const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); + const builderInfo = await checkWebpack5Builder({ mainConfig, storybookVersion }); return builderInfo ? { craVersion, ...builderInfo } : null; }, diff --git a/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts b/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts index 122cb7f7e4f7..05b73100cff3 100644 --- a/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts +++ b/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts @@ -1,27 +1,26 @@ /// ; import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { incompatibleAddons } from './incompatible-addons'; import * as packageVersions from '../helpers/getActualPackageVersions'; +import type { JsPackageManager } from '../../js-package-manager'; jest.mock('../helpers/getActualPackageVersions'); const check = async ({ - packageJson, + packageManager, main: mainConfig = {}, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; + packageManager: Partial; main?: Partial & Record; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return incompatibleAddons.check({ - packageManager: makePackageManager(packageJson), + packageManager: packageManager as any, configDir: '', + mainConfig: mainConfig as any, + storybookVersion, }); }; @@ -42,14 +41,11 @@ describe('incompatible-addons fix', () => { ]) ); - const packageJson = { - dependencies: { - '@storybook/addon-essentials': '^7.0.0', - '@storybook/addon-info': '^6.0.0', - }, - }; await expect( - check({ packageJson, main: { addons: ['@storybook/essentials', '@storybook/addon-info'] } }) + check({ + packageManager: {}, + main: { addons: ['@storybook/essentials', '@storybook/addon-info'] }, + }) ).resolves.toEqual({ incompatibleAddonList: [ { @@ -70,11 +66,8 @@ describe('incompatible-addons fix', () => { ]) ); - const packageJson = { - dependencies: { '@storybook/addon-essentials': '^7.0.0' }, - }; await expect( - check({ packageJson, main: { addons: ['@storybook/essentials'] } }) + check({ packageManager: {}, main: { addons: ['@storybook/essentials'] } }) ).resolves.toBeNull(); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts b/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts index 5620da7d45e1..bb5a45accd19 100644 --- a/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts +++ b/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts @@ -1,7 +1,6 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import type { Fix } from '../types'; -import { getStorybookData } from '../helpers/mainConfigFile'; import { getIncompatibleAddons } from '../helpers/getIncompatibleAddons'; interface IncompatibleAddonsOptions { @@ -12,12 +11,7 @@ export const incompatibleAddons: Fix = { id: 'incompatible-addons', promptOnly: true, - async check({ packageManager, configDir }) { - const { mainConfig } = await getStorybookData({ - packageManager, - configDir, - }); - + async check({ mainConfig }) { const incompatibleAddonList = await getIncompatibleAddons(mainConfig); return incompatibleAddonList.length > 0 ? { incompatibleAddonList } : null; diff --git a/code/lib/cli/src/automigrate/fixes/index.ts b/code/lib/cli/src/automigrate/fixes/index.ts index b38f196bfe0e..4de0e7d63689 100644 --- a/code/lib/cli/src/automigrate/fixes/index.ts +++ b/code/lib/cli/src/automigrate/fixes/index.ts @@ -23,6 +23,7 @@ export * from '../types'; export const allFixes: Fix[] = [ nodeJsRequirement, + newFrameworks, cra5, webpack5, vue3, @@ -30,7 +31,6 @@ export const allFixes: Fix[] = [ builderVite, sbBinary, sbScripts, - newFrameworks, incompatibleAddons, removedGlobalClientAPIs, mdx1to2, diff --git a/code/lib/cli/src/automigrate/fixes/mdx-gfm.test.ts b/code/lib/cli/src/automigrate/fixes/mdx-gfm.test.ts index 55ec1971e7c3..e7d7f0c2a30a 100644 --- a/code/lib/cli/src/automigrate/fixes/mdx-gfm.test.ts +++ b/code/lib/cli/src/automigrate/fixes/mdx-gfm.test.ts @@ -1,6 +1,4 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { mdxgfm } from './mdx-gfm'; jest.mock('globby', () => ({ @@ -9,28 +7,27 @@ jest.mock('globby', () => ({ })); const check = async ({ - packageJson, + packageManager, main: mainConfig, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; + packageManager: any; main: Partial & Record; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return mdxgfm.check({ - packageManager: makePackageManager(packageJson), + packageManager, configDir: '', + mainConfig: mainConfig as any, + storybookVersion, }); }; describe('no-ops', () => { - const packageJson = {}; test('sb > 7.0', async () => { await expect( check({ - packageJson, + packageManager: {}, main: {}, storybookVersion: '6.2.0', }) @@ -39,7 +36,7 @@ describe('no-ops', () => { test('legacyMdx1', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { features: { legacyMdx1: true, @@ -51,7 +48,7 @@ describe('no-ops', () => { test('with addon docs setup', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { addons: [ { @@ -78,7 +75,7 @@ describe('no-ops', () => { test('with addon migration assistant addon added', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { addons: ['@storybook/addon-mdx-gfm'], }, @@ -88,11 +85,10 @@ describe('no-ops', () => { }); describe('continue', () => { - const packageJson = {}; test('nothing configured at all', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { stories: ['**/*.stories.mdx'], }, @@ -102,7 +98,7 @@ describe('continue', () => { test('unconfigured addon-docs', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { stories: ['**/*.stories.mdx'], addons: [ @@ -124,7 +120,7 @@ describe('continue', () => { test('unconfigured addon-essentials', async () => { await expect( check({ - packageJson, + packageManager: {}, main: { stories: ['**/*.stories.mdx'], addons: ['@storybook/addon-essentials'], diff --git a/code/lib/cli/src/automigrate/fixes/mdx-gfm.ts b/code/lib/cli/src/automigrate/fixes/mdx-gfm.ts index b8c215feef06..d138f9578f47 100644 --- a/code/lib/cli/src/automigrate/fixes/mdx-gfm.ts +++ b/code/lib/cli/src/automigrate/fixes/mdx-gfm.ts @@ -4,7 +4,7 @@ import { join } from 'path'; import slash from 'slash'; import glob from 'globby'; import { commonGlobOptions } from '@storybook/core-common'; -import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; +import { updateMainConfig } from '../helpers/mainConfigFile'; import type { Fix } from '../types'; import { getStorybookVersionSpecifier } from '../../helpers'; @@ -19,9 +19,7 @@ interface Options { export const mdxgfm: Fix = { id: 'github-flavored-markdown-mdx', - async check({ configDir, packageManager }) { - const { mainConfig, storybookVersion } = await getStorybookData({ packageManager, configDir }); - + async check({ configDir, mainConfig, storybookVersion }) { if (!semver.gte(storybookVersion, '7.0.0')) { return null; } diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts index 6ce9aa71cab2..a325bb471926 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts @@ -2,9 +2,8 @@ /// ; import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import { missingBabelRc } from './missing-babelrc'; +import type { JsPackageManager } from '../../js-package-manager'; // eslint-disable-next-line global-require, jest/no-mocks-import jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); @@ -27,12 +26,14 @@ const babelContent = JSON.stringify({ }); const check = async ({ - packageJson = {}, - main: mainConfig, + packageManager = { + retrievePackageJson: () => ({}), + }, + main: mainConfig = {}, storybookVersion = '7.0.0', extraFiles, }: { - packageJson?: PackageJson; + packageManager?: any; main?: Partial & Record; storybookVersion?: string; extraFiles?: Record; @@ -42,11 +43,30 @@ const check = async ({ require('fs-extra').__setMockFiles(extraFiles); } - mockStorybookData({ mainConfig, storybookVersion }); - - return missingBabelRc.check({ packageManager: makePackageManager(packageJson) }); + return missingBabelRc.check({ + packageManager, + mainConfig: mainConfig as any, + storybookVersion, + }); }; +const packageManager = { + retrievePackageJson: () => + Promise.resolve({ + devDependencies: {}, + dependencies: {}, + }), +} as Partial; + +const packageManagerWithBabelField = { + retrievePackageJson: () => + Promise.resolve({ + devDependencies: {}, + dependencies: {}, + babel: babelContent, + }), +} as Partial; + describe('missing-babelrc fix', () => { afterEach(jest.restoreAllMocks); @@ -55,84 +75,58 @@ describe('missing-babelrc fix', () => { }); it('skips when babelrc config is present', async () => { - const packageJson = { - devDependencies: { - '@storybook/react': '^7.0.0', - '@storybook/react-webpack5': '^7.0.0', - }, - }; - // different babel extensions await expect( check({ + packageManager, extraFiles: { '.babelrc': babelContent }, - packageJson, main: { framework: '@storybook/react' }, }) ).resolves.toBeNull(); await expect( check({ + packageManager, extraFiles: { '.babelrc.json': babelContent }, - packageJson, main: { framework: '@storybook/react' }, }) ).resolves.toBeNull(); await expect( check({ + packageManager, extraFiles: { 'babel.config.json': babelContent }, - packageJson, main: { framework: '@storybook/react' }, }) ).resolves.toBeNull(); - // babel field in package.json await expect( check({ - packageJson: { ...packageJson, babel: babelContent }, + packageManager: packageManagerWithBabelField, main: { framework: '@storybook/react' }, }) ).resolves.toBeNull(); }); it('skips when using a framework that provides babel config', async () => { - const packageJson = { - devDependencies: { - '@storybook/react': '^7.0.0', - '@storybook/nextjs': '^7.0.0', - }, - }; - await expect( - check({ packageJson, main: { framework: '@storybook/nextjs' } }) + check({ main: { framework: '@storybook/nextjs' }, packageManager }) ).resolves.toBeNull(); }); it('skips when using CRA preset', async () => { - const packageJson = { - devDependencies: { - '@storybook/react': '^7.0.0', - '@storybook/react-webpack5': '^7.0.0', - }, - }; - await expect( check({ - packageJson, main: { framework: '@storybook/react', addons: ['@storybook/preset-create-react-app'] }, + packageManager, }) ).resolves.toBeNull(); }); it('prompts when babelrc file is missing and framework does not provide babel config', async () => { - const packageJson = { - devDependencies: { - '@storybook/react': '^7.0.0', - '@storybook/react-webpack5': '^7.0.0', - }, - }; - await expect( - check({ main: { framework: '@storybook/react-webpack5' }, packageJson }) + check({ + packageManager, + main: { framework: '@storybook/react-webpack5' }, + }) ).resolves.toEqual({ needsBabelRc: true, }); diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts index 1b403a6b0d4b..5c70808657a6 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts @@ -4,7 +4,7 @@ import semver from 'semver'; import { loadPartialConfigAsync } from '@babel/core'; import type { Fix } from '../types'; import { generateStorybookBabelConfigInCWD } from '../../babel-config'; -import { getStorybookData } from '../helpers/mainConfigFile'; +import { getFrameworkPackageName } from '../helpers/mainConfigFile'; interface MissingBabelRcOptions { needsBabelRc: boolean; @@ -23,24 +23,28 @@ const frameworksThatNeedBabelConfig = [ export const missingBabelRc: Fix = { id: 'missing-babelrc', - async check({ configDir, packageManager }) { + async check({ packageManager, mainConfig, storybookVersion }) { const packageJson = await packageManager.retrievePackageJson(); - const { mainConfig, storybookVersion } = await getStorybookData({ configDir, packageManager }); if (!semver.gte(storybookVersion, '7.0.0')) { return null; } - const { framework, addons } = mainConfig; - - const frameworkPackage = typeof framework === 'string' ? framework : framework.name; + const { addons } = mainConfig; const hasCraPreset = - addons && addons.find((addon) => addon === '@storybook/preset-create-react-app'); + addons && + addons.find((addon) => + typeof addon === 'string' + ? addon.endsWith('@storybook/preset-create-react-app') + : addon.name.endsWith('@storybook/preset-create-react-app') + ); + + const frameworkPackageName = getFrameworkPackageName(mainConfig); if ( - frameworkPackage && - frameworksThatNeedBabelConfig.includes(frameworkPackage) && + frameworkPackageName && + frameworksThatNeedBabelConfig.includes(frameworkPackageName) && !hasCraPreset ) { const config = await loadPartialConfigAsync({ 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 0938511e53cf..441af07c311f 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.test.ts @@ -1,9 +1,8 @@ import type { StorybookConfig } from '@storybook/types'; import * as findUp from 'find-up'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; import * as rendererHelpers from '../helpers/detectRenderer'; import { newFrameworks } from './new-frameworks'; +import type { JsPackageManager } from '../../js-package-manager'; jest.mock('find-up'); jest.mock('../helpers/detectRenderer', () => ({ @@ -11,40 +10,70 @@ jest.mock('../helpers/detectRenderer', () => ({ })); const checkNewFrameworks = async ({ - packageJson, + packageManager = {}, main: mainConfig, storybookVersion = '7.0.0', + rendererPackage, }: { - packageJson: PackageJson; - main: Partial & Record; + packageManager?: any; + main?: Partial & Record; storybookVersion?: string; + rendererPackage?: string; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return newFrameworks.check({ - packageManager: makePackageManager(packageJson), + packageManager, + mainConfig: mainConfig as any, + storybookVersion, + rendererPackage, configDir: '', }); }; +const getPackageManager = (packages: Record) => { + return { + getPackageVersion(packageName) { + return new Promise((resolve) => { + Object.entries(packages).forEach(([name, version]) => { + if (packageName === name) { + resolve(version); + } + }); + + resolve(null); + }); + }, + retrievePackageJson: () => + Promise.resolve({ + dependencies: {}, + devDependencies: packages, + }), + getAllDependencies: () => Promise.resolve(packages), + } as Partial; +}; + describe('new-frameworks fix', () => { describe('should no-op', () => { it('in sb < 7', async () => { - const packageJson = { dependencies: { '@storybook/vue': '^6.2.0' } }; + const packageManager = getPackageManager({ + '@storybook/vue': '6.2.0', + }); + await expect( checkNewFrameworks({ - packageJson, - main: {}, + packageManager, storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); }); it('in sb 7 with correct structure already', async () => { - const packageJson = { dependencies: { '@storybook/angular': '^7.0.0' } }; + const packageManager = getPackageManager({ + '@storybook/angular': '7.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/angular', }, @@ -55,26 +84,24 @@ describe('new-frameworks fix', () => { describe('should throw an error', () => { it('in sb 7 with no main.js', async () => { - const packageJson = { dependencies: { '@storybook/vue': '^7.0.0' } }; await expect(() => checkNewFrameworks({ - packageJson, main: undefined, }) ).rejects.toBeTruthy(); }); it('in sb 7 with vite < 3', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0', - '@storybook/builder-vite': 'x.y.z', - vite: '^2.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0', + '@storybook/builder-vite': 'x.y.z', + vite: '2.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, + rendererPackage: '@storybook/react', main: { framework: '@storybook/react', core: { @@ -88,16 +115,15 @@ describe('new-frameworks fix', () => { describe('generic new-frameworks migration', () => { 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', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/builder-webpack5': '6.5.9', + '@storybook/manager-webpack5': '6.5.9', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/react', core: { @@ -133,16 +159,15 @@ describe('new-frameworks fix', () => { }); it('should update to @storybook/react-vite', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/builder-vite': '^0.0.2', - vite: '3.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/builder-vite': '0.0.2', + vite: '3.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/react', core: { @@ -160,16 +185,15 @@ 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', - }, - }; + const packageManager = getPackageManager({ + '@storybook/angular': '7.0.0-alpha.0', + '@storybook/builder-webpack5': '6.5.9', + '@storybook/manager-webpack5': '6.5.9', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/angular', core: { @@ -206,16 +230,15 @@ describe('new-frameworks fix', () => { }); it('should update to @storybook/preact-vite', async () => { - const packageJson = { - dependencies: { - '@storybook/preact': '^7.0.0-alpha.0', - '@storybook/builder-vite': '^0.0.2', - vite: '3.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/preact': '7.0.0-alpha.0', + '@storybook/builder-vite': '0.0.2', + vite: '3.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/preact', core: { @@ -233,12 +256,14 @@ describe('new-frameworks fix', () => { }); it('should update correctly when there is no builder', async () => { - const packageJson = { - dependencies: { '@storybook/vue': '^7.0.0', '@storybook/builder-webpack5': '^7.0.0' }, - }; + const packageManager = getPackageManager({ + '@storybook/vue': '7.0.0', + '@storybook/builder-webpack5': '7.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/vue', }, @@ -253,12 +278,14 @@ describe('new-frameworks fix', () => { }); it('should update when there is no framework field in main', async () => { - const packageJson = { - dependencies: { '@storybook/vue': '^7.0.0', '@storybook/manager-webpack5': '^7.0.0' }, - }; + const packageManager = getPackageManager({ + '@storybook/vue': '7.0.0', + '@storybook/manager-webpack5': '7.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: {}, }) ).resolves.toEqual( @@ -272,12 +299,14 @@ describe('new-frameworks fix', () => { }); it('should update when the framework field has a legacy value', async () => { - const packageJson = { - dependencies: { '@storybook/vue': '^7.0.0', '@storybook/manager-webpack5': '^7.0.0' }, - }; + const packageManager = getPackageManager({ + '@storybook/vue': '7.0.0', + '@storybook/manager-webpack5': '7.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: 'vue', }, @@ -296,16 +325,16 @@ describe('new-frameworks fix', () => { // there should be a prompt, which we mock the response const detectRendererSpy = jest.spyOn(rendererHelpers, 'detectRenderer'); detectRendererSpy.mockReturnValueOnce(Promise.resolve('@storybook/react')); - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0', - '@storybook/vue': '^7.0.0', - '@storybook/builder-vite': 'x.y.z', - }, - }; + + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0', + '@storybook/vue': '7.0.0', + '@storybook/builder-vite': 'x.y.z', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { core: { builder: '@storybook/builder-vite', @@ -322,18 +351,16 @@ describe('new-frameworks fix', () => { }); it('should add framework field in main.js when everything is properly configured, but framework field in main.js is missing', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/react-vite': '^7.0.0-alpha.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/react-vite': '7.0.0-alpha.0', + }); // project contains vite.config.js jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('vite.config.js')); await expect( checkNewFrameworks({ - packageJson, + packageManager, main: {}, }) ).resolves.toEqual( @@ -348,21 +375,21 @@ describe('new-frameworks fix', () => { jest .spyOn(rendererHelpers, 'detectRenderer') .mockReturnValueOnce(Promise.resolve('@storybook/web-components')); - const packageJson = { - dependencies: { - '@storybook/addon-essentials': '^7.0.0-beta.48', - '@storybook/vue': '^7.0.0-beta.48', - '@storybook/builder-vite': '^7.0.0-beta.48', - '@storybook/builder-webpack5': '^7.0.0-beta.48', - '@storybook/core-server': '^7.0.0-beta.48', - '@storybook/manager-webpack5': '^6.5.15', - '@storybook/react': '^7.0.0-beta.48', - '@storybook/web-components': '^7.0.0-beta.48', - }, - }; + + const packageManager = getPackageManager({ + '@storybook/addon-essentials': '7.0.0-beta.48', + '@storybook/vue': '7.0.0-beta.48', + '@storybook/builder-vite': '7.0.0-beta.48', + '@storybook/builder-webpack5': '7.0.0-beta.48', + '@storybook/core-server': '7.0.0-beta.48', + '@storybook/manager-webpack5': '6.5.15', + '@storybook/react': '7.0.0-beta.48', + '@storybook/web-components': '7.0.0-beta.48', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { core: { builder: 'webpack5' }, }, @@ -378,61 +405,57 @@ describe('new-frameworks fix', () => { describe('nextjs migration', () => { it('skips in non-Next.js projects', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0', - '@storybook/react-vite': '^7.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0', + '@storybook/react-vite': '7.0.0', + }); + const main = { framework: '@storybook/react-vite', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('skips if project uses Next.js < 12.0.0', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0', - '@storybook/react-webpack5': '^7.0.0', - next: '^11.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0', + '@storybook/react-webpack5': '7.0.0', + next: '11.0.0', + }); + const main = { framework: '@storybook/react', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('skips if project already has @storybook/nextjs set up', async () => { jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('next.config.js')); - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0', - '@storybook/nextjs': '^7.0.0', - next: '^12.0.0', - }, - }; + + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0', + '@storybook/nextjs': '7.0.0', + next: '12.0.0', + }); + const main = { framework: '@storybook/nextjs', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('should update from @storybook/react-webpack5 to @storybook/nextjs', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/react-webpack5': '^7.0.0-alpha.0', - '@storybook/builder-webpack5': '^7.0.0-alpha.0', - next: '^12.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/react-webpack5': '7.0.0-alpha.0', + '@storybook/builder-webpack5': '7.0.0-alpha.0', + next: '12.0.0', + }); jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('next.config.js')); await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: { name: '@storybook/react-webpack5', options: {} }, }, @@ -448,18 +471,17 @@ describe('new-frameworks fix', () => { it('should remove legacy addons', async () => { jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('next.config.js')); - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/react-webpack5': '^7.0.0-alpha.0', - next: '^12.0.0', - 'storybook-addon-next': '^1.0.0', - 'storybook-addon-next-router': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/react-webpack5': '7.0.0-alpha.0', + next: '12.0.0', + 'storybook-addon-next': '1.0.0', + 'storybook-addon-next-router': '1.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: '@storybook/react-webpack5', addons: ['storybook-addon-next', 'storybook-addon-next-router'], @@ -479,17 +501,17 @@ describe('new-frameworks fix', () => { it('should move storybook-addon-next options and reactOptions to frameworkOptions', async () => { jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('next.config.js')); - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/react-webpack5': '^7.0.0-alpha.0', - next: '^12.0.0', - 'storybook-addon-next': '^1.0.0', - }, - }; + + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/react-webpack5': '7.0.0-alpha.0', + next: '12.0.0', + 'storybook-addon-next': '1.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { framework: { name: '@storybook/react-webpack5', options: { fastRefresh: true } }, addons: [ @@ -528,17 +550,17 @@ describe('new-frameworks fix', () => { it('should migrate to @storybook/react-vite in Next.js project that uses vite builder', async () => { jest.spyOn(findUp, 'default').mockReturnValueOnce(Promise.resolve('next.config.js')); - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - '@storybook/builder-vite': '^7.0.0-alpha.0', - next: '^12.0.0', - 'storybook-addon-next': '^1.0.0', - }, - }; + + const packageManager = getPackageManager({ + '@storybook/react': '7.0.0-alpha.0', + '@storybook/builder-vite': '7.0.0-alpha.0', + next: '12.0.0', + 'storybook-addon-next': '1.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { core: { builder: '@storybook/builder-vite', @@ -558,59 +580,55 @@ describe('new-frameworks fix', () => { describe('SvelteKit migration', () => { it('skips in non-SvelteKit projects', async () => { - const packageJson = { - dependencies: { - svelte: '^3.53.1', - '@storybook/svelte': '^7.0.0', - '@storybook/svelte-vite': '^7.0.0', - }, - }; + const packageManager = getPackageManager({ + svelte: '3.53.1', + '@storybook/svelte': '7.0.0', + '@storybook/svelte-vite': '7.0.0', + }); + const main = { framework: '@storybook/svelte-vite', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('skips if project uses SvelteKit < 1.0.0', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - '@storybook/svelte-vite': '^7.0.0', - '@sveltejs/kit': '^0.9.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + '@storybook/svelte-vite': '7.0.0', + '@sveltejs/kit': '0.9.0', + }); + const main = { framework: '@storybook/svelte-vite', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('skips if project already has @storybook/sveltekit set up', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - '@storybook/sveltekit': '^7.0.0', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + '@storybook/sveltekit': '7.0.0', + '@sveltejs/kit': '1.0.0', + }); + const main = { framework: '@storybook/svelte-vite', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toBeFalsy(); + await expect(checkNewFrameworks({ packageManager, main })).resolves.toBeFalsy(); }); it('from @storybook/svelte-vite', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - '@storybook/svelte-vite': '^7.0.0', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + '@storybook/svelte-vite': '7.0.0', + '@sveltejs/kit': '1.0.0', + }); + const main = { framework: '@storybook/svelte-vite', }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toEqual( + await expect(checkNewFrameworks({ packageManager, main })).resolves.toEqual( expect.objectContaining({ dependenciesToAdd: ['@storybook/sveltekit'], dependenciesToRemove: ['@storybook/svelte-vite'], @@ -620,18 +638,17 @@ describe('new-frameworks fix', () => { }); it('from @storybook/svelte framework and @storybook/builder-vite builder', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - '@storybook/builder-vite': '^7.0.0', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + '@storybook/builder-vite': '7.0.0', + '@sveltejs/kit': '1.0.0', + }); + const main = { framework: '@storybook/svelte', core: { builder: '@storybook/builder-vite' }, }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toEqual( + await expect(checkNewFrameworks({ packageManager, main })).resolves.toEqual( expect.objectContaining({ dependenciesToAdd: ['@storybook/sveltekit'], dependenciesToRemove: ['@storybook/builder-vite'], @@ -641,18 +658,17 @@ describe('new-frameworks fix', () => { }); it('from @storybook/svelte framework and storybook-builder-vite builder', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - 'storybook-builder-vite': '^0.2.5', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + 'storybook-builder-vite': '0.2.5', + '@sveltejs/kit': '1.0.0', + }); + const main = { framework: '@storybook/svelte', core: { builder: 'storybook-builder-vite' }, }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toEqual( + await expect(checkNewFrameworks({ packageManager, main })).resolves.toEqual( expect.objectContaining({ dependenciesToAdd: ['@storybook/sveltekit'], dependenciesToRemove: ['storybook-builder-vite'], @@ -662,19 +678,18 @@ describe('new-frameworks fix', () => { }); it('should migrate and remove svelteOptions', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0', - 'storybook-builder-vite': '^0.2.5', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0', + 'storybook-builder-vite': '0.2.5', + '@sveltejs/kit': '1.0.0', + }); + const main = { framework: '@storybook/svelte', core: { builder: 'storybook-builder-vite' }, svelteOptions: { preprocess: 'preprocess' }, }; - await expect(checkNewFrameworks({ packageJson, main })).resolves.toEqual( + await expect(checkNewFrameworks({ packageManager, main })).resolves.toEqual( expect.objectContaining({ dependenciesToAdd: ['@storybook/sveltekit'], dependenciesToRemove: ['storybook-builder-vite'], @@ -685,16 +700,15 @@ describe('new-frameworks fix', () => { }); it('should migrate to @storybook/svelte-webpack5 in SvelteKit project that uses Webpack5 builder', async () => { - const packageJson = { - dependencies: { - '@storybook/svelte': '^7.0.0-alpha.0', - '@storybook/builder-webpack5': '^7.0.0-alpha.0', - '@sveltejs/kit': '^1.0.0', - }, - }; + const packageManager = getPackageManager({ + '@storybook/svelte': '7.0.0-alpha.0', + '@storybook/builder-webpack5': '7.0.0-alpha.0', + '@sveltejs/kit': '1.0.0', + }); + await expect( checkNewFrameworks({ - packageJson, + packageManager, main: { core: { builder: '@storybook/builder-webpack5', diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index d2be64bf877a..cecf3f852df3 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -1,32 +1,27 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; -import findUp from 'find-up'; import semver from 'semver'; import { frameworkPackages, rendererPackages } from '@storybook/core-common'; import type { Preset } from '@storybook/types'; import type { Fix } from '../types'; -import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; import { getStorybookVersionSpecifier } from '../../helpers'; -import { detectRenderer } from '../helpers/detectRenderer'; import { getNextjsAddonOptions, detectBuilderInfo, packagesMap, } from '../helpers/new-frameworks-utils'; -import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile'; +import { + getFrameworkPackageName, + getRendererPackageNameFromFramework, + updateMainConfig, +} from '../helpers/mainConfigFile'; +import { detectRenderer } from '../helpers/detectRenderer'; const logger = console; -const nextJsConfigFiles = [ - 'next.config.js', - 'next.config.cjs', - 'next.config.mjs', - 'next.config.ts', -]; interface NewFrameworkRunOptions { mainConfigPath: string; - packageJson: PackageJsonWithDepsAndDevDeps; dependenciesToAdd: string[]; dependenciesToRemove: string[]; hasFrameworkInMainConfig: boolean; @@ -63,48 +58,37 @@ export const newFrameworks: Fix = { id: 'new-frameworks', async check({ - rendererPackage: userDefinedRendererPackage, - configDir: userDefinedConfigDir, + configDir, packageManager, + storybookVersion, + mainConfig, + mainConfigPath, + rendererPackage, }) { - const packageJson = await packageManager.retrievePackageJson(); - const { storybookVersion, mainConfig, mainConfigPath, configDir } = await getStorybookData({ - packageManager, - configDir: userDefinedConfigDir, - }); - if (!semver.gte(storybookVersion, '7.0.0')) { return null; } - const frameworkPackage = - typeof mainConfig.framework === 'string' ? mainConfig.framework : mainConfig.framework?.name; - let hasFrameworkInMainConfig = !!frameworkPackage; - - // if --renderer is passed to the command, just use it. - // Useful for monorepo projects to automate the script without getting prompts - let rendererPackage = userDefinedRendererPackage; - if (!rendererPackage) { - if (frameworkPackage && Object.keys(rendererPackages).includes(frameworkPackage)) { - // at some point in 6.4 we introduced a framework field, but filled with a renderer package - rendererPackage = frameworkPackage; - } else if (frameworkPackage && Object.values(rendererPackages).includes(frameworkPackage)) { - // for scenarios where the value is e.g. "react" instead of "@storybook/react" - rendererPackage = Object.keys(rendererPackages).find( - (k) => rendererPackages[k] === frameworkPackage - ); - hasFrameworkInMainConfig = false; - } else { - // detect the renderer package from the user's dependencies, and if multiple are there (monorepo), prompt the user to choose - rendererPackage = await detectRenderer(packageJson); - } + const packageJson = await packageManager.retrievePackageJson(); + + const frameworkPackageName = getFrameworkPackageName(mainConfig); + + const rendererPackageName = + rendererPackage ?? + (await getRendererPackageNameFromFramework(frameworkPackageName)) ?? + (await detectRenderer(packageJson)); + + let hasFrameworkInMainConfig = !!frameworkPackageName; + + if (frameworkPackageName && !!Object.values(rendererPackages).includes(frameworkPackageName)) { + hasFrameworkInMainConfig = false; } const builderConfig = mainConfig.core?.builder; // bail if we can't detect an official renderer const supportedPackages = Object.keys(packagesMap); - if (!supportedPackages.includes(rendererPackage)) { + if (!supportedPackages.includes(rendererPackageName)) { return null; } @@ -113,17 +97,16 @@ export const newFrameworks: Fix = { const builderInfo = await detectBuilderInfo({ mainConfig, configDir, - packageDependencies: allDependencies, + packageManager, }); // if the user has a new framework already, use it let newFrameworkPackage = Object.keys(frameworkPackages).find( - (pkg) => pkg === frameworkPackage + (pkg) => pkg === frameworkPackageName ); if (!newFrameworkPackage) { - newFrameworkPackage = - packagesMap[rendererPackage] && packagesMap[rendererPackage][builderInfo.name]; + newFrameworkPackage = packagesMap[rendererPackageName]?.[builderInfo.name]; } // bail if there is no framework that matches the renderer + builder @@ -131,7 +114,7 @@ export const newFrameworks: Fix = { return null; } - const renderer = rendererPackages[rendererPackage]; + const renderer = rendererPackages[rendererPackageName]; // @ts-expect-error account for renderer options for packages that supported it: reactOptions, angularOptions. (svelteOptions got removed) let rendererOptions = mainConfig[`${renderer}Options`] || {}; @@ -151,11 +134,15 @@ export const newFrameworks: Fix = { let addonOptions: Record = {}; let metaFramework: string | undefined; - if (rendererPackage === '@storybook/react' && allDependencies.next) { - const nextConfigFile = await findUp(nextJsConfigFiles, { cwd: configDir }); + const nextVersion = await packageManager.getPackageVersion('next'); + const svelteKitVersion = await packageManager.getPackageVersion('@sveltejs/kit'); + const viteVersion = await packageManager.getPackageVersion('vite'); + + if (rendererPackageName === '@storybook/react' && nextVersion) { const nextAddonOptions = getNextjsAddonOptions(mainConfig.addons); + const isNextJsCandidate = - (semver.gte(semver.coerce(allDependencies.next).version, '12.0.0') && nextConfigFile) || + (nextVersion && semver.gte(nextVersion, '12.0.0')) || Object.keys(nextAddonOptions).length > 0; if (isNextJsCandidate) { @@ -184,9 +171,9 @@ export const newFrameworks: Fix = { } } } else if ( - rendererPackage === '@storybook/svelte' && - allDependencies['@sveltejs/kit'] && - semver.gte(semver.coerce(allDependencies['@sveltejs/kit']).version, '1.0.0') + rendererPackageName === '@storybook/svelte' && + svelteKitVersion && + semver.gte(svelteKitVersion, '1.0.0') ) { metaFramework = 'sveltekit'; if (newFrameworkPackage === '@storybook/svelte-vite') { @@ -221,12 +208,12 @@ export const newFrameworks: Fix = { return null; } - if (allDependencies.vite && semver.lt(semver.coerce(allDependencies.vite).version, '3.0.0')) { + if (viteVersion && semver.lt(viteVersion, '3.0.0')) { throw new Error(dedent` ❌ Your project should be upgraded to use the framework package ${chalk.bold( newFrameworkPackage )}, but we detected that you are using Vite ${chalk.bold( - allDependencies.vite + viteVersion )}, which is unsupported in ${chalk.bold( 'Storybook 7.0' )}. Please upgrade Vite to ${chalk.bold('3.0.0 or higher')} and rerun this migration. @@ -248,7 +235,6 @@ export const newFrameworks: Fix = { addonOptions, addonsToRemove, builderInfo, - packageJson, renderer, builderConfig, metaFramework, @@ -436,7 +422,6 @@ export const newFrameworks: Fix = { frameworkPackage, frameworkOptions, builderInfo, - packageJson, renderer, addonsToRemove, }, @@ -445,6 +430,7 @@ export const newFrameworks: Fix = { mainConfigPath, skipInstall, }) { + const packageJson = await packageManager.retrievePackageJson(); if (dependenciesToRemove.length > 0) { logger.info(`✅ Removing dependencies: ${dependenciesToRemove.join(', ')}`); if (!dryRun) { diff --git a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts index 5a4f6327e657..84e4f55fca33 100644 --- a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts +++ b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts @@ -1,15 +1,16 @@ /// ; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import { makePackageManager } from '../helpers/testing-helpers'; import { nodeJsRequirement } from './nodejs-requirement'; // eslint-disable-next-line global-require, jest/no-mocks-import jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); const check = async ({ storybookVersion = '7.0.0' }) => { - mockStorybookData({ mainConfig: {}, storybookVersion }); return nodeJsRequirement.check({ - packageManager: makePackageManager({}), + storybookVersion, + packageManager: {} as any, + mainConfig: {} as any, }); }; diff --git a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts index 276d4b9e1ead..4c3006b7a367 100644 --- a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts +++ b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.ts @@ -2,7 +2,6 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; import semver from 'semver'; import type { Fix } from '../types'; -import { getStorybookData } from '../helpers/mainConfigFile'; interface NodeJsRequirementOptions { nodeVersion: string; @@ -12,9 +11,7 @@ export const nodeJsRequirement: Fix = { id: 'nodejs-requirement', promptOnly: true, - async check({ packageManager, configDir }) { - const { storybookVersion } = await getStorybookData({ packageManager, configDir }); - + async check({ storybookVersion }) { if (!semver.gte(storybookVersion, '7.0.0')) { return null; } diff --git a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.test.ts b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.test.ts index a8fa9d050b9c..27d6fe065f3f 100644 --- a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.test.ts +++ b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.test.ts @@ -8,7 +8,7 @@ import { RemovedAPIs, removedGlobalClientAPIs as migration } from './remove-glob // eslint-disable-next-line global-require, jest/no-mocks-import jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra')); -const check = async ({ packageJson = {}, contents }: any) => { +const check = async ({ contents, previewConfigPath }: any) => { if (contents) { // eslint-disable-next-line global-require require('fs-extra').__setMockFiles({ @@ -16,9 +16,15 @@ const check = async ({ packageJson = {}, contents }: any) => { }); } const packageManager = { - retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), + retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {} }), } as JsPackageManager; - return migration.check({ packageManager }); + + return migration.check({ + packageManager, + mainConfig: {} as any, + storybookVersion: '7.0.0', + previewConfigPath, + }); }; describe('removedGlobalClientAPIs fix', () => { @@ -30,14 +36,18 @@ describe('removedGlobalClientAPIs fix', () => { const contents = ` export const parameters = {}; `; - await expect(check({ contents })).resolves.toBeNull(); + await expect( + check({ contents, previewConfigPath: path.join('.storybook', 'preview.js') }) + ).resolves.toBeNull(); }); it('uses 1 removed API', async () => { const contents = ` import { addParameters } from '@storybook/react'; addParameters({}); `; - await expect(check({ contents })).resolves.toEqual( + await expect( + check({ contents, previewConfigPath: path.join('.storybook', 'preview.js') }) + ).resolves.toEqual( expect.objectContaining({ usedAPIs: [RemovedAPIs.addParameters], }) @@ -49,7 +59,9 @@ describe('removedGlobalClientAPIs fix', () => { addParameters({}); addDecorator((storyFn) => storyFn()); `; - await expect(check({ contents })).resolves.toEqual( + await expect( + check({ contents, previewConfigPath: path.join('.storybook', 'preview.js') }) + ).resolves.toEqual( expect.objectContaining({ usedAPIs: expect.arrayContaining([RemovedAPIs.addParameters, RemovedAPIs.addDecorator]), }) diff --git a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts index 9888b22a6be9..4a1304dae1a6 100644 --- a/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts +++ b/code/lib/cli/src/automigrate/fixes/remove-global-client-apis.ts @@ -1,6 +1,5 @@ import chalk from 'chalk'; import dedent from 'ts-dedent'; -import { getStorybookInfo } from '@storybook/core-common'; import { readFile } from 'fs-extra'; import type { Fix } from '../types'; @@ -22,13 +21,9 @@ export const removedGlobalClientAPIs: Fix = { id: 'removedglobalclientapis', promptOnly: true, - async check({ packageManager, configDir }) { - const packageJson = await packageManager.retrievePackageJson(); - - const { previewConfig } = getStorybookInfo(packageJson, configDir); - - if (previewConfig) { - const contents = await readFile(previewConfig, 'utf8'); + async check({ previewConfigPath }) { + if (previewConfigPath) { + const contents = await readFile(previewConfigPath, 'utf8'); const usedAPIs = Object.values(RemovedAPIs).reduce((acc, item) => { if (contents.includes(item)) { @@ -40,7 +35,7 @@ export const removedGlobalClientAPIs: Fix = { if (usedAPIs.length) { return { usedAPIs, - previewPath: previewConfig, + previewPath: previewConfigPath, }; } } diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts index b150dc424406..c0326a9620fe 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.test.ts @@ -1,26 +1,39 @@ -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { sbBinary } from './sb-binary'; const checkStorybookBinary = async ({ - packageJson, + packageManager, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; + packageManager: Partial; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig: {}, storybookVersion }); - return sbBinary.check({ packageManager: makePackageManager(packageJson) }); + return sbBinary.check({ + packageManager: packageManager as any, + storybookVersion, + mainConfig: {} as any, + }); }; describe('storybook-binary fix', () => { describe('sb < 7.0', () => { describe('does nothing', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('6.2.0'); + default: + return null; + } + }, + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + it('should no-op', async () => { await expect( checkStorybookBinary({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); @@ -30,25 +43,43 @@ describe('storybook-binary fix', () => { describe('sb >= 7.0', () => { it('should no-op in NX projects', async () => { - const packageJson = { - dependencies: { '@storybook/react': '^7.0.0', '@nrwl/storybook': '^15.7.1' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0'); + case '@nrwl/storybook': + return Promise.resolve('15.7.1'); + default: + return null; + } + }, + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + await expect( checkStorybookBinary({ - packageJson, + packageManager, }) ).resolves.toBeFalsy(); }); it('should add storybook dependency if not present', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + default: + return null; + } }, - }; + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + await expect( checkStorybookBinary({ - packageJson, + packageManager, }) ).resolves.toEqual( expect.objectContaining({ @@ -59,15 +90,23 @@ describe('storybook-binary fix', () => { }); it('should remove sb dependency if it is present', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - sb: '6.5.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + case 'sb': + return Promise.resolve('6.5.0'); + default: + return null; + } }, - }; + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + await expect( checkStorybookBinary({ - packageJson, + packageManager, }) ).resolves.toEqual( expect.objectContaining({ @@ -78,15 +117,23 @@ describe('storybook-binary fix', () => { }); it('should no op if storybook is present and sb is not present', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', - storybook: '^7.0.0-alpha.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + case 'storybook': + return Promise.resolve('7.0.0-alpha.0'); + default: + return null; + } }, - }; + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + await expect( checkStorybookBinary({ - packageJson, + packageManager, }) ).resolves.toBeNull(); }); diff --git a/code/lib/cli/src/automigrate/fixes/sb-binary.ts b/code/lib/cli/src/automigrate/fixes/sb-binary.ts index 22d0283e3de3..276d10c6178a 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-binary.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-binary.ts @@ -4,7 +4,6 @@ import semver from 'semver'; import type { Fix } from '../types'; import { getStorybookVersionSpecifier } from '../../helpers'; import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; -import { getStorybookData } from '../helpers/mainConfigFile'; interface SbBinaryRunOptions { storybookVersion: string; @@ -25,18 +24,20 @@ const logger = console; export const sbBinary: Fix = { id: 'storybook-binary', - async check({ packageManager, configDir }) { + async check({ packageManager, storybookVersion }) { const packageJson = await packageManager.retrievePackageJson(); - const allDependencies = await packageManager.getAllDependencies(); - const { storybookVersion } = await getStorybookData({ packageManager, configDir }); + + const nrwlStorybookVersion = await packageManager.getPackageVersion('@nrwl/storybook'); + const sbBinaryVersion = await packageManager.getPackageVersion('sb'); + const storybookBinaryVersion = await packageManager.getPackageVersion('storybook'); // Nx provides their own binary, so we don't need to do anything - if (allDependencies['@nrwl/storybook'] || semver.lt(storybookVersion, '7.0.0')) { + if (nrwlStorybookVersion || semver.lt(storybookVersion, '7.0.0')) { return null; } - const hasSbBinary = !!allDependencies.sb; - const hasStorybookBinary = !!allDependencies.storybook; + const hasSbBinary = !!sbBinaryVersion; + const hasStorybookBinary = !!storybookBinaryVersion; if (!hasSbBinary && hasStorybookBinary) { return null; diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts index 1e8d4edf71ca..036cb18ba098 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.test.ts @@ -1,16 +1,18 @@ -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { getStorybookScripts, sbScripts } from './sb-scripts'; const checkSbScripts = async ({ - packageJson, + packageManager, storybookVersion = '7.0.0', }: { - packageJson: PackageJson; + packageManager: Partial; storybookVersion?: string; }) => { - mockStorybookData({ mainConfig: {}, storybookVersion }); - return sbScripts.check({ packageManager: makePackageManager(packageJson) }); + return sbScripts.check({ + packageManager: packageManager as any, + storybookVersion, + mainConfig: {} as any, + }); }; describe('getStorybookScripts', () => { @@ -58,11 +60,22 @@ describe('getStorybookScripts', () => { describe('sb-scripts fix', () => { describe('sb < 7.0', () => { describe('does nothing', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('6.2.0'); + default: + return null; + } + }, + retrievePackageJson: () => Promise.resolve({}), + } as Partial; + it('should no-op', async () => { await expect( checkSbScripts({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); @@ -72,19 +85,30 @@ describe('sb-scripts fix', () => { describe('sb >= 7.0', () => { describe('with old scripts', () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + default: + return null; + } }, - scripts: { - storybook: 'start-storybook -p 6006', - 'build-storybook': 'build-storybook -o build/storybook', - }, - }; + retrievePackageJson: () => + Promise.resolve({ + scripts: { + storybook: 'start-storybook -p 6006', + 'build-storybook': 'build-storybook -o build/storybook', + }, + dependencies: {}, + devDependencies: {}, + }), + } as Partial; + it('should update scripts to new format', async () => { await expect( checkSbScripts({ - packageJson, + packageManager, }) ).resolves.toEqual( expect.objectContaining({ @@ -105,22 +129,32 @@ describe('sb-scripts fix', () => { describe('with old custom scripts', () => { it('should update scripts to new format', async () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + default: + return null; + } }, - scripts: { - 'storybook:ci': 'yarn start-storybook --ci', - 'storybook:build': 'build-storybook -o build/storybook', - 'storybook:build-mocked': 'MOCKS=true yarn storybook:build', - 'test-storybook:ci': - 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "CI=true build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', - }, - }; + retrievePackageJson: () => + Promise.resolve({ + scripts: { + 'storybook:ci': 'yarn start-storybook --ci', + 'storybook:build': 'build-storybook -o build/storybook', + 'storybook:build-mocked': 'MOCKS=true yarn storybook:build', + 'test-storybook:ci': + 'concurrently -k -s first -n "SB,TEST" -c "magenta,blue" "CI=true build-storybook --quiet && npx http-server storybook-static --port 6006 --silent" "wait-on tcp:6006 && yarn test-storybook"', + }, + dependencies: {}, + devDependencies: {}, + }), + } as Partial; await expect( checkSbScripts({ - packageJson, + packageManager, }) ).resolves.toEqual( expect.objectContaining({ @@ -142,19 +176,30 @@ describe('sb-scripts fix', () => { }); describe('already containing new scripts', () => { - const packageJson = { - dependencies: { - '@storybook/react': '^7.0.0-alpha.0', + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + default: + return null; + } }, - scripts: { - storybook: 'storybook dev -p 6006', - 'build-storybook': 'storybook build -o build/storybook', - }, - }; + retrievePackageJson: () => + Promise.resolve({ + scripts: { + storybook: 'storybook dev -p 6006', + 'build-storybook': 'storybook build -o build/storybook', + }, + dependencies: {}, + devDependencies: {}, + }), + } as Partial; + it('should no-op', async () => { await expect( checkSbScripts({ - packageJson, + packageManager, }) ).resolves.toBeFalsy(); }); diff --git a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts index b624d494af5a..8471d3c1a6c3 100644 --- a/code/lib/cli/src/automigrate/fixes/sb-scripts.ts +++ b/code/lib/cli/src/automigrate/fixes/sb-scripts.ts @@ -3,7 +3,6 @@ import { dedent } from 'ts-dedent'; import semver from 'semver'; import type { Fix } from '../types'; import type { PackageJsonWithDepsAndDevDeps } from '../../js-package-manager'; -import { getStorybookData } from '../helpers/mainConfigFile'; interface SbScriptsRunOptions { storybookScripts: Record; @@ -71,10 +70,9 @@ export const getStorybookScripts = (allScripts: Record) => { export const sbScripts: Fix = { id: 'sb-scripts', - async check({ packageManager, configDir }) { + async check({ packageManager, storybookVersion }) { const packageJson = await packageManager.retrievePackageJson(); const { scripts = {} } = packageJson; - const { storybookVersion } = await getStorybookData({ packageManager, configDir }); if (semver.lt(storybookVersion, '7.0.0')) { return null; diff --git a/code/lib/cli/src/automigrate/fixes/vue3.test.ts b/code/lib/cli/src/automigrate/fixes/vue3.test.ts index 6c4594d076df..6d7f61d77186 100644 --- a/code/lib/cli/src/automigrate/fixes/vue3.test.ts +++ b/code/lib/cli/src/automigrate/fixes/vue3.test.ts @@ -1,21 +1,21 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { vue3 } from './vue3'; const checkVue3 = async ({ - packageJson, main: mainConfig = {}, storybookVersion = '7.0.0', + packageManager, }: { - packageJson: PackageJson; main?: Partial & Record; + mainConfig?: Partial; storybookVersion?: string; + packageManager?: Partial; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return vue3.check({ - packageManager: makePackageManager(packageJson), + packageManager: packageManager as any, + storybookVersion, + mainConfig: mainConfig as any, }); }; @@ -24,24 +24,43 @@ describe('vue3 fix', () => { describe('sb < 6.3', () => { describe('vue3 dependency', () => { - const packageJson = { - dependencies: { '@storybook/vue': '^6.2.0', vue: '^3.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/vue': + return Promise.resolve('6.2.0'); + case 'vue': + return Promise.resolve('3.0.0'); + default: + return null; + } + }, + } as Partial; + it('should fail', async () => { await expect( checkVue3({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); }); describe('no vue dependency', () => { - const packageJson = { dependencies: { '@storybook/vue': '^6.2.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/vue': + return Promise.resolve('6.2.0'); + default: + return null; + } + }, + } as Partial; it('should no-op', async () => { await expect( checkVue3({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); @@ -50,14 +69,24 @@ describe('vue3 fix', () => { }); describe('sb 6.3 - 7.0', () => { describe('vue3 dependency', () => { - const packageJson = { - dependencies: { '@storybook/vue': '^6.3.0', vue: '^3.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/vue': + return Promise.resolve('6.3.0'); + case 'vue': + return Promise.resolve('3.0.0'); + default: + return null; + } + }, + } as Partial; + describe('webpack5 builder', () => { it('should no-op', async () => { await expect( checkVue3({ - packageJson, + packageManager, main: { core: { builder: 'webpack5' } }, }) ).resolves.toBeFalsy(); @@ -67,7 +96,7 @@ describe('vue3 fix', () => { it('should no-op', async () => { await expect( checkVue3({ - packageJson, + packageManager, main: { core: { builder: 'storybook-builder-vite' } }, }) ).resolves.toBeFalsy(); @@ -77,12 +106,12 @@ describe('vue3 fix', () => { it('should add webpack5 builder', async () => { await expect( checkVue3({ - packageJson, + packageManager, main: { core: { builder: 'webpack4' } }, storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ - vueVersion: '^3.0.0', + vueVersion: '3.0.0', storybookVersion: '6.3.0', }); }); @@ -91,12 +120,12 @@ describe('vue3 fix', () => { it('should add webpack5 builder', async () => { await expect( checkVue3({ - packageJson, + packageManager, main: {}, storybookVersion: '6.3.0', }) ).resolves.toMatchObject({ - vueVersion: '^3.0.0', + vueVersion: '3.0.0', storybookVersion: '6.3.0', }); }); @@ -104,23 +133,34 @@ describe('vue3 fix', () => { }); describe('no vue dependency', () => { it('should no-op', async () => { + const packageManager = { + getPackageVersion: (packageName) => { + return null; + }, + } as Partial; + await expect( checkVue3({ - packageJson: {}, + packageManager, main: {}, }) ).resolves.toBeFalsy(); }); }); describe('vue2 dependency', () => { + const packageManager = { + getPackageVersion: (packageName) => { + if (packageName === 'vue') { + return Promise.resolve('2.0.0'); + } + return null; + }, + } as Partial; + it('should no-op', async () => { await expect( checkVue3({ - packageJson: { - dependencies: { - vue: '2', - }, - }, + packageManager, main: {}, }) ).resolves.toBeFalsy(); @@ -129,13 +169,23 @@ describe('vue3 fix', () => { }); describe('sb 7.0+', () => { describe('vue3 dependency', () => { - const packageJson = { - dependencies: { '@storybook/vue': '^7.0.0-alpha.0', vue: '^3.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/vue': + return Promise.resolve('7.0.0-alpha.0'); + case 'vue': + return Promise.resolve('3.0.0'); + default: + return null; + } + }, + } as Partial; + it('should no-op', async () => { await expect( checkVue3({ - packageJson, + packageManager, main: {}, }) ).resolves.toBeFalsy(); diff --git a/code/lib/cli/src/automigrate/fixes/vue3.ts b/code/lib/cli/src/automigrate/fixes/vue3.ts index 0d3aaca104af..84bde42d6196 100644 --- a/code/lib/cli/src/automigrate/fixes/vue3.ts +++ b/code/lib/cli/src/automigrate/fixes/vue3.ts @@ -19,16 +19,14 @@ interface Vue3RunOptions { export const vue3: Fix = { id: 'vue3', - async check({ configDir, packageManager }) { - const allDependencies = await packageManager.getAllDependencies(); - const vueVersion = allDependencies.vue; - const vueCoerced = semver.coerce(vueVersion)?.version; + async check({ packageManager, mainConfig, storybookVersion }) { + const vueVersion = await packageManager.getPackageVersion('vue'); - if (!vueCoerced || semver.lt(vueCoerced, '3.0.0')) { + if (!vueVersion || semver.lt(vueVersion, '3.0.0')) { return null; } - const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); + const builderInfo = await checkWebpack5Builder({ mainConfig, storybookVersion }); return builderInfo ? { vueVersion, ...builderInfo } : null; }, diff --git a/code/lib/cli/src/automigrate/fixes/webpack5.test.ts b/code/lib/cli/src/automigrate/fixes/webpack5.test.ts index 15fc94a76800..db03eeb57a08 100644 --- a/code/lib/cli/src/automigrate/fixes/webpack5.test.ts +++ b/code/lib/cli/src/automigrate/fixes/webpack5.test.ts @@ -1,22 +1,22 @@ import type { StorybookConfig } from '@storybook/types'; -import type { PackageJson } from '../../js-package-manager'; -import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers'; +import type { JsPackageManager } from '../../js-package-manager'; import { webpack5 } from './webpack5'; const checkWebpack5 = async ({ - packageJson, + packageManager, main: mainConfig, storybookVersion = '6.3.0', }: { - packageJson: PackageJson; + packageManager: Partial; main?: Partial & Record; storybookVersion?: string; + mainConfig?: Partial; }) => { - mockStorybookData({ mainConfig, storybookVersion }); - return webpack5.check({ - packageManager: makePackageManager(packageJson), + packageManager: packageManager as any, configDir: '', + storybookVersion, + mainConfig: mainConfig as any, }); }; @@ -25,22 +25,44 @@ describe('webpack5 fix', () => { describe('sb < 6.3', () => { describe('webpack5 dependency', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0', webpack: '^5.0.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('6.2.0'); + case 'webpack': + return Promise.resolve('5.0.0'); + default: + return null; + } + }, + } as Partial; + it('should fail', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).rejects.toThrow(); }); }); describe('no webpack5 dependency', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('6.2.0'); + default: + return null; + } + }, + } as Partial; + it('should no-op', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, storybookVersion: '6.2.0', }) ).resolves.toBeFalsy(); @@ -49,12 +71,24 @@ describe('webpack5 fix', () => { }); describe('sb 6.3 - 7.0', () => { describe('webpack5 dependency', () => { - const packageJson = { dependencies: { '@storybook/react': '^6.3.0', webpack: '^5.0.0' } }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('6.3.0'); + case 'webpack': + return Promise.resolve('5.0.0'); + default: + return null; + } + }, + } as Partial; + describe('webpack5 builder', () => { it('should no-op', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, main: { core: { builder: 'webpack5' } }, }) ).resolves.toBeFalsy(); @@ -64,7 +98,7 @@ describe('webpack5 fix', () => { it('should no-op', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, main: { core: { builder: 'storybook-builder-vite' } }, }) ).resolves.toBeFalsy(); @@ -74,11 +108,11 @@ describe('webpack5 fix', () => { it('should add webpack5 builder', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, main: { core: { builder: 'webpack4' } }, }) ).resolves.toMatchObject({ - webpackVersion: '^5.0.0', + webpackVersion: '5.0.0', storybookVersion: '6.3.0', }); }); @@ -87,34 +121,47 @@ describe('webpack5 fix', () => { it('should add webpack5 builder', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, main: {}, }) ).resolves.toMatchObject({ - webpackVersion: '^5.0.0', + webpackVersion: '5.0.0', storybookVersion: '6.3.0', }); }); }); }); describe('no webpack dependency', () => { + const packageManager = { + getPackageVersion: () => { + return null; + }, + } as Partial; + it('should no-op', async () => { await expect( checkWebpack5({ - packageJson: {}, + packageManager, }) ).resolves.toBeFalsy(); }); }); describe('webpack4 dependency', () => { + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case 'webpack': + return Promise.resolve('4.0.0'); + default: + return null; + } + }, + } as Partial; + it('should no-op', async () => { await expect( checkWebpack5({ - packageJson: { - dependencies: { - webpack: '4', - }, - }, + packageManager, }) ).resolves.toBeFalsy(); }); @@ -122,13 +169,23 @@ describe('webpack5 fix', () => { }); describe('sb 7.0+', () => { describe('webpack5 dependency', () => { - const packageJson = { - dependencies: { '@storybook/react': '^7.0.0-alpha.0', webpack: '^5.0.0' }, - }; + const packageManager = { + getPackageVersion: (packageName) => { + switch (packageName) { + case '@storybook/react': + return Promise.resolve('7.0.0-alpha.0'); + case 'webpack': + return Promise.resolve('5.0.0'); + default: + return null; + } + }, + } as Partial; + it('should no-op', async () => { await expect( checkWebpack5({ - packageJson, + packageManager, main: {}, storybookVersion: '7.0.0', }) diff --git a/code/lib/cli/src/automigrate/fixes/webpack5.ts b/code/lib/cli/src/automigrate/fixes/webpack5.ts index c60dc9f0eed1..bcae50749a1e 100644 --- a/code/lib/cli/src/automigrate/fixes/webpack5.ts +++ b/code/lib/cli/src/automigrate/fixes/webpack5.ts @@ -25,20 +25,17 @@ interface Webpack5RunOptions { export const webpack5: Fix = { id: 'webpack5', - async check({ configDir, packageManager }) { - const allDependencies = (await packageManager.retrievePackageJson()).dependencies; - - const webpackVersion = allDependencies.webpack; - const webpackCoerced = semver.coerce(webpackVersion)?.version; + async check({ configDir, packageManager, mainConfig, storybookVersion }) { + const webpackVersion = await packageManager.getPackageVersion('webpack'); if ( - !webpackCoerced || - semver.lt(webpackCoerced, '5.0.0') || - semver.gte(webpackCoerced, '6.0.0') + !webpackVersion || + semver.lt(webpackVersion, '5.0.0') || + semver.gte(webpackVersion, '6.0.0') ) return null; - const builderInfo = await checkWebpack5Builder({ configDir, packageManager }); + const builderInfo = await checkWebpack5Builder({ mainConfig, storybookVersion }); return builderInfo ? { webpackVersion, ...builderInfo } : null; }, diff --git a/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.test.ts b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.test.ts new file mode 100644 index 000000000000..51cb622baf42 --- /dev/null +++ b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.test.ts @@ -0,0 +1,78 @@ +import type { StorybookConfig } from '@storybook/types'; +import { checkWebpack5Builder } from './checkWebpack5Builder'; +import { getBuilderPackageName } from './mainConfigFile'; + +const mockMainConfig: StorybookConfig = { + framework: 'react', + addons: [], + stories: [], +}; + +jest.mock('./mainConfigFile'); + +describe('checkWebpack5Builder', () => { + let loggerWarnSpy: jest.SpyInstance; + let loggerInfoSpy: jest.SpyInstance; + + beforeEach(() => { + loggerWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + loggerInfoSpy = jest.spyOn(console, 'info').mockImplementation(); + }); + + afterEach(() => { + loggerWarnSpy.mockRestore(); + loggerInfoSpy.mockRestore(); + }); + + it('should return null and log a warning if storybook version is below 6.3.0', async () => { + const result = await checkWebpack5Builder({ + mainConfig: mockMainConfig, + storybookVersion: '6.2.9', + }); + expect(result).toBeNull(); + expect(loggerWarnSpy).toHaveBeenCalledWith(expect.any(String)); + }); + + it('should return null if storybook version is 7.0.0 or above', async () => { + const result = await checkWebpack5Builder({ + mainConfig: mockMainConfig, + storybookVersion: '7.0.0', + }); + expect(result).toBeNull(); + expect(loggerWarnSpy).not.toHaveBeenCalled(); + }); + + it('should return null and log a warning if mainConfig is missing', async () => { + const result = await checkWebpack5Builder({ + mainConfig: undefined, + storybookVersion: '6.3.0', + }); + expect(result).toBeNull(); + expect(loggerWarnSpy).toHaveBeenCalledWith(expect.any(String)); + }); + + it('should return null and log an info message if builderPackageName is found but not "webpack4"', async () => { + jest.mocked(getBuilderPackageName).mockReturnValueOnce('webpack5'); + + const result = await checkWebpack5Builder({ + mainConfig: mockMainConfig, + storybookVersion: '6.3.0', + }); + + expect(result).toBeNull(); + expect(loggerInfoSpy).toHaveBeenCalledWith(expect.any(String)); + }); + + it('should return { storybookVersion } if all checks pass', async () => { + jest.mocked(getBuilderPackageName).mockReturnValueOnce('webpack4'); + + const result = await checkWebpack5Builder({ + mainConfig: mockMainConfig, + storybookVersion: '6.3.0', + }); + + expect(result).toEqual({ storybookVersion: '6.3.0' }); + expect(loggerWarnSpy).not.toHaveBeenCalled(); + expect(loggerInfoSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts index b900d40b89bd..8d59d62d0bcc 100644 --- a/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts +++ b/code/lib/cli/src/automigrate/helpers/checkWebpack5Builder.ts @@ -1,17 +1,18 @@ import chalk from 'chalk'; import semver from 'semver'; import dedent from 'ts-dedent'; -import type { GetStorybookData } from './mainConfigFile'; -import { getStorybookData } from './mainConfigFile'; +import type { StorybookConfig } from '@storybook/types'; +import { getBuilderPackageName } from './mainConfigFile'; const logger = console; export const checkWebpack5Builder = async ({ - configDir, - packageManager, -}: Parameters[0]) => { - const { mainConfig, storybookVersion } = await getStorybookData({ configDir, packageManager }); - + mainConfig, + storybookVersion, +}: { + mainConfig: StorybookConfig; + storybookVersion: string; +}) => { if (semver.lt(storybookVersion, '6.3.0')) { logger.warn( dedent` @@ -36,9 +37,9 @@ export const checkWebpack5Builder = async ({ return null; } - const builder = mainConfig.core?.builder; - if (builder && builder !== 'webpack4') { - logger.info(`Found builder ${builder}, skipping`); + const builderPackageName = getBuilderPackageName(mainConfig); + if (builderPackageName && builderPackageName !== 'webpack4') { + logger.info(`Found builder ${builderPackageName}, skipping`); return null; } diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts new file mode 100644 index 000000000000..40770c7cde9d --- /dev/null +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts @@ -0,0 +1,133 @@ +import { + getBuilderPackageName, + getFrameworkPackageName, + getRendererPackageNameFromFramework, +} from './mainConfigFile'; + +describe('getBuilderPackageName', () => { + it('should return null when mainConfig is undefined or null', () => { + const packageName = getBuilderPackageName(undefined); + expect(packageName).toBeNull(); + + const packageName2 = getBuilderPackageName(null); + expect(packageName2).toBeNull(); + }); + + it('should return null when builder package name or path is not found', () => { + const mainConfig = {}; + + const packageName = getBuilderPackageName(mainConfig as any); + expect(packageName).toBeNull(); + }); + + it('should return builder package name when core.builder is a string', () => { + const builderPackage = '@storybook/builder-webpack5'; + const mainConfig = { + core: { + builder: builderPackage, + }, + }; + + const packageName = getBuilderPackageName(mainConfig as any); + expect(packageName).toBe(builderPackage); + }); + + it('should return builder package name when core.builder.name contains valid builder package name', () => { + const builderPackage = '@storybook/builder-webpack5'; + const packageNameOrPath = `/path/to/${builderPackage}`; + const mainConfig = { + core: { + builder: { name: packageNameOrPath }, + }, + }; + + const packageName = getBuilderPackageName(mainConfig as any); + expect(packageName).toBe(builderPackage); + }); + + it(`should return package name or path when core.builder doesn't contain the name of a valid builder package`, () => { + const packageNameOrPath = '@my-org/storybook-builder'; + const mainConfig = { + core: { + builder: packageNameOrPath, + }, + }; + + const packageName = getBuilderPackageName(mainConfig as any); + expect(packageName).toBe(packageNameOrPath); + }); +}); + +describe('getFrameworkPackageName', () => { + it('should return null when mainConfig is undefined or null', () => { + const packageName = getFrameworkPackageName(undefined); + expect(packageName).toBeNull(); + + const packageName2 = getFrameworkPackageName(null); + expect(packageName2).toBeNull(); + }); + + it('should return null when framework package name or path is not found', () => { + const mainConfig = {}; + + const packageName = getFrameworkPackageName(mainConfig as any); + expect(packageName).toBeNull(); + }); + + it('should return framework package name when framework is a string', () => { + const frameworkPackage = '@storybook/react'; + const mainConfig = { + framework: frameworkPackage, + }; + + const packageName = getFrameworkPackageName(mainConfig as any); + expect(packageName).toBe(frameworkPackage); + }); + + it('should return framework package name when framework.name contains valid framework package name', () => { + const frameworkPackage = '@storybook/react-vite'; + const packageNameOrPath = `/path/to/${frameworkPackage}`; + const mainConfig = { + framework: { name: packageNameOrPath }, + }; + + const packageName = getFrameworkPackageName(mainConfig as any); + expect(packageName).toBe(frameworkPackage); + }); + + it(`should return package name or path when framework does not contain the name of a valid framework package`, () => { + const packageNameOrPath = '@my-org/storybook-framework'; + const mainConfig = { + framework: packageNameOrPath, + }; + + const packageName = getFrameworkPackageName(mainConfig as any); + expect(packageName).toBe(packageNameOrPath); + }); +}); + +describe('getRendererPackageNameFromFramework', () => { + it('should return null when given no package name', () => { + const packageName = getRendererPackageNameFromFramework(undefined); + expect(packageName).toBeNull(); + }); + + it('should return the frameworkPackageName if it exists in rendererPackages', () => { + const frameworkPackageName = '@storybook/angular'; + const packageName = getRendererPackageNameFromFramework(frameworkPackageName); + expect(packageName).toBe(frameworkPackageName); + }); + + it('should return the corresponding key of rendererPackages if the value is the same as the frameworkPackageName', () => { + const frameworkPackageName = 'vue'; + const expectedPackageName = '@storybook/vue'; + const packageName = getRendererPackageNameFromFramework(frameworkPackageName); + expect(packageName).toBe(expectedPackageName); + }); + + it('should return null if a frameworkPackageName is known but not available in rendererPackages', () => { + const frameworkPackageName = '@storybook/unknown'; + const packageName = getRendererPackageNameFromFramework(frameworkPackageName); + expect(packageName).toBeNull(); + }); +}); diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index f843f57097f9..ebf10cef9799 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -1,4 +1,10 @@ -import { getStorybookInfo, loadMainConfig } from '@storybook/core-common'; +import { + getStorybookInfo, + loadMainConfig, + rendererPackages, + frameworkPackages, + builderPackages, +} from '@storybook/core-common'; import type { StorybookConfig } from '@storybook/types'; import type { ConfigFile } from '@storybook/csf-tools'; import { readConfig, writeConfig as writeConfigFile } from '@storybook/csf-tools'; @@ -9,6 +15,60 @@ import type { JsPackageManager } from '../../js-package-manager'; const logger = console; +/** + * Given a Storybook configuration object, retrieves the package name or file path of the framework. + * @param mainConfig - The main Storybook configuration object to lookup. + * @returns - The package name of the framework. If not found, returns null. + */ +export const getFrameworkPackageName = (mainConfig?: StorybookConfig) => { + const packageNameOrPath = + typeof mainConfig?.framework === 'string' ? mainConfig.framework : mainConfig?.framework?.name; + + return packageNameOrPath + ? Object.keys(frameworkPackages).find((pkg) => packageNameOrPath.endsWith(pkg)) ?? + packageNameOrPath + : null; +}; + +/** + * Given a Storybook configuration object, retrieves the package name or file path of the builder. + * @param mainConfig - The main Storybook configuration object to lookup. + * @returns - The package name of the builder. If not found, returns null. + */ +export const getBuilderPackageName = (mainConfig?: StorybookConfig) => { + const packageNameOrPath = + typeof mainConfig?.core?.builder === 'string' + ? mainConfig.core.builder + : mainConfig?.core?.builder?.name; + + return packageNameOrPath + ? builderPackages.find((pkg) => packageNameOrPath.endsWith(pkg)) ?? packageNameOrPath + : null; +}; + +/** + * Returns a renderer package name given a framework package name. + * @param frameworkPackageName - The package name of the framework to lookup. + * @returns - The corresponding package name in `rendererPackages`. If not found, returns null. + */ +export const getRendererPackageNameFromFramework = (frameworkPackageName: string) => { + if (frameworkPackageName) { + if (Object.keys(rendererPackages).includes(frameworkPackageName)) { + // at some point in 6.4 we introduced a framework field, but filled with a renderer package + return frameworkPackageName; + } + + if (Object.values(rendererPackages).includes(frameworkPackageName)) { + // for scenarios where the value is e.g. "react" instead of "@storybook/react" + return Object.keys(rendererPackages).find( + (k) => rendererPackages[k] === frameworkPackageName + ); + } + } + + return null; +}; + export const getStorybookData = async ({ packageManager, configDir: userDefinedConfigDir, diff --git a/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.test.ts b/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.test.ts index ee691d396f53..a112619717b3 100644 --- a/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.test.ts +++ b/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.test.ts @@ -3,6 +3,7 @@ import { detectBuilderInfo as _getBuilderInfo, getNextjsAddonOptions, } from './new-frameworks-utils'; +import type { JsPackageManager } from '../../js-package-manager'; jest.mock('find-up'); @@ -10,17 +11,17 @@ type GetBuilderInfoParams = Parameters[0]['mainConfig']; const getBuilderInfo = async ({ mainConfig = {}, - packageDependencies = {}, + packageManager = {}, configDir = '.storybook', }: { - mainConfig: Partial; - packageDependencies?: Record; + mainConfig?: Partial; + packageManager?: Partial; configDir?: string; }) => { return _getBuilderInfo({ mainConfig: mainConfig as any, configDir, - packageDependencies, + packageManager: packageManager as any, }); }; @@ -29,7 +30,9 @@ describe('getBuilderInfo', () => { await expect( getBuilderInfo({ mainConfig: { - core: { builder: '@storybook/builder-webpack5' }, + core: { + builder: '@storybook/builder-webpack5', + }, }, }) ).resolves.toEqual({ name: 'webpack5', options: {} }); @@ -54,6 +57,15 @@ describe('getBuilderInfo', () => { it('should infer webpack5 info from framework', async () => { await expect( getBuilderInfo({ + packageManager: { + getPackageVersion: (packageName) => { + if (packageName === '@storybook/react-webpack5') { + return Promise.resolve('1.0.0'); + } + + return Promise.resolve(null); + }, + }, mainConfig: { framework: '@storybook/react-webpack5', }, @@ -204,7 +216,14 @@ describe('getBuilderInfo', () => { await expect( getBuilderInfo({ mainConfig: {}, - packageDependencies: { '@storybook/builder-vite': '^7.0.0' }, + packageManager: { + getPackageVersion: (packageName) => { + if (packageName === '@storybook/builder-vite') { + return Promise.resolve('7.0.0'); + } + return Promise.resolve(null); + }, + }, }) ).resolves.toEqual({ name: 'vite', @@ -218,7 +237,14 @@ describe('getBuilderInfo', () => { await expect( getBuilderInfo({ mainConfig: {}, - packageDependencies: { '@storybook/builder-webpack5': '^7.0.0' }, + packageManager: { + getPackageVersion: (packageName) => { + if (packageName === '@storybook/builder-webpack5') { + return Promise.resolve('7.0.0'); + } + return Promise.resolve(null); + }, + }, }) ).resolves.toEqual({ name: 'webpack5', diff --git a/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.ts b/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.ts index a6640c8bb147..db84aaa3b245 100644 --- a/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.ts +++ b/code/lib/cli/src/automigrate/helpers/new-frameworks-utils.ts @@ -1,6 +1,8 @@ import { frameworkPackages } from '@storybook/core-common'; import type { Preset, StorybookConfig } from '@storybook/types'; import findUp from 'find-up'; +import type { JsPackageManager } from '../../js-package-manager'; +import { getBuilderPackageName, getFrameworkPackageName } from './mainConfigFile'; const logger = console; @@ -62,32 +64,29 @@ type BuilderType = 'vite' | 'webpack5'; export const detectBuilderInfo = async ({ mainConfig, configDir, - packageDependencies, + packageManager, }: { mainConfig: StorybookConfig & { builder?: string | Preset }; configDir: string; - packageDependencies: Record; + packageManager: JsPackageManager; }): Promise<{ name: BuilderType; options: any }> => { - let builderOptions = {}; let builderName: BuilderType; let builderOrFrameworkName; const { core = {}, framework } = mainConfig; const { builder } = core; - if (builder) { - if (typeof builder === 'string') { - builderOrFrameworkName = builder; - } else { - builderOrFrameworkName = builder.name; + const builderPackageName = getBuilderPackageName(mainConfig); + const frameworkPackageName = getFrameworkPackageName(mainConfig); - builderOptions = builder.options || {}; - } + let builderOptions = typeof builder !== 'string' ? builder?.options ?? {} : {}; + + if (builderPackageName) { + builderOrFrameworkName = builderPackageName; } else if (framework) { - const frameworkName = typeof framework === 'string' ? framework : framework.name; - if (Object.keys(frameworkPackages).includes(frameworkName)) { - builderOrFrameworkName = frameworkName; - builderOptions = typeof framework === 'object' ? framework.options?.builder : {}; + if (Object.keys(frameworkPackages).includes(frameworkPackageName)) { + builderOrFrameworkName = frameworkPackageName; + builderOptions = typeof framework === 'object' ? framework.options?.builder ?? {} : {}; } } @@ -112,15 +111,22 @@ export const detectBuilderInfo = async ({ // if builder is still not detected, rely on package dependencies if (!builderOrFrameworkName) { - if ( - packageDependencies['@storybook/builder-vite'] || - packageDependencies['storybook-builder-vite'] - ) { + const storybookBuilderViteVersion = await packageManager.getPackageVersion( + '@storybook/builder-vite' + ); + const storybookBuilderVite2Version = await packageManager.getPackageVersion( + 'storybook-builder-vite' + ); + const storybookBuilderWebpack5Version = await packageManager.getPackageVersion( + '@storybook/builder-webpack5' + ); + const storybookBuilderManagerWebpack5Version = await packageManager.getPackageVersion( + '@storybook/manager-webpack5' + ); + + if (storybookBuilderViteVersion || storybookBuilderVite2Version) { builderOrFrameworkName = 'vite'; - } else if ( - packageDependencies['@storybook/builder-webpack5'] || - packageDependencies['@storybook/manager-webpack5'] - ) { + } else if (storybookBuilderWebpack5Version || storybookBuilderManagerWebpack5Version) { builderOrFrameworkName = 'webpack5'; } } diff --git a/code/lib/cli/src/automigrate/helpers/testing-helpers.ts b/code/lib/cli/src/automigrate/helpers/testing-helpers.ts index 3651fe472caf..f7b206ccb004 100644 --- a/code/lib/cli/src/automigrate/helpers/testing-helpers.ts +++ b/code/lib/cli/src/automigrate/helpers/testing-helpers.ts @@ -1,6 +1,4 @@ import type { JsPackageManager, PackageJson } from '../../js-package-manager'; -import type { GetStorybookData } from './mainConfigFile'; -import * as mainConfigFile from './mainConfigFile'; jest.mock('./mainConfigFile', () => ({ ...jest.requireActual('./mainConfigFile'), @@ -23,21 +21,3 @@ export const makePackageManager = (packageJson: PackageJson) => { }), } as JsPackageManager; }; - -type GetStorybookDataParams = Awaited>; -export const mockStorybookData = ( - mockData: { - mainConfig: Partial & Record; - storybookVersion: GetStorybookDataParams['storybookVersion']; - } & Partial> -) => { - const defaults: Partial = { - configDir: '', - mainConfigPath: '', - }; - - jest.spyOn(mainConfigFile, 'getStorybookData').mockResolvedValueOnce({ - ...defaults, - ...mockData, - } as GetStorybookDataParams); -}; diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index 01447e371cd4..27ded58a7c42 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -16,6 +16,7 @@ import type { Fix, FixId, FixOptions, FixSummary } from './fixes'; import { FixStatus, PreCheckFailure, allFixes } from './fixes'; import { cleanLog } from './helpers/cleanLog'; import { getMigrationSummary } from './helpers/getMigrationSummary'; +import { getStorybookData } from './helpers/mainConfigFile'; const logger = console; const LOG_FILE_NAME = 'migration-storybook.log'; @@ -211,10 +212,19 @@ export async function runFixes({ let result; try { + const { mainConfig, previewConfigPath } = await getStorybookData({ + configDir, + packageManager, + }); + result = await f.check({ packageManager, configDir, rendererPackage, + mainConfig, + storybookVersion, + previewConfigPath, + mainConfigPath, }); } catch (error) { logger.info(`⚠️ failed to check fix ${chalk.bold(f.id)}`); diff --git a/code/lib/cli/src/automigrate/types.ts b/code/lib/cli/src/automigrate/types.ts index f172af777fc8..740dfa3a0451 100644 --- a/code/lib/cli/src/automigrate/types.ts +++ b/code/lib/cli/src/automigrate/types.ts @@ -1,9 +1,14 @@ +import type { StorybookConfig } from '@storybook/types'; import type { JsPackageManager, PackageManagerName } from '../js-package-manager'; export interface CheckOptions { packageManager: JsPackageManager; rendererPackage?: string; configDir?: string; + mainConfig: StorybookConfig; + storybookVersion: string; + previewConfigPath?: string; + mainConfigPath?: string; } export interface RunOptions { diff --git a/code/lib/cli/src/detect.test.ts b/code/lib/cli/src/detect.test.ts index 0136d4991377..85f29d884d41 100644 --- a/code/lib/cli/src/detect.test.ts +++ b/code/lib/cli/src/detect.test.ts @@ -1,13 +1,11 @@ import * as fs from 'fs'; import { logger } from '@storybook/node-logger'; -import { getBowerJson } from './helpers'; import { detect, detectFrameworkPreset, detectLanguage } from './detect'; import { ProjectType, SupportedLanguage } from './project_types'; -import type { PackageJsonWithMaybeDeps } from './js-package-manager'; +import type { JsPackageManager, PackageJsonWithMaybeDeps } from './js-package-manager'; jest.mock('./helpers', () => ({ isNxProject: jest.fn(), - getBowerJson: jest.fn(), })); jest.mock('fs', () => ({ @@ -235,54 +233,123 @@ const MOCK_FRAMEWORK_FILES: { ]; describe('Detect', () => { - it(`should return type HTML if html option is passed`, () => { - expect(detect({ dependencies: {} }, { html: true })).toBe(ProjectType.HTML); - }); + it(`should return type HTML if html option is passed`, async () => { + const packageManager = { + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: () => Promise.resolve(null), + } as any as JsPackageManager; - it(`should return type UNDETECTED if neither packageJson or bowerJson exist`, () => { - (getBowerJson as jest.Mock).mockImplementation(() => false); - expect(detect(undefined)).toBe(ProjectType.UNDETECTED); + await expect(detect(packageManager, { html: true })).resolves.toBe(ProjectType.HTML); }); - it(`should return language javascript if the TS dependency is present but less than minimum supported`, () => { + it(`should return language javascript if the TS dependency is present but less than minimum supported`, async () => { (logger.warn as jest.MockedFunction).mockClear(); - expect(detectLanguage({ dependencies: { typescript: '1.0.0' } })).toBe( - SupportedLanguage.JAVASCRIPT - ); + + const packageManager = { + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('1.0.0'); + default: + return Promise.resolve(null); + } + }, + } as Partial; + + await expect(detectLanguage(packageManager as any)).resolves.toBe(SupportedLanguage.JAVASCRIPT); expect(logger.warn).toHaveBeenCalledWith( 'Detected TypeScript < 3.8, populating with JavaScript examples' ); }); - it(`should return language typescript-3-8 if the TS dependency is >=3.8 and <4.9`, () => { - expect(detectLanguage({ dependencies: { typescript: '3.8.0' } })).toBe( - SupportedLanguage.TYPESCRIPT_3_8 - ); - expect(detectLanguage({ dependencies: { typescript: '4.8.0' } })).toBe( - SupportedLanguage.TYPESCRIPT_3_8 - ); + it(`should return language typescript-3-8 if the TS dependency is >=3.8 and <4.9`, async () => { + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName: string) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('3.8.0'); + default: + return Promise.resolve(null); + } + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.TYPESCRIPT_3_8); + + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName: string) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('4.8.0'); + default: + return Promise.resolve(null); + } + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.TYPESCRIPT_3_8); }); - it(`should return language typescript-4-9 if the dependency is >TS4.9`, () => { - expect(detectLanguage({ dependencies: { typescript: '4.9.1' } })).toBe( - SupportedLanguage.TYPESCRIPT_4_9 - ); + it(`should return language typescript-4-9 if the dependency is >TS4.9`, async () => { + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName: string) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('4.9.1'); + default: + return Promise.resolve(null); + } + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.TYPESCRIPT_4_9); }); - it(`should return language typescript if the dependency is =TS4.9`, () => { - expect(detectLanguage({ dependencies: { typescript: '4.9.0' } })).toBe( - SupportedLanguage.TYPESCRIPT_4_9 - ); + it(`should return language typescript if the dependency is =TS4.9`, async () => { + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName: string) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('4.9.0'); + default: + return Promise.resolve(null); + } + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.TYPESCRIPT_4_9); }); - it(`should return language typescript if the dependency is =TS4.9beta`, () => { - expect(detectLanguage({ dependencies: { typescript: '^4.9.0-beta' } })).toBe( - SupportedLanguage.TYPESCRIPT_4_9 - ); + it(`should return language typescript if the dependency is =TS4.9beta`, async () => { + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: (packageName: string) => { + switch (packageName) { + case 'typescript': + return Promise.resolve('4.9.0-beta'); + default: + return Promise.resolve(null); + } + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.TYPESCRIPT_3_8); }); - it(`should return language javascript by default`, () => { - expect(detectLanguage()).toBe(SupportedLanguage.JAVASCRIPT); + it(`should return language javascript by default`, async () => { + await expect( + detectLanguage({ + retrievePackageJson: () => Promise.resolve({ dependencies: {}, devDependencies: {} }), + getPackageVersion: () => { + return Promise.resolve(null); + }, + } as Partial as JsPackageManager) + ).resolves.toBe(SupportedLanguage.JAVASCRIPT); }); describe('detectFrameworkPreset should return', () => { diff --git a/code/lib/cli/src/detect.ts b/code/lib/cli/src/detect.ts index 8f8271ad8e4d..2c23dc471e7a 100644 --- a/code/lib/cli/src/detect.ts +++ b/code/lib/cli/src/detect.ts @@ -14,8 +14,8 @@ import { unsupportedTemplate, CoreBuilder, } from './project_types'; -import { commandLog, getBowerJson, isNxProject } from './helpers'; -import type { JsPackageManager, PackageJson, PackageJsonWithMaybeDeps } from './js-package-manager'; +import { commandLog, isNxProject } from './helpers'; +import type { JsPackageManager, PackageJsonWithMaybeDeps } from './js-package-manager'; const viteConfigFiles = ['vite.config.ts', 'vite.config.js', 'vite.config.mjs']; const webpackConfigFiles = ['webpack.config.js']; @@ -158,65 +158,55 @@ export function detectPnp() { return pathExistsSync(join(process.cwd(), '.pnp.cjs')); } -export function detectLanguage(packageJson?: PackageJson) { +export async function detectLanguage(packageManager: JsPackageManager) { let language = SupportedLanguage.JAVASCRIPT; - // TODO: we may need to also detect whether a jsconfig.json file is present - // in a monorepo root directory - if (!packageJson || fs.existsSync('jsconfig.json')) { + if (fs.existsSync('jsconfig.json')) { return language; } + const typescriptVersion = await packageManager.getPackageVersion('typescript'); + const prettierVersion = await packageManager.getPackageVersion('prettier'); + const babelPluginTransformTypescriptVersion = await packageManager.getPackageVersion( + '@babel/plugin-transform-typescript' + ); + const typescriptEslintParserVersion = await packageManager.getPackageVersion( + '@typescript-eslint/parser' + ); + + const eslintPluginStorybookVersion = await packageManager.getPackageVersion( + 'eslint-plugin-storybook' + ); + if ( - hasDependency(packageJson, 'typescript', (version) => - semver.gte(semver.coerce(version), '4.9.0') - ) && - (!hasDependency(packageJson, 'prettier') || - hasDependency(packageJson, 'prettier', (version) => - semver.gte(semver.coerce(version), '2.8.0') - )) && - (!hasDependency(packageJson, '@babel/plugin-transform-typescript') || - hasDependency(packageJson, '@babel/plugin-transform-typescript', (version) => - semver.gte(semver.coerce(version), '7.20.0') - )) && - (!hasDependency(packageJson, '@typescript-eslint/parser') || - hasDependency(packageJson, '@typescript-eslint/parser', (version) => - semver.gte(semver.coerce(version), '5.44.0') - )) && - (!hasDependency(packageJson, 'eslint-plugin-storybook') || - hasDependency(packageJson, 'eslint-plugin-storybook', (version) => - semver.gte(semver.coerce(version), '0.6.8') - )) + typescriptVersion && + semver.gte(typescriptVersion, '4.9.0') && + (!prettierVersion || semver.gte(prettierVersion, '2.8.0')) && + (!babelPluginTransformTypescriptVersion || + semver.gte(babelPluginTransformTypescriptVersion, '7.20.0')) && + (!typescriptEslintParserVersion || semver.gte(typescriptEslintParserVersion, '5.44.0')) && + (!eslintPluginStorybookVersion || semver.gte(eslintPluginStorybookVersion, '0.6.8')) ) { language = SupportedLanguage.TYPESCRIPT_4_9; - } else if ( - hasDependency(packageJson, 'typescript', (version) => - semver.gte(semver.coerce(version), '3.8.0') - ) - ) { + } else if (typescriptVersion && semver.gte(typescriptVersion, '3.8.0')) { language = SupportedLanguage.TYPESCRIPT_3_8; - } else if ( - hasDependency(packageJson, 'typescript', (version) => - semver.lt(semver.coerce(version), '3.8.0') - ) - ) { + } else if (typescriptVersion && semver.lt(typescriptVersion, '3.8.0')) { logger.warn('Detected TypeScript < 3.8, populating with JavaScript examples'); } return language; } -export function detect( - packageJson: PackageJson, +export async function detect( + packageManager: JsPackageManager, options: { force?: boolean; html?: boolean } = {} ) { - const bowerJson = getBowerJson(); - - if (!packageJson && !bowerJson) { + const packageJson = await packageManager.retrievePackageJson(); + if (!packageJson) { return ProjectType.UNDETECTED; } - if (isNxProject(packageJson)) { + if (await isNxProject(packageManager)) { return ProjectType.NX; } @@ -224,5 +214,5 @@ export function detect( return ProjectType.HTML; } - return detectFrameworkPreset(packageJson || bowerJson); + return detectFrameworkPreset(packageJson); } diff --git a/code/lib/cli/src/generators/ANGULAR/index.ts b/code/lib/cli/src/generators/ANGULAR/index.ts index 62ba7f0fb41b..8e76390d8170 100644 --- a/code/lib/cli/src/generators/ANGULAR/index.ts +++ b/code/lib/cli/src/generators/ANGULAR/index.ts @@ -13,16 +13,8 @@ const generator: Generator<{ projectName: string }> = async ( options, commandOptions ) => { - const angularVersionFromDependencies = semver.coerce( - (await packageManager.retrievePackageJson()).dependencies['@angular/core'] - )?.version; - - const angularVersionFromDevDependencies = semver.coerce( - (await packageManager.retrievePackageJson()).devDependencies['@angular/core'] - )?.version; - - const angularVersion = angularVersionFromDependencies || angularVersionFromDevDependencies; - const isWebpack5 = semver.gte(angularVersion, '12.0.0'); + const angularVersion = await packageManager.getPackageVersion('@angular/core'); + const isWebpack5 = angularVersion && semver.gte(angularVersion, '12.0.0'); const updatedOptions = isWebpack5 ? { ...options, builder: CoreBuilder.Webpack5 } : options; const angularJSON = new AngularJSON(); diff --git a/code/lib/cli/src/generators/REACT/index.ts b/code/lib/cli/src/generators/REACT/index.ts index c37ebc323f99..86fcd790a312 100644 --- a/code/lib/cli/src/generators/REACT/index.ts +++ b/code/lib/cli/src/generators/REACT/index.ts @@ -5,7 +5,7 @@ import type { Generator } from '../types'; const generator: Generator = async (packageManager, npmOptions, options) => { // Add prop-types dependency if not using TypeScript - const language = detectLanguage(); + const language = await detectLanguage(packageManager); const extraPackages = language === SupportedLanguage.JAVASCRIPT ? ['prop-types'] : []; await baseGenerator(packageManager, npmOptions, options, 'react', { diff --git a/code/lib/cli/src/generators/REACT_SCRIPTS/index.ts b/code/lib/cli/src/generators/REACT_SCRIPTS/index.ts index 8948aea95500..46846ccc1bc5 100644 --- a/code/lib/cli/src/generators/REACT_SCRIPTS/index.ts +++ b/code/lib/cli/src/generators/REACT_SCRIPTS/index.ts @@ -25,8 +25,7 @@ const generator: Generator = async (packageManager, npmOptions, options) => { } : {}; - const packageJson = await packageManager.retrievePackageJson(); - const craVersion = semver.coerce(packageJson.dependencies['react-scripts'])?.version; + const craVersion = await packageManager.getPackageVersion('react-scripts'); const isCra5OrHigher = craVersion && semver.gte(craVersion, '5.0.0'); const updatedOptions = isCra5OrHigher ? { ...options, builder: CoreBuilder.Webpack5 } : options; diff --git a/code/lib/cli/src/helpers.ts b/code/lib/cli/src/helpers.ts index f928fe2c9983..ebe272d7ef0e 100644 --- a/code/lib/cli/src/helpers.ts +++ b/code/lib/cli/src/helpers.ts @@ -18,16 +18,6 @@ import storybookMonorepoPackages from './versions'; const logger = console; -export function getBowerJson() { - const bowerJsonPath = path.resolve('bower.json'); - if (!fs.existsSync(bowerJsonPath)) { - return false; - } - - const jsonContent = fs.readFileSync(bowerJsonPath, 'utf8'); - return JSON.parse(jsonContent); -} - export function readFileAsJson(jsonPath: string, allowComments?: boolean) { const filePath = path.resolve(jsonPath); if (!fs.existsSync(filePath)) { @@ -274,6 +264,7 @@ export function getStorybookVersionSpecifier(packageJson: PackageJsonWithDepsAnd return allDeps[storybookPackage]; } -export function isNxProject(packageJSON: PackageJson) { - return !!packageJSON.devDependencies?.nx || fs.existsSync('nx.json'); +export async function isNxProject(packageManager: JsPackageManager) { + const nxVersion = await packageManager.getPackageVersion('nx'); + return nxVersion ?? fs.existsSync('nx.json'); } diff --git a/code/lib/cli/src/initiate.ts b/code/lib/cli/src/initiate.ts index 6d62f6061ca7..1e711c0b7f5a 100644 --- a/code/lib/cli/src/initiate.ts +++ b/code/lib/cli/src/initiate.ts @@ -50,14 +50,7 @@ const installStorybook = async ( skipInstall: options.skipInstall, }; - let packageJson; - try { - packageJson = await packageManager.readPackageJson(); - } catch (err) { - // - } - - const language = detectLanguage(packageJson); + const language = await detectLanguage(packageManager); const pnp = detectPnp(); const generatorOptions = { @@ -263,8 +256,6 @@ async function doInitiate(options: CommandOptions, pkg: PackageJson): Promise; + public abstract getPackageVersion(packageName: string, basePath?: string): Promise; // NOTE: for some reason yarn prefers the npm registry in // local development, so always use npm diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.ts b/code/lib/cli/src/js-package-manager/NPMProxy.ts index b626232988a0..0eb4802e9670 100644 --- a/code/lib/cli/src/js-package-manager/NPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/NPMProxy.ts @@ -4,6 +4,7 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import { existsSync, readFileSync } from 'fs'; import path from 'path'; +import semver from 'semver'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; import type { InstallationMetadata, PackageMetadata } from './types'; @@ -80,7 +81,10 @@ export class NPMProxy extends JsPackageManager { return this.executeCommand({ command: 'npm', args: ['--version'] }); } - public async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + public async getPackageVersion( + packageName: string, + basePath = process.cwd() + ): Promise { const packageJsonPath = await findUpSync( (dir) => { const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); @@ -94,7 +98,7 @@ export class NPMProxy extends JsPackageManager { } const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; - return packageJson.version; + return semver.coerce(packageJson.version)?.version ?? null; } getInstallArgs(): string[] { diff --git a/code/lib/cli/src/js-package-manager/PNPMProxy.ts b/code/lib/cli/src/js-package-manager/PNPMProxy.ts index 0af34ad9580d..b7feb0d439d0 100644 --- a/code/lib/cli/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/PNPMProxy.ts @@ -3,6 +3,7 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import path from 'path'; import fs from 'fs'; +import semver from 'semver'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; import type { InstallationMetadata, PackageMetadata } from './types'; @@ -130,9 +131,10 @@ export class PNPMProxy extends JsPackageManager { 'utf-8' ); - return JSON.parse(packageJSON).version; + return semver.coerce(JSON.parse(packageJSON).version)?.version ?? null; } catch (error) { console.error('Error while fetching package version in Yarn PnP mode:', error); + return null; } } @@ -150,7 +152,7 @@ export class PNPMProxy extends JsPackageManager { const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); - return packageJson.version; + return semver.coerce(packageJson.version)?.version ?? null; } protected getResolutions(packageJson: PackageJson, versions: Record) { diff --git a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts index 3e8d23fc5e02..e9038339af16 100644 --- a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts @@ -2,6 +2,7 @@ import dedent from 'ts-dedent'; import { sync as findUpSync } from 'find-up'; import { existsSync, readFileSync } from 'fs'; import path from 'path'; +import semver from 'semver'; import { createLogStream } from '../utils'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -62,7 +63,10 @@ export class Yarn1Proxy extends JsPackageManager { return this.executeCommand({ command: `yarn`, args: [command, ...args], cwd }); } - public async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + public async getPackageVersion( + packageName: string, + basePath = process.cwd() + ): Promise { const packageJsonPath = await findUpSync( (dir) => { const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); @@ -76,7 +80,7 @@ export class Yarn1Proxy extends JsPackageManager { } const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; - return packageJson.version; + return semver.coerce(packageJson.version)?.version ?? null; } public async findInstallations(pattern: string[]) { diff --git a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts index d1fb0d048ef1..d402179aa5ed 100644 --- a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts @@ -4,6 +4,7 @@ import fs, { existsSync, readFileSync } from 'fs'; import path from 'path'; import { NodeFS, VirtualFS, ZipOpenFS } from '@yarnpkg/fslib'; import { getLibzipSync } from '@yarnpkg/libzip'; +import semver from 'semver'; import { createLogStream } from '../utils'; import { JsPackageManager } from './JsPackageManager'; import type { PackageJson } from './PackageJson'; @@ -153,9 +154,10 @@ export class Yarn2Proxy extends JsPackageManager { const virtualFile = virtualFs.readJsonSync( path.join(pkg.packageLocation, 'package.json') as any ); - return virtualFile.version; + return semver.coerce(virtualFile.version)?.version ?? null; } catch (error) { console.error('Error while fetching package version in Yarn PnP mode:', error); + return null; } } @@ -172,7 +174,7 @@ export class Yarn2Proxy extends JsPackageManager { } const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); - return packageJson.version; + return semver.coerce(packageJson.version)?.version ?? null; } protected getResolutions(packageJson: PackageJson, versions: Record) { diff --git a/code/lib/core-common/src/utils/get-storybook-info.ts b/code/lib/core-common/src/utils/get-storybook-info.ts index e19ccdda9b21..7fea27ffedaa 100644 --- a/code/lib/core-common/src/utils/get-storybook-info.ts +++ b/code/lib/core-common/src/utils/get-storybook-info.ts @@ -45,6 +45,8 @@ export const frameworkPackages: Record = { 'storybook-solidjs-vite': 'solid', }; +export const builderPackages = ['@storybook/builder-webpack5', '@storybook/builder-vite']; + const logger = console; const findDependency = ( From 6c06430ae3ba2d64c8d8680fad274b882412d3f3 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 19 May 2023 16:35:14 +0200 Subject: [PATCH 04/24] Fix lint --- code/lib/cli/src/automigrate/fixes/add-react.test.ts | 8 +++++++- code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/add-react.test.ts b/code/lib/cli/src/automigrate/fixes/add-react.test.ts index 42bb20b60e2e..4602a30d3ecd 100644 --- a/code/lib/cli/src/automigrate/fixes/add-react.test.ts +++ b/code/lib/cli/src/automigrate/fixes/add-react.test.ts @@ -1,3 +1,4 @@ +import type { StorybookConfig } from '@storybook/types'; import type { JsPackageManager, PackageJson } from '../../js-package-manager'; import { addReact } from './add-react'; @@ -5,7 +6,12 @@ const checkAddReact = async (packageJson: PackageJson) => { const packageManager = { retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }), } as JsPackageManager; - return addReact.check({ packageManager }); + + return addReact.check({ + packageManager, + mainConfig: {} as StorybookConfig, + storybookVersion: '7.0.0', + }); }; describe('addReact fix', () => { diff --git a/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts b/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts index cb242c9b0626..54a527848ae6 100644 --- a/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts +++ b/code/lib/cli/src/automigrate/fixes/eslint-plugin.test.ts @@ -41,6 +41,8 @@ const checkEslint = async ({ }); return eslintPlugin.check({ packageManager: makePackageManager(packageJson), + mainConfig: {} as any, + storybookVersion: '7.0.0', }); }; From f3bc515e6d36e509ed86160a1fb7c129c62f5c43 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 19 May 2023 17:02:20 +0200 Subject: [PATCH 05/24] Remove unused import --- code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts index 84e4f55fca33..d86b266b6081 100644 --- a/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts +++ b/code/lib/cli/src/automigrate/fixes/nodejs-requirement.test.ts @@ -1,6 +1,5 @@ /// ; -import { makePackageManager } from '../helpers/testing-helpers'; import { nodeJsRequirement } from './nodejs-requirement'; // eslint-disable-next-line global-require, jest/no-mocks-import From ff9caf8bfefaeb96cae268715059b380a36a3991 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 22 May 2023 17:15:11 +0200 Subject: [PATCH 06/24] Take backslash paths into account --- .../helpers/mainConfigFile.test.ts | 24 +++++++++++++++++++ .../src/automigrate/helpers/mainConfigFile.ts | 24 +++++++++++++------ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts index 40770c7cde9d..f31ca41f0a0f 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts @@ -45,6 +45,19 @@ describe('getBuilderPackageName', () => { expect(packageName).toBe(builderPackage); }); + it('should return builder package name when core.builder.name contains windows backslash paths', () => { + const builderPackage = '@storybook/builder-webpack5'; + const packageNameOrPath = 'c:\\path\\to\\@storybook\\builder-webpack5'; + const mainConfig = { + core: { + builder: { name: packageNameOrPath }, + }, + }; + + const packageName = getBuilderPackageName(mainConfig as any); + expect(packageName).toBe(builderPackage); + }); + it(`should return package name or path when core.builder doesn't contain the name of a valid builder package`, () => { const packageNameOrPath = '@my-org/storybook-builder'; const mainConfig = { @@ -95,6 +108,17 @@ describe('getFrameworkPackageName', () => { expect(packageName).toBe(frameworkPackage); }); + it('should return builder package name when framework.name contains windows backslash paths', () => { + const builderPackage = '@storybook/react-vite'; + const packageNameOrPath = 'c:\\path\\to\\@storybook\\react-vite'; + const mainConfig = { + framework: { name: packageNameOrPath }, + }; + + const packageName = getFrameworkPackageName(mainConfig as any); + expect(packageName).toBe(builderPackage); + }); + it(`should return package name or path when framework does not contain the name of a valid framework package`, () => { const packageNameOrPath = '@my-org/storybook-framework'; const mainConfig = { diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index ebf10cef9799..b5240a6b4f5c 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -11,6 +11,7 @@ import { readConfig, writeConfig as writeConfigFile } from '@storybook/csf-tools import chalk from 'chalk'; import semver from 'semver'; import dedent from 'ts-dedent'; +import path from 'path'; import type { JsPackageManager } from '../../js-package-manager'; const logger = console; @@ -24,10 +25,15 @@ export const getFrameworkPackageName = (mainConfig?: StorybookConfig) => { const packageNameOrPath = typeof mainConfig?.framework === 'string' ? mainConfig.framework : mainConfig?.framework?.name; - return packageNameOrPath - ? Object.keys(frameworkPackages).find((pkg) => packageNameOrPath.endsWith(pkg)) ?? - packageNameOrPath - : null; + if (!packageNameOrPath) { + return null; + } + + const normalizedPath = path.normalize(packageNameOrPath).replace(new RegExp(/\\/, 'g'), '/'); + + return ( + Object.keys(frameworkPackages).find((pkg) => normalizedPath.endsWith(pkg)) || packageNameOrPath + ); }; /** @@ -41,9 +47,13 @@ export const getBuilderPackageName = (mainConfig?: StorybookConfig) => { ? mainConfig.core.builder : mainConfig?.core?.builder?.name; - return packageNameOrPath - ? builderPackages.find((pkg) => packageNameOrPath.endsWith(pkg)) ?? packageNameOrPath - : null; + if (!packageNameOrPath) { + return null; + } + + const normalizedPath = path.normalize(packageNameOrPath).replace(new RegExp(/\\/, 'g'), '/'); + + return builderPackages.find((pkg) => normalizedPath.endsWith(pkg)) || packageNameOrPath; }; /** From 4fc7e9daff9a522b168c97bbecf0cb759f924fbd Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 22 May 2023 17:33:54 +0200 Subject: [PATCH 07/24] Await project type detection --- code/lib/cli/src/detect.ts | 1 + code/lib/cli/src/initiate.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/code/lib/cli/src/detect.ts b/code/lib/cli/src/detect.ts index 2c23dc471e7a..6b887a625664 100644 --- a/code/lib/cli/src/detect.ts +++ b/code/lib/cli/src/detect.ts @@ -202,6 +202,7 @@ export async function detect( options: { force?: boolean; html?: boolean } = {} ) { const packageJson = await packageManager.retrievePackageJson(); + if (!packageJson) { return ProjectType.UNDETECTED; } diff --git a/code/lib/cli/src/initiate.ts b/code/lib/cli/src/initiate.ts index 1e711c0b7f5a..447c48736ce9 100644 --- a/code/lib/cli/src/initiate.ts +++ b/code/lib/cli/src/initiate.ts @@ -268,7 +268,7 @@ async function doInitiate(options: CommandOptions, pkg: PackageJson): Promise Date: Tue, 23 May 2023 08:35:28 +0200 Subject: [PATCH 08/24] Await language detection --- scripts/tasks/sandbox-parts.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/tasks/sandbox-parts.ts b/scripts/tasks/sandbox-parts.ts index cd15ac2c81ec..3150c4709154 100644 --- a/scripts/tasks/sandbox-parts.ts +++ b/scripts/tasks/sandbox-parts.ts @@ -374,7 +374,10 @@ export const addStories: Task['run'] = async ( // Ensure that we match the right stories in the stories directory const packageJson = await import(join(cwd, 'package.json')); - updateStoriesField(mainConfig, detectLanguage(packageJson) === SupportedLanguage.JAVASCRIPT); + updateStoriesField( + mainConfig, + (await detectLanguage(packageJson)) === SupportedLanguage.JAVASCRIPT + ); const isCoreRenderer = template.expected.renderer.startsWith('@storybook/') && From 79c97242db6234a8796de3f6963c075bf7e68c01 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 23 May 2023 08:51:06 +0200 Subject: [PATCH 09/24] Pass packageManager instead of packageJSON to detectLanguage --- scripts/tasks/sandbox-parts.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tasks/sandbox-parts.ts b/scripts/tasks/sandbox-parts.ts index 3150c4709154..22eb2dc789df 100644 --- a/scripts/tasks/sandbox-parts.ts +++ b/scripts/tasks/sandbox-parts.ts @@ -371,12 +371,12 @@ export const addStories: Task['run'] = async ( const storiesPath = await findFirstPath([join('src', 'stories'), 'stories'], { cwd }); const mainConfig = await readMainConfig({ cwd }); + const packageManager = JsPackageManagerFactory.getPackageManager(); // Ensure that we match the right stories in the stories directory - const packageJson = await import(join(cwd, 'package.json')); updateStoriesField( mainConfig, - (await detectLanguage(packageJson)) === SupportedLanguage.JAVASCRIPT + (await detectLanguage(packageManager)) === SupportedLanguage.JAVASCRIPT ); const isCoreRenderer = From 82d4e7a07980e8f891ab1b89bc253a4221ef1391 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 May 2023 13:53:44 +0200 Subject: [PATCH 10/24] Fix pnp/workspace related issues and read exact installed versions from packages --- code/frameworks/nextjs/src/utils.ts | 10 +++- .../fixes/incompatible-addons.test.ts | 52 +++++++++---------- .../automigrate/fixes/incompatible-addons.ts | 4 +- .../helpers/getActualPackageVersions.ts | 26 ---------- .../helpers/getIncompatibleAddons.ts | 43 ++++++++------- code/lib/cli/src/detect.ts | 5 +- code/lib/cli/src/generators/ANGULAR/index.ts | 2 +- code/lib/cli/src/generators/baseGenerator.ts | 2 +- code/lib/cli/src/helpers.ts | 3 +- .../js-package-manager/JsPackageManager.ts | 5 ++ .../cli/src/js-package-manager/NPMProxy.ts | 17 +++--- .../cli/src/js-package-manager/PNPMProxy.ts | 17 +++--- .../cli/src/js-package-manager/Yarn1Proxy.ts | 15 ++++-- .../cli/src/js-package-manager/Yarn2Proxy.ts | 18 ++++--- code/lib/cli/src/upgrade.ts | 4 +- .../src/utils/get-storybook-info.ts | 16 +++--- code/lib/core-server/package.json | 1 + code/yarn.lock | 1 + 18 files changed, 124 insertions(+), 117 deletions(-) delete mode 100644 code/lib/cli/src/automigrate/helpers/getActualPackageVersions.ts diff --git a/code/frameworks/nextjs/src/utils.ts b/code/frameworks/nextjs/src/utils.ts index 9ba051ab7e9e..23244ee9d5b4 100644 --- a/code/frameworks/nextjs/src/utils.ts +++ b/code/frameworks/nextjs/src/utils.ts @@ -94,7 +94,15 @@ export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?: * scopedResolve('styled-jsx') === '/some/path/node_modules/styled-jsx' */ export const scopedResolve = (id: string): string => { - const scopedModulePath = require.resolve(id, { paths: [path.resolve()] }); + // id = css-loader/package.json + let scopedModulePath; + + try { + scopedModulePath = require.resolve(id, { paths: [path.resolve()] }); + } catch (e) { + scopedModulePath = require.resolve(id); + } + const moduleFolderStrPosition = scopedModulePath.lastIndexOf( id.replace(/\//g /* all '/' occurances */, path.sep) ); diff --git a/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts b/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts index 05b73100cff3..46978eba8396 100644 --- a/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts +++ b/code/lib/cli/src/automigrate/fixes/incompatible-addons.test.ts @@ -2,11 +2,8 @@ import type { StorybookConfig } from '@storybook/types'; import { incompatibleAddons } from './incompatible-addons'; -import * as packageVersions from '../helpers/getActualPackageVersions'; import type { JsPackageManager } from '../../js-package-manager'; -jest.mock('../helpers/getActualPackageVersions'); - const check = async ({ packageManager, main: mainConfig = {}, @@ -28,22 +25,20 @@ describe('incompatible-addons fix', () => { afterEach(jest.restoreAllMocks); it('should show incompatible addons', async () => { - jest.spyOn(packageVersions, 'getActualPackageVersions').mockReturnValueOnce( - Promise.resolve([ - { - name: '@storybook/addon-essentials', - version: '7.0.0', - }, - { - name: '@storybook/addon-info', - version: '5.3.21', - }, - ]) - ); - await expect( check({ - packageManager: {}, + packageManager: { + getPackageVersion(packageName, basePath) { + switch (packageName) { + case '@storybook/addon-essentials': + return Promise.resolve('7.0.0'); + case '@storybook/addon-info': + return Promise.resolve('5.3.21'); + default: + return Promise.resolve(null); + } + }, + }, main: { addons: ['@storybook/essentials', '@storybook/addon-info'] }, }) ).resolves.toEqual({ @@ -57,17 +52,20 @@ describe('incompatible-addons fix', () => { }); it('no-op when there are no incompatible addons', async () => { - jest.spyOn(packageVersions, 'getActualPackageVersions').mockReturnValueOnce( - Promise.resolve([ - { - name: '@storybook/addon-essentials', - version: '7.0.0', - }, - ]) - ); - await expect( - check({ packageManager: {}, main: { addons: ['@storybook/essentials'] } }) + check({ + packageManager: { + getPackageVersion(packageName, basePath) { + switch (packageName) { + case '@storybook/addon-essentials': + return Promise.resolve('7.0.0'); + default: + return Promise.resolve(null); + } + }, + }, + main: { addons: ['@storybook/essentials'] }, + }) ).resolves.toBeNull(); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts b/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts index bb5a45accd19..3455408f279d 100644 --- a/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts +++ b/code/lib/cli/src/automigrate/fixes/incompatible-addons.ts @@ -11,8 +11,8 @@ export const incompatibleAddons: Fix = { id: 'incompatible-addons', promptOnly: true, - async check({ mainConfig }) { - const incompatibleAddonList = await getIncompatibleAddons(mainConfig); + async check({ mainConfig, packageManager }) { + const incompatibleAddonList = await getIncompatibleAddons(mainConfig, packageManager); return incompatibleAddonList.length > 0 ? { incompatibleAddonList } : null; }, diff --git a/code/lib/cli/src/automigrate/helpers/getActualPackageVersions.ts b/code/lib/cli/src/automigrate/helpers/getActualPackageVersions.ts deleted file mode 100644 index 77f741def3d6..000000000000 --- a/code/lib/cli/src/automigrate/helpers/getActualPackageVersions.ts +++ /dev/null @@ -1,26 +0,0 @@ -import * as fs from 'fs-extra'; -import path from 'path'; - -export const getActualPackageVersions = async (packages: string[]) => { - return Promise.all(packages.map(getActualPackageVersion)); -}; - -export const getActualPackageVersion = async (packageName: string) => { - try { - const packageJson = await getActualPackageJson(packageName); - return { - name: packageName, - version: packageJson.version, - }; - } catch (err) { - return { name: packageName, version: null }; - } -}; - -export const getActualPackageJson = async (packageName: string) => { - const resolvedPackageJson = require.resolve(path.join(packageName, 'package.json'), { - paths: [process.cwd()], - }); - const packageJson = await fs.readJson(resolvedPackageJson); - return packageJson; -}; diff --git a/code/lib/cli/src/automigrate/helpers/getIncompatibleAddons.ts b/code/lib/cli/src/automigrate/helpers/getIncompatibleAddons.ts index fcc4f79e9f4f..d6fc28ed776b 100644 --- a/code/lib/cli/src/automigrate/helpers/getIncompatibleAddons.ts +++ b/code/lib/cli/src/automigrate/helpers/getIncompatibleAddons.ts @@ -1,9 +1,12 @@ import type { StorybookConfig } from '@storybook/types'; import semver from 'semver'; -import { getActualPackageVersions } from './getActualPackageVersions'; import { getAddonNames } from './mainConfigFile'; +import { JsPackageManagerFactory } from '../../js-package-manager'; -export const getIncompatibleAddons = async (mainConfig: StorybookConfig) => { +export const getIncompatibleAddons = async ( + mainConfig: StorybookConfig, + packageManager = JsPackageManagerFactory.getPackageManager() +) => { // TODO: Keep this up to date with https://github.com/storybookjs/storybook/issues/20529 in case more addons get added const incompatibleList = { '@storybook/addon-knobs': '6.4.0', @@ -39,29 +42,29 @@ export const getIncompatibleAddons = async (mainConfig: StorybookConfig) => { return []; } - const addonVersions = await getActualPackageVersions(addons); + const addonVersions = await Promise.all( + addons.map( + async (addon) => + ({ + name: addon, + version: await packageManager.getPackageVersion(addon), + } as { name: keyof typeof incompatibleList; version: string }) + ) + ); const incompatibleAddons: { name: string; version: string }[] = []; - addonVersions.forEach( - ({ - name, - version: installedVersion, - }: { - name: keyof typeof incompatibleList; - version: string; - }) => { - if (installedVersion === null) return; + addonVersions.forEach(({ name, version: installedVersion }) => { + if (installedVersion === null) return; - const addonVersion = incompatibleList[name]; - try { - if (semver.lte(semver.coerce(installedVersion), semver.coerce(addonVersion))) { - incompatibleAddons.push({ name, version: installedVersion }); - } - } catch (err) { - // we tried our best but if we can't compare, we just no-op for that addon + const addonVersion = incompatibleList[name]; + try { + if (semver.lte(semver.coerce(installedVersion), semver.coerce(addonVersion))) { + incompatibleAddons.push({ name, version: installedVersion }); } + } catch (err) { + // we tried our best but if we can't compare, we just no-op for that addon } - ); + }); return incompatibleAddons; }; diff --git a/code/lib/cli/src/detect.ts b/code/lib/cli/src/detect.ts index 6b887a625664..7a91210fc6fc 100644 --- a/code/lib/cli/src/detect.ts +++ b/code/lib/cli/src/detect.ts @@ -3,8 +3,7 @@ import findUp from 'find-up'; import semver from 'semver'; import { logger } from '@storybook/node-logger'; -import { pathExistsSync } from 'fs-extra'; -import { join, resolve } from 'path'; +import { resolve } from 'path'; import prompts from 'prompts'; import type { TemplateConfiguration, TemplateMatcher } from './project_types'; import { @@ -155,7 +154,7 @@ export function isStorybookInstantiated(configDir = resolve(process.cwd(), '.sto } export function detectPnp() { - return pathExistsSync(join(process.cwd(), '.pnp.cjs')); + return findUp.sync(['.pnp.js', '.pnp.cjs']); } export async function detectLanguage(packageManager: JsPackageManager) { diff --git a/code/lib/cli/src/generators/ANGULAR/index.ts b/code/lib/cli/src/generators/ANGULAR/index.ts index 8e76390d8170..3c4d9a1f0ca9 100644 --- a/code/lib/cli/src/generators/ANGULAR/index.ts +++ b/code/lib/cli/src/generators/ANGULAR/index.ts @@ -71,7 +71,7 @@ const generator: Generator<{ projectName: string }> = async ( }, 'angular', { - ...(useCompodoc && { extraPackages: ['@compodoc/compodoc'] }), + ...(useCompodoc && { extraPackages: ['@compodoc/compodoc', '@storybook/addon-docs'] }), addScripts: false, componentsDestinationPath: root ? `${root}/src/stories` : undefined, storybookConfigFolder: storybookFolder, diff --git a/code/lib/cli/src/generators/baseGenerator.ts b/code/lib/cli/src/generators/baseGenerator.ts index 999c533f0025..6b8b02b5692a 100644 --- a/code/lib/cli/src/generators/baseGenerator.ts +++ b/code/lib/cli/src/generators/baseGenerator.ts @@ -246,7 +246,7 @@ export async function baseGenerator( addons.push('@storybook/addon-interactions'); addonPackages.push( '@storybook/addon-interactions', - '@storybook/testing-library@^0.0.14-next.1' + '@storybook/testing-library@^0.1.1-future.2' ); } diff --git a/code/lib/cli/src/helpers.ts b/code/lib/cli/src/helpers.ts index ebe272d7ef0e..96b87bd35415 100644 --- a/code/lib/cli/src/helpers.ts +++ b/code/lib/cli/src/helpers.ts @@ -6,6 +6,7 @@ import chalk from 'chalk'; import { satisfies } from 'semver'; import stripJsonComments from 'strip-json-comments'; +import findUp from 'find-up'; import { getCliDir, getRendererDir } from './dirs'; import type { JsPackageManager, @@ -266,5 +267,5 @@ export function getStorybookVersionSpecifier(packageJson: PackageJsonWithDepsAnd export async function isNxProject(packageManager: JsPackageManager) { const nxVersion = await packageManager.getPackageVersion('nx'); - return nxVersion ?? fs.existsSync('nx.json'); + return nxVersion ?? findUp.sync('nx.json'); } diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index edaf759d49bf..cc759dabcf06 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -49,6 +49,11 @@ export abstract class JsPackageManager { public readonly cwd?: string; + public abstract getPackageJSON( + packageName: string, + basePath?: string + ): Promise; + public abstract getPackageVersion(packageName: string, basePath?: string): Promise; // NOTE: for some reason yarn prefers the npm registry in diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.ts b/code/lib/cli/src/js-package-manager/NPMProxy.ts index 0eb4802e9670..237fed5fa03f 100644 --- a/code/lib/cli/src/js-package-manager/NPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/NPMProxy.ts @@ -81,10 +81,7 @@ export class NPMProxy extends JsPackageManager { return this.executeCommand({ command: 'npm', args: ['--version'] }); } - public async getPackageVersion( - packageName: string, - basePath = process.cwd() - ): Promise { + public async getPackageJSON(packageName: string, basePath?: string): Promise { const packageJsonPath = await findUpSync( (dir) => { const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); @@ -97,8 +94,16 @@ export class NPMProxy extends JsPackageManager { return null; } - const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; - return semver.coerce(packageJson.version)?.version ?? null; + const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); + return packageJson; + } + + public async getPackageVersion( + packageName: string, + basePath = process.cwd() + ): Promise { + const packageJson = await this.getPackageJSON(packageName, basePath); + return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; } getInstallArgs(): string[] { diff --git a/code/lib/cli/src/js-package-manager/PNPMProxy.ts b/code/lib/cli/src/js-package-manager/PNPMProxy.ts index b7feb0d439d0..25e9ff0cbdf4 100644 --- a/code/lib/cli/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/PNPMProxy.ts @@ -111,7 +111,7 @@ export class PNPMProxy extends JsPackageManager { } } - async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + public async getPackageJSON(packageName: string, basePath?: string): Promise { const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); if (pnpapiPath) { @@ -126,12 +126,11 @@ export class PNPMProxy extends JsPackageManager { const pkgLocator = pnpApi.findPackageLocator(resolvedPath); const pkg = pnpApi.getPackageInformation(pkgLocator); - const packageJSON = fs.readFileSync( - path.join(pkg.packageLocation, 'package.json'), - 'utf-8' + const packageJSON = JSON.parse( + fs.readFileSync(path.join(pkg.packageLocation, 'package.json'), 'utf-8') ); - return semver.coerce(JSON.parse(packageJSON).version)?.version ?? null; + return packageJSON; } catch (error) { console.error('Error while fetching package version in Yarn PnP mode:', error); return null; @@ -150,9 +149,13 @@ export class PNPMProxy extends JsPackageManager { return null; } - const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); + return JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); + } + + async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const packageJSON = await this.getPackageJSON(packageName, basePath); - return semver.coerce(packageJson.version)?.version ?? null; + return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; } protected getResolutions(packageJson: PackageJson, versions: Record) { diff --git a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts index e9038339af16..f64dd2360a2f 100644 --- a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts @@ -63,10 +63,10 @@ export class Yarn1Proxy extends JsPackageManager { return this.executeCommand({ command: `yarn`, args: [command, ...args], cwd }); } - public async getPackageVersion( + public async getPackageJSON( packageName: string, basePath = process.cwd() - ): Promise { + ): Promise { const packageJsonPath = await findUpSync( (dir) => { const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); @@ -79,8 +79,15 @@ export class Yarn1Proxy extends JsPackageManager { return null; } - const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; - return semver.coerce(packageJson.version)?.version ?? null; + return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; + } + + public async getPackageVersion( + packageName: string, + basePath = process.cwd() + ): Promise { + const packageJson = await this.getPackageJSON(packageName, basePath); + return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; } public async findInstallations(pattern: string[]) { diff --git a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts index d402179aa5ed..5a44de87802f 100644 --- a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts @@ -123,7 +123,7 @@ export class Yarn2Proxy extends JsPackageManager { } } - async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + async getPackageJSON(packageName: string, basePath = process.cwd()): Promise { const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); if (pnpapiPath) { @@ -151,12 +151,11 @@ export class Yarn2Proxy extends JsPackageManager { baseFs: zipOpenFs, }); - const virtualFile = virtualFs.readJsonSync( - path.join(pkg.packageLocation, 'package.json') as any - ); - return semver.coerce(virtualFile.version)?.version ?? null; + return virtualFs.readJsonSync(path.join(pkg.packageLocation, 'package.json') as any); } catch (error) { - console.error('Error while fetching package version in Yarn PnP mode:', error); + if (error.code !== 'MODULE_NOT_FOUND') { + console.error('Error while fetching package version in Yarn PnP mode:', error); + } return null; } } @@ -174,7 +173,12 @@ export class Yarn2Proxy extends JsPackageManager { } const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); - return semver.coerce(packageJson.version)?.version ?? null; + return packageJson; + } + + async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + const packageJSON = await this.getPackageJSON(packageName, basePath); + return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; } protected getResolutions(packageJson: PackageJson, versions: Record) { diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index af8e53ce6a95..6cb24a457eb3 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -189,7 +189,7 @@ export const doUpgrade = async ({ } const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr }); - const beforeVersion = await getStorybookCoreVersion(); + const beforeVersion = await getStorybookCoreVersion(packageManager); commandLog(`Checking for latest versions of '@storybook/*' packages`); @@ -238,7 +238,7 @@ export const doUpgrade = async ({ automigrationResults = await automigrate({ dryRun, yes, packageManager: pkgMgr, configDir }); } if (!options.disableTelemetry) { - const afterVersion = await getStorybookCoreVersion(); + const afterVersion = await getStorybookCoreVersion(packageManager); const { preCheckFailure, fixResults } = automigrationResults || {}; const automigrationTelemetry = { automigrationResults: preCheckFailure ? null : fixResults, diff --git a/code/lib/core-common/src/utils/get-storybook-info.ts b/code/lib/core-common/src/utils/get-storybook-info.ts index 7fea27ffedaa..9689dd62bf70 100644 --- a/code/lib/core-common/src/utils/get-storybook-info.ts +++ b/code/lib/core-common/src/utils/get-storybook-info.ts @@ -1,6 +1,7 @@ import path from 'path'; import fse from 'fs-extra'; import type { CoreCommon_StorybookInfo, PackageJson } from '@storybook/types'; +import type { JsPackageManager } from 'lib/cli/src'; import { getStorybookConfiguration } from './get-storybook-configuration'; export const rendererPackages: Record = { @@ -52,17 +53,17 @@ const logger = console; const findDependency = ( { dependencies, devDependencies, peerDependencies }: PackageJson, predicate: (entry: [string, string | undefined]) => string -) => [ - Object.entries(dependencies || {}).find(predicate), - Object.entries(devDependencies || {}).find(predicate), - Object.entries(peerDependencies || {}).find(predicate), -]; +) => + [ + Object.entries(dependencies || {}).find(predicate), + Object.entries(devDependencies || {}).find(predicate), + Object.entries(peerDependencies || {}).find(predicate), + ] as const; const getRendererInfo = (packageJson: PackageJson) => { // Pull the viewlayer from dependencies in package.json const [dep, devDep, peerDep] = findDependency(packageJson, ([key]) => rendererPackages[key]); const [pkg, version] = dep || devDep || peerDep || []; - const renderer = pkg ? rendererPackages[pkg] : undefined; if (dep && devDep && dep[0] === devDep[0]) { logger.warn( @@ -77,10 +78,7 @@ const getRendererInfo = (packageJson: PackageJson) => { return { version, - framework: renderer, frameworkPackage: pkg, - renderer, - rendererPackage: pkg, }; }; diff --git a/code/lib/core-server/package.json b/code/lib/core-server/package.json index 2cf1c4cd6e95..a1fbb4948b15 100644 --- a/code/lib/core-server/package.json +++ b/code/lib/core-server/package.json @@ -94,6 +94,7 @@ "telejson": "^7.0.3", "tiny-invariant": "^1.3.1", "ts-dedent": "^2.0.0", + "util": "^0.12.4", "util-deprecate": "^1.0.2", "watchpack": "^2.2.0", "ws": "^8.2.3" diff --git a/code/yarn.lock b/code/yarn.lock index d18353969a0f..721b5e8537ac 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -6402,6 +6402,7 @@ __metadata: tiny-invariant: ^1.3.1 ts-dedent: ^2.0.0 typescript: ~4.9.3 + util: ^0.12.4 util-deprecate: ^1.0.2 watchpack: ^2.2.0 ws: ^8.2.3 From c226dcd062de45c422b915a830405dc1c9e8f1c2 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 May 2023 15:29:55 +0200 Subject: [PATCH 11/24] Get proper installed Storybook version --- code/lib/cli/src/add.ts | 6 ++++-- .../src/automigrate/helpers/mainConfigFile.ts | 5 ++--- code/lib/cli/src/automigrate/index.ts | 16 ++++++++-------- code/lib/cli/src/utils.ts | 15 +++++++++++++++ .../core-common/src/utils/get-storybook-info.ts | 1 - 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/code/lib/cli/src/add.ts b/code/lib/cli/src/add.ts index efced649c957..7b9aca413e16 100644 --- a/code/lib/cli/src/add.ts +++ b/code/lib/cli/src/add.ts @@ -11,6 +11,7 @@ import { useNpmWarning, type PackageManagerName, } from './js-package-manager'; +import { getStorybookVersion } from './utils'; const logger = console; @@ -83,7 +84,7 @@ export async function add( const packageJson = await packageManager.retrievePackageJson(); const [addonName, versionSpecifier] = getVersionSpecifier(addon); - const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson); + const { mainConfig } = getStorybookInfo(packageJson); if (!mainConfig) { logger.error('Unable to find storybook main.js config'); return; @@ -97,8 +98,9 @@ export async function add( // add to package.json const isStorybookAddon = addonName.startsWith('@storybook/'); + const storybookVersion = await getStorybookVersion(packageManager); const version = versionSpecifier || (isStorybookAddon ? storybookVersion : latestVersion); - const addonWithVersion = `${addonName}@${version}`; + const addonWithVersion = `${addonName}@^${version}`; logger.log(`Installing ${addonWithVersion}`); await packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]); diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index b5240a6b4f5c..d1ccbff10d32 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -9,10 +9,10 @@ import type { StorybookConfig } from '@storybook/types'; import type { ConfigFile } from '@storybook/csf-tools'; import { readConfig, writeConfig as writeConfigFile } from '@storybook/csf-tools'; import chalk from 'chalk'; -import semver from 'semver'; import dedent from 'ts-dedent'; import path from 'path'; import type { JsPackageManager } from '../../js-package-manager'; +import { getStorybookVersion } from '../../utils'; const logger = console; @@ -93,8 +93,7 @@ export const getStorybookData = async ({ configDir: configDirFromScript, previewConfig: previewConfigPath, } = getStorybookInfo(packageJson, userDefinedConfigDir); - const storybookVersion = - storybookVersionSpecifier && semver.coerce(storybookVersionSpecifier)?.version; + const storybookVersion = await getStorybookVersion(packageManager); const configDir = userDefinedConfigDir || configDirFromScript || '.storybook'; diff --git a/code/lib/cli/src/automigrate/index.ts b/code/lib/cli/src/automigrate/index.ts index 27ded58a7c42..3e8e5af0e461 100644 --- a/code/lib/cli/src/automigrate/index.ts +++ b/code/lib/cli/src/automigrate/index.ts @@ -8,7 +8,6 @@ import dedent from 'ts-dedent'; import { join } from 'path'; import { getStorybookInfo, loadMainConfig } from '@storybook/core-common'; -import semver from 'semver'; import { JsPackageManagerFactory, useNpmWarning } from '../js-package-manager'; import type { PackageManagerName } from '../js-package-manager'; @@ -17,6 +16,7 @@ import { FixStatus, PreCheckFailure, allFixes } from './fixes'; import { cleanLog } from './helpers/cleanLog'; import { getMigrationSummary } from './helpers/getMigrationSummary'; import { getStorybookData } from './helpers/mainConfigFile'; +import { getStorybookVersion } from '../utils'; const logger = console; const LOG_FILE_NAME = 'migration-storybook.log'; @@ -158,14 +158,14 @@ export async function runFixes({ const fixResults = {} as Record; const fixSummary: FixSummary = { succeeded: [], failed: {}, manual: [], skipped: [] }; - const { - configDir: inferredConfigDir, - mainConfig: mainConfigPath, - version: storybookVersion, - } = getStorybookInfo(await packageManager.retrievePackageJson(), userSpecifiedConfigDir); + const { configDir: inferredConfigDir, mainConfig: mainConfigPath } = getStorybookInfo( + await packageManager.retrievePackageJson(), + userSpecifiedConfigDir + ); + + const storybookVersion = await getStorybookVersion(packageManager); - const sbVersionCoerced = storybookVersion && semver.coerce(storybookVersion)?.version; - if (!sbVersionCoerced) { + if (!storybookVersion) { logger.info(dedent` [Storybook automigrate] ❌ Unable to determine storybook version so the automigrations will be skipped. 🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag. diff --git a/code/lib/cli/src/utils.ts b/code/lib/cli/src/utils.ts index 4ae92e61ad66..1aedbe047d59 100644 --- a/code/lib/cli/src/utils.ts +++ b/code/lib/cli/src/utils.ts @@ -2,6 +2,8 @@ import type { WriteStream } from 'fs-extra'; import { move, remove, writeFile, readFile, createWriteStream } from 'fs-extra'; import { join } from 'path'; import tempy from 'tempy'; +import { rendererPackages } from '@storybook/core-common'; +import type { JsPackageManager } from './js-package-manager'; export function parseList(str: string): string[] { return str @@ -10,6 +12,19 @@ export function parseList(str: string): string[] { .filter((item) => item.length > 0); } +export async function getStorybookVersion(packageManager: JsPackageManager) { + const packages = ( + await Promise.all( + Object.keys(rendererPackages).map(async (pkg) => ({ + name: pkg, + version: await packageManager.getPackageVersion(pkg), + })) + ) + ).filter(({ version }) => !!version); + + return packages[0]?.version; +} + export function getEnvConfig(program: Record, configEnv: Record): void { Object.keys(configEnv).forEach((fieldName) => { const envVarName = configEnv[fieldName]; diff --git a/code/lib/core-common/src/utils/get-storybook-info.ts b/code/lib/core-common/src/utils/get-storybook-info.ts index 9689dd62bf70..bcdb4eee3970 100644 --- a/code/lib/core-common/src/utils/get-storybook-info.ts +++ b/code/lib/core-common/src/utils/get-storybook-info.ts @@ -1,7 +1,6 @@ import path from 'path'; import fse from 'fs-extra'; import type { CoreCommon_StorybookInfo, PackageJson } from '@storybook/types'; -import type { JsPackageManager } from 'lib/cli/src'; import { getStorybookConfiguration } from './get-storybook-configuration'; export const rendererPackages: Record = { From 82f819ebc90c6682cbca0b2a0f8522c58a0f69fe Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 May 2023 15:30:21 +0200 Subject: [PATCH 12/24] Fix eslint plugin setup --- code/lib/cli/src/automigrate/helpers/eslintPlugin.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/code/lib/cli/src/automigrate/helpers/eslintPlugin.ts b/code/lib/cli/src/automigrate/helpers/eslintPlugin.ts index 3d31d1111498..548b0856a74c 100644 --- a/code/lib/cli/src/automigrate/helpers/eslintPlugin.ts +++ b/code/lib/cli/src/automigrate/helpers/eslintPlugin.ts @@ -54,7 +54,7 @@ export async function configureEslintPlugin(eslintFile: string, packageManager: const eslintConfig = (await readJson(eslintFile)) as { extends?: string[] }; const existingConfigValue = Array.isArray(eslintConfig.extends) ? eslintConfig.extends - : [eslintConfig.extends]; + : [eslintConfig.extends].filter(Boolean); eslintConfig.extends = [...(existingConfigValue || []), 'plugin:storybook/recommended']; const eslintFileContents = await readFile(eslintFile, 'utf8'); @@ -63,8 +63,13 @@ export async function configureEslintPlugin(eslintFile: string, packageManager: } else { const eslint = await readConfig(eslintFile); const extendsConfig = eslint.getFieldValue(['extends']) || []; - const existingConfigValue = Array.isArray(extendsConfig) ? extendsConfig : [extendsConfig]; - eslint.setFieldValue(['extends'], [...existingConfigValue, 'plugin:storybook/recommended']); + const existingConfigValue = Array.isArray(extendsConfig) + ? extendsConfig + : [extendsConfig].filter(Boolean); + eslint.setFieldValue( + ['extends'], + [...(existingConfigValue || []), 'plugin:storybook/recommended'] + ); await writeConfig(eslint); } From 5e2a3571b34043de56931896e96dd59a170a7220 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 31 May 2023 17:21:08 +0200 Subject: [PATCH 13/24] Fix isNxProject function by returning boolean instead of number --- code/lib/cli/src/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/helpers.ts b/code/lib/cli/src/helpers.ts index 96b87bd35415..c86abe299bf2 100644 --- a/code/lib/cli/src/helpers.ts +++ b/code/lib/cli/src/helpers.ts @@ -267,5 +267,5 @@ export function getStorybookVersionSpecifier(packageJson: PackageJsonWithDepsAnd export async function isNxProject(packageManager: JsPackageManager) { const nxVersion = await packageManager.getPackageVersion('nx'); - return nxVersion ?? findUp.sync('nx.json'); + return !!nxVersion ?? findUp.sync('nx.json'); } From f07c4c0c2853adbf7ca8dbe4821b5871ea8c0463 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 1 Jun 2023 12:44:41 +0200 Subject: [PATCH 14/24] Fix error log --- code/lib/cli/src/js-package-manager/PNPMProxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/js-package-manager/PNPMProxy.ts b/code/lib/cli/src/js-package-manager/PNPMProxy.ts index 25e9ff0cbdf4..6a42a447f96f 100644 --- a/code/lib/cli/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/PNPMProxy.ts @@ -132,7 +132,7 @@ export class PNPMProxy extends JsPackageManager { return packageJSON; } catch (error) { - console.error('Error while fetching package version in Yarn PnP mode:', error); + console.error('Error while fetching package version in PNPM PnP mode:', error); return null; } } From 6aca79c12b928fa151eb1611b515ba887b9396e0 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 1 Jun 2023 12:46:21 +0200 Subject: [PATCH 15/24] Fix test --- code/lib/cli/src/helpers.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/lib/cli/src/helpers.test.ts b/code/lib/cli/src/helpers.test.ts index 4f5c3d37afee..22b737b8e310 100644 --- a/code/lib/cli/src/helpers.test.ts +++ b/code/lib/cli/src/helpers.test.ts @@ -22,6 +22,10 @@ jest.mock('fs-extra', () => ({ pathExists: jest.fn(), })); +jest.mock('find-up', () => ({ + sync: jest.fn(), +})); + jest.mock('path', () => { const path = jest.requireActual('path'); return { From a67383a055237b93655068c9f4a0b6a601ed09ac Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 1 Jun 2023 14:05:54 +0200 Subject: [PATCH 16/24] Fix return value of detectPnp --- code/lib/cli/src/detect.ts | 4 ++-- code/lib/cli/src/initiate.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/code/lib/cli/src/detect.ts b/code/lib/cli/src/detect.ts index 7a91210fc6fc..ccddf46c9c3c 100644 --- a/code/lib/cli/src/detect.ts +++ b/code/lib/cli/src/detect.ts @@ -153,8 +153,8 @@ export function isStorybookInstantiated(configDir = resolve(process.cwd(), '.sto return fs.existsSync(configDir); } -export function detectPnp() { - return findUp.sync(['.pnp.js', '.pnp.cjs']); +export async function detectPnp() { + return !!findUp.sync(['.pnp.js', '.pnp.cjs']); } export async function detectLanguage(packageManager: JsPackageManager) { diff --git a/code/lib/cli/src/initiate.ts b/code/lib/cli/src/initiate.ts index 447c48736ce9..cf2d163e907a 100644 --- a/code/lib/cli/src/initiate.ts +++ b/code/lib/cli/src/initiate.ts @@ -51,7 +51,7 @@ const installStorybook = async ( }; const language = await detectLanguage(packageManager); - const pnp = detectPnp(); + const pnp = await detectPnp(); const generatorOptions = { language, From ec27b9bd685491e0643d2c663e97c05808abf2ca Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 2 Jun 2023 09:37:06 +0200 Subject: [PATCH 17/24] Fix types --- code/lib/cli/src/upgrade.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index 6cb24a457eb3..af8e53ce6a95 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -189,7 +189,7 @@ export const doUpgrade = async ({ } const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr }); - const beforeVersion = await getStorybookCoreVersion(packageManager); + const beforeVersion = await getStorybookCoreVersion(); commandLog(`Checking for latest versions of '@storybook/*' packages`); @@ -238,7 +238,7 @@ export const doUpgrade = async ({ automigrationResults = await automigrate({ dryRun, yes, packageManager: pkgMgr, configDir }); } if (!options.disableTelemetry) { - const afterVersion = await getStorybookCoreVersion(packageManager); + const afterVersion = await getStorybookCoreVersion(); const { preCheckFailure, fixResults } = automigrationResults || {}; const automigrationTelemetry = { automigrationResults: preCheckFailure ? null : fixResults, From cf68d655a64e3c57282fe66b476c18249fc86c2b Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 2 Jun 2023 10:25:45 +0200 Subject: [PATCH 18/24] Remove comment --- code/frameworks/nextjs/src/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/code/frameworks/nextjs/src/utils.ts b/code/frameworks/nextjs/src/utils.ts index 23244ee9d5b4..9aa0e04b95a1 100644 --- a/code/frameworks/nextjs/src/utils.ts +++ b/code/frameworks/nextjs/src/utils.ts @@ -94,7 +94,6 @@ export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?: * scopedResolve('styled-jsx') === '/some/path/node_modules/styled-jsx' */ export const scopedResolve = (id: string): string => { - // id = css-loader/package.json let scopedModulePath; try { From 8f557f7c70f717c8c5486f96aca17a9ac357702c Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 2 Jun 2023 12:26:28 +0200 Subject: [PATCH 19/24] Add todo comment --- code/frameworks/nextjs/src/utils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/code/frameworks/nextjs/src/utils.ts b/code/frameworks/nextjs/src/utils.ts index 9aa0e04b95a1..c119db2dbff7 100644 --- a/code/frameworks/nextjs/src/utils.ts +++ b/code/frameworks/nextjs/src/utils.ts @@ -97,6 +97,7 @@ export const scopedResolve = (id: string): string => { let scopedModulePath; try { + // TODO: Remove in next major release (SB 8.0) and use the statement in the catch block per default instead scopedModulePath = require.resolve(id, { paths: [path.resolve()] }); } catch (e) { scopedModulePath = require.resolve(id); From 0a88f95ff794c8d99e1cbbd5350a0bfcdae2aa5c Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 15 Jun 2023 10:41:18 +0200 Subject: [PATCH 20/24] await removeDependencies --- .../js-package-manager/JsPackageManager.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index cc759dabcf06..f64c21989a8b 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -249,7 +249,7 @@ export abstract class JsPackageManager { packageJson?: PackageJson; }, dependencies: string[] - ): void { + ): Promise { const { skipInstall } = options; if (skipInstall) { @@ -264,15 +264,15 @@ export abstract class JsPackageManager { } }); - this.writePackageJson(packageJson); - } else { - try { - this.runRemoveDeps(dependencies); - } catch (e) { - logger.error('An error occurred while removing dependencies.'); - logger.log(e.message); - throw new HandledError(e); - } + return this.writePackageJson(packageJson); + } + + try { + return this.runRemoveDeps(dependencies); + } catch (e) { + logger.error('An error occurred while removing dependencies.'); + logger.log(e.message); + throw new HandledError(e); } } From f786b0fd3bb98b43cbd65b45e0c38e177ec04456 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 15 Jun 2023 12:54:27 +0200 Subject: [PATCH 21/24] fix async logic in removeDependencies --- .../js-package-manager/JsPackageManager.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index f64c21989a8b..eac9ed6e39b0 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -243,7 +243,7 @@ export abstract class JsPackageManager { * `@storybook/addon-actions`, * ]); */ - public removeDependencies( + public async removeDependencies( options: { skipInstall?: boolean; packageJson?: PackageJson; @@ -264,15 +264,15 @@ export abstract class JsPackageManager { } }); - return this.writePackageJson(packageJson); - } - - try { - return this.runRemoveDeps(dependencies); - } catch (e) { - logger.error('An error occurred while removing dependencies.'); - logger.log(e.message); - throw new HandledError(e); + await this.writePackageJson(packageJson); + } else { + try { + await this.runRemoveDeps(dependencies); + } catch (e) { + logger.error('An error occurred while removing dependencies.'); + logger.log(e.message); + throw new HandledError(e); + } } } From a884dcd8a929ab7450b8c92c5b3aeddcbf40a5dd Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 15 Jun 2023 17:01:34 +0200 Subject: [PATCH 22/24] account for correct cwd in package proxies --- .../lib/cli/src/js-package-manager/JsPackageManager.ts | 4 ++-- code/lib/cli/src/js-package-manager/NPMProxy.ts | 10 +++++----- code/lib/cli/src/js-package-manager/PNPMProxy.ts | 7 +++++-- code/lib/cli/src/js-package-manager/Yarn1Proxy.ts | 7 ++----- code/lib/cli/src/js-package-manager/Yarn2Proxy.ts | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index eac9ed6e39b0..933a204a829b 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -73,7 +73,7 @@ export abstract class JsPackageManager { } constructor(options?: JsPackageManagerOptions) { - this.cwd = options?.cwd; + this.cwd = options?.cwd || process.cwd(); } /** @@ -97,7 +97,7 @@ export abstract class JsPackageManager { } packageJsonPath(): string { - return this.cwd ? path.resolve(this.cwd, 'package.json') : path.resolve('package.json'); + return path.resolve(this.cwd, 'package.json'); } async readPackageJson(): Promise { diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.ts b/code/lib/cli/src/js-package-manager/NPMProxy.ts index 237fed5fa03f..519030d34a59 100644 --- a/code/lib/cli/src/js-package-manager/NPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/NPMProxy.ts @@ -81,7 +81,10 @@ export class NPMProxy extends JsPackageManager { return this.executeCommand({ command: 'npm', args: ['--version'] }); } - public async getPackageJSON(packageName: string, basePath?: string): Promise { + public async getPackageJSON( + packageName: string, + basePath = this.cwd + ): Promise { const packageJsonPath = await findUpSync( (dir) => { const possiblePath = path.join(dir, 'node_modules', packageName, 'package.json'); @@ -98,10 +101,7 @@ export class NPMProxy extends JsPackageManager { return packageJson; } - public async getPackageVersion( - packageName: string, - basePath = process.cwd() - ): Promise { + public async getPackageVersion(packageName: string, basePath = this.cwd): Promise { const packageJson = await this.getPackageJSON(packageName, basePath); return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; } diff --git a/code/lib/cli/src/js-package-manager/PNPMProxy.ts b/code/lib/cli/src/js-package-manager/PNPMProxy.ts index 6a42a447f96f..4ddaeb6fcda3 100644 --- a/code/lib/cli/src/js-package-manager/PNPMProxy.ts +++ b/code/lib/cli/src/js-package-manager/PNPMProxy.ts @@ -111,7 +111,10 @@ export class PNPMProxy extends JsPackageManager { } } - public async getPackageJSON(packageName: string, basePath?: string): Promise { + public async getPackageJSON( + packageName: string, + basePath = this.cwd + ): Promise { const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); if (pnpapiPath) { @@ -152,7 +155,7 @@ export class PNPMProxy extends JsPackageManager { return JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); } - async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + async getPackageVersion(packageName: string, basePath = this.cwd): Promise { const packageJSON = await this.getPackageJSON(packageName, basePath); return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; diff --git a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts index f64dd2360a2f..3793b7f54528 100644 --- a/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn1Proxy.ts @@ -65,7 +65,7 @@ export class Yarn1Proxy extends JsPackageManager { public async getPackageJSON( packageName: string, - basePath = process.cwd() + basePath = this.cwd ): Promise { const packageJsonPath = await findUpSync( (dir) => { @@ -82,10 +82,7 @@ export class Yarn1Proxy extends JsPackageManager { return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record; } - public async getPackageVersion( - packageName: string, - basePath = process.cwd() - ): Promise { + public async getPackageVersion(packageName: string, basePath = this.cwd): Promise { const packageJson = await this.getPackageJSON(packageName, basePath); return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null; } diff --git a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts index 5a44de87802f..742bcf4a8fd3 100644 --- a/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts +++ b/code/lib/cli/src/js-package-manager/Yarn2Proxy.ts @@ -123,7 +123,7 @@ export class Yarn2Proxy extends JsPackageManager { } } - async getPackageJSON(packageName: string, basePath = process.cwd()): Promise { + async getPackageJSON(packageName: string, basePath = this.cwd): Promise { const pnpapiPath = findUpSync(['.pnp.js', '.pnp.cjs'], { cwd: basePath }); if (pnpapiPath) { @@ -176,7 +176,7 @@ export class Yarn2Proxy extends JsPackageManager { return packageJson; } - async getPackageVersion(packageName: string, basePath = process.cwd()): Promise { + async getPackageVersion(packageName: string, basePath = this.cwd): Promise { const packageJSON = await this.getPackageJSON(packageName, basePath); return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null; } From 33f8cc2902a075376efee989e7bff73df92e6420 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 15 Jun 2023 17:02:07 +0200 Subject: [PATCH 23/24] pass sandbox directory when adding stories --- scripts/tasks/sandbox-parts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tasks/sandbox-parts.ts b/scripts/tasks/sandbox-parts.ts index 222c8754a1c8..65b88986777b 100644 --- a/scripts/tasks/sandbox-parts.ts +++ b/scripts/tasks/sandbox-parts.ts @@ -372,7 +372,7 @@ export const addStories: Task['run'] = async ( const storiesPath = await findFirstPath([join('src', 'stories'), 'stories'], { cwd }); const mainConfig = await readMainConfig({ cwd }); - const packageManager = JsPackageManagerFactory.getPackageManager(); + const packageManager = JsPackageManagerFactory.getPackageManager({}, sandboxDir); // Ensure that we match the right stories in the stories directory updateStoriesField( From 2c3be335b974c5905dbacad42ea833d1ac596dfe Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 15 Jun 2023 18:49:43 +0200 Subject: [PATCH 24/24] prebundle boxen --- code/lib/cli/package.json | 2 +- code/lib/core-server/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/cli/package.json b/code/lib/cli/package.json index fd98dd0deff9..9e1596c456c2 100644 --- a/code/lib/cli/package.json +++ b/code/lib/cli/package.json @@ -67,7 +67,6 @@ "@types/semver": "^7.3.4", "@yarnpkg/fslib": "^2.10.3", "@yarnpkg/libzip": "^2.3.0", - "boxen": "^5.1.2", "chalk": "^4.1.0", "commander": "^6.2.1", "cross-spawn": "^7.0.3", @@ -102,6 +101,7 @@ "@types/puppeteer-core": "^2.1.0", "@types/semver": "^7.3.4", "@types/util-deprecate": "^1.0.0", + "boxen": "^5.1.2", "slash": "^5.0.0", "strip-json-comments": "^3.1.1", "typescript": "~4.9.3" diff --git a/code/lib/core-server/package.json b/code/lib/core-server/package.json index 58a01ebd9f4f..a299407e4b18 100644 --- a/code/lib/core-server/package.json +++ b/code/lib/core-server/package.json @@ -74,7 +74,6 @@ "@types/pretty-hrtime": "^1.0.0", "@types/semver": "^7.3.4", "better-opn": "^3.0.2", - "boxen": "^5.1.2", "chalk": "^4.1.0", "cli-table3": "^0.6.1", "compression": "^1.7.4", @@ -104,6 +103,7 @@ "@types/node-fetch": "^2.5.7", "@types/serve-favicon": "^2.5.2", "@types/ws": "^8", + "boxen": "^5.1.2", "jest-os-detection": "^1.3.1", "jest-specific-snapshot": "^8.0.0", "node-fetch": "^3.3.1",