Skip to content

Conversation

@kasperpeulen
Copy link
Contributor

@kasperpeulen kasperpeulen commented Nov 5, 2025

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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/core team 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

  • Bug Fixes
    • Improved component detection and validation, now handling identifier-based renders
    • Enhanced error reporting with richer messages and source-location context
    • Stronger re-export handling with depth limits and aggregated error summaries
    • Reduced noisy logging for import-resolution failures (now debug-level)

@nx-cloud
Copy link

nx-cloud bot commented Nov 5, 2025

View your CI Pipeline Execution ↗ for commit fa39aa1

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 45s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-05 14:41:58 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

Updates improve render-path resolution to accept identifier renders, restructure how story/meta render functions are chosen, and significantly enhance react-docgen traversal and error aggregation. Component detection now relies on csf.meta?.component, and import-resolution logging was lowered from error to debug.

Changes

Cohort / File(s) Summary
Render path resolution
code/renderers/react/src/componentManifest/generateCodeSnippet.ts
getRenderPath now returns the render value directly as a NodePath and accepts identifiers as component names; assigns componentName when render is an identifier; validates render value is an arrow function, function expression, or identifier and throws for other expressions. Computes metaRenderPath first, then storyRenderPath, and sets storyFn from resolved render path with fallback to meta render.
React docgen traversal & error aggregation
code/renderers/react/src/componentManifest/reactDocgen.ts
Reworked to aggregate errors alongside docgens during re-export traversal. getExportPaths uses guarded parsing; gatherDocgensForPath returns { docgens, errors } with explicit error records (path, code, name, message), handles node_modules, file read failures, max re-export depth, and composes richer error messages including playground links. getReactDocgen now returns success or structured error built from aggregated results. getMatchingDocgen returns early when no docgens. Added imports: logger and dedent.
Component detection & error messages
code/renderers/react/src/componentManifest/generator.ts
Missing react-docgen detection now checks csf._meta?.component/csf.meta?.component rather than component reference presence; error naming and messages updated to use csf.meta.component and append source location details (entry.importPath and full story file contents) to the error output.
Import resolution logging
code/renderers/react/src/componentManifest/getComponentImports.ts
Exception logging level in the import-resolution catch block changed from error to debug.

Sequence Diagram(s)

sequenceDiagram
    participant Generator as generator.ts
    participant Snippet as generateCodeSnippet.ts
    participant Docgen as getReactDocgen
    participant Gather as gatherDocgensForPath
    participant Parse as parseWithReactDocgen
    participant Match as getMatchingDocgen

    Generator->>Docgen: request docgen for story file
    Docgen->>Gather: gatherDocgensForPath(path, depth=0)
    Gather->>Parse: parseWithReactDocgen(path)
    Parse-->>Gather: { docgens?, parseError? }

    alt Parse success
        Gather->>Gather: traverse re-exports (depth ≤ 5)
        Gather-->>Docgen: { docgens[], errors[] }
    else Parse failure or file errors
        Gather-->>Docgen: { docgens[], errors[] } (includes parse/file/node_modules errors)
    end

    Docgen->>Match: getMatchingDocgen(docgens)
    alt docgen found
        Match-->>Docgen: matched docgen
        Docgen-->>Generator: { type: "success", data: DocObj }
    else no docgen
        Match-->>Docgen: return early
        Docgen-->>Generator: { type: "error", error: aggregatedError }
    end

    Generator->>Snippet: request code snippet
    Snippet->>Snippet: resolve metaRenderPath first
    Snippet->>Snippet: resolve storyRenderPath
    Snippet->>Snippet: validate render value (arrow|func|identifier)
    Snippet-->>Generator: snippet or throw
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review focus:
    • reactDocgen.ts — new return shapes, error aggregation, re-export traversal, depth limits, and message composition.
    • generateCodeSnippet.ts — validation logic for identifier renders and changes to render-path resolution order; ensure identifier-to-componentName assignment is correct in all cases.
    • generator.ts — confirm csf.meta?.component usage is safe across code paths and that appended source details are formatted correctly.
    • Ensure logging level change in getComponentImports.ts is intentional and doesn't hide actionable errors.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d1fc3e and b59e2ee.

📒 Files selected for processing (4)
  • code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1 hunks)
  • code/renderers/react/src/componentManifest/generator.ts (1 hunks)
  • code/renderers/react/src/componentManifest/getComponentImports.ts (1 hunks)
  • code/renderers/react/src/componentManifest/reactDocgen.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 (use yarn lint:js:cmd <file>)

