Skip to content

Allow AI setup for React Rsbuild projects#34888

Closed
agustiobvious wants to merge 2 commits into
storybookjs:nextfrom
agustiobvious:codex/ai-setup-react-rsbuild
Closed

Allow AI setup for React Rsbuild projects#34888
agustiobvious wants to merge 2 commits into
storybookjs:nextfrom
agustiobvious:codex/ai-setup-react-rsbuild

Conversation

@agustiobvious
Copy link
Copy Markdown

@agustiobvious agustiobvious commented May 22, 2026

Summary

  • allow storybook ai setup for React projects using storybook-builder-rsbuild
  • keep the AI setup gate narrow by still requiring the React renderer
  • extract the support check into a small helper with coverage for Vite, Rsbuild, non-React Rsbuild, and unsupported React builders

Why

storybook ai setup currently exits for React + Rsbuild projects even though the generated setup prompt is not Vite-specific. The current guard only allows @storybook/builder-vite, so projects using storybook-react-rsbuild receive the unsupported-project message and no setup instructions.

Validation

  • git diff --check
  • npx --yes -p vitest@4.1.5 -p vite@7.0.4 vitest run --config /tmp/storybook-ai-vitest.config.mjs

The focused Vitest run used a minimal temp config because this shallow checkout did not have Yarn/Corepack dependencies installed.

Summary by CodeRabbit

  • New Features

    • AI-assisted Storybook setup now supports React projects with the Rsbuild builder in addition to Vite.
  • Improvements

    • Clearer error messages listing supported renderer/builder combinations when AI setup is unsupported.
    • Test runner now only includes special project annotations when using the Vite-style builder.
    • Storybook startup skips adding nonexistent builder-specific preset files to avoid spurious errors.

Review Change Stack

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","build","dependencies"]

🚫

PR is not labeled with one of: ["ci:normal","ci:merged","ci:daily","ci:docs"]

🚫 PR title must be in the format of "Area: Summary", With both Area and Summary starting with a capital letter Good examples: - "Docs: Describe Canvas Doc Block" - "Svelte: Support Svelte v4" Bad examples: - "add new api docs" - "fix: Svelte 4 support" - "Vue: improve docs"
🚫 PR description is missing the mandatory "#### Manual testing" section. Please add it so that reviewers know how to manually test your changes.

Generated by 🚫 dangerJS against f92a7da

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

Centralizes AI setup renderer/builder support checks into a new module with tests and integrates it into aiSetup; additionally, makes Vitest project-annotation gating builder-aware and guards adding a builder preset in core load on file existence.

Changes

AI Setup Project Support Detection

Layer / File(s) Summary
Support detection contract and tests
code/lib/cli-storybook/src/ai/supported-project.ts, code/lib/cli-storybook/src/ai/supported-project.test.ts
isAiSetupSupportedProject enforces @storybook/react with a supported builder (Vite or Rsbuild); getUnsupportedAiSetupProjectMessage formats rejection text. Tests cover supported and unsupported renderer/builder combinations and message content.
aiSetup integration
code/lib/cli-storybook/src/ai/index.ts
Replaces prior inline renderer/builder gating with isAiSetupSupportedProject; logs getUnsupportedAiSetupProjectMessage and exits early for unsupported projects.

Vitest plugin builder-aware gating

Layer / File(s) Summary
Vitest plugin builder check
code/addons/vitest/src/vitest-plugin/index.ts
Derives builderName from core.builder and only calls requiresProjectAnnotations(...) when the builder corresponds to Vite; otherwise disables project-annotation requirement without invoking the function.

Core server builder preset guard

