From 308bee070601afd8b7ca13e26b5f8fdd7a619195 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 28 Feb 2024 11:47:43 +0100 Subject: [PATCH] rework the logic based on reviews --- .../getIncompatibleStorybookPackages.test.ts | 140 ++++++++------ .../getIncompatibleStorybookPackages.ts | 174 ++++++++---------- code/lib/cli/src/doctor/index.ts | 70 ++++--- code/lib/cli/src/doctor/utils.ts | 33 ++++ 4 files changed, 236 insertions(+), 181 deletions(-) create mode 100644 code/lib/cli/src/doctor/utils.ts diff --git a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.test.ts b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.test.ts index e354497930f1..40955d363578 100644 --- a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.test.ts +++ b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.test.ts @@ -1,22 +1,28 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; +import type { AnalysedPackage } from './getIncompatibleStorybookPackages'; import { getIncompatibleStorybookPackages, getIncompatiblePackagesSummary, + checkPackageCompatibility, } from './getIncompatibleStorybookPackages'; -import pkgUp from 'read-pkg-up'; import type { JsPackageManager } from '@storybook/core-common'; +import * as doctorUtils from './utils'; + vi.mock('chalk', () => { return { default: { yellow: (str: string) => str, cyan: (str: string) => str, + bold: (str: string) => str, }, }; }); -vi.mock('read-pkg-up', () => ({ - default: vi.fn(), +vi.mock('./utils', () => ({ + getPackageJsonPath: vi.fn(() => Promise.resolve('package.json')), + getPackageJsonOfDependency: vi.fn(() => Promise.resolve({})), + PackageJsonNotFoundError: Error, })); const packageManagerMock = { @@ -27,71 +33,96 @@ const packageManagerMock = { latestVersion: vi.fn(() => Promise.resolve('8.0.0')), } as Partial; -describe('getIncompatibleStorybookPackages', () => { - beforeEach(() => { - vi.resetAllMocks(); - }); - - it('returns an array of incompatible packages', async () => { - vi.mocked(pkgUp).mockResolvedValueOnce({ - packageJson: { - name: '@storybook/addon-essentials', - version: '7.0.0', - dependencies: { - '@storybook/core-common': '7.0.0', - }, +describe('checkPackageCompatibility', () => { + it('returns that an package is incompatible', async () => { + const packageName = 'my-storybook-package'; + vi.mocked(doctorUtils.getPackageJsonOfDependency).mockResolvedValueOnce({ + name: packageName, + version: '1.0.0', + dependencies: { + '@storybook/core-common': '7.0.0', }, - path: '', + } as any); + const result = await checkPackageCompatibility(packageName, { + currentStorybookVersion: '8.0.0', + packageManager: packageManagerMock as JsPackageManager, }); + expect(result).toEqual({ + packageName: 'my-storybook-package', + packageVersion: '1.0.0', + hasIncompatibleDependencies: true, + homepage: undefined, + availableUpdate: undefined, + }); + }); - vi.mocked(packageManagerMock.latestVersion)?.mockResolvedValueOnce('8.0.0'); - - const result = await getIncompatibleStorybookPackages({ + it('returns that an package is compatible', async () => { + const packageName = 'my-storybook-package'; + vi.mocked(doctorUtils.getPackageJsonOfDependency).mockResolvedValueOnce({ + name: packageName, + version: '1.0.0', + dependencies: { + '@storybook/core-common': '8.0.0', + }, + } as any); + const result = await checkPackageCompatibility(packageName, { currentStorybookVersion: '8.0.0', packageManager: packageManagerMock as JsPackageManager, }); + expect(result).toEqual({ + packageName: 'my-storybook-package', + packageVersion: '1.0.0', + hasIncompatibleDependencies: false, + homepage: undefined, + availableUpdate: undefined, + }); + }); - expect(packageManagerMock.latestVersion).toHaveBeenCalled(); - expect(result).toEqual([ - { - packageName: '@storybook/addon-essentials', - packageVersion: '7.0.0', - hasIncompatibleDependencies: true, - homepage: undefined, - availableUpdate: true, - latestVersionOfPackage: '8.0.0', + it('returns that an package is incompatible and because it is core, can be upgraded', async () => { + const packageName = '@storybook/addon-essentials'; + vi.mocked(doctorUtils.getPackageJsonOfDependency).mockResolvedValueOnce({ + name: packageName, + version: '7.0.0', + dependencies: { + '@storybook/core-common': '7.0.0', }, - ]); + } as any); + const result = await checkPackageCompatibility(packageName, { + currentStorybookVersion: '8.0.0', + packageManager: packageManagerMock as JsPackageManager, + }); + expect(result).toEqual({ + packageName: '@storybook/addon-essentials', + packageVersion: '7.0.0', + hasIncompatibleDependencies: true, + homepage: undefined, + availableUpdate: '8.0.0', + }); }); +}); - it('returns an array of incompatible packages without upgrade check', async () => { - vi.mocked(pkgUp).mockResolvedValueOnce({ - packageJson: { - name: '@storybook/addon-essentials', - version: '7.0.0', - dependencies: { - '@storybook/core-common': '7.0.0', - }, +describe('getIncompatibleStorybookPackages', () => { + it('returns an array of incompatible packages', async () => { + vi.mocked(doctorUtils.getPackageJsonOfDependency).mockResolvedValueOnce({ + name: '@storybook/addon-essentials', + version: '7.0.0', + dependencies: { + '@storybook/core-common': '7.0.0', }, - path: '', - }); + } as any); const result = await getIncompatibleStorybookPackages({ currentStorybookVersion: '8.0.0', packageManager: packageManagerMock as JsPackageManager, - skipUpgradeCheck: true, }); - expect(packageManagerMock.latestVersion).not.toHaveBeenCalled(); - expect(result).toEqual([ { packageName: '@storybook/addon-essentials', packageVersion: '7.0.0', hasIncompatibleDependencies: true, homepage: undefined, - availableUpdate: false, - latestVersionOfPackage: undefined, + availableUpdate: '8.0.0', }, ]); }); @@ -99,19 +130,24 @@ describe('getIncompatibleStorybookPackages', () => { describe('getIncompatiblePackagesSummary', () => { it('generates a summary message for incompatible packages', () => { - const analysedPackages = [ + const analysedPackages: AnalysedPackage[] = [ { packageName: 'storybook-react', packageVersion: '1.0.0', hasIncompatibleDependencies: true, - latestVersionOfPackage: '2.0.0', - availableUpdate: true, + }, + { + packageName: '@storybook/addon-essentials', + packageVersion: '7.0.0', + hasIncompatibleDependencies: true, + availableUpdate: '8.0.0', }, ]; - const summary = getIncompatiblePackagesSummary(analysedPackages, '7.0.0'); + const summary = getIncompatiblePackagesSummary(analysedPackages, '8.0.0'); expect(summary).toMatchInlineSnapshot(` - "The following addons are likely incompatible with Storybook 7.0.0: - - storybook-react@1.0.0 (2.0.0 available!) + "The following packages are incompatible with Storybook 8.0.0 as they depend on different major versions of Storybook packages: + - storybook-react@1.0.0 + - @storybook/addon-essentials@7.0.0 (8.0.0 available!) Please consider updating your packages or contacting the maintainers for compatibility details. diff --git a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts index e07ca9ec3489..33dcc99738d3 100644 --- a/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts +++ b/code/lib/cli/src/doctor/getIncompatibleStorybookPackages.ts @@ -1,121 +1,99 @@ /* eslint-disable local-rules/no-uncategorized-errors */ import chalk from 'chalk'; import semver from 'semver'; -import readPkgUp from 'read-pkg-up'; import type { JsPackageManager } from '@storybook/core-common'; import { JsPackageManagerFactory, versions as storybookCorePackages } from '@storybook/core-common'; +import { PackageJsonNotFoundError, getPackageJsonOfDependency } from './utils'; -type AnalysedPackage = { +export type AnalysedPackage = { packageName: string; packageVersion?: string; homepage?: string; hasIncompatibleDependencies?: boolean; - latestVersionOfPackage?: string; - availableUpdate?: boolean; + availableUpdate?: string; }; -export const getIncompatibleStorybookPackages = async ({ - currentStorybookVersion, - packageManager = JsPackageManagerFactory.getPackageManager(), - skipUpgradeCheck = false, - skipErrors = false, -}: { +type Context = { currentStorybookVersion: string; - packageManager?: JsPackageManager; + packageManager: JsPackageManager; skipUpgradeCheck?: boolean; skipErrors?: boolean; -}): Promise => { - const allDeps = await packageManager.getAllDependencies(); - const storybookLikeDeps = Object.keys(allDeps).filter((dep) => dep.includes('storybook')); +}; - if (storybookLikeDeps.length === 0) { - throw new Error('No storybook dependencies found in the package.json'); - } +const isPackageIncompatible = (installedVersion: string, currentStorybookVersion: string) => { + return !semver.satisfies(currentStorybookVersion, installedVersion); +}; - const isPackageIncompatible = (installedVersion: string) => { - const dependencyMajor = semver.coerce(installedVersion)!.major; - const storybookMajor = semver.coerce(currentStorybookVersion)!.major; - return dependencyMajor !== storybookMajor; - }; +export const checkPackageCompatibility = async (dependency: string, context: Context) => { + const { currentStorybookVersion, skipErrors } = context; + try { + const { + version: packageVersion, + name, + dependencies, + peerDependencies, + homepage, + } = await getPackageJsonOfDependency(dependency); + + const hasIncompatibleDependencies = !!Object.entries({ + ...dependencies, + ...peerDependencies, + }) + .filter(([dep]) => Object.keys(storybookCorePackages).includes(dep)) + .find(([, version]) => { + // prevent issues with "tag" based versions e.g. "latest" or "next" instead of actual numbers + return ( + version && + semver.validRange(version) && + isPackageIncompatible(version, currentStorybookVersion) + ); + }); - const checkCompatibility = async (dependency: string): Promise => { - try { - const resolvedPath = require.resolve(dependency); - const result = await readPkgUp({ cwd: resolvedPath }); + const isCorePackage = Object.keys(storybookCorePackages).includes(name); - if (!result?.packageJson) { - throw new Error(`No package.json found for ${dependency}`); - } + let availableUpdate; - const { - packageJson: { version: versionSpecifier, name, dependencies, peerDependencies, homepage }, - } = result; - const coercedVersion = new semver.SemVer(versionSpecifier); - const packageVersion = coercedVersion.version; - - const hasIncompatibleDependencies = !!Object.entries({ - ...dependencies, - ...peerDependencies, - }) - .filter(([dep]) => Object.keys(storybookCorePackages).includes(dep)) - .find(([, version]) => { - // prevent issues with "tag" based versions e.g. "latest" or "next" instead of actual numbers - return version && semver.validRange(version) && isPackageIncompatible(version); - }); - - let latestVersionOfPackage; - - if (!skipUpgradeCheck) { - try { - const isStorybookPreRelease = currentStorybookVersion.includes('-'); - // if the user is on a pre-release, we try to get the existing prereleases of all packages - if (isStorybookPreRelease) { - // this is mostly a guess that makes it work for external addons which use the next/latest release strategy - const constraint = currentStorybookVersion.includes('-') - ? `^${coercedVersion.major + 1}.0.0-alpha.0` - : `^${coercedVersion.major + 1}.0.0`; - - latestVersionOfPackage = await packageManager.latestVersion(name, constraint); - } else { - latestVersionOfPackage = await packageManager.latestVersion(name); - } - } catch (err) { - // things might not work when defining the prerelease constraint, so we fall back to "latest" - latestVersionOfPackage = await packageManager.latestVersion(name); - } - } + // For now, we notify about updates only for core packages (which will match the currently installed storybook version) + // In the future, we can use packageManager.latestVersion(name, constraint) for all packages + if (isCorePackage && semver.gt(currentStorybookVersion, packageVersion)) { + availableUpdate = currentStorybookVersion; + } - return { - packageName: name, - packageVersion, - homepage, - hasIncompatibleDependencies, - latestVersionOfPackage, - availableUpdate: !!( - latestVersionOfPackage && semver.gt(latestVersionOfPackage, packageVersion) - ), - }; - } catch (err) { - // For the reviewers: When running sb doctor, this error message is only shown in the log file. - // Do we want it? maybe not? it's currently under a flag because this is also used in storybook dev and we do not want to show errors there - // We can choose to silently fail, but this has proven quite useful as some of our addons - // have faulty package.json files: @storybook/addon-onboarding, @storybook/addon-coverage - if (!skipErrors) { - console.error( - `Error checking compatibility for ${dependency}, please report an issue:\n`, - err - ); - } - return { packageName: dependency }; + return { + packageName: name, + packageVersion, + homepage, + hasIncompatibleDependencies, + availableUpdate, + }; + } catch (err) { + if (!(err instanceof PackageJsonNotFoundError) && !skipErrors) { + console.log(`Error checking compatibility for ${dependency}, please report an issue:\n`, err); } - }; + return { packageName: dependency }; + } +}; + +export const getIncompatibleStorybookPackages = async ( + context: Omit & Partial> +): Promise => { + const packageManager = context.packageManager ?? JsPackageManagerFactory.getPackageManager(); + + const allDeps = await packageManager.getAllDependencies(); + const storybookLikeDeps = Object.keys(allDeps).filter((dep) => dep.includes('storybook')); + + if (storybookLikeDeps.length === 0) { + throw new Error('No storybook dependencies found in the package.json'); + } - return Promise.all(storybookLikeDeps.map((dep) => checkCompatibility(dep))); + return Promise.all( + storybookLikeDeps.map((dep) => checkPackageCompatibility(dep, { ...context, packageManager })) + ); }; export const getIncompatiblePackagesSummary = ( dependencyAnalysis: AnalysedPackage[], - currentVersion: string + currentStorybookVersion: string ) => { const summaryMessage: string[] = []; @@ -125,18 +103,14 @@ export const getIncompatiblePackagesSummary = ( if (incompatiblePackages.length > 0) { summaryMessage.push( - `The following addons are likely incompatible with Storybook ${currentVersion}:` + `The following packages are incompatible with Storybook ${chalk.bold( + currentStorybookVersion + )} as they depend on different major versions of Storybook packages:` ); incompatiblePackages.forEach( - ({ - packageName: addonName, - packageVersion: addonVersion, - homepage, - availableUpdate, - latestVersionOfPackage, - }) => { + ({ packageName: addonName, packageVersion: addonVersion, homepage, availableUpdate }) => { const packageDescription = `${chalk.cyan(addonName)}@${chalk.cyan(addonVersion)}`; - const updateMessage = availableUpdate ? ` (${latestVersionOfPackage} available!)` : ''; + const updateMessage = availableUpdate ? ` (${availableUpdate} available!)` : ''; const packageRepo = homepage ? `\n Repo: ${chalk.yellow(homepage)}` : ''; summaryMessage.push(`- ${packageDescription}${updateMessage}${packageRepo}`); diff --git a/code/lib/cli/src/doctor/index.ts b/code/lib/cli/src/doctor/index.ts index efd1b74642c5..a2d538c15703 100644 --- a/code/lib/cli/src/doctor/index.ts +++ b/code/lib/cli/src/doctor/index.ts @@ -14,6 +14,8 @@ import { getIncompatiblePackagesSummary, getIncompatibleStorybookPackages, } from './getIncompatibleStorybookPackages'; +import { getDuplicatedDepsWarnings } from './getDuplicatedDepsWarnings'; +import { isPrerelease } from './utils'; const logger = console; const LOG_FILE_NAME = 'doctor-storybook.log'; @@ -46,23 +48,25 @@ type DoctorOptions = { packageManager?: PackageManagerName; }; -const logDiagnostic = (title: string, message: string) => { - logger.info( - boxen(message, { - borderStyle: 'round', - padding: 1, - title, - borderColor: '#F1618C', - }) - ); -}; - export const doctor = async ({ configDir: userSpecifiedConfigDir, packageManager: pkgMgr, }: DoctorOptions = {}) => { augmentLogsToFile(); + let foundIssues = false; + const logDiagnostic = (title: string, message: string) => { + foundIssues = true; + logger.info( + boxen(message, { + borderStyle: 'round', + padding: 1, + title, + borderColor: '#F1618C', + }) + ); + }; + logger.info('🩺 The doctor is checking the health of your Storybook..'); const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr }); @@ -118,28 +122,36 @@ export const doctor = async ({ 'storybook', ]); - const mismatchingVersionMessage = getMismatchingVersionsWarnings( - installationMetadata, - allDependencies - ); - if (mismatchingVersionMessage) { - logDiagnostic('Diagnostics', [mismatchingVersionMessage].join('\n\n-------\n\n')); + // If we found incompatible packages, we let the users fix that first + // If they run doctor again and there are still issues, we show the other warnings + if (!incompatiblePackagesMessage) { + const mismatchingVersionMessage = getMismatchingVersionsWarnings( + installationMetadata, + allDependencies + ); + if (mismatchingVersionMessage) { + logDiagnostic('Diagnostics', [mismatchingVersionMessage].join('\n\n-------\n\n')); + } else { + const list = installationMetadata + ? getDuplicatedDepsWarnings(installationMetadata) + : getDuplicatedDepsWarnings(); + if (list) { + logDiagnostic('Duplicated dependencies found', list?.join('\n')); + } + } } - // CHECK: Temporarily disable multiple versions warning as the incompatible packages mostly covers this - // else { - // const list = installationMetadata - // ? getDuplicatedDepsWarnings(installationMetadata) - // : getDuplicatedDepsWarnings(); - // if (list) { - // diagnosticMessages.push(list?.join('\n')); - // } - // } - logger.info(); - const foundIssues = incompatiblePackagesMessage || mismatchingVersionMessage; + const doctorCommand = isPrerelease(storybookVersion) + ? 'npx storybook@next doctor' + : 'npx storybook@latest doctor'; + + logger.info( + `👉 You can always recheck the health of your project by running:\n${chalk.cyan(doctorCommand)}` + ); + logger.info(); if (foundIssues) { - logger.info(`You can find the full logs in ${chalk.cyan(LOG_FILE_PATH)}`); + logger.info(`Full logs are available in ${chalk.cyan(LOG_FILE_PATH)}`); await move(TEMP_LOG_FILE_PATH, join(process.cwd(), LOG_FILE_NAME), { overwrite: true }); } else { diff --git a/code/lib/cli/src/doctor/utils.ts b/code/lib/cli/src/doctor/utils.ts new file mode 100644 index 000000000000..f4e861e07b8d --- /dev/null +++ b/code/lib/cli/src/doctor/utils.ts @@ -0,0 +1,33 @@ +import path from 'path'; +import readPkgUp from 'read-pkg-up'; + +export const getPackageJsonPath = (dependency: string) => { + try { + return require.resolve(dependency, { paths: [process.cwd()] }); + } catch (err) { + return require.resolve(path.join(dependency, 'package.json'), { + paths: [process.cwd()], + }); + } +}; + +export class PackageJsonNotFoundError extends Error { + constructor() { + super(`No package.json found`); + } +} + +export const getPackageJsonOfDependency = async (dependency: string) => { + const resolvedPath = getPackageJsonPath(dependency); + const result = await readPkgUp({ cwd: resolvedPath }); + + if (!result?.packageJson) { + throw new PackageJsonNotFoundError(); + } + + return result.packageJson; +}; + +export const isPrerelease = (version: string) => { + return version.includes('-'); +};