-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
React: Experimental code examples #32794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds an experimental CSF enricher hook ( Changes
Sequence DiagramsequenceDiagram
participant Presets as Preset System
participant Renderer as React Renderer (enrichCsf)
participant Docs as Docs Preset
participant Builder as Builder / CSF Plugin
participant CSFTools as CSF Tools
Presets->>Renderer: apply('experimental_enrichCsf')
Renderer-->>Presets: returns enrichCsf (async)
Presets->>Docs: apply('docs') → collects csfPluginOptions
Docs-->>Builder: provide csfPluginOptions (includes enrichCsf)
Builder->>CSFTools: call enrichCsf(csf, csfSource, { enrichCsf })
alt enrichCsf provided
CSFTools->>Renderer: invoke enrichCsf(csf, csfSource) and await result
rect #E8F6EF
Renderer->>Renderer: compute code snippets, format with Prettier
Renderer->>Renderer: mutate CSF AST (params.docs.source.code)
end
Renderer-->>CSFTools: return enriched CSF
end
CSFTools->>Builder: continue formatting/return enriched CSF
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
code/builders/builder-vite/src/plugins/csf-plugin.ts (1)
15-21: Type and gate enrichCsf before passing to the plugin.Avoid passing undefined and make intent explicit.
Apply within this block:
- const enrichCsf = await presets.apply('experimental_enrichCsf'); + const enrichCsf = await presets.apply('experimental_enrichCsf');And conditionally include the option:
- return vite({ - ...docsOptions?.csfPluginOptions, - enrichCsf, - }) as Plugin; + return vite({ + ...docsOptions?.csfPluginOptions, + ...(enrichCsf && { enrichCsf }), + }) as Plugin;If you prefer stronger typing, add (outside this hunk):
+import type { CsfEnricher } from 'storybook/internal/types';and:
-const enrichCsf = await presets.apply('experimental_enrichCsf'); +const enrichCsf = (await presets.apply('experimental_enrichCsf')) as CsfEnricher | undefined;code/addons/docs/src/preset.ts (1)
45-46: Type and conditionally pass enrichCsf into the CSF plugin.Keeps options clean and clarifies intent.
Apply:
- const enrichCsf = await options.presets.apply('experimental_enrichCsf'); + const enrichCsf = (await options.presets.apply('experimental_enrichCsf')) as CsfEnricher | undefined;and:
- ...(csfPluginOptions - ? [ - (await import('@storybook/csf-plugin')).webpack({ - ...csfPluginOptions, - enrichCsf, - }), - ] - : []), + ...(csfPluginOptions + ? [ + (await import('@storybook/csf-plugin')).webpack({ + ...csfPluginOptions, + ...(enrichCsf && { enrichCsf }), + }), + ] + : []),Note: This uses CsfEnricher, which you already import as a type at Line 6.
Also applies to: 105-112
code/core/src/csf-tools/enrichCsf.ts (1)
146-149: Guard external enricher execution to avoid crashing the pipeline.Wrap user code with try/catch and log a warning.
Import (outside this hunk):
+import { logger } from 'storybook/internal/node-logger';Then adjust here:
- options?.enrichCsf?.(csf, csfSource); + if (options?.enrichCsf) { + try { + options.enrichCsf(csf, csfSource); + } catch (err) { + logger.warn(`experimental_enrichCsf failed: ${String((err as Error)?.message ?? err)}`); + } + }As per coding guidelines (use Storybook loggers).
code/renderers/react/src/enrichCsf.ts (2)
28-28: Tighten typings for local arraysAvoid implicit
any[]to keep TS strict.Apply:
- const parameters = []; + const parameters: Array<t.ObjectProperty | t.SpreadElement> = []; ... - const extraDocsParameters = []; + const extraDocsParameters: t.ObjectProperty[] = [];Also applies to: 48-48
14-84: Optional: insert after each story declaration for locality (keep current behavior if out of scope)Appending all assignments at EOF is safe but less local for diffs and tooling. Consider inserting right after each story’s export node.
If desired, replace:
csf._ast.program.body.push(addParameter);
with:storyExport.insertAfter(addParameter as any);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
code/addons/docs/src/preset.ts(3 hunks)code/builders/builder-vite/src/plugins/csf-plugin.ts(1 hunks)code/core/src/csf-tools/enrichCsf.ts(2 hunks)code/core/src/types/modules/core-common.ts(5 hunks)code/renderers/react/src/enrichCsf.ts(1 hunks)code/renderers/react/src/preset.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/preset.tscode/renderers/react/src/enrichCsf.tscode/addons/docs/src/preset.tscode/core/src/types/modules/core-common.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/csf-tools/enrichCsf.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/preset.tscode/renderers/react/src/enrichCsf.tscode/addons/docs/src/preset.tscode/core/src/types/modules/core-common.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/csf-tools/enrichCsf.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/preset.tscode/renderers/react/src/enrichCsf.tscode/addons/docs/src/preset.tscode/core/src/types/modules/core-common.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/csf-tools/enrichCsf.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/preset.tscode/renderers/react/src/enrichCsf.tscode/addons/docs/src/preset.tscode/core/src/types/modules/core-common.tscode/builders/builder-vite/src/plugins/csf-plugin.tscode/core/src/csf-tools/enrichCsf.ts
🧬 Code graph analysis (5)
code/renderers/react/src/enrichCsf.ts (4)
code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)code/core/src/types/modules/core-common.ts (1)
PresetPropertyFn(632-637)code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1027)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(23-247)
code/addons/docs/src/preset.ts (3)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(9-85)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/core/src/types/modules/core-common.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1027)
code/builders/builder-vite/src/plugins/csf-plugin.ts (3)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(9-85)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
code/core/src/csf-tools/enrichCsf.ts (1)
code/core/src/types/modules/core-common.ts (1)
CsfEnricher(364-364)
⏰ 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: Core Unit Tests, windows-latest
🔇 Additional comments (9)
code/renderers/react/src/preset.ts (1)
13-14: Export alias looks good.Public API wiring is clear and minimal.
code/core/src/csf-tools/enrichCsf.ts (1)
9-10: Option field addition is correct.Shape matches intended hook contract.
code/core/src/types/modules/core-common.ts (5)
4-6: Type-only import: verify no runtime cycle.Using
import type { CsfFile } ...should erase at runtime; confirm your TS/Babel config preserves type-only imports.You can sanity-check emitted JS has no
require('storybook/internal/csf-tools')from this file.
110-112: Generic apply overload LGTM.Enables
apply('experimental_enrichCsf')with correct typing.
364-365: CsfEnricher type is well-scoped.Signature matches usage across core and renderers.
380-381: Config field addition LGTM.
experimental_enrichCsf?: CsfEnricheraligns with preset export.
481-482: Feature flag LGTM.
experimentalCodeExamples?: booleanmatches renderer gating.code/renderers/react/src/enrichCsf.ts (2)
1-8: Gracefully handle non‑variable story exports; avoid hard invariants; add loggingFunction‑declared stories (
export function Story() {}) will throw due to the invariants. Wrap snippet generation and analyze declarations defensively; log and skip on failure.Apply:
import { type NodePath, recast, types as t } from 'storybook/internal/babel'; import { type CsfFile } from 'storybook/internal/csf-tools'; import type { PresetPropertyFn } from 'storybook/internal/types'; import invariant from 'tiny-invariant'; +import { logger } from 'storybook/internal/node-logger'; import { getCodeSnippet } from './componentManifest/generateCodeSnippet'; ... - if (csfSource._meta?.component) { - const snippet = recast.print( - getCodeSnippet(storyExport, csfSource._metaNode, csfSource._meta?.component) - ).code; - - const declaration = storyExport.get('declaration') as NodePath<t.Declaration>; - invariant(declaration.isVariableDeclaration(), 'Expected variable declaration'); - - const declarator = declaration.get('declarations')[0] as NodePath<t.VariableDeclarator>; - const init = declarator.get('init') as NodePath<t.Expression>; - invariant(init.isExpression(), 'Expected story initializer to be an expression'); + if (csfSource._meta?.component) { + let snippet: string | null = null; + try { + snippet = recast.print( + getCodeSnippet(storyExport, csfSource._metaNode, csfSource._meta.component) + ).code; + } catch (err) { + logger.debug( + `experimental_enrichCsf: skipping story "${key}": ${ + err instanceof Error ? err.message : String(err) + }` + ); + } + + const declaration = storyExport.get('declaration') as NodePath<t.Declaration>; + const isVarDecl = declaration.isVariableDeclaration(); + const declarator = isVarDecl + ? (declaration.get('declarations')[0] as NodePath<t.VariableDeclarator>) + : null; + const init = declarator ? (declarator.get('init') as NodePath<t.Expression>) : null; const parameters = []; - const isCsfFactory = - t.isCallExpression(init.node) && - t.isMemberExpression(init.node.callee) && - t.isIdentifier(init.node.callee.object) && - init.node.callee.object.name === 'meta'; + const isCsfFactory = + !!init && + t.isCallExpression(init.node) && + t.isMemberExpression(init.node.callee) && + t.isIdentifier(init.node.callee.object) && + init.node.callee.object.name === 'meta';Also applies to: 17-27, 28-34
⛔ Skipped due to learnings
Learnt from: CR PR: storybookjs/storybook#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-13T13:33:14.659Z Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)Learnt from: CR PR: storybookjs/storybook#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-13T13:33:14.659Z Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
1-85: No issues found. The code is correct for the story patterns used in the codebase.The sanity check verified that function-declared story exports (
export function StoryName() {}) do not exist anywhere in the codebase. All stories consistently use the CSF pattern with variable declarations (export const StoryName: StoryObj = { ... }), which exactly matches what the invariant on line 24 expects. The guard is appropriate and safe.
| if (!features.experimentalCodeExamples) { | ||
| return; | ||
| } | ||
| return (csf: CsfFile, csfSource: CsfFile) => { | ||
| Object.entries(csf._storyPaths).forEach(([key, storyExport]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Compose with existing enricher and return input when feature is off
Preserve previous experimental_enrichCsf value and avoid returning undefined when disabled.
Apply:
- if (!features.experimentalCodeExamples) {
- return;
- }
- return (csf: CsfFile, csfSource: CsfFile) => {
+ if (!features.experimentalCodeExamples) {
+ return input;
+ }
+ return (csf: CsfFile, csfSource: CsfFile) => {
+ if (typeof input === 'function') {
+ input(csf, csfSource);
+ }📝 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.
| if (!features.experimentalCodeExamples) { | |
| return; | |
| } | |
| return (csf: CsfFile, csfSource: CsfFile) => { | |
| Object.entries(csf._storyPaths).forEach(([key, storyExport]) => { | |
| if (!features.experimentalCodeExamples) { | |
| return input; | |
| } | |
| return (csf: CsfFile, csfSource: CsfFile) => { | |
| if (typeof input === 'function') { | |
| input(csf, csfSource); | |
| } | |
| Object.entries(csf._storyPaths).forEach(([key, storyExport]) => { |
🤖 Prompt for AI Agents
In code/renderers/react/src/enrichCsf.ts around lines 11–15, the enricher
currently returns undefined when features.experimentalCodeExamples is false and
discards any existing experimental_enrichCsf; change it to preserve and return
the previous experimental_enrichCsf (or an identity enricher that returns the
inputs) when the feature is off, and when the feature is on compose the new
logic with the existing experimental_enrichCsf so the previous enricher still
runs and the function always returns a valid enricher function (csf, csfSource)
=> {...}.
| const originalParameters = t.memberExpression(baseStoryObject, t.identifier('parameters')); | ||
| parameters.push(t.spreadElement(originalParameters)); | ||
| const optionalDocs = t.optionalMemberExpression( | ||
| originalParameters, | ||
| t.identifier('docs'), | ||
| false, | ||
| true | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix runtime errors from spreading possibly undefined; ensure new snippet overrides existing
{ ...Story.parameters }, { ...Story.parameters?.docs }, and { ...Story.parameters?.docs?.source } will throw when the right‑hand side is nullish. Also, code precedes the spread, letting older docs.source.code overwrite the new snippet.
Apply:
- const originalParameters = t.memberExpression(baseStoryObject, t.identifier('parameters'));
- parameters.push(t.spreadElement(originalParameters));
+ const originalParameters = t.memberExpression(baseStoryObject, t.identifier('parameters'));
+ parameters.push(
+ t.spreadElement(
+ t.logicalExpression('??', originalParameters, t.objectExpression([]))
+ )
+ );
const optionalDocs = t.optionalMemberExpression(
originalParameters,
t.identifier('docs'),
false,
true
);
const extraDocsParameters = [];
if (snippet) {
const optionalSource = t.optionalMemberExpression(
optionalDocs,
t.identifier('source'),
false,
true
);
extraDocsParameters.push(
t.objectProperty(
t.identifier('source'),
t.objectExpression([
- t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)),
- t.spreadElement(optionalSource),
+ t.spreadElement(
+ t.logicalExpression('??', optionalSource, t.objectExpression([]))
+ ),
+ t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)),
])
)
);
}
// docs: { description: { story: %%description%% } },
if (extraDocsParameters.length > 0) {
parameters.push(
t.objectProperty(
t.identifier('docs'),
- t.objectExpression([t.spreadElement(optionalDocs), ...extraDocsParameters])
+ t.objectExpression([
+ t.spreadElement(
+ t.logicalExpression('??', optionalDocs, t.objectExpression([]))
+ ),
+ ...extraDocsParameters,
+ ])
)
);
const addParameter = t.expressionStatement(
t.assignmentExpression('=', originalParameters, t.objectExpression(parameters))
);
csf._ast.program.body.push(addParameter);
}Also applies to: 51-66, 71-79
🤖 Prompt for AI Agents
In code/renderers/react/src/enrichCsf.ts around lines 40-47 (and likewise update
the blocks at 51-66 and 71-79), the code spreads possibly-nullish expressions
like Story.parameters, Story.parameters.docs and Story.parameters.docs.source
which will throw at runtime; also the new snippet's `code` is placed before the
spread so existing docs.source.code can overwrite it. Fix by changing spreads to
spread a non-null object (e.g., spread (Story.parameters ?? {}) /
(Story.parameters?.docs ?? {}) / (Story.parameters?.docs?.source ?? {})) so the
RHS is never nullish, and reorder the object properties so the generated `code`
property comes after the spread (or place the spread first and then set `code`)
to ensure the new snippet overrides any existing value.
|
View your CI Pipeline Execution ↗ for commit fa08b04
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/types/modules/core-common.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/types/modules/core-common.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/types/modules/core-common.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/types/modules/core-common.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/types/modules/core-common.ts
🧬 Code graph analysis (1)
code/core/src/types/modules/core-common.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1027)
⏰ 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: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/core/src/types/modules/core-common.ts (1)
4-4: LGTM!The type-only import of
CsfFileis correctly added to support the newCsfEnrichertype definition.
| apply<T extends keyof StorybookConfigRaw>( | ||
| extension: T, | ||
| config?: T, | ||
| args?: unknown | ||
| ): Promise<StorybookConfigRaw[T]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect type for config parameter.
The config parameter is typed as T, but since T extends keyof StorybookConfigRaw, this means config would be typed as a string literal (e.g., 'typescript' or 'features'). The config should represent the configuration value, not the key name.
Apply this diff to fix the type:
apply<T extends keyof StorybookConfigRaw>(
extension: T,
- config?: T,
+ config?: StorybookConfigRaw[T],
args?: unknown
): Promise<StorybookConfigRaw[T]>;📝 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.
| apply<T extends keyof StorybookConfigRaw>( | |
| extension: T, | |
| config?: T, | |
| args?: unknown | |
| ): Promise<StorybookConfigRaw[T]>; | |
| apply<T extends keyof StorybookConfigRaw>( | |
| extension: T, | |
| config?: StorybookConfigRaw[T], | |
| args?: unknown | |
| ): Promise<StorybookConfigRaw[T]>; |
🤖 Prompt for AI Agents
In code/core/src/types/modules/core-common.ts around lines 110 to 114, the
second parameter `config` is incorrectly typed as the key generic `T` (a string
literal) instead of the corresponding config value type; change the type of
`config` to StorybookConfigRaw[T] (keeping it optional) so the method becomes
apply<T extends keyof StorybookConfigRaw>(extension: T, config?:
StorybookConfigRaw[T], args?: unknown): Promise<StorybookConfigRaw[T]> to ensure
callers pass the actual config value for the given key.
| export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => void; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc documentation for public API.
This is a new public type that users will implement. Add documentation explaining the purpose of each parameter and the expected behavior of enrichment functions.
Apply this diff to add documentation:
+/**
+ * A function that enriches a CSF (Component Story Format) file before per-story enrichment.
+ *
+ * @param csf - The CSF file to be enriched (will be mutated)
+ * @param csfSource - The source CSF file for reference (read-only)
+ */
export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => void;📝 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.
| export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => void; | |
| /** | |
| * A function that enriches a CSF (Component Story Format) file before per-story enrichment. | |
| * | |
| * @param csf - The CSF file to be enriched (will be mutated) | |
| * @param csfSource - The source CSF file for reference (read-only) | |
| */ | |
| export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => void; | |
🤖 Prompt for AI Agents
In code/core/src/types/modules/core-common.ts around lines 368 to 369, the
exported CsfEnricher type lacks JSDoc for a public API; add a JSDoc block
immediately above the type that briefly describes its purpose (a
user-implemented enricher for augmenting a CsfFile), documents the two
parameters (csf: the target CsfFile to mutate/enrich, csfSource: the
original/source CsfFile providing reference data), states that the function
performs synchronous enrichment and may mutate the csf in-place, and clarifies
it returns void (no value).
| addons?: Preset[]; | ||
| core?: CoreConfig; | ||
| componentManifestGenerator?: ComponentManifestGenerator; | ||
| experimental_enrichCsf?: CsfEnricher; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc documentation for experimental configuration option.
This experimental configuration option needs documentation explaining its purpose and usage, especially since it's part of the public configuration API.
Apply this diff to add documentation:
+ /**
+ * @experimental This feature is experimental and may change in future versions.
+ *
+ * A function to enrich CSF files before per-story enrichment. This allows for
+ * custom transformations of the CSF AST, such as generating code examples.
+ *
+ * @example
+ *
+ * ```ts
+ * experimental_enrichCsf: (csf, csfSource) => {
+ * // Custom enrichment logic
+ * }
+ * ```
+ */
experimental_enrichCsf?: CsfEnricher;🤖 Prompt for AI Agents
In code/core/src/types/modules/core-common.ts around line 384, the
experimental_enrichCsf field lacks JSDoc; add a JSDoc block immediately above
the field that documents purpose, expected signature and parameters, shows a
short usage example matching the provided diff (function signature: (csf,
csfSource) => { /* Custom enrichment logic */ }), and mark it as experimental;
ensure formatting follows existing JSDoc style and TypeScript comment
conventions so IDEs and generated docs pick it up.
e2269ac to
91d3927
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
code/core/src/types/modules/core-common.ts (3)
366-366: Add JSDoc documentation for public API type.This public type lacks documentation explaining its purpose and parameters. Users implementing this interface need guidance on expected behavior.
Apply this diff to add documentation:
+/** + * A function that enriches a CSF (Component Story Format) file before per-story enrichment. + * + * @param csf - The CSF file to be enriched (will be mutated) + * @param csfSource - The source CSF file for reference (read-only) + */ export type CsfEnricher = (csf: CsfFile, csfSource: CsfFile) => void;
382-382: Add JSDoc documentation for experimental configuration option.This experimental field needs documentation explaining its purpose, especially as it's part of the public configuration API that users will reference.
Apply this diff to add documentation:
+ /** + * @experimental This feature is experimental and may change in future versions. + * + * A function to enrich CSF files before per-story enrichment. This allows for + * custom transformations of the CSF AST, such as generating code examples. + * + * @example + * ```ts + * experimental_enrichCsf: (csf, csfSource) => { + * // Custom enrichment logic + * } + * ``` + */ experimental_enrichCsf?: CsfEnricher;
483-483: Add JSDoc documentation for experimental feature flag.This boolean flag needs documentation explaining what functionality it enables, its default value, and that it's experimental.
Apply this diff to add documentation:
+ /** + * @experimental This feature is experimental and may change in future versions. + * + * Enable experimental code examples generation for components. + * + * @default false + */ experimentalCodeExamples?: boolean;
🧹 Nitpick comments (1)
code/core/src/types/modules/core-common.ts (1)
110-112: Clarify the overload comment.The overload itself is sound—it enables better type inference when
configandargsaren't required. However, the comment "The second and third parameter are not needed" is somewhat ambiguous.Consider this clearer wording:
- /** The second and third parameter are not needed. And make type inference easier. */ + /** Overload for calling apply with only an extension name, enabling stricter type inference. */ apply<T extends keyof StorybookConfigRaw>(extension: T): Promise<StorybookConfigRaw[T]>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/common/presets.ts(1 hunks)code/core/src/types/modules/core-common.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/common/presets.tscode/core/src/types/modules/core-common.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/common/presets.tscode/core/src/types/modules/core-common.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/common/presets.tscode/core/src/types/modules/core-common.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/common/presets.tscode/core/src/types/modules/core-common.ts
🧬 Code graph analysis (1)
code/core/src/types/modules/core-common.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1027)
⏰ 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: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/types/modules/core-common.ts (1)
4-4: LGTM: Import required for CsfEnricher type.The
CsfFileimport is necessary for the newCsfEnrichertype definition.code/core/src/common/presets.ts (1)
324-324: LGTM: Config parameter is now optional and already in active use.The codebase already calls
presets.apply()without config arguments in multiple places (e.g.,code/builders/builder-vite/src/plugins/csf-plugin.ts:15,code/addons/docs/src/preset.ts:45), and existing preset implementations likeenrichCsfalready handle the optional input parameter correctly.
824c900 to
7c6b418
Compare
7c6b418 to
d903424
Compare
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
code/renderers/react/src/enrichCsf.ts (2)
48-92: Fix runtime errors from spreading nullish values and ensure snippet overrides existing code.Three issues:
- Line 52: Spreading
originalParameterswill throw ifStory.parametersisundefined.- Lines 73-74: The
codeproperty precedes the spread, allowing existingdocs.source.codeto overwrite the generated snippet.- Line 85: Spreading
optionalDocswill throw ifStory.parameters.docsisundefined.Apply this diff to fix all three issues:
const originalParameters = t.memberExpression( baseStoryObject, t.identifier('parameters') ); - parameters.push(t.spreadElement(originalParameters)); + parameters.push( + t.spreadElement( + t.logicalExpression('??', originalParameters, t.objectExpression([])) + ) + ); const optionalDocs = t.optionalMemberExpression( originalParameters, t.identifier('docs'), false, true ); const extraDocsParameters = []; if (snippet) { const optionalSource = t.optionalMemberExpression( optionalDocs, t.identifier('source'), false, true ); extraDocsParameters.push( t.objectProperty( t.identifier('source'), t.objectExpression([ - t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)), - t.spreadElement(optionalSource), + t.spreadElement( + t.logicalExpression('??', optionalSource, t.objectExpression([])) + ), + t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)), ]) ) ); } // docs: { description: { story: %%description%% } }, if (extraDocsParameters.length > 0) { parameters.push( t.objectProperty( t.identifier('docs'), - t.objectExpression([t.spreadElement(optionalDocs), ...extraDocsParameters]) + t.objectExpression([ + t.spreadElement( + t.logicalExpression('??', optionalDocs, t.objectExpression([])) + ), + ...extraDocsParameters, + ]) ) );
11-15: Preserve composability by returning input when feature is disabled.When
experimentalCodeExamplesis false, returningundefineddiscards any existingexperimental_enrichCsffrom prior presets. Additionally, when the feature is enabled, the existing enricher (if any) should be invoked before applying the new enrichment logic.Apply this diff to preserve composability:
export const enrichCsf: PresetPropertyFn<'experimental_enrichCsf'> = async (input, options) => { const features = await options.presets.apply('features'); if (!features.experimentalCodeExamples) { - return; + return input; } - return async (csf: CsfFile, csfSource: CsfFile) => { + return async (csf: CsfFile, csfSource: CsfFile) => { + if (typeof input === 'function') { + await input(csf, csfSource); + } await Promise.all( Object.entries(csf._storyPaths).map(async ([key, storyExport]) => {
🧹 Nitpick comments (1)
code/renderers/react/src/enrichCsf.ts (1)
26-27: TODO: Read user's Prettier configuration.The hardcoded filepath
join(process.cwd(), 'component.tsx')is used for Prettier formatting. Consider resolving the actual story file path or reading the user's Prettier config to ensure correct formatting.Do you want me to open an issue to track implementing proper Prettier config resolution, or would you like me to propose a solution?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
code/.storybook/main.ts(1 hunks)code/addons/docs/src/preset.ts(3 hunks)code/builders/builder-vite/src/plugins/csf-plugin.ts(1 hunks)code/core/src/common/presets.ts(1 hunks)code/core/src/common/utils/formatter.ts(1 hunks)code/core/src/csf-tools/enrichCsf.ts(2 hunks)code/core/src/types/modules/core-common.ts(4 hunks)code/lib/csf-plugin/src/rollup-based-plugin.ts(1 hunks)code/lib/csf-plugin/src/webpack-loader.ts(1 hunks)code/renderers/react/src/docs/jsxDecorator.tsx(2 hunks)code/renderers/react/src/enrichCsf.ts(1 hunks)code/renderers/react/src/entry-preview-docs.ts(1 hunks)code/renderers/react/src/preset.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- code/lib/csf-plugin/src/webpack-loader.ts
- code/lib/csf-plugin/src/rollup-based-plugin.ts
- code/builders/builder-vite/src/plugins/csf-plugin.ts
- code/core/src/common/presets.ts
- code/.storybook/main.ts
- code/core/src/csf-tools/enrichCsf.ts
- code/core/src/types/modules/core-common.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/renderers/react/src/enrichCsf.tscode/core/src/common/utils/formatter.tscode/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/entry-preview-docs.tscode/renderers/react/src/preset.tscode/addons/docs/src/preset.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/renderers/react/src/enrichCsf.tscode/core/src/common/utils/formatter.tscode/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/entry-preview-docs.tscode/renderers/react/src/preset.tscode/addons/docs/src/preset.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/renderers/react/src/enrichCsf.tscode/core/src/common/utils/formatter.tscode/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/entry-preview-docs.tscode/renderers/react/src/preset.tscode/addons/docs/src/preset.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/enrichCsf.tscode/core/src/common/utils/formatter.tscode/renderers/react/src/docs/jsxDecorator.tsxcode/renderers/react/src/entry-preview-docs.tscode/renderers/react/src/preset.tscode/addons/docs/src/preset.ts
🧠 Learnings (2)
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/renderers/react/src/docs/jsxDecorator.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/renderers/react/src/entry-preview-docs.ts
🧬 Code graph analysis (4)
code/renderers/react/src/enrichCsf.ts (5)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(277-1017)code/core/src/common/utils/formatter.ts (1)
getPrettier(1-6)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(23-247)
code/renderers/react/src/docs/jsxDecorator.tsx (1)
code/core/src/types/modules/core-common.ts (1)
StorybookConfigRaw(367-520)
code/renderers/react/src/entry-preview-docs.ts (2)
code/core/src/types/modules/core-common.ts (1)
StorybookConfigRaw(367-520)code/renderers/react/src/docs/jsxDecorator.tsx (1)
jsxDecorator(238-269)
code/addons/docs/src/preset.ts (3)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(11-97)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)
⏰ 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: Core Unit Tests, windows-latest
🔇 Additional comments (9)
code/core/src/common/utils/formatter.ts (1)
1-6: LGTM: Export enables broader formatter usage.The visibility change allows
getPrettierto be consumed by enrichment logic without altering its behavior or error handling.code/addons/docs/src/preset.ts (2)
45-45: LGTM: Enrichment wiring is correctly positioned.Retrieving
enrichCsfbefore plugin configuration ensures the enricher is available when needed.
106-111: LGTM: Enricher properly merged into plugin options.The conditional block correctly passes
enrichCsfto the CSF plugin whencsfPluginOptionsis provided.code/renderers/react/src/preset.ts (1)
13-13: LGTM: Experimental API alias follows convention.The re-export pattern matches the
experimental_componentManifestGeneratoron line 11 and properly signals the experimental nature of the feature.code/renderers/react/src/docs/jsxDecorator.tsx (2)
7-7: LGTM: Type import supports feature flag usage.The type-only import enables proper typing for the global
FEATURESvariable without runtime overhead.
235-236: LGTM: Ambient declaration follows established pattern.The global
FEATURESdeclaration matches the pattern used inentry-preview-docs.tsand correctly uses theno-vareslint-disable for ambient declarations.code/renderers/react/src/entry-preview-docs.ts (3)
2-2: LGTM: Type import enables feature flag typing.The import provides proper typing for the global
FEATURESvariable used in the conditional logic.
7-8: LGTM: Ambient declaration consistent with jsxDecorator.tsx.The global
FEATURESdeclaration follows the same pattern established injsxDecorator.tsx.
10-12: LGTM: Feature flag correctly toggles code generation approach.When
experimentalCodeExamplesis enabled, the decorator array is empty (disabling runtime JSX decoration). Otherwise, the traditionaljsxDecoratoris included. This properly gates the experimental feature.
|
Failed to publish canary version of this pull request, triggered by @kasperpeulen. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/18782658854 |
Closes #
What I did
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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Chores