Layer / File(s) Summary
Guard builder preset file existence
code/core/src/core-server/load.ts
Imports existsSync and conditionally pushes the computed builder preset.js into corePresets only if the file exists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@code/core/src/core-server/load.ts`:
- Around line 78-81: The existsSync call is given a file:// URL-derived path,
causing false negatives; update the code that computes builderPresetDir/preset
path (where builderPresetDir and builderPreset are built) to detect file: URLs
(e.g., builderName starting with 'file:') and convert them to a filesystem path
using fileURLToPath from 'url' before joining to 'preset.js' and calling
existsSync; ensure you import fileURLToPath and apply it to builderPresetDir (or
the builderName source) so builderPreset becomes a valid local path for
existsSync to check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e670b4e-ada0-44a2-932b-2f2a08b92b41

📥 Commits

Reviewing files that changed from the base of the PR and between bc5ed34 and f92a7da.

📒 Files selected for processing (2)
  • code/addons/vitest/src/vitest-plugin/index.ts
  • code/core/src/core-server/load.ts

Comment on lines +78 to +81
const builderPreset = join(builderPresetDir, 'preset.js');
if (existsSync(builderPreset)) {
corePresets.push(builderPreset);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does Node.js fs.existsSync accept a file:///... string path directly, or should file URLs be passed as URL objects / converted with fileURLToPath?

💡 Result:

fs.existsSync does NOT accept a file:///... URL string directly as a path; pass a WHATWG URL object (e.g., new URL('file:///...')) or convert the file URL to a filesystem path with fileURLToPath(). Node’s fs API docs explicitly say the path parameter can be a WHATWG URL object using the file: protocol (not a URL string) for fs functions like fs.access() [1]. An issue discussion in the Node.js repo notes that fs APIs support URL inputs only when passed as URL objects, not plain URL strings [2]. The same issue also reflects the rationale that file systems may contain characters like : that would make treating file://... strings as paths ambiguous, implying Node keeps string inputs as plain paths [2]. Recommended pattern: 1) If you already have a file URL string: - Convert: existsSync(fileURLToPath(urlString)) 2) If you already have a URL object: - Use: existsSync(urlObject) Example: const { existsSync } = require('node:fs'); const { fileURLToPath } = require('node:url'); const fileUrlString = 'file:///C:/path/to/file.txt'; const ok = existsSync(fileURLToPath(fileUrlString)); If you pass 'file:///C:/path/to/file.txt' directly to existsSync as a string, it will be treated as a literal path string (not a URL), which won’t match real filesystem paths.

Citations:


Fix existsSync when builderName is a file:// URL

When builderName is a file://... URL string, the code ends up calling fs.existsSync() with a file://... string (not a filesystem path), which can false-negative and skip loading the builder preset. Convert file: URLs to paths with fileURLToPath() before building the preset.js path / calling existsSync.

Suggested fix
-import { existsSync } from 'node:fs';
+import { existsSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
@@
-    const isResolved = builderName.startsWith('file:') || isAbsolute(builderName);
-    const builderPresetDir = isResolved ? dirname(builderName) : resolvePackageDir(builderName);
+    const isFileUrl = builderName.startsWith('file:');
+    const isResolved = isFileUrl || isAbsolute(builderName);
+    const builderPresetDir = isResolved
+      ? isFileUrl
+        ? dirname(fileURLToPath(builderName))
+        : dirname(builderName)
+      : resolvePackageDir(builderName);
     const builderPreset = join(builderPresetDir, 'preset.js');
     if (existsSync(builderPreset)) {
       corePresets.push(builderPreset);
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/core/src/core-server/load.ts` around lines 78 - 81, The existsSync call
is given a file:// URL-derived path, causing false negatives; update the code
that computes builderPresetDir/preset path (where builderPresetDir and
builderPreset are built) to detect file: URLs (e.g., builderName starting with
'file:') and convert them to a filesystem path using fileURLToPath from 'url'
before joining to 'preset.js' and calling existsSync; ensure you import
fileURLToPath and apply it to builderPresetDir (or the builderName source) so
builderPreset becomes a valid local path for existsSync to check.

@agustiobvious
Copy link
Copy Markdown
Author

is this something wanted upstream anyways? happy to close this otherwise

@valentinpalkovic
Copy link
Copy Markdown
Contributor

Hi @agustiobvious,

Thank you for your contribution!

storybook ai setup indeed is dependent on @storybook/builder-vite and @storybook/addon-vitest since the self-healing flow doesn't work without it. Hence, I am closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants