Skip to content

Commit

Permalink
Merge pull request #175 from storybookjs/yann/refactor-improvements
Browse files Browse the repository at this point in the history
Build: Refactor, fix issues reported by eslint and format md files
  • Loading branch information
yannbf authored Nov 1, 2024
2 parents b737baa + 3152382 commit 079456b
Show file tree
Hide file tree
Showing 26 changed files with 50 additions and 58 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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ This plugin does not support MDX files.
| [`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. | | <ul><li>recommended</li><li>flat/recommended</li></ul> |
| [`storybook/prefer-pascal-case`](./docs/rules/prefer-pascal-case.md) | Stories should use PascalCase | 🔧 | <ul><li>recommended</li><li>flat/recommended</li></ul> |
| [`storybook/story-exports`](./docs/rules/story-exports.md) | A story file must contain at least one story export | | <ul><li>recommended</li><li>flat/recommended</li><li>csf</li><li>flat/csf</li></ul> |
| [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/jest` | 🔧 | <ul><li>addon-interactions</li><li>flat/addon-interactions</li><li>recommended</li><li>flat/recommended</li></ul> |
| [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/test` or `@storybook/jest` | 🔧 | <ul><li>addon-interactions</li><li>flat/addon-interactions</li><li>recommended</li><li>flat/recommended</li></ul> |
| [`storybook/use-storybook-testing-library`](./docs/rules/use-storybook-testing-library.md) | Do not use testing-library directly on stories | 🔧 | <ul><li>addon-interactions</li><li>flat/addon-interactions</li><li>recommended</li><li>flat/recommended</li></ul> |

<!-- RULES-LIST:END -->
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
5 changes: 3 additions & 2 deletions 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 Expand Up @@ -146,7 +146,8 @@ export = createStorybookRule({
const parsedFile = JSON.parse(file)
packageJson.dependencies = parsedFile.dependencies || {}
packageJson.devDependencies = parsedFile.devDependencies || {}
} catch (e) {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (err) {
throw new Error(
dedent`The provided path in your eslintrc.json - ${path} is not a valid path to a package.json file or your package.json file is not in the same folder as ESLint is running from.
Expand Down
3 changes: 2 additions & 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 Expand Up @@ -137,6 +137,7 @@ export = createStorybookRule({
excludeStories: getDescriptor(meta, 'excludeStories'),
includeStories: getDescriptor(meta, 'includeStories'),
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (err) {
//
}
Expand Down
3 changes: 2 additions & 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 Expand Up @@ -69,6 +69,7 @@ export = createStorybookRule({
excludeStories: getDescriptor(meta, 'excludeStories'),
includeStories: getDescriptor(meta, 'includeStories'),
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
} catch (err) {
//
}
Expand Down
6 changes: 2 additions & 4 deletions lib/rules/use-storybook-expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,15 @@ export = createStorybookRule<TDefaultOptions, string>({
meta: {
type: 'suggestion',
fixable: 'code',
hasSuggestions: true,
schema: [],
severity: 'error',
docs: {
description: 'Use expect from `@storybook/test` or `@storybook/jest`',
categories: [CategoryId.ADDON_INTERACTIONS, CategoryId.RECOMMENDED],
recommended: 'strict',
},
messages: {
updateImports: 'Update imports',
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 }
}

// 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',
// }
4 changes: 3 additions & 1 deletion tests/integrations/flat-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ describe('Integration with flat config', () => {
cp.execSync('pnpm i -f', { stdio: 'inherit' })
})
afterEach(() => {
originalCwd && process.chdir(originalCwd)
if (originalCwd) {
process.chdir(originalCwd)
}
})

it('should work with config', () => {
Expand Down
4 changes: 3 additions & 1 deletion tests/integrations/legacy-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ describe('Integration with legacy config', () => {
cp.execSync('pnpm i -f', { stdio: 'inherit' })
})
afterEach(() => {
originalCwd && process.chdir(originalCwd)
if (originalCwd) {
process.chdir(originalCwd)
}
})

it('should work with config', () => {
Expand Down
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
Loading

0 comments on commit 079456b

Please sign in to comment.