From 7f367f7b15896594641be8f51ec2c072765aefb5 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 22 Aug 2023 14:42:43 +0200 Subject: [PATCH 1/4] replace generic errors with categorized ones --- .../core-common/src/utils/validate-config.ts | 28 ++----- .../core-events/src/errors/server-errors.ts | 75 +++++++++++++++++++ .../core-events/src/errors/storybook-error.ts | 6 +- code/lib/core-server/src/build-static.ts | 11 +-- .../core-server/src/utils/server-statics.ts | 9 +-- 5 files changed, 91 insertions(+), 38 deletions(-) diff --git a/code/lib/core-common/src/utils/validate-config.ts b/code/lib/core-common/src/utils/validate-config.ts index 3c5d7201724..39d3f5ff44d 100644 --- a/code/lib/core-common/src/utils/validate-config.ts +++ b/code/lib/core-common/src/utils/validate-config.ts @@ -1,5 +1,9 @@ import { join } from 'path'; -import { dedent } from 'ts-dedent'; +import { + CouldNotEvaluateFrameworkError, + MissingFrameworkFieldError, + InvalidFrameworkNameError, +} from '@storybook/core-events/server-errors'; import { frameworkPackages } from './get-storybook-info'; const renderers = ['html', 'preact', 'react', 'server', 'svelte', 'vue', 'vue3', 'web-components']; @@ -9,28 +13,15 @@ const rendererNames = [...renderers, ...renderers.map((renderer) => `@storybook/ export function validateFrameworkName( frameworkName: string | undefined ): asserts frameworkName is string { - const automigrateMessage = `Please run 'npx storybook@next automigrate' to automatically fix your config. - - See the migration guide for more information: - https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api\n`; - // Throw error if there is no framework field // TODO: maybe this error should not be thrown if we allow empty Storybooks that only use "refs" for composition if (!frameworkName) { - throw new Error(dedent` - Could not find a 'framework' field in Storybook config. - - ${automigrateMessage} - `); + throw new MissingFrameworkFieldError(); } // Account for legacy scenario where the framework was referring to a renderer if (rendererNames.includes(frameworkName)) { - throw new Error(dedent` - Invalid value of '${frameworkName}' in the 'framework' field of Storybook config. - - ${automigrateMessage} - `); + throw new InvalidFrameworkNameError({ frameworkName }); } // If we know about the framework, we don't need to validate it @@ -42,9 +33,6 @@ export function validateFrameworkName( try { require.resolve(join(frameworkName, 'preset')); } catch (err) { - throw new Error(dedent` - Could not evaluate the ${frameworkName} package from the 'framework' field of Storybook config. - - Are you sure it's a valid package and is installed?`); + throw new CouldNotEvaluateFrameworkError({ frameworkName }); } } diff --git a/code/lib/core-events/src/errors/server-errors.ts b/code/lib/core-events/src/errors/server-errors.ts index 695b4cb0930..f12926d0f4a 100644 --- a/code/lib/core-events/src/errors/server-errors.ts +++ b/code/lib/core-events/src/errors/server-errors.ts @@ -44,3 +44,78 @@ export class NxProjectDetectedError extends StorybookError { `; } } + +export class MissingFrameworkFieldError extends StorybookError { + readonly category = Category.CORE_COMMON; + + readonly code = 1; + + public readonly documentation = + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api'; + + template() { + return dedent` + Could not find a 'framework' field in Storybook config. + + Please run 'npx storybook@next automigrate' to automatically fix your config. + `; + } +} + +export class InvalidFrameworkNameError extends StorybookError { + readonly category = Category.CORE_COMMON; + + readonly code = 2; + + public readonly documentation = + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api'; + + constructor(public data: { frameworkName: string }) { + super(); + } + + template() { + return dedent` + Invalid value of '${this.data.frameworkName}' in the 'framework' field of Storybook config. + + Please run 'npx storybook@next automigrate' to automatically fix your config. + `; + } +} + +export class CouldNotEvaluateFrameworkError extends StorybookError { + readonly category = Category.CORE_COMMON; + + readonly code = 3; + + constructor(public data: { frameworkName: string }) { + super(); + } + + template() { + return dedent` + Could not evaluate the '${this.data.frameworkName}' package from the 'framework' field of Storybook config. + + Are you sure it's a valid package and is installed? + `; + } +} + +export class ConflictingStaticDirConfigError extends StorybookError { + readonly category = Category.CORE_SERVER; + + readonly code = 1; + + public readonly documentation = + 'https://storybook.js.org/docs/react/configure/images-and-assets#serving-static-files-via-storybook-configuration'; + + template() { + return dedent` + Storybook encountered a conflict when trying to serve statics. You have configured both: + * Storybook's option in the config file: 'staticDirs' + * Storybook's (deprecated) CLI flag: '--staticDir' or '-s' + + Please remove the CLI flag from your storybook script and use only the 'staticDirs' option instead. + `; + } +} diff --git a/code/lib/core-events/src/errors/storybook-error.ts b/code/lib/core-events/src/errors/storybook-error.ts index 3a0abbf463f..40158190d93 100644 --- a/code/lib/core-events/src/errors/storybook-error.ts +++ b/code/lib/core-events/src/errors/storybook-error.ts @@ -26,7 +26,7 @@ export abstract class StorybookError extends Error { * - If a string, uses the provided URL for documentation (external or FAQ links). * - If `false` (default), no documentation link is added. */ - public documentation: boolean | string = false; + public documentation: boolean | string | string[] = false; /** * Flag used to easily determine if the error originates from Storybook. @@ -51,8 +51,10 @@ export abstract class StorybookError extends Error { page = `https://storybook.js.org/error/${this.name}`; } else if (typeof this.documentation === 'string') { page = this.documentation; + } else if (Array.isArray(this.documentation)) { + page = `\n${this.documentation.map((doc) => `\t- ${doc}`).join('\n')}`; } - return this.template() + (page != null ? `\n\nMore info: ${page}` : ''); + return this.template() + (page != null ? `\n\nMore info: ${page}\n` : ''); } } diff --git a/code/lib/core-server/src/build-static.ts b/code/lib/core-server/src/build-static.ts index 4f22e5c8ec1..2267839dcd5 100644 --- a/code/lib/core-server/src/build-static.ts +++ b/code/lib/core-server/src/build-static.ts @@ -1,9 +1,7 @@ import chalk from 'chalk'; import { copy, emptyDir, ensureDir } from 'fs-extra'; import { dirname, isAbsolute, join, resolve } from 'path'; -import { dedent } from 'ts-dedent'; import { global } from '@storybook/global'; - import { logger } from '@storybook/node-logger'; import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry'; import type { @@ -22,6 +20,7 @@ import { normalizeStories, resolveAddonName, } from '@storybook/core-common'; +import { ConflictingStaticDirConfigError } from '@storybook/core-events/server-errors'; import isEqual from 'lodash/isEqual.js'; import { outputStats } from './utils/output-stats'; @@ -125,13 +124,7 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption }; if (options.staticDir && !isEqual(staticDirs, defaultStaticDirs)) { - throw new Error(dedent` - Conflict when trying to read staticDirs: - * Storybook's configuration option: 'staticDirs' - * Storybook's CLI flag: '--staticDir' or '-s' - - Choose one of them, but not both. - `); + throw new ConflictingStaticDirConfigError(); } const effects: Promise[] = []; diff --git a/code/lib/core-server/src/utils/server-statics.ts b/code/lib/core-server/src/utils/server-statics.ts index 60938469294..b2d5a5e3cbc 100644 --- a/code/lib/core-server/src/utils/server-statics.ts +++ b/code/lib/core-server/src/utils/server-statics.ts @@ -1,6 +1,7 @@ import { logger } from '@storybook/node-logger'; import type { Options, StorybookConfig } from '@storybook/types'; import { getDirectoryFromWorkingDir } from '@storybook/core-common'; +import { ConflictingStaticDirConfigError } from '@storybook/core-events/server-errors'; import chalk from 'chalk'; import express from 'express'; import { pathExists } from 'fs-extra'; @@ -17,13 +18,7 @@ export async function useStatics(router: any, options: Options) { const faviconPath = await options.presets.apply('favicon'); if (options.staticDir && !isEqual(staticDirs, defaultStaticDirs)) { - throw new Error(dedent` - Conflict when trying to read staticDirs: - * Storybook's configuration option: 'staticDirs' - * Storybook's CLI flag: '--staticDir' or '-s' - - Choose one of them, but not both. - `); + throw new ConflictingStaticDirConfigError(); } const statics = [ From af502451d213a5bcbdac5d5638712f41b93fd6d1 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 22 Aug 2023 17:09:02 +0200 Subject: [PATCH 2/4] fix test --- .../src/errors/storybook-error.test.ts | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/code/lib/core-events/src/errors/storybook-error.test.ts b/code/lib/core-events/src/errors/storybook-error.test.ts index 328c27a827e..dc26a50f967 100644 --- a/code/lib/core-events/src/errors/storybook-error.test.ts +++ b/code/lib/core-events/src/errors/storybook-error.test.ts @@ -26,16 +26,35 @@ describe('StorybookError', () => { const error = new TestError(); error.documentation = true; const expectedMessage = - 'This is a test error.\n\nMore info: https://storybook.js.org/error/SB_TEST_CATEGORY_0123'; + 'This is a test error.\n\nMore info: https://storybook.js.org/error/SB_TEST_CATEGORY_0123\n'; expect(error.message).toBe(expectedMessage); }); it('should generate the correct message with external documentation link', () => { const error = new TestError(); error.documentation = 'https://example.com/docs/test-error'; - const expectedMessage = - 'This is a test error.\n\nMore info: https://example.com/docs/test-error'; - expect(error.message).toBe(expectedMessage); + expect(error.message).toMatchInlineSnapshot(` + "This is a test error. + + More info: https://example.com/docs/test-error + " + `); + }); + + it('should generate the correct message with multiple external documentation links', () => { + const error = new TestError(); + error.documentation = [ + 'https://example.com/docs/first-error', + 'https://example.com/docs/second-error', + ]; + expect(error.message).toMatchInlineSnapshot(` + "This is a test error. + + More info: + - https://example.com/docs/first-error + - https://example.com/docs/second-error + " + `); }); it('should have default documentation value of false', () => { From ad2d4398a8e1f16dbd8065b67acc75a548efb4f8 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 22 Aug 2023 18:24:10 +0200 Subject: [PATCH 3/4] add override presets for framework presets --- code/lib/core-server/package.json | 8 +++++++- code/lib/core-server/src/build-dev.ts | 9 +++++++-- code/lib/core-server/src/build-static.ts | 9 +++++++-- .../src/presets/common-override-preset.ts | 14 ++++++++++++++ 4 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 code/lib/core-server/src/presets/common-override-preset.ts diff --git a/code/lib/core-server/package.json b/code/lib/core-server/package.json index 305d010bb28..ac658435f81 100644 --- a/code/lib/core-server/package.json +++ b/code/lib/core-server/package.json @@ -36,6 +36,11 @@ "node": "./dist/presets/common-preset.js", "require": "./dist/presets/common-preset.js" }, + "./dist/presets/common-override-preset": { + "types": "./dist/presets/common-override-preset.d.ts", + "node": "./dist/presets/common-override-preset.js", + "require": "./dist/presets/common-override-preset.js" + }, "./public/favicon.svg": "./public/favicon.svg", "./package.json": "./package.json" }, @@ -119,7 +124,8 @@ "entries": [ "./src/index.ts", "./src/presets/babel-cache-preset.ts", - "./src/presets/common-preset.ts" + "./src/presets/common-preset.ts", + "./src/presets/common-override-preset.ts" ], "platform": "node" }, diff --git a/code/lib/core-server/src/build-dev.ts b/code/lib/core-server/src/build-dev.ts index ee045cde1a3..7785afdee7b 100644 --- a/code/lib/core-server/src/build-dev.ts +++ b/code/lib/core-server/src/build-dev.ts @@ -80,7 +80,9 @@ export async function buildDevStandalone( // We hope to remove this in SB8 let presets = await loadAllPresets({ corePresets, - overridePresets: [], + overridePresets: [ + require.resolve('@storybook/core-server/dist/presets/common-override-preset'), + ], ...options, }); @@ -112,7 +114,10 @@ export async function buildDevStandalone( ...corePresets, require.resolve('@storybook/core-server/dist/presets/babel-cache-preset'), ], - overridePresets: previewBuilder.overridePresets ?? [], + overridePresets: [ + ...(previewBuilder.overridePresets || []), + require.resolve('@storybook/core-server/dist/presets/common-override-preset'), + ], ...options, }); diff --git a/code/lib/core-server/src/build-static.ts b/code/lib/core-server/src/build-static.ts index 4f22e5c8ec1..a9cfcaefbfc 100644 --- a/code/lib/core-server/src/build-static.ts +++ b/code/lib/core-server/src/build-static.ts @@ -85,7 +85,9 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption require.resolve('@storybook/core-server/dist/presets/common-preset'), ...corePresets, ], - overridePresets: [], + overridePresets: [ + require.resolve('@storybook/core-server/dist/presets/common-override-preset'), + ], ...options, }); @@ -103,7 +105,10 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption ...corePresets, require.resolve('@storybook/core-server/dist/presets/babel-cache-preset'), ], - overridePresets: previewBuilder.overridePresets || [], + overridePresets: [ + ...(previewBuilder.overridePresets || []), + require.resolve('@storybook/core-server/dist/presets/common-override-preset'), + ], ...options, }); diff --git a/code/lib/core-server/src/presets/common-override-preset.ts b/code/lib/core-server/src/presets/common-override-preset.ts new file mode 100644 index 00000000000..55a55bb0f11 --- /dev/null +++ b/code/lib/core-server/src/presets/common-override-preset.ts @@ -0,0 +1,14 @@ +import type { PresetProperty, StorybookConfig } from '@storybook/types'; + +export const framework: PresetProperty<'framework', StorybookConfig> = async (config) => { + // This will get called with the values from the user's main config, but before + // framework preset from framework packages e.g. react-webpack5 gets called. + // This means we can add default values to the framework config, before it's requested by other packages. + const name = typeof config === 'string' ? config : config?.name; + const options = typeof config === 'string' ? {} : config?.options || {}; + + return { + name, + options, + }; +}; From 69d3f500e747986ff81c05550d74261cc2540326 Mon Sep 17 00:00:00 2001 From: storybook-bot <32066757+storybook-bot@users.noreply.github.com> Date: Wed, 23 Aug 2023 09:51:29 +0000 Subject: [PATCH 4/4] Write changelog for 7.4.0-alpha.1 --- CHANGELOG.prerelease.md | 18 ++++++++++++++++++ code/package.json | 3 ++- docs/versions/next.json | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.prerelease.md b/CHANGELOG.prerelease.md index 071fd1a3d01..75f971a30a8 100644 --- a/CHANGELOG.prerelease.md +++ b/CHANGELOG.prerelease.md @@ -1,3 +1,21 @@ +## 7.4.0-alpha.1 + +- Build: Migrate @storybook/scripts to strict-ts - [#23818](https://github.com/storybookjs/storybook/pull/23818), thanks [@stilt0n](https://github.com/stilt0n)! +- CLI: Exclude addon-styling from upgrade - [#23841](https://github.com/storybookjs/storybook/pull/23841), thanks [@Integrayshaun](https://github.com/Integrayshaun)! +- Core: Add error categorization framework - [#23653](https://github.com/storybookjs/storybook/pull/23653), thanks [@yannbf](https://github.com/yannbf)! +- Core: Fix error thrown if `docs.defaultName` is unset - [#23893](https://github.com/storybookjs/storybook/pull/23893), thanks [@stilt0n](https://github.com/stilt0n)! +- Core: Fix race-condition relating to `addons.setConfig` - [#23802](https://github.com/storybookjs/storybook/pull/23802), thanks [@ndelangen](https://github.com/ndelangen)! +- Maintenance: Categorize server errors - [#23912](https://github.com/storybookjs/storybook/pull/23912), thanks [@yannbf](https://github.com/yannbf)! +- Maintenance: Move filtering of sidebar into the state - [#23911](https://github.com/storybookjs/storybook/pull/23911), thanks [@ndelangen](https://github.com/ndelangen)! +- Maintenance: Revert "WebpackBuilder: Remove need for `react` as peerDependency" - [#23882](https://github.com/storybookjs/storybook/pull/23882), thanks [@vanessayuenn](https://github.com/vanessayuenn)! +- Manager API: Fix `api.getAddonState`default value - [#23804](https://github.com/storybookjs/storybook/pull/23804), thanks [@sookmax](https://github.com/sookmax)! +- Preset: Add common preset overrides mechanism - [#23915](https://github.com/storybookjs/storybook/pull/23915), thanks [@yannbf](https://github.com/yannbf)! +- Publish: Don't distribute src files or unnecessary template files - [#23853](https://github.com/storybookjs/storybook/pull/23853), thanks [@shilman](https://github.com/shilman)! +- UI: Add an experimental API for adding sidebar filter functions at runtime - [#23722](https://github.com/storybookjs/storybook/pull/23722), thanks [@ndelangen](https://github.com/ndelangen)! +- UI: Removal of experimental components - [#23907](https://github.com/storybookjs/storybook/pull/23907), thanks [@ndelangen](https://github.com/ndelangen)! +- Vue3: Add support for Global Apps install - [#23772](https://github.com/storybookjs/storybook/pull/23772), thanks [@chakAs3](https://github.com/chakAs3)! +- Vue3: Use slot value directly if it's a string in source decorator - [#23784](https://github.com/storybookjs/storybook/pull/23784), thanks [@nasvillanueva](https://github.com/nasvillanueva)! + ## 7.4.0-alpha.0 - Index: Fix `*.story.*` CSF indexing - [#23852](https://github.com/storybookjs/storybook/pull/23852), thanks [@shilman](https://github.com/shilman)! diff --git a/code/package.json b/code/package.json index 3ae4a217a37..d533bef9549 100644 --- a/code/package.json +++ b/code/package.json @@ -327,5 +327,6 @@ "Dependency Upgrades" ] ] - } + }, + "deferredNextVersion": "7.4.0-alpha.1" } diff --git a/docs/versions/next.json b/docs/versions/next.json index 13e20888799..48a70749ba4 100644 --- a/docs/versions/next.json +++ b/docs/versions/next.json @@ -1 +1 @@ -{"version":"7.4.0-alpha.0","info":{"plain":"- Index: Fix `*.story.*` CSF indexing - [#23852](https://github.com/storybookjs/storybook/pull/23852), thanks [@shilman](https://github.com/shilman)!"}} +{"version":"7.4.0-alpha.1","info":{"plain":"- Build: Migrate @storybook/scripts to strict-ts - [#23818](https://github.com/storybookjs/storybook/pull/23818), thanks [@stilt0n](https://github.com/stilt0n)!\n- CLI: Exclude addon-styling from upgrade - [#23841](https://github.com/storybookjs/storybook/pull/23841), thanks [@Integrayshaun](https://github.com/Integrayshaun)!\n- Core: Add error categorization framework - [#23653](https://github.com/storybookjs/storybook/pull/23653), thanks [@yannbf](https://github.com/yannbf)!\n- Core: Fix error thrown if `docs.defaultName` is unset - [#23893](https://github.com/storybookjs/storybook/pull/23893), thanks [@stilt0n](https://github.com/stilt0n)!\n- Core: Fix race-condition relating to `addons.setConfig` - [#23802](https://github.com/storybookjs/storybook/pull/23802), thanks [@ndelangen](https://github.com/ndelangen)!\n- Maintenance: Categorize server errors - [#23912](https://github.com/storybookjs/storybook/pull/23912), thanks [@yannbf](https://github.com/yannbf)!\n- Maintenance: Move filtering of sidebar into the state - [#23911](https://github.com/storybookjs/storybook/pull/23911), thanks [@ndelangen](https://github.com/ndelangen)!\n- Maintenance: Revert \"WebpackBuilder: Remove need for `react` as peerDependency\" - [#23882](https://github.com/storybookjs/storybook/pull/23882), thanks [@vanessayuenn](https://github.com/vanessayuenn)!\n- Manager API: Fix `api.getAddonState`default value - [#23804](https://github.com/storybookjs/storybook/pull/23804), thanks [@sookmax](https://github.com/sookmax)!\n- Preset: Add common preset overrides mechanism - [#23915](https://github.com/storybookjs/storybook/pull/23915), thanks [@yannbf](https://github.com/yannbf)!\n- Publish: Don't distribute src files or unnecessary template files - [#23853](https://github.com/storybookjs/storybook/pull/23853), thanks [@shilman](https://github.com/shilman)!\n- UI: Add an experimental API for adding sidebar filter functions at runtime - [#23722](https://github.com/storybookjs/storybook/pull/23722), thanks [@ndelangen](https://github.com/ndelangen)!\n- UI: Removal of experimental components - [#23907](https://github.com/storybookjs/storybook/pull/23907), thanks [@ndelangen](https://github.com/ndelangen)!\n- Vue3: Add support for Global Apps install - [#23772](https://github.com/storybookjs/storybook/pull/23772), thanks [@chakAs3](https://github.com/chakAs3)!\n- Vue3: Use slot value directly if it's a string in source decorator - [#23784](https://github.com/storybookjs/storybook/pull/23784), thanks [@nasvillanueva](https://github.com/nasvillanueva)!"}}