Files:

  • code/renderers/react/src/componentManifest/getComponentImports.ts
  • code/renderers/react/src/componentManifest/reactDocgen.ts
  • code/renderers/react/src/componentManifest/generator.ts
  • code/renderers/react/src/componentManifest/generateCodeSnippet.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/componentManifest/getComponentImports.ts
  • code/renderers/react/src/componentManifest/reactDocgen.ts
  • code/renderers/react/src/componentManifest/generator.ts
  • code/renderers/react/src/componentManifest/generateCodeSnippet.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/componentManifest/getComponentImports.ts
  • code/renderers/react/src/componentManifest/reactDocgen.ts
  • code/renderers/react/src/componentManifest/generator.ts
  • code/renderers/react/src/componentManifest/generateCodeSnippet.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/renderers/react/src/componentManifest/getComponentImports.ts
  • code/renderers/react/src/componentManifest/reactDocgen.ts
  • code/renderers/react/src/componentManifest/generator.ts
  • code/renderers/react/src/componentManifest/generateCodeSnippet.ts
🧠 Learnings (6)
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to {code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size

Applied to files:

  • code/renderers/react/src/componentManifest/getComponentImports.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 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.*`

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 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/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
🧬 Code graph analysis (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)
code/core/src/node-logger/index.ts (1)
  • logger (49-75)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)
code/renderers/react/src/componentManifest/utils.ts (3)
  • cachedResolveImport (93-93)
  • cached (41-82)
  • cachedReadFileSync (89-89)
⏰ 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). (2)
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/renderers/react/src/componentManifest/getComponentImports.ts (1)

201-202: Lower log level keeps expected misses quiet

Line [201]: lowering this log to debug keeps the expected import-resolution misses from surfacing as errors while the upstream generator now aggregates actionable failures. Looks good.

code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)

124-141: Identifier render support is a welcome addition

Line [124]: recognizing identifier-valued render properties and validating the shape tightens the CSF4 path and plugs the common Template pattern nicely.

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

Comment on lines +249 to +260
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'),
};
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)

250-262: Remove full source code from error messages.

Lines 252-261 include ${e.code} in the error message, which can dump entire source files into the manifest. Additionally, when the errors array is empty, the join produces an empty string. This is the same issue flagged in the previous review.

Apply this diff to fix both issues:

     if (!docgen) {
+      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: errors
-          .map(
-            (e) => dedent`
-            File: ${e.path}
-            Error:
-            ${e.message}
-            Code:
-            ${e.code}`
-          )
-          .join('\n\n'),
+        message: formattedMessage,
       };
       return { type: 'error', error };
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b59e2ee and fa39aa1.

📒 Files selected for processing (1)
  • code/renderers/react/src/componentManifest/reactDocgen.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 (use yarn lint:js:cmd <file>)

Files:

  • code/renderers/react/src/componentManifest/reactDocgen.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/componentManifest/reactDocgen.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/componentManifest/reactDocgen.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.694Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.919Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 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.*`

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 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/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested

Applied to files:

  • code/renderers/react/src/componentManifest/reactDocgen.ts
🧬 Code graph analysis (1)
code/renderers/react/src/componentManifest/reactDocgen.ts (1)
code/renderers/react/src/componentManifest/utils.ts (3)
  • cachedResolveImport (93-93)
  • cached (41-82)
  • cachedReadFileSync (89-89)
🔇 Additional comments (7)
code/renderers/react/src/componentManifest/reactDocgen.ts (7)

6-6: LGTM: Proper imports added.

The logger import follows coding guidelines for server-side code, and dedent will improve error message formatting.

Also applies to: 16-16


38-40: LGTM: Defensive early return.

The early return when docgens is empty prevents unnecessary processing and makes the function's logic clearer.


99-104: LGTM: Resilient parse error handling.

Wrapping babelParse in try-catch and returning an empty array prevents parse exceptions from bubbling up, making the function more resilient.


109-124: LGTM: Improved error handling for import resolution.

The refactoring improves readability, and using logger.debug for import resolution failures is appropriate as these are expected scenarios that shouldn't clutter error logs.


137-169: Excellent error message for monorepo users.

The comprehensive error message with tsconfig path alias examples directly addresses the feedback about helping monorepo users understand why node_modules is problematic. The guidance is clear and actionable.


171-186: LGTM: Clear file read error handling.

The structured error handling for file read failures is clear and helpful, with an appropriate placeholder for the code field.


207-215: LGTM: Effective error aggregation.

The aggregation of both docgens and errors from recursive calls enables comprehensive error reporting while preserving successfully parsed components.

Comment on lines +188 to 205
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.
`,
},
],
};
}
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.

Comment on lines +216 to 233
} 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,
],
};
}
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.

@kasperpeulen kasperpeulen merged commit 4642640 into next Nov 5, 2025
60 of 62 checks passed
@kasperpeulen kasperpeulen deleted the kasper/manifest-auto-component-import-barrel branch November 5, 2025 16:04
@github-actions github-actions bot mentioned this pull request Nov 5, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants