Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion code/lib/eslint-plugin/scripts/update-lib-configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ function formatCategory(category: TCategory) {
// This file is bundled in an index.js file at the root
// so the reference is relative to the src directory
extends: './configs/${extendsCategoryId}',
rules: ${formatRules(category.rules)}
overrides: [{
files: [${STORIES_GLOBS.join(', ')}],
rules: ${formatRules(category.rules)}
},]
}
`;
}
Expand Down
1 change: 1 addition & 0 deletions code/lib/eslint-plugin/scripts/update-lib-flat-configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function formatCategory(category: TCategory) {
...config,
{
name: 'storybook:${category.categoryId}:rules',
files: [${STORIES_GLOBS.join(', ')}],
rules: ${formatRules(category.rules)}
}
]
Expand Down
17 changes: 11 additions & 6 deletions code/lib/eslint-plugin/src/configs/csf-strict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ export default {
// This file is bundled in an index.js file at the root
// so the reference is relative to the src directory
extends: './configs/csf',
rules: {
'react-hooks/rules-of-hooks': 'off',
'import/no-anonymous-default-export': 'off',
'storybook/no-stories-of': 'error',
'storybook/no-title-property-in-meta': 'error',
} as const,
overrides: [
{
files: ['**/*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '**/*.story.@(ts|tsx|js|jsx|mjs|cjs)'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there @cylewaitforit, thanks for your contribution!
Won't this block users from specifying their own files to apply these rules? As it now has hardcode file paths? Perhaps we can do differently and leave things as they were, and only move these general, conflicting rules to a new entry that's always added, and has the hardcoded paths:

        'react-hooks/rules-of-hooks': 'off',
        'import/no-anonymous-default-export': 'off',

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @yannbf, thanks for reviewing. This updates csf-strict to follow the same pattern used by the other configs (see https://github.com/storybookjs/storybook/blob/next/code/lib/eslint-plugin/src/configs/recommended.ts and https://github.com/storybookjs/storybook/blob/next/code/lib/eslint-plugin/src/configs/csf.ts). Users should still have the ability to make any overrides in their own config after extending.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yannbf Another option, if you’re concerned about this legacy config, might be to only leave the fix for the Flat Config and instead mark the legacy config options as deprecated for removal in V10 of this storybook eslint plugin.

Overriding files types for a config on the user side is very straight forward in the flat config and can be easily verified with the eslint config inspector.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yannbf Are the legacy configs staying in eslint for Storybook 10 or will they be removed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there, I'm so sorry this has become stale for so long.
We don't have plans to drop ESLint v8 support as there are still tons of users on that version.
Additionally, I spent some time today on this PR and made changes to address my concern, by separating the normal rules (without any overrides) to the extra overrides (which would use the overrides), so that users can define any file name for stories.. but to be honest, this plugin's purpose is to help users follow standards, and the story file name is a standard 😅 so I'll just go on and accept your PR as is! Thank you so much for your contribution 🙏

If users report this as a breaking change I'll revisit this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yannbf! Appreciate the time and thoughtfulness you gave to the review.

rules: {
'react-hooks/rules-of-hooks': 'off',
'import/no-anonymous-default-export': 'off',
'storybook/no-stories-of': 'error',
'storybook/no-title-property-in-meta': 'error',
} as const,
},
],
};
1 change: 1 addition & 0 deletions code/lib/eslint-plugin/src/configs/flat/csf-strict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default [
...config,
{
name: 'storybook:csf-strict:rules',
files: ['**/*.stories.@(ts|tsx|js|jsx|mjs|cjs)', '**/*.story.@(ts|tsx|js|jsx|mjs|cjs)'],
rules: {
'react-hooks/rules-of-hooks': 'off',
'import/no-anonymous-default-export': 'off',
Expand Down