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
25 changes: 17 additions & 8 deletions code/renderers/react/src/componentManifest/generateCodeSnippet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,26 @@ export function getCodeSnippet(
? metaPath.get('properties').filter((p) => p.isObjectProperty())
: [];

const getRenderPath = (object: NodePath<t.ObjectProperty>[]) =>
object
.filter((p) => keyOf(p.node) === 'render')
.map((p) => p.get('value'))
.find(
(v): v is NodePath<t.ArrowFunctionExpression | t.FunctionExpression> =>
v.isArrowFunctionExpression() || v.isFunctionExpression()
const getRenderPath = (object: NodePath<t.ObjectProperty>[]) => {
const renderPath = object.find((p) => keyOf(p.node) === 'render')?.get('value');

if (renderPath?.isIdentifier()) {
componentName = renderPath.node.name;
}
if (
renderPath &&
!(renderPath.isArrowFunctionExpression() || renderPath.isFunctionExpression())
) {
throw renderPath.buildCodeFrameError(
'Expected render to be an arrow function or function expression'
);
}

return renderPath;
};

const renderPath = getRenderPath(storyProps);
const metaRenderPath = getRenderPath(metaProps);
const renderPath = getRenderPath(storyProps);

storyFn ??= renderPath ?? metaRenderPath;

Expand Down
12 changes: 7 additions & 5 deletions code/renderers/react/src/componentManifest/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,23 @@ export const componentManifestGenerator: PresetPropertyFn<
} satisfies Partial<ComponentManifest>;

if (!component?.reactDocgen) {
const error = !component
const error = !csf._meta?.component
? {
name: 'No meta.component specified',
message: 'Specify meta.component for the component to be included in the manifest.',
name: 'No component found',
message:
'We could not detect the component from your story file. Specify meta.component.',
}
: {
name: 'No component import found',
message: `No component file found for the "${component.componentName}" component.`,
message: `No component file found for the "${csf.meta.component}" component.`,
};
return {
...base,
error: {
name: error.name,
message:
csf._metaStatementPath?.buildCodeFrameError(error.message).message ?? error.message,
(csf._metaStatementPath?.buildCodeFrameError(error.message).message ??
error.message) + `\n\n${entry.importPath}:\n${storyFile}`,
},
Comment on lines +150 to 152
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid embedding the full story source in manifest errors

Line [151]: appending ${storyFile} injects the entire story file into the manifest error payload. For sizable stories this can add hundreds of kilobytes per entry and leaks every line of the source to downstream consumers. The code-frame fallback already gives localized context; we should keep that and drop the raw file dump.

-              (csf._metaStatementPath?.buildCodeFrameError(error.message).message ??
-                error.message) + `\n\n${entry.importPath}:\n${storyFile}`,
+              csf._metaStatementPath?.buildCodeFrameError(error.message).message ?? error.message,
📝 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.

Suggested change
(csf._metaStatementPath?.buildCodeFrameError(error.message).message ??
error.message) + `\n\n${entry.importPath}:\n${storyFile}`,
},
csf._metaStatementPath?.buildCodeFrameError(error.message).message ?? error.message,
},
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.ts around lines 150 to
152, the manifest error currently appends the full story source (`${storyFile}`)
to the error message which leaks the entire file and inflates the manifest;
remove the raw story file dump and only include the import path and/or the csf
code-frame fallback (e.g., keep `${entry.importPath}` and the
csf._metaStatementPath buildCodeFrameError message) so the error provides
localized context without embedding the full source content.

};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const getComponents = ({
});
}
} catch (e) {
logger.error(e);
logger.debug(e);
}
if (path) {
const reactDocgen = getReactDocgen(path, component);
Expand Down
207 changes: 132 additions & 75 deletions code/renderers/react/src/componentManifest/reactDocgen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { dirname, sep } from 'node:path';

import { babelParse, types as t } from 'storybook/internal/babel';
import { getProjectRoot, supportedExtensions } from 'storybook/internal/common';
import { logger } from 'storybook/internal/node-logger';

import * as find from 'empathic/find';
import {
Expand All @@ -12,6 +13,7 @@ import {
makeFsImporter,
parse,
} from 'react-docgen';
import { dedent } from 'ts-dedent';
import * as TsconfigPaths from 'tsconfig-paths';

import { type ComponentRef } from './getComponentImports';
Expand All @@ -33,6 +35,9 @@ const defaultResolver = new docgenResolver.FindExportedDefinitionsResolver();
const handlers = [...defaultHandlers, actualNameHandler, exportNameHandler];

export function getMatchingDocgen(docgens: DocObj[], component: ComponentRef) {
if (docgens.length === 0) {
return;
}
if (docgens.length === 1) {
return docgens[0];
}
Expand Down Expand Up @@ -91,75 +96,141 @@ export const parseWithReactDocgen = cached(

const getExportPaths = cached(
(code: string, filePath: string) => {
const ast = (() => {
try {
return babelParse(code);
} catch (_) {
return undefined;
}
})();

if (!ast) {
return [] as string[];
let ast;
try {
ast = babelParse(code);
} catch (_) {
return [];
}

const basedir = dirname(filePath);
const body = ast.program.body;
return body
.flatMap((n) =>
t.isExportAllDeclaration(n)
? [n.source.value]
: t.isExportNamedDeclaration(n) && !!n.source && !n.declaration
? [n.source.value]
.flatMap((statement) =>
t.isExportAllDeclaration(statement)
? [statement.source.value]
: t.isExportNamedDeclaration(statement) && !!statement.source && !statement.declaration
? [statement.source.value]
: []
)
.map((s) => matchPath(s, basedir))
.map((s) => {
.map((id) => matchPath(id, basedir))
.flatMap((id) => {
try {
return cachedResolveImport(s, { basedir });
} catch {
return undefined;
return [cachedResolveImport(id, { basedir })];
} catch (e) {
logger.debug(e);
return [];
}
})
.filter((p): p is string => !!p && !p.includes('node_modules'));
});
},
{ name: 'getExportPaths' }
);

const gatherDocgensForPath = cached(
(
filePath: string,
path: string,
depth: number
): { docgens: DocObj[]; analyzed: { path: string; code: string }[] } => {
if (depth > 5 || filePath.includes('node_modules')) {
return { docgens: [], analyzed: [] };
): {
docgens: DocObj[];
errors: { path: string; code: string; name: string; message: string }[];
} => {
if (path.includes('node_modules')) {
return {
docgens: [],
errors: [
{
path,
code: '/* File in node_modules */',
name: 'Component file in node_modules',
message: dedent`
Component files in node_modules are not supported.
The distributed files in node_modules usually don't contain the necessary comments or types needed to analyze component information.
Configure TypeScript path aliases to map your package name to the source file instead.

Example (tsconfig.json):
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"@design-system/button": ["src/components/Button.tsx"],
"@design-system/*": ["src/components/*"]
}
}
}

Then import using:
import { Button } from '@design-system/button'

Storybook resolves tsconfig paths automatically.
`,
},
],
};
}

let code: string | undefined;
let code;
try {
code = cachedReadFileSync(filePath, 'utf-8') as string;
} catch {}
code = cachedReadFileSync(path, 'utf-8') as string;
} catch {
return {
docgens: [],
errors: [
{
path,
code: '/* File not found or unreadable */',
name: 'Component file could not be read',
message: `Could not read the component file located at "${path}".\nPrefer relative imports if possible.`,
},
],
};
}

if (!code) {
return { docgens: [], analyzed: [{ path: filePath, code: '/* File not found */' }] };
if (depth > 5) {
return {
docgens: [],
errors: [
{
path,
code,
name: 'Max re-export depth exceeded',
message: dedent`
Traversal stopped after 5 steps while following re-exports starting from this file.
This usually indicates a deep or circular re-export chain. Try one of the following:
- Import the component file directly (e.g., src/components/Button.tsx),
- Reduce the number of re-export hops.
`,
},
],
};
}
Comment on lines +188 to 205
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid including full source in error objects.

Line 194 includes the full source code in the error object's code field. This can bloat the manifest, especially for large files that hit the depth limit. Use a placeholder comment like the approach in lines 143 and 180.

Apply this diff:

     if (depth > 5) {
       return {
         docgens: [],
         errors: [
           {
             path,
-            code,
+            code: '/* Max re-export depth exceeded */',
             name: 'Max re-export depth exceeded',
             message: dedent`
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/reactDocgen.ts around lines 188 to
205, the error object returned when max re-export depth is exceeded currently
places the full source into the `code` field which can bloat the manifest;
replace the full source with a short placeholder comment (e.g., "/* source
omitted due to size */" or the same placeholder used on lines 143/180) so the
error still conveys context without embedding large file contents, and ensure
the `code` property remains a string and tests/type checks still pass.


const reexportResults = getExportPaths(code, filePath).map((p) =>
gatherDocgensForPath(p, depth + 1)
);
const fromReexports = reexportResults.flatMap((r) => r.docgens);
const analyzedChildren = reexportResults.flatMap((r) => r.analyzed);
const exportPaths = getExportPaths(code, path).map((p) => gatherDocgensForPath(p, depth + 1));
const docgens = exportPaths.flatMap((r) => r.docgens);
const errors = exportPaths.flatMap((r) => r.errors);

let locals: DocObj[];
try {
locals = parseWithReactDocgen(code as string, filePath);
} catch {
locals = [];
return {
docgens: [...parseWithReactDocgen(code, path), ...docgens],
errors,
};
} catch (e) {
const message = e instanceof Error ? e.message : String(e);
return {
docgens,
errors: [
{
path,
code,
name: 'No component definition found',
message: dedent`
${message}
You can debug your component file in this playground: https://react-docgen.dev/playground
`,
},
...errors,
],
};
}
Comment on lines +216 to 233
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid including full source in error objects.

Line 223 includes the full source code in the error object. This creates the same manifest bloat issue as line 194. Use a placeholder comment instead.

Apply this diff:

     } catch (e) {
       const message = e instanceof Error ? e.message : String(e);
       return {
         docgens,
         errors: [
           {
             path,
-            code,
+            code: '/* Component parse failed */',
             name: 'No component definition found',
             message: dedent`
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/reactDocgen around lines 216 to
233, the error object returned on catch currently includes the full source
(causing manifest bloat); change the error entry so it does not embed the file
source — replace the real source field with a small placeholder string (e.g. "/*
source omitted */" or similar comment) or remove the source property entirely,
mirroring the fix applied at line 194, and return the modified errors array with
the placeholder instead of the full code.


return {
docgens: [...locals, ...fromReexports],
analyzed: [{ path: filePath, code }, ...analyzedChildren],
};
},
{ name: 'gatherDocgensWithTrace', key: (filePath) => filePath }
);
Expand All @@ -171,39 +242,25 @@ export const getReactDocgen = cached(
):
| { type: 'success'; data: DocObj }
| { type: 'error'; error: { name: string; message: string } } => {
if (path.includes('node_modules')) {
return {
type: 'error',
error: {
name: 'Component file in node_modules',
message: `Component files in node_modules are not supported. Please import your component file directly.`,
},
};
}

const docgenWithInfo = gatherDocgensForPath(path, 0);
const docgens = docgenWithInfo.docgens;

const noCompDefError = {
type: 'error' as const,
error: {
name: 'No component definition found',
message:
`Could not find a component definition.\n` +
`Prefer relative imports if possible.\n` +
`Avoid pointing to transpiled files.\n` +
`You can debug your component file in this playground: https://react-docgen.dev/playground\n\n` +
docgenWithInfo.analyzed.map(({ path, code }) => `File: ${path}\n${code}`).join('\n'),
},
};

if (!docgens || docgens.length === 0) {
return noCompDefError;
}
const { docgens, errors } = gatherDocgensForPath(path, 0);

const docgen = getMatchingDocgen(docgens, component);

if (!docgen) {
return noCompDefError;
const error = {
name: errors.at(-1)?.name ?? 'No component definition found',
message: errors
.map(
(e) => dedent`
File: ${e.path}
Error:
${e.message}
Code:
${e.code}`
)
.join('\n\n'),
};
Comment on lines +251 to +262
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep docgen errors concise and non-empty

Line [249]: when docgens is empty we currently join an empty errors array, yielding a blank message. When errors is non-empty we echo ${e.code}, which is the entire source file captured earlier—ballooning the manifest and exposing full component source. Let’s format the error without dumping the file contents and add a fallback message when no per-file errors exist.

-      const error = {
-        name: errors.at(-1)?.name ?? 'No component definition found',
-        message: errors
-          .map(
-            (e) => dedent`
-            File: ${e.path}
-            Error:
-            ${e.message}
-            Code:
-            ${e.code}`
-          )
-          .join('\n\n'),
-      };
+      const formattedMessage =
+        errors.length > 0
+          ? errors
+              .map(
+                (e) => dedent`
+                  File: ${e.path}
+                  Error:
+                  ${e.message}`
+              )
+              .join('\n\n')
+          : dedent`
+              No component definition found.
+              We attempted to parse "${path}" but react-docgen did not return any components.`;
+      const error = {
+        name: errors.at(-1)?.name ?? 'No component definition found',
+        message: formattedMessage,
+      };
📝 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.

Suggested change
name: errors.at(-1)?.name ?? 'No component definition found',
message: errors
.map(
(e) => dedent`
File: ${e.path}
Error:
${e.message}
Code:
${e.code}`
)
.join('\n\n'),
};
const formattedMessage =
errors.length > 0
? errors
.map(
(e) => dedent`
File: ${e.path}
Error:
${e.message}`
)
.join('\n\n')
: dedent`
No component definition found.
We attempted to parse "${path}" but react-docgen did not return any components.`;
const error = {
name: errors.at(-1)?.name ?? 'No component definition found',
message: formattedMessage,
};
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/reactDocgen.ts around lines 249 to
260, the error message currently joins potentially-empty errors (producing a
blank string) and injects the full source via e.code (exposing large component
source). Change the message generation to: if errors is empty set a fallback
like "No docgen errors captured"; otherwise map each error to a concise block
showing File: <path> and Error: <message> and do NOT include the full e.code —
either omit it entirely or include a very small excerpt (e.g., first ~200 chars)
if an excerpt is helpful. Ensure the final message.join uses this concise
representation so the manifest stays small and non-empty.

return { type: 'error', error };
}
return { type: 'success', data: docgen };
},
Expand Down
Loading