diff --git a/README.md b/README.md index eb2ac2d..9d6fde7 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,18 @@ npm install eslint-plugin-storybook --save-dev yarn add eslint-plugin-storybook --dev ``` +And finally, add this to your .eslintignore file: + +``` +// Inside your .eslintignore file +!.storybook +!.storybook/* +``` + +This allows for this plugin to also lint your configuration files inside the .storybook folder, so that you always have a correct configuration and don't face any issues regarding mistyped addon names, for instance. + +For more info, check this [ESLint documentation](https://eslint.org/docs/latest/user-guide/configuring/ignoring-code#:~:text=In%20addition%20to,contents%2C%20are%20ignored). + ## Usage Use `.eslintrc.*` file to configure rules. See also: https://eslint.org/docs/user-guide/configuring @@ -96,20 +108,21 @@ This plugin does not support MDX files. **Configurations**: csf, csf-strict, addon-interactions, recommended -| Name | Description | 🔧 | Included in configurations | -| ------------------------------------------------------------------------------------------ | ----------------------------------------------------------- | --- | -------------------------------------------------------- | -| [`storybook/await-interactions`](./docs/rules/await-interactions.md) | Interactions should be awaited | 🔧 | | -| [`storybook/context-in-play-function`](./docs/rules/context-in-play-function.md) | Pass a context when invoking play function of another story | | | -| [`storybook/csf-component`](./docs/rules/csf-component.md) | The component property should be set | | | -| [`storybook/default-exports`](./docs/rules/default-exports.md) | Story files should have a default export | 🔧 | | -| [`storybook/hierarchy-separator`](./docs/rules/hierarchy-separator.md) | Deprecated hierarchy separator in title property | 🔧 | | -| [`storybook/no-redundant-story-name`](./docs/rules/no-redundant-story-name.md) | A story should not have a redundant name property | 🔧 | | -| [`storybook/no-stories-of`](./docs/rules/no-stories-of.md) | storiesOf is deprecated and should not be used | | | -| [`storybook/no-title-property-in-meta`](./docs/rules/no-title-property-in-meta.md) | Do not define a title in meta | 🔧 | | -| [`storybook/prefer-pascal-case`](./docs/rules/prefer-pascal-case.md) | Stories should use PascalCase | 🔧 | | -| [`storybook/story-exports`](./docs/rules/story-exports.md) | A story file must contain at least one story export | | | -| [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/jest` | 🔧 | | -| [`storybook/use-storybook-testing-library`](./docs/rules/use-storybook-testing-library.md) | Do not use testing-library directly on stories | 🔧 | | +| Name | Description | 🔧 | Included in configurations | +| ------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------- | --- | -------------------------------------------------------- | +| [`storybook/await-interactions`](./docs/rules/await-interactions.md) | Interactions should be awaited | 🔧 | | +| [`storybook/context-in-play-function`](./docs/rules/context-in-play-function.md) | Pass a context when invoking play function of another story | | | +| [`storybook/csf-component`](./docs/rules/csf-component.md) | The component property should be set | | | +| [`storybook/default-exports`](./docs/rules/default-exports.md) | Story files should have a default export | 🔧 | | +| [`storybook/hierarchy-separator`](./docs/rules/hierarchy-separator.md) | Deprecated hierarchy separator in title property | 🔧 | | +| [`storybook/no-redundant-story-name`](./docs/rules/no-redundant-story-name.md) | A story should not have a redundant name property | 🔧 | | +| [`storybook/no-stories-of`](./docs/rules/no-stories-of.md) | storiesOf is deprecated and should not be used | | | +| [`storybook/no-title-property-in-meta`](./docs/rules/no-title-property-in-meta.md) | Do not define a title in meta | 🔧 | | +| [`storybook/no-uninstalled-addons`](./docs/rules/no-uninstalled-addons.md) | This rule identifies storybook addons that are invalid because they are either not installed or contain a typo in their name. | | | +| [`storybook/prefer-pascal-case`](./docs/rules/prefer-pascal-case.md) | Stories should use PascalCase | 🔧 | | +| [`storybook/story-exports`](./docs/rules/story-exports.md) | A story file must contain at least one story export | | | +| [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/jest` | 🔧 | | +| [`storybook/use-storybook-testing-library`](./docs/rules/use-storybook-testing-library.md) | Do not use testing-library directly on stories | 🔧 | | diff --git a/docs/rules/no-uninstalled-addons.md b/docs/rules/no-uninstalled-addons.md new file mode 100644 index 0000000..44b9433 --- /dev/null +++ b/docs/rules/no-uninstalled-addons.md @@ -0,0 +1,80 @@ +# no-uninstalled-addons + + + +**Included in these configurations**: + + + +## Rule Details + +This rule checks if all addons in the storybook main.js file are properly listed in the root package.json of the npm project. + +For instance, if the `@storybook/addon-links` is in the `.storybook/main.js` but is not listed in the `package.json` of the project, this rule will notify the user to add the addon to the package.json and install it. + +As an important side note, this rule will check for the package.json in the same level of the .storybook folder. + +Another very important side note: your ESLint config must allow the linting of the .storybook folder. By default, ESLint ignores all dot-files so this folder will be ignored. In order to allow this rule to lint the .storybook/main.js file, it's important to configure ESLint to lint this file. This can be achieved by writing something like: + +``` +// Inside your .eslintignore file +!.storybook +.storybook/* +!.storybook/main.js +``` + +For more info, check this [ESLint documentation](https://eslint.org/docs/latest/user-guide/configuring/ignoring-code#:~:text=In%20addition%20to,contents%2C%20are%20ignored). + +Examples of **incorrect** code for this rule: + +```js +// in .storybook/main.js +module.exports = { + addons: [ + '@storybook/addon-links', + '@storybook/addon-essentials', + '@storybook/addon-interactions', + ], +} + +// package.json +{ + "devDependencies": { + "@storybook/addon-links": "0.0.1", + "@storybook/addon-essentials": "0.0.1", + ' + } +} +``` + +Examples of **correct** code for this rule: + +```js +// in .storybook/main.js +module.exports = { + addons: [ + '@storybook/addon-links', + '@storybook/addon-essentials', + '@storybook/addon-interactions', + ], +} + +// package.json +{ + "devDependencies": { + "@storybook/addon-links": "0.0.1", + "@storybook/addon-essentials": "0.0.1", + "@storybook/addon-interactions": "0.0.1" + } +} +``` + +### Options + +## When Not To Use It + +This rule is very handy to be used because if the user tries to start storybook but has forgotten to install the plugin, storybook will throw very weird errors that will give no clue to the user to what's going wrong. To prevent that, this rule should be always on. + +## Further Reading + +Check the issue in GitHub: https://github.com/storybookjs/eslint-plugin-storybook/issues/95 diff --git a/lib/configs/addon-interactions.ts b/lib/configs/addon-interactions.ts index 71df19f..1b98da3 100644 --- a/lib/configs/addon-interactions.ts +++ b/lib/configs/addon-interactions.ts @@ -16,5 +16,11 @@ export = { 'storybook/use-storybook-testing-library': 'error', }, }, + { + files: ['main.@(js|cjs|mjs|ts)'], + rules: { + 'storybook/no-uninstalled-addons': 'error', + }, + }, ], } diff --git a/lib/configs/csf.ts b/lib/configs/csf.ts index 165e92d..278c6e8 100644 --- a/lib/configs/csf.ts +++ b/lib/configs/csf.ts @@ -17,5 +17,11 @@ export = { 'storybook/story-exports': 'error', }, }, + { + files: ['main.@(js|cjs|mjs|ts)'], + rules: { + 'storybook/no-uninstalled-addons': 'error', + }, + }, ], } diff --git a/lib/configs/recommended.ts b/lib/configs/recommended.ts index 602652d..722e02b 100644 --- a/lib/configs/recommended.ts +++ b/lib/configs/recommended.ts @@ -21,5 +21,11 @@ export = { 'storybook/use-storybook-testing-library': 'error', }, }, + { + files: ['main.@(js|cjs|mjs|ts)'], + rules: { + 'storybook/no-uninstalled-addons': 'error', + }, + }, ], } diff --git a/lib/rules/no-uninstalled-addons.ts b/lib/rules/no-uninstalled-addons.ts new file mode 100644 index 0000000..4f952fc --- /dev/null +++ b/lib/rules/no-uninstalled-addons.ts @@ -0,0 +1,174 @@ +/** + * @fileoverview This rule identifies storybook addons that are invalid because they are either not installed or contain a typo in their name. + * @author Andre Santos + */ + +import { readFileSync } from 'fs' +import { resolve } from 'path' + +import { createStorybookRule } from '../utils/create-storybook-rule' +import { CategoryId } from '../utils/constants' +import { + isObjectExpression, + isProperty, + isIdentifier, + isArrayExpression, + isLiteral, +} from '../utils/ast' +import { Property } from '@typescript-eslint/types/dist/ast-spec' +import { BigIntLiteral, BooleanLiteral, NullLiteral, StringLiteral } from 'typescript' +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils' + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +export = createStorybookRule({ + name: 'no-uninstalled-addons', + defaultOptions: [], + meta: { + type: 'problem', + docs: { + description: + 'This rule identifies storybook addons that are invalid because they are either not installed or contain a typo in their name.', + categories: [CategoryId.RECOMMENDED], + recommended: 'error', // or 'error' + }, + messages: { + addonIsNotInstalled: `The {{ addonName }} is not installed. Did you forget to install it?`, + }, + + schema: [], // Add a schema if the rule has options. Otherwise remove this + }, + + create(context) { + // variables should be defined here + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + // this will not only exclude the nullables but it will also exclude the type undefined from them, so that TS does not complain + function excludeNullable(item: T | undefined): item is T { + return !!item + } + + type MergeDepsWithDevDeps = (packageJson: Record) => string[] + const mergeDepsWithDevDeps: MergeDepsWithDevDeps = (packageJson) => { + const deps = Object.keys(packageJson.dependencies || {}) + const devDeps = Object.keys(packageJson.devDependencies || {}) + return [...deps, ...devDeps] + } + + type IsAddonInstalled = (addon: string, installedAddons: string[]) => boolean + const isAddonInstalled: IsAddonInstalled = (addon, installedAddons) => { + return installedAddons.includes(addon) + } + + type AreThereAddonsNotInstalled = ( + addons: string[], + installedSbAddons: string[] + ) => false | { name: string }[] + const areThereAddonsNotInstalled: AreThereAddonsNotInstalled = (addons, installedSbAddons) => { + const result = addons + .filter((addon) => !isAddonInstalled(addon, installedSbAddons)) + .map((addon) => ({ name: addon })) + return result.length ? result : false + } + + type GetPackageJson = (path: string) => Record + + const getPackageJson: GetPackageJson = (path) => { + const packageJson = { + devDependencies: {}, + dependencies: {}, + } + try { + const file = readFileSync(path, 'utf8') + const parsedFile = JSON.parse(file) + packageJson.dependencies = parsedFile.dependencies || {} + packageJson.devDependencies = parsedFile.devDependencies || {} + } catch (e) { + console.error( + 'Could not fetch package.json - it is probably not in the same directory as the .storybook folder' + ) + } + + return packageJson + } + + const extractAllAddonsFromTheStorybookConfig = (addonsProperty: Property | undefined) => { + if (addonsProperty && isArrayExpression(addonsProperty.value)) { + // extract all nodes taht are a string inside the addons array + const nodesWithAddons = addonsProperty.value.elements + .map((elem) => (isLiteral(elem) ? { value: elem.value, node: elem } : undefined)) + .filter(excludeNullable) + + const listOfAddonsInString = nodesWithAddons.map((elem) => elem.value) as string[] + + // extract all nodes that are an object inside the addons array + const nodesWithAddonsInObj = addonsProperty.value.elements + .map((elem) => (isObjectExpression(elem) ? elem : { properties: [] })) + .map((elem) => { + const property: Property = elem.properties.find( + (prop) => isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'name' + ) as Property + return isLiteral(property?.value) + ? { value: property.value.value, node: property.value } + : undefined + }) + .filter(excludeNullable) + + const listOfAddonsInObj = nodesWithAddonsInObj.map((elem) => elem.value) as string[] + + const listOfAddons = [...listOfAddonsInString, ...listOfAddonsInObj] + const listOfAddonElements = [...nodesWithAddons, ...nodesWithAddonsInObj] + return { listOfAddons, listOfAddonElements } + } + + return { listOfAddons: [], listOfAddonElements: [] } + } + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + AssignmentExpression: function (node) { + // when this is running for .storybook/main.js, we get the path to the folder which contains the package.json of the + // project. This will be handy for monorepos that may be running ESLint in a node process in another folder. + const projectRoot = context.getPhysicalFilename + ? resolve(context.getPhysicalFilename(), '../../') + : './' + + const packageJsonObject = getPackageJson(`${projectRoot}/package.json`) + const depsAndDevDeps = mergeDepsWithDevDeps(packageJsonObject) + + if (isObjectExpression(node.right)) { + const addonsProp = node.right.properties.find( + (prop) => isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'addons' + ) as Property | undefined + + const { listOfAddons, listOfAddonElements } = + extractAllAddonsFromTheStorybookConfig(addonsProp) + + const result = areThereAddonsNotInstalled(listOfAddons, depsAndDevDeps) + + if (result) { + const elemsWithErrors = listOfAddonElements.filter( + (elem) => !!result.find((addon) => addon.name === elem.value) + ) + + elemsWithErrors.forEach((elem) => { + context.report({ + node: elem.node, + messageId: 'addonIsNotInstalled', + data: { addonName: elem.value }, + }) + }) + } + } + }, + } + }, +}) diff --git a/tests/lib/rules/no-uninstalled-addons.test.ts b/tests/lib/rules/no-uninstalled-addons.test.ts new file mode 100644 index 0000000..09d13b1 --- /dev/null +++ b/tests/lib/rules/no-uninstalled-addons.test.ts @@ -0,0 +1,222 @@ +/** + * @fileoverview This rule identifies storybook addons that are invalid because they are either not installed or contain a typo in their name. + * @author Andre Santos + */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils' + +import rule from '../../../lib/rules/no-uninstalled-addons' +import ruleTester from '../../utils/rule-tester' + +jest.mock('fs', () => ({ + ...jest.requireActual('fs'), + readFileSync: (a: string, b: string) => ` + { + "name": "react-repro", + "version": "1.0.0", + "main": "index.js", + "license": "MIT", + "dependencies": { + "react": "^18.2.0", + "react-dom": "^18.2.0" + }, + "packageManager": "yarn@3.2.1", + "devDependencies": { + "@babel/core": "^7.18.5", + "@mdx-js/react": "^1.6.22", + "@storybook/addon-actions": "^6.5.9", + "@storybook/addon-docs": "^6.5.9", + "@storybook/addon-essentials": "^6.5.9", + "@storybook/addon-interactions": "^6.5.9", + "@storybook/addon-links": "^6.5.9", + "@storybook/builder-webpack4": "^6.5.9", + "@storybook/manager-webpack4": "^6.5.9", + "@storybook/react": "^6.5.9", + "@storybook/testing-library": "^0.0.13", + "storybook-addon-valid-addon": "0.0.1", + "addon-without-the-prefix": "^0.0.1", + "babel-loader": "^8.2.5", + "prop-types": "^15.8.1" + } + } + `, +})) + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +ruleTester.run('no-uninstalled-addons', rule, { + /** + * This is an example test for a rule that reports an error in case a named export is called 'wrong' + * Use https://eslint.org/docs/developer-guide/working-with-rules for Eslint API + * And delete this entire comment block + */ + valid: [ + ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + ] + } + `, + ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + "storybook-addon-valid-addon", + ] + } + `, + ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + "addon-without-the-prefix", + ] + } + `, + ` + module.exports = { + addons: [ + { + name: "@storybook/addon-links", + }, + "@storybook/addon-essentials", + "@storybook/addon-interactions", + { + name: "addon-without-the-prefix", + } + ] + } + `, + ], + invalid: [ + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + '@storybook/not-installed-addon' + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + // type: AST_NODE_TYPES.AssignmentExpression, + type: AST_NODE_TYPES.Literal, + data: { + addonName: '@storybook/not-installed-addon', + }, + }, + ], + }, + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + { + name: '@storybook/not-installed-addon' + } + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.Literal, + data: { + addonName: '@storybook/not-installed-addon', + }, + }, + ], + }, + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + { + name: "@storybook/addon-esentials", + }, + "@storybook/addon-interactions", + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.Literal, + data: { + addonName: '@storybook/addon-esentials', + }, + }, + ], + }, + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/adon-essentials", + "@storybook/addon-interactions", + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.Literal, + data: { + addonName: '@storybook/adon-essentials', + }, + }, + ], + }, + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + "addon-withut-the-prefix", + "@storybook/addon-esentials", + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.Literal, + data: { + addonName: 'addon-withut-the-prefix', + }, + }, + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.Literal, + data: { + addonName: '@storybook/addon-esentials', + }, + }, + ], + }, + ], +}) diff --git a/tools/update-configs.ts b/tools/update-configs.ts index beb7ac7..af630f2 100644 --- a/tools/update-configs.ts +++ b/tools/update-configs.ts @@ -19,10 +19,12 @@ const externalRuleOverrides = { 'import/no-anonymous-default-export': 'off', } -function formatRules(rules: TCategory['rules']) { +function formatRules(rules: TCategory['rules'], exclude?: string[]) { const obj = rules.reduce( (setting, rule) => { - setting[rule.ruleId] = rule.meta.docs.recommended || 'error' + if (!exclude?.includes(rule.ruleId)) { + setting[rule.ruleId] = rule.meta.docs.recommended || 'error' + } return setting }, { ...externalRuleOverrides } @@ -31,12 +33,21 @@ function formatRules(rules: TCategory['rules']) { return JSON.stringify(obj, null, 2) } +function formatSingleRule(rules: TCategory['rules'], ruleId: string) { + const ruleOpt = rules.find((rule) => rule.ruleId === ruleId)?.meta.docs.recommended || 'error' + + return JSON.stringify({ [ruleId]: ruleOpt }, null, 2) +} + const SUPPORTED_EXTENSIONS = ['ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs'] const STORIES_GLOBS = [ `'*.stories.@(${SUPPORTED_EXTENSIONS.join('|')})'`, `'*.story.@(${SUPPORTED_EXTENSIONS.join('|')})'`, ] +// Other files that will be linted +const MAIN_JS_FILE = [`'main.@(js|cjs|mjs|ts)'`] + function formatCategory(category: TCategory) { const extendsCategoryId = extendsCategories[category.categoryId] if (extendsCategoryId == null) { @@ -51,7 +62,10 @@ function formatCategory(category: TCategory) { ], overrides: [{ files: [${STORIES_GLOBS.join(', ')}], - rules: ${formatRules(category.rules)} + rules: ${formatRules(category.rules, ['storybook/no-uninstalled-addons'])} + }, { + files: [${MAIN_JS_FILE.join(', ')}], + rules: ${formatSingleRule(category.rules, 'storybook/no-uninstalled-addons')} }] } `