From 5ec27e8433206b5a96d5c0521d565512055e05bd Mon Sep 17 00:00:00 2001 From: Andre Santos Date: Sun, 26 Jun 2022 11:26:43 +0200 Subject: [PATCH] feat(no-uninstalled-addons): add uninstalled plugin rule --- README.md | 29 +-- docs/rules/no-uninstalled-addons.md | 71 ++++++ lib/configs/addon-interactions.ts | 7 +- lib/configs/csf.ts | 7 +- lib/configs/recommended.ts | 8 +- lib/rules/no-uninstalled-addons.ts | 185 +++++++++++++++ tests/lib/rules/no-uninstalled-addons.test.ts | 213 ++++++++++++++++++ tools/update-configs.ts | 6 +- 8 files changed, 508 insertions(+), 18 deletions(-) create mode 100644 docs/rules/no-uninstalled-addons.md create mode 100644 lib/rules/no-uninstalled-addons.ts create mode 100644 tests/lib/rules/no-uninstalled-addons.test.ts diff --git a/README.md b/README.md index eb2ac2d..ffe8990 100644 --- a/README.md +++ b/README.md @@ -96,20 +96,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..0cc8aa0 --- /dev/null +++ b/docs/rules/no-uninstalled-addons.md @@ -0,0 +1,71 @@ +# 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. + +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 + +TODO: check if the package.json location should be an option. + +## 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..8fa84c7 100644 --- a/lib/configs/addon-interactions.ts +++ b/lib/configs/addon-interactions.ts @@ -7,7 +7,12 @@ export = { plugins: ['storybook'], overrides: [ { - files: ['*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '*.story.@(ts|tsx|js|jsx|mjs|cjs)'], + files: [ + '*.stories.@(ts|tsx|js|jsx|mjs|cjs)', + '*.story.@(ts|tsx|js|jsx|mjs|cjs)', + 'main.js', + 'main.js', + ], rules: { 'import/no-anonymous-default-export': 'off', 'storybook/await-interactions': 'error', diff --git a/lib/configs/csf.ts b/lib/configs/csf.ts index 165e92d..8d44d0d 100644 --- a/lib/configs/csf.ts +++ b/lib/configs/csf.ts @@ -7,7 +7,12 @@ export = { plugins: ['storybook'], overrides: [ { - files: ['*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '*.story.@(ts|tsx|js|jsx|mjs|cjs)'], + files: [ + '*.stories.@(ts|tsx|js|jsx|mjs|cjs)', + '*.story.@(ts|tsx|js|jsx|mjs|cjs)', + 'main.js', + 'main.js', + ], rules: { 'import/no-anonymous-default-export': 'off', 'storybook/csf-component': 'warn', diff --git a/lib/configs/recommended.ts b/lib/configs/recommended.ts index 602652d..aec2659 100644 --- a/lib/configs/recommended.ts +++ b/lib/configs/recommended.ts @@ -7,7 +7,12 @@ export = { plugins: ['storybook'], overrides: [ { - files: ['*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '*.story.@(ts|tsx|js|jsx|mjs|cjs)'], + files: [ + '*.stories.@(ts|tsx|js|jsx|mjs|cjs)', + '*.story.@(ts|tsx|js|jsx|mjs|cjs)', + 'main.js', + 'main.js', + ], rules: { 'import/no-anonymous-default-export': 'off', 'storybook/await-interactions': 'error', @@ -15,6 +20,7 @@ export = { 'storybook/default-exports': 'error', 'storybook/hierarchy-separator': 'warn', 'storybook/no-redundant-story-name': 'warn', + 'storybook/no-uninstalled-addons': 'error', 'storybook/prefer-pascal-case': 'warn', 'storybook/story-exports': 'error', 'storybook/use-storybook-expect': 'error', diff --git a/lib/rules/no-uninstalled-addons.ts b/lib/rules/no-uninstalled-addons.ts new file mode 100644 index 0000000..f030813 --- /dev/null +++ b/lib/rules/no-uninstalled-addons.ts @@ -0,0 +1,185 @@ +/** + * @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' + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +export = createStorybookRule({ + name: 'no-uninstalled-addons', + defaultOptions: [], + meta: { + type: 'problem', // `problem`, `suggestion`, or `layout` + docs: { + description: + 'This rule identifies storybook addons that are invalid because they are either not installed or contain a typo in their name.', + // Add the categories that suit this rule. + categories: [CategoryId.RECOMMENDED], + recommended: 'error', // or 'error' + }, + messages: { + // find out how to make this message dynamic + 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 + //---------------------------------------------------------------------- + + 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 + } + + //---------------------------------------------------------------------- + // 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(), '../../') + : './' + + try { + } catch (e) {} + 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 + + if (addonsProp) { + if (isArrayExpression(addonsProp.value)) { + // extract all nodes taht are a string inside the addons array + const listOfAddonsInString: string[] = addonsProp.value.elements + .map((elem) => (isLiteral(elem) ? elem.value : undefined)) + .filter((elem) => !!elem) as string[] + + // extract all nodes that are an object inside the addons array + const listOfAddonsInObj = addonsProp.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) ? property.value.value : '' + }) + .filter((elem) => !!elem) as string[] + + const listOfAddons = [...listOfAddonsInString, ...listOfAddonsInObj] + + const result = areThereAddonsNotInstalled(listOfAddons, depsAndDevDeps) + + result + ? context.report({ + node, + messageId: 'addonIsNotInstalled', + data: { addonName: result[0].name }, + }) + : null + } + } + } + }, + } + }, +}) + +/** + * Notes about this new feature + * + * + * The issues that this rule is trying to solve are: + * 1 - Addon is listed in the main.js file of Storybook, but is not installed + * 2 - Addon is listed in the main.js file of Storybook, but it contains a typo in its name + * + * Obs: + * + * addons: [ + * // usual way to register addons + * '@storybook/addon-actions', + * { + * + * // alternative way to register addons + * name: '@storybook/addon-actions', + * options: { + * docs: false + * } + * ] + * + * Not every addon is starts with @storybook/addon or storybook-addon. But most of them do and this is a recommended way to register them. + * + * The solution: + * + * - When the addon is listed but not installed or there is a typo but it is not prefixed in the recommended way: + * The addon ${addonName} is not installed. Did you forget to install it? + * + * - When the addon is listed but has a typo and it is prefixed in the recommended way: + * The addon ${addonName} is not installed. Did you mean CORRECT_NAME instead of NAME_WITH_TYPO? + */ 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..90f7376 --- /dev/null +++ b/tests/lib/rules/no-uninstalled-addons.test.ts @@ -0,0 +1,213 @@ +/** + * @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, + 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.AssignmentExpression, + 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.AssignmentExpression, + 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.AssignmentExpression, + data: { + addonName: '@storybook/adon-essentials', + }, + }, + ], + }, + { + code: ` + module.exports = { + addons: [ + "@storybook/addon-links", + "@storybook/addon-essentials", + "@storybook/addon-interactions", + "addon-withut-the-prefix", + ] + } + `, + errors: [ + { + messageId: 'addonIsNotInstalled', // comes from the rule file + type: AST_NODE_TYPES.AssignmentExpression, + data: { + addonName: 'addon-withut-the-prefix', + }, + }, + ], + }, + ], +}) diff --git a/tools/update-configs.ts b/tools/update-configs.ts index beb7ac7..648ec96 100644 --- a/tools/update-configs.ts +++ b/tools/update-configs.ts @@ -35,8 +35,12 @@ const SUPPORTED_EXTENSIONS = ['ts', 'tsx', 'js', 'jsx', 'mjs', 'cjs'] const STORIES_GLOBS = [ `'*.stories.@(${SUPPORTED_EXTENSIONS.join('|')})'`, `'*.story.@(${SUPPORTED_EXTENSIONS.join('|')})'`, + `'main.js'`, ] +// Other files that will be linted +const OTHER_FILES = [`'main.js'`] + function formatCategory(category: TCategory) { const extendsCategoryId = extendsCategories[category.categoryId] if (extendsCategoryId == null) { @@ -50,7 +54,7 @@ function formatCategory(category: TCategory) { 'storybook' ], overrides: [{ - files: [${STORIES_GLOBS.join(', ')}], + files: [${STORIES_GLOBS.concat(OTHER_FILES).join(', ')}], rules: ${formatRules(category.rules)} }] }