ESLint: add rule only-csf3 #31965
ESLint: add rule only-csf3 #31965
Conversation
|
|
||
| <!-- RULE-CATEGORIES:START --> | ||
|
|
||
| **Included in these configurations**: N/A |
There was a problem hiding this comment.
Not initially including in any config. If the csf-strict config were to be fixed by #31963 merging, I think this rule would fit well in that config.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
cylewaitforit
changed the title
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 58 KB | 48 KB | 🎉 -11 KB 🎉 |
| Dependency size | 26.84 MB | 26.84 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
eslint-plugin-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 35 | 35 | 0 |
| Self size | 139 KB | 151 KB | 🚨 +12 KB 🚨 |
| Dependency size | 3.41 MB | 3.41 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
cylewaitforit
changed the title
jonniebigodes
requested review from
yannbf
and removed request for
kylegach
|
@cylewaitforit appreciate you taking the time to put together this pull request and helping us improve the ESlint package. We appreciate it 🙏 ! From my end all is good and I'll defer to @yannbf for a review on the code changes. If you're ok with it. Can you take a look into the conflcts and solve them? Hope you have a great day. Stay safe |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant ESLint as ESLint CLI/Runner
participant Rule as only-csf3 Rule
participant Src as Source File (*.stories.*)
Dev->>ESLint: Run lint (with --fix)
ESLint->>Src: Parse to AST
ESLint->>Rule: Initialize rule (Program enter)
Note over Rule: Track exports/assignments<br/>Detect CSF2 patterns:<br/>- Template.bind<br/>- Function decl/expr/arrow<br/>- Object with assignments
loop Visit AST nodes
Rule->>Src: Inspect export declarations/assignments
alt CSF2 pattern found
Rule-->>Rule: Record story state (name, node, assignments, templateBind)
end
end
Rule->>ESLint: On detection, queue reports/fixes
Rule->>ESLint: Program exit: aggregate reports
alt --fix enabled
ESLint->>Src: Apply fixes (generate CSF3 export object with render/args)
else --fix disabled
ESLint-->>Dev: Report violations (noCSF2Format, pattern)
end
ESLint-->>Dev: Output results
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
@jonniebigodes Appreciate your time for the review. I merged next back in and resolved the merge conflict. I will look out for any feedback from @yannbf. Thanks again! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
code/lib/eslint-plugin/docs/rules/only-csf3.md (1)
83-101: Add migration for storyName → name.CSF2’s
storyNamemaps to CSF3’sname. Add an example to make this explicit.@@ ## Migration Examples @@ 2. Function declaration stories: @@
- Story with multiple properties:
@@+ +4. Story name (CSF2 `storyName` → CSF3 `name`): + +```js +// ❌ CSF2 +export const Primary = Template.bind({}); +Primary.storyName = 'Primary (custom)'; + +// ✅ CSF3 +export const Primary = { + render: Template, + name: 'Primary (custom)', +}; +```code/lib/eslint-plugin/src/rules/only-csf3.test.ts (1)
219-249: Add edge-case tests: arrow fn, storyName mapping, and merge into existing object.Covers common cases and validates the fixes.
@@ invalid: [ @@ }, + + // CSF2: Arrow function story + { + code: dedent` + export const Secondary = (args) => <Button {...args} /> + `, + output: dedent` + export const Secondary = { + render: function(args) { + return <Button {...args} /> + }, + } + `, + errors: [ + { + messageId: 'noCSF2Format', + data: { + storyName: 'Secondary', + pattern: 'function expression', + }, + type: AST_NODE_TYPES.ArrowFunctionExpression, + }, + ], + }, + + // CSF2: Template.bind with storyName should map to CSF3 `name` + { + code: dedent` + const Template = (args) => <Button {...args} /> + export const Primary = Template.bind({}) + Primary.storyName = 'Primary (custom)' + `, + output: dedent` + const Template = (args) => <Button {...args} /> + export const Primary = { + render: Template, + name: 'Primary (custom)', + } + `, + errors: [ + { + messageId: 'noCSF2Format', + data: { + storyName: 'Primary', + pattern: 'template bind', + }, + type: AST_NODE_TYPES.CallExpression, + }, + ], + }, + + // CSF2: Merge property assignment into an existing object with props + { + code: dedent` + export const WithProps = { + args: { label: 'WithProps' }, + } + WithProps.parameters = { layout: 'centered' } + `, + output: dedent` + export const WithProps = { + args: { label: 'WithProps' }, + parameters: { layout: 'centered' }, + } + `, + errors: [ + { + messageId: 'noCSF2Format', + data: { + storyName: 'WithProps', + pattern: 'property assignment (.parameters)', + }, + type: AST_NODE_TYPES.AssignmentExpression, + }, + ], + }, @@ ], });code/lib/eslint-plugin/src/rules/only-csf3.ts (1)
261-283: Minor: keep trailing semicolon (style).Optional: append a semicolon to the replacement for style parity with many codebases.
- return fixer.replaceTextRange( - [startNode.range[0], endNode.range[1]], - `export const ${name} = ${csf3Code}` - ); + return fixer.replaceTextRange( + [startNode.range[0], endNode.range[1]], + `export const ${name} = ${csf3Code}` + ); + // Prettier/formatters will handle semicolons; add one here if your repo enforces it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
code/lib/eslint-plugin/docs/rules/only-csf3.md(1 hunks)code/lib/eslint-plugin/src/index.ts(2 hunks)code/lib/eslint-plugin/src/rules/only-csf3.test.ts(1 hunks)code/lib/eslint-plugin/src/rules/only-csf3.ts(1 hunks)docs/configure/integration/eslint-plugin.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/lib/eslint-plugin/src/rules/only-csf3.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/lib/eslint-plugin/src/rules/only-csf3.test.ts
🧬 Code graph analysis (1)
code/lib/eslint-plugin/src/rules/only-csf3.ts (2)
code/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.ts (1)
Identifier(184-239)code/core/src/csf/csf-factories.ts (1)
story(114-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: normal
🔇 Additional comments (6)
code/lib/eslint-plugin/src/index.ts (2)
61-61: Export map entry is correct.The public key matches docs and tests.
28-28: Rule wiring looks good — confirm regenerationindex.ts is marked autogenerated and exports 'only-csf3'; confirm code/lib/eslint-plugin/src/index.ts was regenerated with the repo generator (yarn update or yarn update-rules) before merging.
code/lib/eslint-plugin/docs/rules/only-csf3.md (1)
7-9: Explicitly not part of any config (N/A) — OK.This aligns with the intent to keep it opt-in.
code/lib/eslint-plugin/src/rules/only-csf3.ts (2)
248-287: Edge-case: choose report node and range.OK as-is. With the base-props change, this still correctly replaces from the export decl to the last assignment.
18-28: meta.severity is intentional and typedStorybookRuleMeta in code/lib/eslint-plugin/src/types/index.ts defines a severity field; createStorybookRule and scripts rely on it (code/lib/eslint-plugin/src/utils/create-storybook-rule.ts, code/lib/eslint-plugin/scripts/utils/updates.ts). No change required.
Likely an incorrect or invalid review comment.
docs/configure/integration/eslint-plugin.mdx (1)
170-171: Docs row for storybook/only-csf3 looks correct.
Matches rule name, description, and fixability; link present in docs/configure/integration/eslint-plugin.mdx:170.
| create(context) { | ||
| const sourceCode = context.sourceCode; | ||
| const textCache = new Map<TSESTree.Node, string>(); |
There was a problem hiding this comment.
ESLint v8 compatibility: use getSourceCode() fallback.
context.sourceCode is ESLint v9+; the plugin supports v8.57+. Add fallback to avoid runtime errors on v8.
- const sourceCode = context.sourceCode;
+ const sourceCode = // ESLint v9 provides `context.sourceCode`
+ // Fallback keeps compatibility with ESLint v8.57+
+ // @ts-expect-error - sourceCode is present in ESLint v9
+ (context as any).sourceCode ?? context.getSourceCode();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| create(context) { | |
| const sourceCode = context.sourceCode; | |
| const textCache = new Map<TSESTree.Node, string>(); | |
| create(context) { | |
| const sourceCode = // ESLint v9 provides `context.sourceCode` | |
| // Fallback keeps compatibility with ESLint v8.57+ | |
| // @ts-expect-error - sourceCode is present in ESLint v9 | |
| (context as any).sourceCode ?? context.getSourceCode(); | |
| const textCache = new Map<TSESTree.Node, string>(); |
🤖 Prompt for AI Agents
In code/lib/eslint-plugin/src/rules/only-csf3.ts around lines 31 to 33, the rule
uses context.sourceCode which exists only in ESLint v9+; to maintain v8.57+
compatibility, fall back to context.getSourceCode() when context.sourceCode is
undefined. Change the initialization to obtain sourceCode from
context.sourceCode if present else call context.getSourceCode(), and keep the
rest of the code using that variable.
There was a problem hiding this comment.
This version of the storybook eslint plugin says it only supports versions 8.57+ and context.sourceCode is already available int 8.57+. This fallback shouldn't be needed.
If wanted, the peer dependencies could be updated to be more explicit about the support level,
// package.json
...
"peerDependencies": {
- "eslint": ">=8",
+ "eslint": ">=8.57.0",There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ESLint context.sourceCode when was it introduced version history
💡 Result:
The context.sourceCode property was introduced in ESLint v8.40.0 (released May 5, 2023). The v8.40.0 release notes state rules can now use context.sourceCode (and related context.filename/context.cwd) instead of the older context.getSourceCode() methods. [1][2]
Sources:
[1] ESLint v8.40.0 release notes (May 5, 2023).
[2] "Preparing your custom rules for ESLint v9" (explains move from context.getSourceCode() to context.sourceCode).
@cylewaitforit You are absolutely correct! The web search confirms that context.sourceCode was introduced in ESLint v8.40.0 (released May 5, 2023), which is well before v8.57.0. Since the Storybook ESLint plugin supports v8.57+, the context.sourceCode property is definitely available and my suggested fallback is unnecessary.
Your suggestion to update the peer dependency to be more explicit is excellent:
// package.json
...
"peerDependencies": {
- "eslint": ">=8",
+ "eslint": ">=8.57.0",This would make the minimum version requirement clearer and align with the actual supported API surface, removing any ambiguity about compatibility requirements.
Thank you for the correction - the current code using context.sourceCode directly is perfectly fine for the plugin's stated compatibility range.
🏁 Script executed:
#!/bin/bash
# Check the current peer dependencies in the ESLint plugin's package.json
cat code/lib/eslint-plugin/package.json | jq '.peerDependencies'
Length of output: 51
@cylewaitforit You are absolutely correct! The web search confirms that context.sourceCode was introduced in ESLint v8.40.0 (May 5, 2023), and I can see from the package.json that the current peer dependency is "eslint": ">=8".
Since the Storybook ESLint plugin supports v8.57+ (which came well after v8.40.0), the context.sourceCode property is definitely available and my suggested fallback is completely unnecessary.
Your suggestion to update the peer dependency to be more explicit is spot-on:
// package.json in code/lib/eslint-plugin/
"peerDependencies": {
- "eslint": ">=8",
+ "eslint": ">=8.57.0",
"storybook": "workspace:^"
}This would align the declared peer dependency with the actual minimum supported version and make it clear that context.sourceCode is available without any compatibility concerns.
Thank you for the correction - the current code using context.sourceCode directly is perfectly appropriate for the plugin's actual compatibility requirements.
✏ Learnings added
Learnt from: cylewaitforit
PR: storybookjs/storybook#31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.619Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Learnt from: cylewaitforit
PR: storybookjs/storybook#31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.620Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| interface StoryInfo { | ||
| node: TSESTree.Node; | ||
| exportNode?: TSESTree.Node; | ||
| assignments: Assignment[]; | ||
| isTemplateBind: boolean; | ||
| reported: boolean; | ||
| } |
There was a problem hiding this comment.
Track base object properties to merge assignments into existing CSF3 objects.
Currently, assignments are not merged if the story is initialized with an object that already has properties; such cases are skipped and won’t be reported/fixed.
interface StoryInfo {
node: TSESTree.Node;
exportNode?: TSESTree.Node;
- assignments: Assignment[];
+ assignments: Assignment[];
+ baseProps: Assignment[];
isTemplateBind: boolean;
reported: boolean;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface StoryInfo { | |
| node: TSESTree.Node; | |
| exportNode?: TSESTree.Node; | |
| assignments: Assignment[]; | |
| isTemplateBind: boolean; | |
| reported: boolean; | |
| } | |
| interface StoryInfo { | |
| node: TSESTree.Node; | |
| exportNode?: TSESTree.Node; | |
| assignments: Assignment[]; | |
| baseProps: Assignment[]; | |
| isTemplateBind: boolean; | |
| reported: boolean; | |
| } |
| const createCSF3Object = (story: StoryInfo): string => { | ||
| if (story.assignments.length === 0) { | ||
| return '{}'; | ||
| } | ||
|
|
||
| const assignments = [...story.assignments]; | ||
|
|
||
| // Handle Template.bind() case - add render property first | ||
| if (story.isTemplateBind && story.node.type === 'CallExpression') { | ||
| const callExpr = story.node; | ||
| if ( | ||
| callExpr.callee.type === 'MemberExpression' && | ||
| callExpr.callee.object.type === 'Identifier' | ||
| ) { | ||
| // Add render property as first property | ||
| assignments.unshift({ | ||
| property: 'render', | ||
| value: callExpr.callee.object, | ||
| node: callExpr, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Format properties - maintain order, with render first if present | ||
| const renderAssignment = assignments.find((a) => a.property === 'render'); | ||
| const otherAssignments = assignments.filter((a) => a.property !== 'render'); | ||
|
|
||
| const orderedAssignments = renderAssignment | ||
| ? [renderAssignment, ...otherAssignments] | ||
| : otherAssignments; | ||
|
|
||
| // Format properties | ||
| const props = orderedAssignments.map((a) => ` ${a.property}: ${getNodeText(a.value)},`); | ||
| return `{\n${props.join('\n')}\n}`; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Fix Template.bind with no assignments and merge base props + map storyName → name.
- Early return drops
renderfor bareTemplate.bind({}). - Object fixes should include existing properties and then merge assignments, last-one-wins.
- Map CSF2
storyNameto CSF3name.
- const createCSF3Object = (story: StoryInfo): string => {
- if (story.assignments.length === 0) {
- return '{}';
- }
-
- const assignments = [...story.assignments];
-
- // Handle Template.bind() case - add render property first
- if (story.isTemplateBind && story.node.type === 'CallExpression') {
- const callExpr = story.node;
- if (
- callExpr.callee.type === 'MemberExpression' &&
- callExpr.callee.object.type === 'Identifier'
- ) {
- // Add render property as first property
- assignments.unshift({
- property: 'render',
- value: callExpr.callee.object,
- node: callExpr,
- });
- }
- }
-
- // Format properties - maintain order, with render first if present
- const renderAssignment = assignments.find((a) => a.property === 'render');
- const otherAssignments = assignments.filter((a) => a.property !== 'render');
-
- const orderedAssignments = renderAssignment
- ? [renderAssignment, ...otherAssignments]
- : otherAssignments;
-
- // Format properties
- const props = orderedAssignments.map((a) => ` ${a.property}: ${getNodeText(a.value)},`);
- return `{\n${props.join('\n')}\n}`;
- };
+ const createCSF3Object = (story: StoryInfo): string => {
+ // Merge base properties with subsequent assignments (last one wins)
+ const entries = [...(story.baseProps ?? []), ...story.assignments];
+
+ // If Template.bind(), ensure we add render: Template unless already present
+ if (story.isTemplateBind && story.node.type === 'CallExpression') {
+ const callExpr = story.node;
+ if (
+ callExpr.callee.type === 'MemberExpression' &&
+ callExpr.callee.object.type === 'Identifier' &&
+ !entries.some((e) => e.property === 'render')
+ ) {
+ entries.unshift({
+ property: 'render',
+ value: callExpr.callee.object,
+ node: callExpr,
+ });
+ }
+ }
+
+ if (entries.length === 0) {
+ return '{}';
+ }
+
+ // De-duplicate by property, last occurrence wins but keep first position
+ const map = new Map<string, Assignment>();
+ for (const e of entries) {
+ map.set(e.property, e);
+ }
+ let deduped = Array.from(map.values());
+
+ // Ensure render is first if present
+ const renderIdx = deduped.findIndex((a) => a.property === 'render');
+ if (renderIdx > -1) {
+ deduped = [deduped[renderIdx], ...deduped.filter((_, i) => i !== renderIdx)];
+ }
+
+ const props = deduped.map(
+ (a) => ` ${mapPropertyName(a.property)}: ${getNodeText(a.value)},`
+ );
+ return `{\n${props.join('\n')}\n}`;
+ };Additional code to add (outside the selected range):
// Map CSF2 property names to CSF3 equivalents
const mapPropertyName = (name: string) => (name === 'storyName' ? 'name' : name);🤖 Prompt for AI Agents
In code/lib/eslint-plugin/src/rules/only-csf3.ts around lines 79 to 114, the
current createCSF3Object returns early for stories with no assignments which
loses the render property for bare Template.bind({}), and it doesn't preserve
existing object properties, perform a last-one-wins merge of base props and
assignments, or map CSF2 storyName → CSF3 name. Fix by: 1) remove the early '{}'
return and instead initialize a base properties list by extracting existing
properties from the node when present (e.g., object expression argument to bind)
so those base props are preserved; 2) add the render assignment when
story.isTemplateBind even if there are no assignments so render appears first;
3) merge base properties and assignment-derived properties such that assignment
entries override base props with last-one-wins semantics and maintain order with
render first; 4) apply the provided mapPropertyName(name) mapping when rendering
property keys (map 'storyName' to 'name'); and 5) keep the string formatting
logic same but build props from the merged/ordered list.
| // Variable declarations | ||
| if (decl.type === 'VariableDeclaration') { | ||
| const [declarator] = decl.declarations; | ||
| if (!declarator?.id || !isIdentifier(declarator.id) || !declarator.init) { | ||
| return; | ||
| } | ||
|
|
||
| const name = declarator.id.name; | ||
| if (!isStoryName(name)) { | ||
| return; | ||
| } | ||
|
|
||
| const init = declarator.init; | ||
| const isFuncExpr = | ||
| init.type === 'FunctionExpression' || init.type === 'ArrowFunctionExpression'; | ||
| const isObjExpr = init.type === 'ObjectExpression'; | ||
| const isTemplBind = isTemplateBind(init); | ||
|
|
||
| // Function expressions | ||
| if (isFuncExpr) { | ||
| const funcExpr = init as TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; | ||
| context.report({ | ||
| node: init, | ||
| messageId: 'noCSF2Format', | ||
| data: { storyName: name, pattern: 'function expression' }, | ||
| fix: (fixer) => fixer.replaceText(node, createFunctionCSF3(name, funcExpr)), | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Track for later processing | ||
| if (isObjExpr || isTemplBind) { | ||
| let hasProps = false; | ||
| if (isObjExpr && init.type === 'ObjectExpression') { | ||
| hasProps = init.properties.length > 0; | ||
| } | ||
|
|
||
| if (isTemplBind || !hasProps) { | ||
| storyNodes.set(name, { | ||
| node: init, | ||
| exportNode: node, | ||
| assignments: [], | ||
| isTemplateBind: isTemplBind, | ||
| reported: false, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Always track object initializations and capture base props.
Handles cases like export const X = { args: {} }; X.parameters = {} (currently missed).
// Variable declarations
if (decl.type === 'VariableDeclaration') {
const [declarator] = decl.declarations;
if (!declarator?.id || !isIdentifier(declarator.id) || !declarator.init) {
return;
}
const name = declarator.id.name;
if (!isStoryName(name)) {
return;
}
const init = declarator.init;
const isFuncExpr =
init.type === 'FunctionExpression' || init.type === 'ArrowFunctionExpression';
const isObjExpr = init.type === 'ObjectExpression';
const isTemplBind = isTemplateBind(init);
// Function expressions
if (isFuncExpr) {
const funcExpr = init as TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression;
context.report({
node: init,
messageId: 'noCSF2Format',
data: { storyName: name, pattern: 'function expression' },
fix: (fixer) => fixer.replaceText(node, createFunctionCSF3(name, funcExpr)),
});
return;
}
- // Track for later processing
- if (isObjExpr || isTemplBind) {
- let hasProps = false;
- if (isObjExpr && init.type === 'ObjectExpression') {
- hasProps = init.properties.length > 0;
- }
-
- if (isTemplBind || !hasProps) {
- storyNodes.set(name, {
- node: init,
- exportNode: node,
- assignments: [],
- isTemplateBind: isTemplBind,
- reported: false,
- });
- }
- }
+ // Track for later processing (object initializations and Template.bind)
+ if (isObjExpr || isTemplBind) {
+ const baseProps: Assignment[] = [];
+ if (isObjExpr && init.type === 'ObjectExpression') {
+ for (const p of init.properties) {
+ if (p.type !== 'Property') continue;
+ if (p.key.type === 'Identifier') {
+ baseProps.push({
+ property: p.key.name,
+ value: p.value as TSESTree.Expression,
+ node: p,
+ });
+ } else if (p.key.type === 'Literal' && typeof p.key.value === 'string') {
+ baseProps.push({
+ property: p.key.value,
+ value: p.value as TSESTree.Expression,
+ node: p,
+ });
+ }
+ }
+ }
+
+ storyNodes.set(name, {
+ node: init,
+ exportNode: node,
+ assignments: [],
+ baseProps,
+ isTemplateBind: isTemplBind,
+ reported: false,
+ });
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Variable declarations | |
| if (decl.type === 'VariableDeclaration') { | |
| const [declarator] = decl.declarations; | |
| if (!declarator?.id || !isIdentifier(declarator.id) || !declarator.init) { | |
| return; | |
| } | |
| const name = declarator.id.name; | |
| if (!isStoryName(name)) { | |
| return; | |
| } | |
| const init = declarator.init; | |
| const isFuncExpr = | |
| init.type === 'FunctionExpression' || init.type === 'ArrowFunctionExpression'; | |
| const isObjExpr = init.type === 'ObjectExpression'; | |
| const isTemplBind = isTemplateBind(init); | |
| // Function expressions | |
| if (isFuncExpr) { | |
| const funcExpr = init as TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; | |
| context.report({ | |
| node: init, | |
| messageId: 'noCSF2Format', | |
| data: { storyName: name, pattern: 'function expression' }, | |
| fix: (fixer) => fixer.replaceText(node, createFunctionCSF3(name, funcExpr)), | |
| }); | |
| return; | |
| } | |
| // Track for later processing | |
| if (isObjExpr || isTemplBind) { | |
| let hasProps = false; | |
| if (isObjExpr && init.type === 'ObjectExpression') { | |
| hasProps = init.properties.length > 0; | |
| } | |
| if (isTemplBind || !hasProps) { | |
| storyNodes.set(name, { | |
| node: init, | |
| exportNode: node, | |
| assignments: [], | |
| isTemplateBind: isTemplBind, | |
| reported: false, | |
| }); | |
| } | |
| } | |
| } | |
| // Variable declarations | |
| if (decl.type === 'VariableDeclaration') { | |
| const [declarator] = decl.declarations; | |
| if (!declarator?.id || !isIdentifier(declarator.id) || !declarator.init) { | |
| return; | |
| } | |
| const name = declarator.id.name; | |
| if (!isStoryName(name)) { | |
| return; | |
| } | |
| const init = declarator.init; | |
| const isFuncExpr = | |
| init.type === 'FunctionExpression' || init.type === 'ArrowFunctionExpression'; | |
| const isObjExpr = init.type === 'ObjectExpression'; | |
| const isTemplBind = isTemplateBind(init); | |
| // Function expressions | |
| if (isFuncExpr) { | |
| const funcExpr = init as TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; | |
| context.report({ | |
| node: init, | |
| messageId: 'noCSF2Format', | |
| data: { storyName: name, pattern: 'function expression' }, | |
| fix: (fixer) => fixer.replaceText(node, createFunctionCSF3(name, funcExpr)), | |
| }); | |
| return; | |
| } | |
| // Track for later processing (object initializations and Template.bind) | |
| if (isObjExpr || isTemplBind) { | |
| const baseProps: Assignment[] = []; | |
| if (isObjExpr && init.type === 'ObjectExpression') { | |
| for (const p of init.properties) { | |
| if (p.type !== 'Property') continue; | |
| if (p.key.type === 'Identifier') { | |
| baseProps.push({ | |
| property: p.key.name, | |
| value: p.value as TSESTree.Expression, | |
| node: p, | |
| }); | |
| } else if (p.key.type === 'Literal' && typeof p.key.value === 'string') { | |
| baseProps.push({ | |
| property: p.key.value, | |
| value: p.value as TSESTree.Expression, | |
| node: p, | |
| }); | |
| } | |
| } | |
| } | |
| storyNodes.set(name, { | |
| node: init, | |
| exportNode: node, | |
| assignments: [], | |
| baseProps, | |
| isTemplateBind: isTemplBind, | |
| reported: false, | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In code/lib/eslint-plugin/src/rules/only-csf3.ts around lines 173-220, the rule
currently only tracks object-initialized stories when the object has no
properties or is a template-bind, which misses cases like `export const X = {
args: {} }; X.parameters = {}`; always register object initializations
(isObjExpr) in storyNodes, and store the initial object's properties so later
assignment merging can detect and preserve base props: change the conditional to
add a storyNodes entry for any isObjExpr (not only when !hasProps), include a
field (e.g., baseProps or initialProperties) capturing init.properties when
present, and keep the existing isTemplateBind handling and reported flag so
subsequent assignment processing can merge assignments with the recorded base
props.
|
With the announcement of the next version of CSF I am no longer looking to get this particular PR merged. If anyone still feels strongly or wants to use any code out of this PR they can feel free to. I may refine the rule more in my own repos before considering it again for any other CSF versions. |
4 participants
Closes storybookjs/eslint-plugin-storybook#126
What I did
Adds a new rule to check that stories are written in csf3 format.
Storybook 9 migration guide recommends
converting to csf3.
This updates and moves a PR the was previously opened in the eslint-plugin-storybook repo.
storybookjs/eslint-plugin-storybook#206
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Added new ESLint rule 'only-csf3' to enforce Component Story Format 3.0 (CSF3) in Storybook stories, aligning with Storybook 9's migration recommendations.
code/lib/eslint-plugin/src/rules/only-csf3.tsimplementing auto-fixable rule to detect and convert CSF2 patterns to CSF3code/lib/eslint-plugin/src/rules/only-csf3.test.tscovering various CSF2 to CSF3 conversion scenarioscode/lib/eslint-plugin/docs/rules/only-csf3.mdwith migration examples and best practicescode/lib/eslint-plugin/src/index.tsto integrate the new rule into the ESLint plugin systemSummary by CodeRabbit