Build: Add Rsbuild-based sandboxes #33039
Conversation
valentinpalkovic
changed the title
|
View your CI Pipeline Execution ↗ for commit cc42c4c
☁️ Nx Cloud last updated this comment at |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughUpdates CI workflow parallelism and adds --loglevel=debug to Storybook init; introduces RsBuild sandbox templates and a nullable skipMocking flag propagated through sandbox task logic to skip mocks/docs; broadens Storybook import detection; refactors framework discovery to use getStorybookInfo (adds configDir, renderer, builder); adjusts Yarn RSBuild handling and related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Dev/CI
participant Registry as Template Registry
participant Init as Sandbox Init Task
participant Mocks as Mocking/Docs Setup
Dev->>Registry: request template (e.g. `react-rsbuild/default-ts`)
Registry-->>Init: return template with modifications.skipMocking
Init->>Init: evaluate template.modifications?.skipMocking
alt skipMocking == true
Init->>Init: write preview config and return early (skip mocks/docs)
else skipMocking == false/undefined
Init->>Mocks: configure mocks, link stories (include mocking files)
Mocks-->>Init: mocks applied, stories linked
end
Init-->>Dev: sandbox ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/lib/create-storybook/src/scaffold-new-project.ts (1)
88-99: Add explicit cases for 'yarn1', 'yarn2', and 'bun' package managers.The
packageManagerToCoercedName()function accepts allPackageManagerNameenum values (npm, yarn1, yarn2, pnpm, bun) but only explicitly handles 'npm' and 'pnpm', silently coercing 'yarn1', 'yarn2', and 'bun' to 'yarn'. Add explicit cases or throw an error for unsupported managers:case 'yarn1': return 'yarn'; case 'yarn2': return 'yarn'; case 'bun': // Either handle bun support or throw: throw new Error(`Unsupported package manager: ${packageManager}`); return 'yarn'; // or appropriate fallback default: throw new Error(`Unknown package manager: ${packageManager}`);
🧹 Nitpick comments (3)
code/lib/create-storybook/src/commands/UserPreferencesCommand.ts (1)
176-179: Consider clarifying the message terminology.The message says "Skipping feature validation" but the code is actually skipping feature auto-selection or determination, not validation. The validation step (via
FeatureCompatibilityService.validateTestFeatureCompatibility) runs earlier in theexecutemethod (lines 68-72) regardless of whether features are explicitly provided.Consider this more accurate wording:
- logger.warn(dedent` - Skipping feature validation since the following features were explicitly selected: - ${Array.from(this.commandOptions.features).join(', ')} - `); + logger.warn(dedent` + Skipping feature auto-selection since the following features were explicitly provided: + ${Array.from(this.commandOptions.features).join(', ')} + `);code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (1)
166-172: Use enum members as keys for better type safety.The
PROXY_MAPuses string literal keys instead of enum members. While this works if the enum values match exactly, using enum members as keys provides better type safety and maintainability.Apply this diff:
private static PROXY_MAP: Record<PackageManagerName, PackageManagerProxy> = { - npm: NPMProxy, - pnpm: PNPMProxy, - yarn1: Yarn1Proxy, - yarn2: Yarn2Proxy, - bun: BUNProxy, + [PackageManagerName.NPM]: NPMProxy, + [PackageManagerName.PNPM]: PNPMProxy, + [PackageManagerName.YARN1]: Yarn1Proxy, + [PackageManagerName.YARN2]: Yarn2Proxy, + [PackageManagerName.BUN]: BUNProxy, };scripts/tasks/sandbox.ts (1)
144-147: Update the comment to reflect the new flag-based logic.The condition now uses the
skipMockingflag instead of checking for 'Bench' in the template name. Consider updating the comment for clarity.- // not if sandbox is bench - if (!details.template.modifications?.skipMocking) { + if (!details.template.modifications?.skipMocking) { await addGlobalMocks(details, options); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.circleci/config.yml(5 hunks).circleci/src/workflows/daily.yml(2 hunks).circleci/src/workflows/merged.yml(1 hunks).circleci/src/workflows/normal.yml(1 hunks)code/core/src/common/js-package-manager/BUNProxy.ts(2 hunks)code/core/src/common/js-package-manager/JsPackageManager.ts(1 hunks)code/core/src/common/js-package-manager/JsPackageManagerFactory.test.ts(5 hunks)code/core/src/common/js-package-manager/JsPackageManagerFactory.ts(5 hunks)code/core/src/common/js-package-manager/NPMProxy.ts(2 hunks)code/core/src/common/js-package-manager/PNPMProxy.ts(2 hunks)code/core/src/common/js-package-manager/Yarn1Proxy.ts(2 hunks)code/core/src/common/js-package-manager/Yarn2Proxy.ts(2 hunks)code/core/src/csf-tools/ConfigFile.ts(1 hunks)code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts(0 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)code/lib/create-storybook/src/bin/run.ts(3 hunks)code/lib/create-storybook/src/commands/UserPreferencesCommand.ts(2 hunks)code/lib/create-storybook/src/scaffold-new-project.ts(1 hunks)scripts/tasks/sandbox-parts.ts(1 hunks)scripts/tasks/sandbox.ts(1 hunks)scripts/utils/yarn.ts(1 hunks)
💤 Files with no reviewable changes (1)
- code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/core/src/common/js-package-manager/Yarn1Proxy.tscode/core/src/common/js-package-manager/Yarn2Proxy.ts.circleci/config.ymlscripts/utils/yarn.tscode/core/src/common/js-package-manager/JsPackageManager.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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:
scripts/tasks/sandbox-parts.tscode/core/src/csf-tools/ConfigFile.ts
📚 Learning: 2025-11-05T09:38:47.712Z
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.712Z
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.
Applied to files:
code/lib/create-storybook/src/commands/UserPreferencesCommand.tscode/core/src/csf-tools/ConfigFile.ts.circleci/config.yml
📚 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/lib/create-storybook/src/commands/UserPreferencesCommand.ts
📚 Learning: 2025-10-01T15:24:01.060Z
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.
Applied to files:
code/core/src/csf-tools/ConfigFile.ts
📚 Learning: 2025-09-24T13:04:58.631Z
Learnt from: cylewaitforit
Repo: storybookjs/storybook PR: 31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.631Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Applied to files:
code/core/src/csf-tools/ConfigFile.ts
🧬 Code graph analysis (2)
code/lib/create-storybook/src/bin/run.ts (1)
code/core/src/manager/settings/shortcuts.tsx (1)
Feature(141-141)
code/core/src/common/js-package-manager/JsPackageManagerFactory.test.ts (5)
code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (1)
JsPackageManagerFactory(30-202)code/core/src/common/js-package-manager/NPMProxy.ts (1)
NPMProxy(70-299)code/core/src/common/js-package-manager/PNPMProxy.ts (1)
PNPMProxy(40-296)code/core/src/common/js-package-manager/Yarn1Proxy.ts (1)
Yarn1Proxy(37-242)code/core/src/common/js-package-manager/Yarn2Proxy.ts (1)
Yarn2Proxy(81-342)
⏰ 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: merged
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (31)
.circleci/src/workflows/merged.yml (1)
37-37: Parallelism increases appear well-coordinated across workflows.The incremental increases (+2 for each job) align with the broader CI expansion for RSBuild sandbox support. The +10-14% scale per job is reasonable for handling additional template variants.
Also applies to: 41-41, 49-49, 53-53
.circleci/src/workflows/normal.yml (1)
37-37: Consistent parallelism scaling in normal workflow tier.The +1 increment per job maintains proportional scaling across the three workflow tiers. This lightweight increase is appropriate for the standard PR check workflow.
Also applies to: 41-41, 49-49, 53-53
.circleci/src/workflows/daily.yml (1)
34-34: Daily workflow parallelism scaling is appropriately aggressive.The +4 increment per job reflects the comprehensive nature of daily testing. This maintains proper proportion relative to the merged (+2) and normal (+1) tiers.
Also applies to: 45-45, 53-53, 57-57
.circleci/config.yml (2)
928-929: Verify that executor class reduction tosmallis intentional and won't impact test reliability.The test-init-features job was downgraded from
medium+tosmall(approximately 50% resource reduction). This job runs Verdaccio registry, installs Storybook dependencies, and executes vitest. Verify that:
- The workload does not exceed small class resource limits (memory, CPU)
- Test execution time is acceptable
- There's no regression in test reliability or flakiness
If the downgrade was inadvertent or causes issues, consider reverting to
medium+or testing with the current resource limits first.
953-953: Command update looks correct but verify feature removal is intentional.The command was updated to use
--features docs test a11y --loglevel=debug, removing the priordevfeature. Per the AI summary, this change removes dev feature while retaining docs, test, and a11y features. Confirm that:
- Removing the
devfeature is intentional (not a regression)- The retained features (docs, test, a11y) are sufficient for the test-init-features job
- The
--loglevel=debugflag was added to improve observability during test executioncode/lib/create-storybook/src/commands/UserPreferencesCommand.ts (1)
10-10: LGTM!The import is necessary for the dedent usage in the warning message below and is correctly structured.
scripts/utils/yarn.ts (1)
141-164: Update inline comment to explain RSBuild template handling.The comment on line 143 only references React prerelease peer dependency issues, but RSBuild templates added at lines 147-150 are production templates (
default-ts), not prereleases. This mismatch makes the reasoning unclear for future maintainers.RSBuild templates are being treated identically to prerelease templates (discarding YN0060
INCOMPATIBLE_PEER_DEPENDENCYerrors), but there's no inline documentation explaining why. Please clarify either:
- If RSBuild has known peer dependency issues that require this treatment, update the comment to document that reason
- If this was applied speculatively, verify it's necessary before merging
Example updated comment:
- // React prereleases will have INCOMPATIBLE_PEER_DEPENDENCY errors because of transitive dependencies not allowing v19 betas + // React prereleases and RSBuild templates have peer dependency issues that need to be ignored during Verdaccio testingcode/lib/create-storybook/src/scaffold-new-project.ts (1)
5-5: LGTM! Import consolidation is clean.Adding
loggerto the existing import from 'storybook/internal/node-logger' properly supports the logger usage at lines 141 and 144.code/core/src/common/js-package-manager/PNPMProxy.ts (1)
15-15: LGTM! Consistent enum-based typing.The import of
PackageManagerNameand its usage in thetypefield correctly migrates from string literal to enum-based typing, maintaining runtime compatibility while improving type safety.Also applies to: 41-41
code/core/src/common/js-package-manager/Yarn1Proxy.ts (1)
15-15: LGTM! Consistent enum-based typing.The changes correctly adopt the
PackageManagerNameenum for type identification, consistent with the broader refactoring across all package manager proxies.Also applies to: 38-38
code/core/src/common/js-package-manager/JsPackageManagerFactory.test.ts (1)
8-8: LGTM! Test updates match the enum-based API changes.All test cases correctly updated to use
PackageManagerNameenum constants instead of string literals when specifying theforceoption. This ensures type safety and consistency with the refactored API.Also applies to: 34-36, 87-89, 177-179, 305-307
code/core/src/common/js-package-manager/BUNProxy.ts (1)
15-15: LGTM! Consistent enum-based typing.The changes correctly migrate BUNProxy to use the
PackageManagerNameenum, maintaining consistency with all other package manager proxy implementations.Also applies to: 70-70
code/core/src/common/js-package-manager/JsPackageManager.ts (1)
22-28: LGTM! Well-structured enum migration.The introduction of the
PackageManagerNameenum properly replaces the previous string union type while maintaining runtime compatibility. All enum values correctly map to their expected string representations, and the abstracttypeproperty appropriately uses the enum type.Also applies to: 70-70
code/core/src/common/js-package-manager/Yarn2Proxy.ts (1)
18-18: LGTM! Consistent enum-based typing.Yarn2Proxy correctly adopts the
PackageManagerNameenum, completing the consistent migration across all package manager proxy implementations.Also applies to: 82-82
code/core/src/common/js-package-manager/NPMProxy.ts (1)
16-16: LGTM! Consistent enum-based typing.NPMProxy correctly adopts the
PackageManagerNameenum, maintaining consistency with all other package manager proxy implementations in this refactoring.Also applies to: 71-71
code/core/src/csf-tools/ConfigFile.ts (1)
1177-1192: No actionable issues found—the broadened import detection is safe and intentional.The change from
startsWith('@storybook')toincludes('storybook')is secure because the function requires two conditions to be true simultaneously:
- Import path contains the substring
'storybook'- AND the imported specifier is named
'definePreview'The second condition effectively constrains the matching. Since
definePreviewis a specific Storybook API (not a common export name), packages with "storybook" in their path would need to explicitly export this identifier to trigger a match. The expanded matching now supports:
- The non-scoped
'storybook'package itself- Third-party or custom implementations that provide
definePreview- Monorepo internal packages with 'storybook' in their name
This is intentional and aligns with the CSF4 factory pattern design. False positives are minimal and unlikely in practice.
code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (3)
8-10: LGTM: Import changes correctly reflect enum refactor.The separation of
JsPackageManageras a type-only import andPackageManagerNameas a value import is appropriate for the enum-based refactor.
91-120: LGTM: Enum-based returns improve type safety.All return statements correctly use
PackageManagerNameenum members, improving type safety over the previous string literal approach.
179-201: LGTM: Package manager inference correctly updated.The method correctly returns
PackageManagerNameenum members with proper undefined handling, maintaining the original logic while improving type safety.code/lib/create-storybook/src/bin/run.ts (5)
51-60: Good filtering of internal ProjectType values.Filtering out
UNDETECTED,NX, andUNSUPPORTEDis appropriate, as these represent internal states that shouldn't be user-selectable options.
28-89: Excellent refactor to enforce input validation at the CLI layer.The consistent use of
addOption()with explicit choices enforces validation before the application logic runs, preventing invalid values from propagating into the codebase. This is a significant improvement in robustness and user experience.
80-89: No action required — loglevel choices are correct.The CLI choices ('trace', 'debug', 'info', 'warn', 'error', 'silent') match the logger's supported
LogLeveltype exactly as defined incode/core/src/node-logger/logger/logger.ts. The logger will accept all provided values without issues.
42-50: No issues found with parser choices.The parser choices hardcoded in the code (
babel,babylon,flow,ts,tsx) match exactly what jscodeshift supports as built-in parser identifiers. The list is complete and accurate.
1-7: ****All four imports (
Feature,PackageManagerName,ProjectType,SupportedBuilder) are confirmed runtime enums, not TypeScript type aliases. Each is defined asexport enum, which compiles to runtime JavaScript objects.Object.values()works correctly on enums at runtime. The concern raised is unfounded.Likely an incorrect or invalid review comment.
code/lib/cli-storybook/src/sandbox-templates.ts (6)
83-83: LGTM! Clean addition of skipMocking flag.The new optional flag provides explicit control over mocking behavior, replacing the previous name-based heuristic.
105-105: LGTM! Type-safe extension for RSBuild.The BaseTemplates type now correctly includes RSBuild as a builder option, maintaining type safety for template names.
536-549: LGTM! Consistent RSBuild template definitions.All RSBuild templates follow a consistent pattern with appropriate configuration:
- All use
skipMocking: trueanduseCsfFactory: true- Appropriate task skipping for new templates
- Consistent use of RSBuild CLI with TypeScript templates
Also applies to: 582-595, 674-687
866-866: LGTM! Proper migration to explicit skipMocking flag.All bench templates now explicitly set
skipMocking: true, replacing the previous name-based check. This improves clarity and maintainability.Also applies to: 884-884, 902-902, 921-921, 939-939
975-975: LGTM! Appropriate cadence distribution for new RSBuild templates.The RSBuild templates are distributed across cadences with a sensible rollout strategy:
- React in
normal(most common framework)- Vue in
merged(broader coverage)- HTML/Lit in
daily(less common use cases)Also applies to: 986-986, 1009-1010
481-495: ****The non-scoped package names are correct and intentional. The repository (rspack-contrib/storybook-rsbuild) contains the Storybook Rsbuild builder and UI framework integrations, and framework packages such as storybook-react-rsbuild and storybook-vue3-rsbuild include storybook-builder-rsbuild for you. These are official packages registered in the Storybook integrations ecosystem, using non-scoped naming by design—distinct from the @storybook/* scoped packages for other builders.
scripts/tasks/sandbox-parts.ts (1)
836-839: LGTM! Clean refactor to use explicit skipMocking flag.The function now checks the
skipMockingflag instead of searching for 'Bench' in the template name. This is more maintainable and aligns with the explicit configuration approach.
| const packageManager = new this.PROXY_MAP[force]({ | ||
| cwd, | ||
| configDir, | ||
| storiesPaths, | ||
| }); | ||
| this.cache.set(cacheKey, packageManager as unknown as JsPackageManager); | ||
| return packageManager as unknown as JsPackageManager; | ||
| } | ||
|
|
||
| // Option 2: Detect package managers based on some heuristics | ||
| const packageManagerType = this.getPackageManagerType(cwd); | ||
| const packageManager = new this.PROXY_MAP[packageManagerType]({ cwd, configDir, storiesPaths }); | ||
| this.cache.set(cacheKey, packageManager); | ||
| return packageManager; | ||
| this.cache.set(cacheKey, packageManager as unknown as JsPackageManager); | ||
| return packageManager as unknown as JsPackageManager; | ||
| } |
There was a problem hiding this comment.
Address type incompatibility instead of double-casting.
The double cast pattern as unknown as JsPackageManager (lines 154-155, 161-162) is a code smell that suppresses TypeScript's type checking and could hide real type errors.
This suggests the proxy classes may not properly implement or extend JsPackageManager. Consider:
- Ensuring all proxy classes implement a common
JsPackageManagerinterface or extend a base class - Defining
PackageManagerProxyto include the constructor signature that returnsJsPackageManager - Using a generic factory helper to avoid the casting
Example approach:
-type PackageManagerProxy =
- | typeof NPMProxy
- | typeof PNPMProxy
- | typeof Yarn1Proxy
- | typeof Yarn2Proxy
- | typeof BUNProxy;
+type PackageManagerProxy = new (config: {
+ cwd: string;
+ configDir: string;
+ storiesPaths?: string[];
+}) => JsPackageManager;Then the casts would be unnecessary:
- const packageManager = new this.PROXY_MAP[force]({
- cwd,
- configDir,
- storiesPaths,
- });
- this.cache.set(cacheKey, packageManager as unknown as JsPackageManager);
- return packageManager as unknown as JsPackageManager;
+ const packageManager = new this.PROXY_MAP[force]({ cwd, configDir, storiesPaths });
+ this.cache.set(cacheKey, packageManager);
+ return packageManager;Committable suggestion skipped: line range outside the PR's diff.
This reverts commit ca8270b.
valentinpalkovic
force-pushed
the
valentin/add-rsbuild-sandboxes
branch
from
e783ba4 to
f280b95
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
code/lib/cli-storybook/src/sandbox-templates.ts (3)
83-83: Consider documenting theskipMockingflag.The new
skipMockingflag lacks inline documentation explaining its purpose and when it should be used. Adding a JSDoc comment would help maintainers understand when to set this flag for new templates.Example documentation:
modifications?: { skipTemplateStories?: boolean; + /** Whether to skip mocking setup during template initialization (typically used for RSBuild templates) */ skipMocking?: boolean;
674-687: Consider naming consistency for the Lit/Web Components template.The template key is
lit-rsbuild/default-tsbut the display name is "Web Components Latest (RSBuild | TypeScript)". Other Lit templates in the codebase use "Lit Latest" in their names (see lines 651, 663). Consider using "Lit Latest (RSBuild | TypeScript)" for consistency, unless the distinction is intentional.
481-687: Consider extracting common RSBuild template configuration.All four RSBuild templates share identical
modificationsandskipTasksconfiguration:
useCsfFactory: trueskipMocking: trueskipTasks: ['e2e-tests', 'bench', 'vitest-integration']While the current explicit approach is clear, you could reduce duplication by extracting common configuration:
const rsbuildCommonConfig = { modifications: { useCsfFactory: true, skipMocking: true, }, skipTasks: ['e2e-tests', 'bench', 'vitest-integration'] as SkippableTask[], };Then use spread syntax:
'react-rsbuild/default-ts': { ...rsbuildCommonConfig, name: 'React Latest (RSBuild | TypeScript)', // ... other unique properties }However, this is optional—the current explicit approach may be preferable for clarity and independent modification of each template.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.circleci/config.yml(5 hunks).circleci/src/workflows/daily.yml(2 hunks).circleci/src/workflows/merged.yml(1 hunks).circleci/src/workflows/normal.yml(1 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .circleci/src/workflows/normal.yml
- .circleci/src/workflows/daily.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
.circleci/config.yml
📚 Learning: 2025-11-05T09:38:47.712Z
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.712Z
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.
Applied to files:
.circleci/config.yml
⏰ 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: Core Unit Tests, windows-latest
- GitHub Check: merged
🔇 Additional comments (10)
.circleci/config.yml (5)
1174-1200: Daily workflow parallelism increases are proportional.Parallelism increments for the daily workflow (+1 worker per job) are smaller than merged workflow adjustments, consistent with the cadence-based sandbox selection strategy.
1288-1310: Merged workflow parallelism increases match workflow file.The parallelism increases in the
mergedworkflow section align with the changes in.circleci/src/workflows/merged.yml.
1383-1406: Normal workflow uses lower parallelism tier appropriately.The
normalworkflow applies proportionally lower parallelism increases (+1 per job vs. +2 in merged workflow), consistent with the tiered scaling strategy across workflows.
926-929: Resource class downgrade has been implemented intentionally; confirm job reliability before closure.The executor class change from
medium+tosmallis confirmed and appears deliberate (part of "streamlined setup" per commit message). However, since the job runs Storybook initialization, npm package installation, and vitest, verify that these steps complete reliably under reduced resources. Check CircleCI run logs or re-run the job to confirm consistent performance before considering this optimization complete.
947-959: Command and features verified as correct.All three features (
docs,test,a11y) are valid, supported, and properly implemented:
create-storybookCLI is exported and available--featuresflag is defined and operational- Feature enum values (Feature.DOCS, Feature.TEST, Feature.A11Y) exist and are used throughout addon configuration and generator execution
The circular config command will execute as expected.
.circleci/src/workflows/merged.yml (1)
36-56: Correct template count: 4 RSBuild templates enabled (not 8).The git diff shows 4 RSBuild templates removed
inDevelopment: trueand added to workflows:
react-rsbuild/default-ts→normalcadencevue3-rsbuild/default-ts→mergedcadencehtml-rsbuild/default-tsandlit-rsbuild/default-ts→dailycadenceThe uniform +2 parallelism increases across all four jobs (despite only 1 template added to merged) suggests the parallelism adjustments may address template distribution across multiple cadences or other load factors. Verify that the parallelism values align with actual template counts in each cadence to ensure proper resource allocation.
code/lib/cli-storybook/src/sandbox-templates.ts (4)
105-108: LGTM!The addition of 'RSBuild' to the builder type follows the established pattern and correctly extends the template naming convention.
866-866: LGTM!The addition of
skipMocking: trueto all bench templates is consistent with the new RSBuild templates and ensures uniform behavior across benchmark configurations.Also applies to: 884-884, 902-902, 921-921, 939-939
975-975: LGTM!The distribution of RSBuild templates across different cadences (normal, merged, daily) provides a reasonable graduated testing strategy, with the most common framework (React) receiving the most frequent testing.
Also applies to: 986-986, 1009-1010
481-495: No issues found—review comment is based on incorrect assumptions.The package names (
storybook-react-rsbuild,storybook-builder-rsbuild) are intentional and correct. They are explicitly documented as community packages (outside the monorepo) incode/core/src/common/utils/get-storybook-info.tsand consistently used across the codebase in framework and builder mappings.Regarding
typeCheck: Most TypeScript templates in the codebase omit this setting. Thereact-vite/default-tstemplate at line 377 is the exception withtypeCheck: true, not the baseline. RSBuild templates follow the standard pattern of other frameworks. RSBuild is included in the "normal" export list and is not marked as experimental, indicating production readiness.The code is correct as written.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/lib/cli-storybook/src/sandbox-templates.ts (2)
84-93: Document the newskipMockingflag inmodificationsAdding
skipMocking?: booleanis a clean, backward‑compatible extension ofTemplate.modifications. To keep this self‑documenting, consider adding a short JSDoc note describing what is being mocked and how CI is expected to interpret this flag.Example:
modifications?: { skipTemplateStories?: boolean; + /** Skip applying CLI mocking / sandbox fixtures for this template. */ skipMocking?: boolean;
108-112: RSBuild added toBaseTemplates.nametype; keep the comment above in syncThe update to include
'RSBuild'in the builder union for thenametemplate literal keeps type‑safety aligned with the new RSBuild templates.The inline example above still only mentions “Webpack” and “Vite”; you may want to reflect RSBuild there too:
- * (<"Webpack"|"Vite"> | <"JavaScript"|"TypeScript">) + * (<"Webpack"|"Vite"|"RSBuild"> | <"JavaScript"|"TypeScript">)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.circleci/config.yml(4 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)scripts/tasks/sandbox-parts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/tasks/sandbox-parts.ts
- .circleci/config.yml
⏰ 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: merged
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/lib/cli-storybook/src/sandbox-templates.ts (3)
485-499: RSBuild base templates look consistent; verify framework/builder identifiersThe four new RSBuild templates (React, Vue 3, HTML, Web Components):
- Use the expected
namepattern ((... (RSBuild | TypeScript))), matching the updatedBaseTemplatestype.- Follow existing patterns for scripts,
expected.renderer, andskipTasks.- Use
modificationswithuseCsfFactory: trueandskipMocking: true, which fits the new flag design.One thing to double‑check: the
expected.frameworkandexpected.builderstrings (storybook-*-rsbuild,storybook-builder-rsbuild) must match whatever the init command actually writes intomain.ts/main.jsand the builder name used in config. A mismatch will cause the assertions that compareexpectedvs. discovered values to fail.I’d recommend validating one of these locally/CI by generating a sandbox with each RSBuild template and confirming the framework and builder detected by the assertion logic match these strings.
Also applies to: 540-553, 592-605, 684-697
879-887: Bench templates opting out of mocking explicitlySetting
skipMocking: truein the bench templates’modificationsaligns them with the new flag and makes the behavior explicit for performance‑sensitive runs. This is consistent across the React Vite/Webpack and nodocs/test‑build variants.Given these templates already override
modificationsinstead of extending the base template’smodifications, the change is localized and shouldn’t affect other behavior.Also applies to: 897-905, 914-922, 933-941, 951-959
976-995: Cadence arrays now exercise RSBuild templates at appropriate frequenciesAdding:
'react-rsbuild/default-ts'tonormal,'vue3-rsbuild/default-ts'tomerged, and'lit-rsbuild/default-ts'and'html-rsbuild/default-ts'todailyfits the existing pattern of gradually widening coverage for newer stacks. The keys line up with the new
baseTemplatesentries andTemplateKeytyping viaallTemplates.As long as CI capacity was accounted for (which it looks like from the PR description), this distribution looks good.
Also applies to: 997-1006, 1008-1031
…alentin/add-rsbuild-sandboxes
valentinpalkovic
changed the title
valentinpalkovic
added
ci:daily
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/lib/cli-storybook/src/sandbox-templates.ts (1)
84-93: Document the newskipMockingflag for future maintainersThe new
modifications.skipMocking?: booleanis central to controlling mocking behavior (seeextendPreview), but it’s currently undocumented in the type. Adding a brief JSDoc-style comment explaining that it disablesstorybook/testmocks injection for that template would make this contract clearer and easier to reuse correctly.Also applies to: 108-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)scripts/tasks/sandbox-parts.ts(1 hunks)scripts/utils/yarn.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/utils/yarn.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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:
scripts/tasks/sandbox-parts.ts
⏰ 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: merged
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
scripts/tasks/sandbox-parts.ts (1)
819-842: skipMocking guard correctly externalizes mocking behaviorSwitching from a
template.name.includes('Bench')check totemplate.modifications?.skipMockingmakes mocking opt‑out explicit and reusable. The earlywriteConfig(previewConfig)ensures any prior preview edits (CSF factory imports, tags, etc.) are still persisted for templates that skip mocking.This looks correct; just ensure all templates that relied on the old bench‑name heuristic (and any RSBuild sandboxes that shouldn’t mock) now set
skipMocking: truein theirmodifications, which appears to be the case insandbox-templates.ts.code/lib/cli-storybook/src/sandbox-templates.ts (3)
881-971: Verification confirms changes are correctAll five bench templates properly declare
modifications.skipMocking: true, andsandbox-parts.tscorrectly gates mocking setup on this flag (line 839) with an early return. The implementation is consistently data-driven and allows non-bench templates to opt out independently.
979-999: Cadence placement and CI infrastructure confirmed; verify stability expectationsRSBuild sandbox placement and CircleCI parallelism are correctly configured:
react-rsbuild/default-tsplaced innormalcadence (line 997)vue3-rsbuild/default-tsplaced inmergedcadence (line 1008)lit-rsbuild/default-tsandhtml-rsbuild/default-tsplaced indailycadence (lines 1031-1032)CircleCI parallelism is set across workflows:
normal: 14,merged: 21,daily: 38(.circleci/src/workflows/). Yarn infrastructure already includes explicit handling for all four RSBuild variants (scripts/utils/yarn.ts lines 151-154).Confirm this cadence split meets your RSBuild stability expectations (React covered most frequently, Vue/HTML moderately, Lit/HTML only in
daily).
485-499: Fix template key mismatches inscripts/utils/yarn.tsand confirm RSBuild framework story linking behaviorBoth concerns are confirmed:
Template key mismatches in
scripts/utils/yarn.ts(lines 152, 154):
- Line 152:
'vue-rsbuild/default-ts'should be'vue3-rsbuild/default-ts'(matchessandbox-templates.ts:540)- Line 154:
'web-components-rsbuild/default-ts'should be'lit-rsbuild/default-ts'(matchessandbox-templates.ts:686)- Lines 151 and 153 are correct.
RSBuild frameworks excluded from
isCoreFrameworkcheck:
All RSBuild templates useexpected.frameworkvalues like'storybook-react-rsbuild'(not prefixed with@storybook/). TheisCoreFrameworkcheck inscripts/tasks/sandbox-parts.ts:652requires the@storybook/prefix, so RSBuild frameworks will not get framework-level template stories linked. Verify this is intentional or adjust framework naming/logic accordingly.⛔ Skipped due to 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.712Z 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: JReinhold Repo: storybookjs/storybook PR: 32607 File: code/package.json:243-243 Timestamp: 2025-10-02T09:22:13.215Z Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
|
|
||
| type BaseTemplates = Template & { | ||
| name: `${string} ${`v${number}` | 'Latest' | 'Prerelease'} (${'Webpack' | 'Vite'} | ${ | ||
| name: `${string} ${`v${number}` | 'Latest' | 'Prerelease'} (${'Webpack' | 'Vite' | 'RSBuild'} | ${ |
There was a problem hiding this comment.
by the way, it's Rsbuild not RSBuild 😆
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/tasks/sandbox-parts.ts (1)
727-737: Consider using a template property instead of hardcoding framework names.The current approach hardcodes three RSBUILD framework names, which requires code changes whenever new RSBUILD frameworks are added. Consider one of these alternatives:
- Add a template property like
template.excludeDocsAddonand check that flag- Create a constant array of RSBUILD frameworks at the module level
- Use pattern matching like
template.expected.framework.includes('-rsbuild')Example with a constant array:
+const RSBUILD_FRAMEWORKS_EXCLUDE_DOCS = [ + 'storybook-vue3-rsbuild', + 'storybook-web-components-rsbuild', + 'storybook-html-rsbuild', +]; + export const addStories: Task['run'] = async ( { sandboxDir, template, key }, { addon: extraAddons, disableDocs }.filter((addon: string) => { // RSBUILD frameworks are not configured to ignore docs addon stories, which are React based - if ( - template.expected.framework === 'storybook-vue3-rsbuild' || - template.expected.framework === 'storybook-web-components-rsbuild' || - template.expected.framework === 'storybook-html-rsbuild' - ) { + if (RSBUILD_FRAMEWORKS_EXCLUDE_DOCS.includes(template.expected.framework)) { return addon !== 'docs'; } return true; })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/tasks/sandbox-parts.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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:
scripts/tasks/sandbox-parts.ts
📚 Learning: 2025-11-05T09:38:47.712Z
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.712Z
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.
Applied to files:
scripts/tasks/sandbox-parts.ts
📚 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:
scripts/tasks/sandbox-parts.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:
scripts/tasks/sandbox-parts.ts
⏰ 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: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
scripts/tasks/sandbox-parts.ts (1)
850-853: Good improvement: semantic flag over name-based checks.Replacing the name-based
template.name.includes('Bench')check withtemplate.modifications?.skipMockingis a clear improvement in maintainability and intent. The semantic flag makes the purpose explicit and is less brittle than string matching.
…ramework metadata retrieval
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
code/lib/cli-storybook/src/sandbox-templates.ts (5)
109-109: Fix casing: use 'Rsbuild' instead of 'RSBuild'.The type union should use
'Rsbuild'for consistency with the official brand name. This issue was previously flagged in past review comments.Apply this diff:
- name: `${string} ${`v${number}` | 'Latest' | 'Prerelease'} (${'Webpack' | 'Vite' | 'RSBuild'} | ${ + name: `${string} ${`v${number}` | 'Latest' | 'Prerelease'} (${'Webpack' | 'Vite' | 'Rsbuild'} | ${
486-486: Fix casing: use 'Rsbuild' instead of 'RSBuild'.The template name should use
'Rsbuild'for consistency with the official brand name. This issue was previously flagged.Apply this diff:
- name: 'React Latest (RSBuild | TypeScript)', + name: 'React Latest (Rsbuild | TypeScript)',
540-540: Fix casing: use 'Rsbuild' instead of 'RSBuild'.The template name should use
'Rsbuild'for consistency with the official brand name.Apply this diff:
- name: 'Vue Latest (RSBuild | TypeScript)', + name: 'Vue Latest (Rsbuild | TypeScript)',
592-592: Fix casing: use 'Rsbuild' instead of 'RSBuild'.The template name should use
'Rsbuild'for consistency with the official brand name.Apply this diff:
- name: 'HTML Latest (RSBuild | TypeScript)', + name: 'HTML Latest (Rsbuild | TypeScript)',
691-691: Fix casing and consider naming consistency: use 'Rsbuild' instead of 'RSBuild'.The template name should use
'Rsbuild'for consistency with the official brand name.Additionally, this template uses "Web Components Latest" while other Lit templates (lines 667, 679) use "Lit Latest" in their names. Consider whether this naming difference is intentional or if it should be aligned for consistency.
Apply this diff for the casing fix:
- name: 'Web Components Latest (RSBuild | TypeScript)', + name: 'Web Components Latest (Rsbuild | TypeScript)',If you want to align with other Lit template naming:
- name: 'Web Components Latest (Rsbuild | TypeScript)', + name: 'Lit Latest (Rsbuild | TypeScript)',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)
⏰ 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: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/lib/cli-storybook/src/sandbox-templates.ts (3)
489-491: Verify the framework/builder package names are correct.The expected framework and builder use community package names (
storybook-react-rsbuild,storybook-builder-rsbuild) without the@storybook/prefix. Ensure these match the actual package names published by the Rsbuild community.
891-891: LGTM! Bench templates correctly use skipMocking flag.The addition of
skipMocking: trueto bench templates aligns with the architectural change to control mocking via the template flag rather than template name-based checks. This provides a cleaner, more explicit approach.Also applies to: 909-909, 927-927, 946-946, 964-964
1000-1000: LGTM! RSBuild templates appropriately distributed across CI cadences.The distribution of RSBuild templates across cadences is reasonable:
react-rsbuild/default-tsin normal (most frequent testing)vue3-rsbuild/default-tsin merged (regular testing)lit-rsbuild/default-tsandhtml-rsbuild/default-tsin daily (comprehensive coverage)This provides good coverage while balancing CI resource usage.
Also applies to: 1011-1011, 1034-1035
| builder: 'storybook-builder-rsbuild', | ||
| }, | ||
| modifications: { | ||
| extraDependencies: ['prop-types', 'storybook-react-rsbuild@^3.0.0-beta.1'], |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the beta version constraint and availability.
The template depends on storybook-react-rsbuild@^3.0.0-beta.1. Ensure this version exists and is stable enough for sandbox usage.
🌐 Web query:
Does storybook-react-rsbuild version 3.0.0-beta.1 or higher exist on npm?
💡 Result:
No — there is no storybook-react-rsbuild package release 3.0.0-beta.1 (or any 3.x) on npm as of today (Nov 20, 2025). Current published versions are in the 2.x range (latest ~2.1.6). [1][2]
Sources:
- npm package page for storybook-react-rsbuild. [1]
- jsDelivr / package index showing latest 2.x version. [2]
Update storybook-react-rsbuild to an available version.
The specified version 3.0.0-beta.1 does not exist on npm; the latest published version is in the 2.x range (~2.1.6). Update line 494 to use an available version constraint, such as ^2.1.0.
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/sandbox-templates.ts around line 494, the
extraDependencies entry uses the non-existent version
"storybook-react-rsbuild@^3.0.0-beta.1"; change it to a published 2.x constraint
such as "storybook-react-rsbuild@^2.1.0" (or ^2.1.6) so npm can resolve the
package; update that string only and keep surrounding formatting intact.
| builder: 'storybook-builder-rsbuild', | ||
| }, | ||
| modifications: { | ||
| extraDependencies: ['storybook-html-rsbuild@^3.0.0-beta.1'], |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the beta version constraint and availability.
Ensure storybook-html-rsbuild@^3.0.0-beta.1 exists and is stable enough for sandbox usage.
🌐 Web query:
Does storybook-html-rsbuild version 3.0.0-beta.1 or higher exist on npm?
💡 Result:
No. There is no storybook-html-rsbuild 3.0.0-beta.1 (or any 3.x) published on npm as of Nov 20, 2025 — the package is published at v2.x (e.g. 2.1.0). [1][2]
Sources:
- storybook-html-rsbuild on Libraries.io (shows v2.1.0). [1]
- Storybook Rsbuild docs (framework table showing storybook-html-rsbuild ≈ ^2.0.0 for Storybook v9). [2]
Update storybook-html-rsbuild to an available version.
The specified version 3.0.0-beta.1 does not exist on npm. The latest published version is in the 2.x range (e.g., 2.1.0), and the Storybook Rsbuild docs show ^2.0.0 for Storybook v9. Update line 600 to use storybook-html-rsbuild@^2.0.0 or ^2.1.0.
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/sandbox-templates.ts around line 600, the
extraDependencies entry pins storybook-html-rsbuild to a non-existent version
(3.0.0-beta.1); update that entry to a published 2.x range such as
storybook-html-rsbuild@^2.0.0 or storybook-html-rsbuild@^2.1.0 so the dependency
resolves correctly (replace the version string accordingly).
| builder: 'storybook-builder-rsbuild', | ||
| }, | ||
| modifications: { | ||
| extraDependencies: ['storybook-web-components-rsbuild@^3.0.0-beta.1'], |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the beta version constraint and availability.
Ensure storybook-web-components-rsbuild@^3.0.0-beta.1 exists and is stable enough for sandbox usage.
🌐 Web query:
Does storybook-web-components-rsbuild version 3.0.0-beta.1 or higher exist on npm?
💡 Result:
No — there is no 3.0.0-beta.1 (or higher) release on npm. The package's latest published version appears to be 2.0.1. [1][2]
Sources:
- Libraries.io package listing (shows storybook-web-components-rsbuild v2.0.1). [1]
- Storybook Rsbuild docs referencing the storybook-web-components-rsbuild package. [2]
Fix non-existent package version dependency.
The specified version storybook-web-components-rsbuild@^3.0.0-beta.1 does not exist on npm; the latest published version is 2.0.1. Update line 699 to use an available version (e.g., 2.0.1) or confirm that 3.0.0-beta.1 is planned for imminent release before merging.
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/sandbox-templates.ts around line 699, the
extraDependencies entry references a non-existent package version
"storybook-web-components-rsbuild@^3.0.0-beta.1"; update this to a published
version (for example "[email protected]") or replace with
the correct version string if you have confirmation of an imminent 3.0.0-beta.1
release, so the dependency resolves on npm.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/lib/cli-storybook/src/sandbox-templates.ts (1)
485-504: Rsbuild templates referencestorybook-*-rsbuild@^3.0.0-beta.1which may not be published.All four Rsbuild templates pin extra dependencies to
^3.0.0-beta.1:
- Line 494:
storybook-react-rsbuild@^3.0.0-beta.1- Line 554:
storybook-vue3-rsbuild@^3.0.0-beta.1- Line 612:
storybook-html-rsbuild@^3.0.0-beta.1- Line 717:
storybook-web-components-rsbuild@^3.0.0-beta.1Previous review already flagged these as non-existent on npm; leaving them as-is will cause sandbox installs to fail until those betas are actually published. Either:
- Update all four to a currently published 2.x range (e.g.
^2.x.y), or- Confirm that the 3.0.0-beta.1 family is now published and aligned with this PR’s Storybook version before merging.
Example diff (using placeholder versions you should replace with the actual latest 2.x line you want):
- extraDependencies: ['prop-types', 'storybook-react-rsbuild@^3.0.0-beta.1'], + extraDependencies: ['prop-types', 'storybook-react-rsbuild@^2.1.0'], @@ - extraDependencies: ['storybook-vue3-rsbuild@^3.0.0-beta.1'], + extraDependencies: ['storybook-vue3-rsbuild@^2.1.2'], @@ - extraDependencies: ['storybook-html-rsbuild@^3.0.0-beta.1'], + extraDependencies: ['storybook-html-rsbuild@^2.1.0'], @@ - extraDependencies: ['storybook-web-components-rsbuild@^3.0.0-beta.1'], + extraDependencies: ['storybook-web-components-rsbuild@^2.0.1'],Check whether the following packages and versions exist on npm and what the latest compatible 2.x versions are: - storybook-react-rsbuild@^3.0.0-beta.1 - storybook-vue3-rsbuild@^3.0.0-beta.1 - storybook-html-rsbuild@^3.0.0-beta.1 - storybook-web-components-rsbuild@^3.0.0-beta.1Also applies to: 545-564, 603-622, 708-727
🧹 Nitpick comments (1)
code/lib/cli-storybook/src/sandbox-templates.ts (1)
30-38: Name template updated for Rsbuild; doc comment is slightly stale.The
BaseTemplates['name']template now correctly includes'RsBuild'alongside'Webpack' | 'Vite', matching the new Rsbuild-based entries. The doc comment above still mentions only Webpack/Vite, so consider updating it to reflect Rsbuild as well for future readers.Also applies to: 108-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/lib/cli-storybook/src/sandbox-templates.ts(14 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-05T09:38:47.712Z
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.712Z
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.
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
⏰ 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: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/lib/cli-storybook/src/sandbox-templates.ts (3)
84-87:skipMockingflag wiring looks correct.Adding
skipMocking?: booleantoTemplate['modifications']keeps the type aligned with how sandboxes now decide whether to generate/apply mocks; no issues from a typing or shape perspective.
908-916: Bench templates opting out of mocking look intentional.Setting
skipMocking: trueacross all bench templates’modificationsis consistent with the new flag and makes sense for performance/behavior isolation in benchmark runs; structurally this is fine.Also applies to: 932-934, 948-952, 967-971, 985-989
1024-1025: Cadence placement for Rsbuild templates seems reasonable but worth confirming.The cadence tiers now exercise Rsbuild templates as:
normal:react-rsbuild/default-tsmerged: addsvue3-rsbuild/default-tsdaily: addslit-rsbuild/default-tsandhtml-rsbuild/default-tsThis gradient (React at higher cadence, Vue/Lit/HTML at lower) looks intentional, but please double‑check that this matches your desired CI coverage for each framework before merging.
Also applies to: 1035-1036, 1058-1059
- Introduced a new story for module mocking in `moduleMocking.stories.ts`. - Updated the e2e test to navigate to the correct story path for module mocking. - Enhanced the `addStoriesEntry` function to include a `skipMocking` parameter for better story management.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/tasks/sandbox-parts.ts (1)
754-764: Consider a more maintainable approach for filtering docs addon.The current implementation hard-codes three specific RSBUILD framework names. If more RSBUILD frameworks are added in the future, they would need to be added to this list manually.
Consider refactoring to check for RSBUILD frameworks more generically:
.filter((addon: string) => { - // RSBUILD frameworks are not configured to ignore docs addon stories, which are React based - if ( - template.expected.framework === 'storybook-vue3-rsbuild' || - template.expected.framework === 'storybook-web-components-rsbuild' || - template.expected.framework === 'storybook-html-rsbuild' - ) { - return addon !== 'docs'; - } - return true; + // RSBUILD frameworks are not configured to ignore docs addon stories, which are React based + const isNonReactRsbuild = + template.expected.framework.includes('-rsbuild') && + template.expected.renderer !== '@storybook/react'; + if (isNonReactRsbuild) { + return addon !== 'docs'; + } + return true; })This would automatically handle future RSBUILD frameworks without requiring code changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/e2e-tests/module-mocking.spec.ts(1 hunks)scripts/tasks/sandbox-parts.ts(11 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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:
scripts/tasks/sandbox-parts.tscode/e2e-tests/module-mocking.spec.ts
📚 Learning: 2025-11-05T09:38:47.712Z
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.712Z
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.
Applied to files:
scripts/tasks/sandbox-parts.tscode/e2e-tests/module-mocking.spec.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:
scripts/tasks/sandbox-parts.ts
📚 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:
scripts/tasks/sandbox-parts.ts
🧬 Code graph analysis (1)
scripts/tasks/sandbox-parts.ts (2)
code/core/src/csf-tools/ConfigFile.ts (1)
ConfigFile(140-1148)code/core/src/csf-tools/CsfFile.ts (1)
stories(968-970)
⏰ 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: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (5)
scripts/tasks/sandbox-parts.ts (4)
388-420: LGTM! Clean parameter threading.The
skipMockingparameter is properly threaded through the function signature and passed toaddStoriesEntry. The implementation is clean and consistent.
877-880: LGTM! More explicit control mechanism.Changing from
template.name.includes('Bench')totemplate.modifications?.skipMockingprovides clearer intent and is more maintainable than inferring behavior from naming conventions. The optional chaining properly handles cases wheremodificationsis undefined.
614-614: LGTM! Consistent skipMocking propagation.The
skipMockingflag is properly sourced fromtemplate.modifications?.skipMockingand consistently threaded through alllinkPackageStoriescalls throughout the file. The implementation is thorough and maintains consistency across all call sites.Also applies to: 653-653, 668-668, 687-687, 703-703, 717-717, 724-724, 771-771
364-378: Verification complete: extglob patterns are supported and the code is correct.Storybook uses picomatch (v2.3.0), which supports extglob patterns including the
!(*Mocking)*syntax by default. The pattern**/!(*Mocking)*will correctly exclude the 4 Mocking story files (NodeModuleMocking, ModuleSpyMocking, ModuleMocking, and ModuleAutoMocking) as intended. No changes needed.code/e2e-tests/module-mocking.spec.ts (1)
60-60: Story path change verified—no issues found.The story file exists at
code/core/template/stories/moduleMocking.stories.ts, and the Storybook configuration correctly derives the title ascore/moduleMockingusing the titlePrefix'core'and the filenamemoduleMocking. The exported story nameBasicnormalizes to'basic', matching the test's navigation call.
valentinpalkovic
force-pushed
the
valentin/add-rsbuild-sandboxes
branch
from
0cfcf8c to
b366e4b
Compare
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
Telemetry
Tests
✏️ Tip: You can customize this high-level summary in your review settings.