Skip to content

Commit

Permalink
chore: rework how severities are defined when creating rules
Browse files Browse the repository at this point in the history
  • Loading branch information
yannbf committed Nov 1, 2024
1 parent 8d281d4 commit 7d80b4c
Show file tree
Hide file tree
Showing 23 changed files with 39 additions and 52 deletions.
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { CategoryId } from '../utils/constants'

module.exports = {
meta: {
severity: 'error', // whether the rule should yield 'warn' or 'error'
docs: {
categories: [CategoryId.RECOMMENDED], // You should always use an existing category from the CategoryId enum], or create a new one there
excludeFromConfig: true, // If the rule is not ready to be shipped in any category, set this flag to true, otherwise remove it
Expand Down Expand Up @@ -82,7 +83,7 @@ ruleTester.run('my-rule-name', rule, {
When you make changes to rules or create/delete rules, the configuration files and documentation have to be updated. For that, run the following command:

```sh
yarn update-all
pnpm run update-all
```

### Useful resources
Expand Down
1 change: 1 addition & 0 deletions lib/configs/flat/addon-interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export = [
name: 'storybook:addon-interactions:setup',
plugins: {
get storybook() {
// eslint-disable-next-line @typescript-eslint/no-require-imports
return require('../../index')
},
},
Expand Down
1 change: 1 addition & 0 deletions lib/configs/flat/csf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export = [
name: 'storybook:csf:setup',
plugins: {
get storybook() {
// eslint-disable-next-line @typescript-eslint/no-require-imports
return require('../../index')
},
},
Expand Down
1 change: 1 addition & 0 deletions lib/configs/flat/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export = [
name: 'storybook:recommended:setup',
plugins: {
get storybook() {
// eslint-disable-next-line @typescript-eslint/no-require-imports
return require('../../index')
},
},
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/await-interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ export = createStorybookRule({
name: 'await-interactions',
defaultOptions: [],
meta: {
severity: 'error',
docs: {
description: 'Interactions should be awaited',
categories: [CategoryId.ADDON_INTERACTIONS, CategoryId.RECOMMENDED],
recommended: 'strict',
},
messages: {
interactionShouldBeAwaited: 'Interaction should be awaited: {{method}}',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/context-in-play-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ export = createStorybookRule({
defaultOptions: [],
meta: {
type: 'problem',
severity: 'error',
docs: {
description: 'Pass a context when invoking play function of another story',
categories: [CategoryId.RECOMMENDED, CategoryId.ADDON_INTERACTIONS],
recommended: 'strict',
},
messages: {
passContextToPlayFunction: 'Pass a context when invoking play function of another story',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/csf-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ export = createStorybookRule({
defaultOptions: [],
meta: {
type: 'suggestion',
severity: 'warn',
docs: {
description: 'The component property should be set',
categories: [CategoryId.CSF],
recommended: 'recommended',
},
messages: {
missingComponentProperty: 'Missing component property.',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/default-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export = createStorybookRule({
defaultOptions: [],
meta: {
type: 'problem',
severity: 'error',
docs: {
description: 'Story files should have a default export',
categories: [CategoryId.CSF, CategoryId.RECOMMENDED],
recommended: 'strict',
},
messages: {
shouldHaveDefaultExport: 'The file should have a default export.',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/hierarchy-separator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ export = createStorybookRule({
type: 'problem',
fixable: 'code',
hasSuggestions: true,
severity: 'warn',
docs: {
description: 'Deprecated hierarchy separator in title property',
categories: [CategoryId.CSF, CategoryId.RECOMMENDED],
recommended: 'recommended',
},
messages: {
useCorrectSeparators: 'Use correct separators',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/meta-inline-properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ export = createStorybookRule({
defaultOptions: [{ csfVersion: 3 }],
meta: {
type: 'problem',
severity: 'error',
docs: {
description: 'Meta should only have inline properties',
categories: [CategoryId.CSF, CategoryId.RECOMMENDED],
excludeFromConfig: true,
recommended: 'strict',
},
messages: {
metaShouldHaveInlineProperties: 'Meta should only have inline properties: {{property}}',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-redundant-story-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ export = createStorybookRule({
type: 'suggestion',
fixable: 'code',
hasSuggestions: true,
severity: 'warn',
docs: {
description: 'A story should not have a redundant name property',
categories: [CategoryId.CSF, CategoryId.RECOMMENDED],
recommended: 'recommended',
},
messages: {
removeRedundantName: 'Remove redundant name',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-stories-of.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ export = createStorybookRule({
defaultOptions: [],
meta: {
type: 'problem',
severity: 'error',
docs: {
description: 'storiesOf is deprecated and should not be used',
categories: [CategoryId.CSF_STRICT],
recommended: 'strict',
},
messages: {
doNotUseStoriesOf: 'storiesOf is deprecated and should not be used',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-title-property-in-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ export = createStorybookRule({
type: 'problem',
fixable: 'code',
hasSuggestions: true,
severity: 'error',
docs: {
description: 'Do not define a title in meta',
categories: [CategoryId.CSF_STRICT],
recommended: 'strict',
},
messages: {
removeTitleInMeta: 'Remove title property from meta',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-uninstalled-addons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ export = createStorybookRule({
],
meta: {
type: 'problem',
severity: 'error',
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: 'strict',
},
messages: {
addonIsNotInstalled: `The {{ addonName }} is not installed in {{packageJsonPath}}. Did you forget to install it or is your package.json in a different location?`,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/prefer-pascal-case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ export = createStorybookRule({
type: 'suggestion',
fixable: 'code',
hasSuggestions: true,
severity: 'warn',
docs: {
description: 'Stories should use PascalCase',
categories: [CategoryId.RECOMMENDED],
recommended: 'stylistic',
},
messages: {
convertToPascalCase: 'Use pascal case',
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/story-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ export = createStorybookRule({
defaultOptions: [],
meta: {
type: 'problem',
severity: 'error',
docs: {
description: 'A story file must contain at least one story export',
categories: [CategoryId.RECOMMENDED, CategoryId.CSF],
recommended: 'strict',
},
messages: {
shouldHaveStoryExport: 'The file should have at least one story export',
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/use-storybook-expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ export = createStorybookRule<TDefaultOptions, string>({
type: 'suggestion',
fixable: 'code',
schema: [],
severity: 'error',
docs: {
description: 'Use expect from `@storybook/test` or `@storybook/jest`',
categories: [CategoryId.ADDON_INTERACTIONS, CategoryId.RECOMMENDED],
recommended: 'strict',
},
messages: {
useExpectFromStorybook:
'Do not use global expect directly in the story. You should import it from `@storybook/test` or `@storybook/jest` instead.',
'Do not use global expect directly in the story. You should import it from `@storybook/test` (preferrably) or `@storybook/jest` instead.',
},
},

Expand Down
4 changes: 2 additions & 2 deletions lib/rules/use-storybook-testing-library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ export = createStorybookRule({
type: 'suggestion',
fixable: 'code',
hasSuggestions: true,
severity: 'error',
docs: {
description: 'Do not use testing-library directly on stories',
categories: [CategoryId.ADDON_INTERACTIONS, CategoryId.RECOMMENDED],
recommended: 'strict',
},
schema: [],
messages: {
updateImports: 'Update imports',
dontUseTestingLibraryDirectly:
'Do not use `{{library}}` directly in the story. You should import the functions from `@storybook/testing-library` instead.',
'Do not use `{{library}}` directly in the story. You should import the functions from `@storybook/test` (preferrably) or `@storybook/testing-library` instead.',
},
},

Expand Down
23 changes: 9 additions & 14 deletions lib/types/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import { TSESLint } from '@typescript-eslint/utils'
import { CategoryId } from '../utils/constants'
import {
RuleRecommendation,
RuleRecommendationAcrossConfigs,
} from '@typescript-eslint/utils/ts-eslint'

export type RuleModule = TSESLint.RuleModule<'', []> & {
meta: { hasSuggestions?: boolean; docs: { recommendedConfig?: 'error' | 'warn' } }
meta: { hasSuggestions?: boolean; severity: 'off' | 'warn' | 'error' }
}

// These 2 types are copied from @typescript-eslint/experimental-utils' CreateRuleMeta
Expand All @@ -20,23 +16,21 @@ export type StorybookRuleMetaDocs = TSESLint.RuleMetaDataDocs & {
* Which configs the rule should be part of
*/
categories?: CategoryId[]

/**
* If a string config name, which starting config this rule is enabled in.
* If an object, which settings it has enabled in each of those configs.
*/
recommended?: RuleRecommendation | RuleRecommendationAcrossConfigs<unknown[]>
}

export type StorybookRuleMeta<TMessageIds extends string> = TSESLint.RuleMetaData<
export type StorybookRuleMeta<TMessageIds extends string = ''> = TSESLint.RuleMetaData<
TMessageIds,
StorybookRuleMetaDocs
>
> & {
/**
* Severity of the rule to be defined in eslint config
*/
severity: 'off' | 'warn' | 'error'
}

// Comment out for testing purposes:
// const docs: StorybookRuleMetaDocs = {
// description: 'bla',
// recommended: 'strict',
// }

// const meta: StorybookRuleMeta<'someId'> = {
Expand All @@ -46,4 +40,5 @@ export type StorybookRuleMeta<TMessageIds extends string> = TSESLint.RuleMetaDat
// type: 'problem',
// schema: [],
// docs,
// severity: 'error',
// }
2 changes: 1 addition & 1 deletion tools/generate-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ const generateRule = async () => {
defaultOptions: [],
meta: {
type: 'problem', // \`problem\`, \`suggestion\`, or \`layout\`
severity: 'error', // or 'warn'
docs: {
description: '${ruleDescription}',
// Add the categories that suit this rule.
categories: [CategoryId.RECOMMENDED],
recommended: 'recommended',
},
messages: {
anyMessageIdHere: 'Fill me in',
Expand Down
1 change: 1 addition & 0 deletions tools/update-lib-flat-configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function formatCategory(category: TCategory) {
name: 'storybook:${category.categoryId}:setup',
plugins: {
get storybook() {
// eslint-disable-next-line @typescript-eslint/no-require-imports
return require('../../index')
}
}
Expand Down
23 changes: 5 additions & 18 deletions tools/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,12 @@ import fs from 'fs'
import path from 'path'

import { createStorybookRule } from '../../lib/utils/create-storybook-rule'
import { StorybookRuleMeta } from '../../lib/types'

const ROOT = path.resolve(__dirname, '../../lib/rules')

export type TRule = ReturnType<typeof createStorybookRule> & {
meta: {
docs: {
categories?: string[]
category?: string
excludeFromConfig?: boolean
}
}
meta: StorybookRuleMeta
}

const rules = fs
Expand All @@ -21,28 +16,20 @@ const rules = fs
.map((file) => path.basename(file, '.ts'))
.map((name) => {
const rule = require(path.join(ROOT, name)) as TRule
const meta = { ...rule.meta }
const meta: StorybookRuleMeta = { ...rule.meta }
if (meta.docs && !meta.docs.categories) {
meta.docs = { ...meta.docs }
meta.docs.categories = []

if (meta.docs.category) {
meta.docs.categories.push(meta.docs.category)
delete meta.docs.category
}

if (meta.docs.recommended) {
meta.docs.categories.push('recommended')
}
}

return {
ruleId: `storybook/${name}`,
name,
meta,
}
})
// We might have rules which are almost ready but should not be shipped
.filter((rule) => !rule.meta.docs.excludeFromConfig)
.filter((rule) => !rule.meta.docs?.excludeFromConfig)

export type TRules = typeof rules

Expand Down
4 changes: 2 additions & 2 deletions tools/utils/updates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function formatRules(rules: TCategory['rules'], exclude?: string[]) {
const obj = rules.reduce(
(setting, rule) => {
if (!exclude?.includes(rule.ruleId)) {
setting[rule.ruleId] = rule.meta.docs.recommended || 'error'
setting[rule.ruleId] = rule.meta.severity || 'error'
}
return setting
},
Expand All @@ -26,7 +26,7 @@ export function formatRules(rules: TCategory['rules'], exclude?: string[]) {
}

export function formatSingleRule(rules: TCategory['rules'], ruleId: string) {
const ruleOpt = rules.find((rule) => rule.ruleId === ruleId)?.meta.docs.recommended || 'error'
const ruleOpt = rules.find((rule) => rule.ruleId === ruleId)?.meta.severity || 'error'

return JSON.stringify({ [ruleId]: ruleOpt }, null, 2)
}
Expand Down

0 comments on commit 7d80b4c

Please sign in to comment.