Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Nov 12, 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

  • Chores
    • Updated Storybook type system and configuration across stories.
    • Added a visual-testing delay for more consistent chromatic captures.
    • Improved error objects to include richer stack information for clearer diagnostics.
    • Adjusted test/story assertions to expect the updated error identifier.

@yannbf yannbf self-assigned this Nov 12, 2025
@yannbf yannbf added build Internal-facing build tooling & test updates ci:normal labels Nov 12, 2025
@nx-cloud
Copy link

nx-cloud bot commented Nov 12, 2025

View your CI Pipeline Execution ↗ for commit aef4ddf

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

☁️ Nx Cloud last updated this comment at 2025-11-15 16:24:18 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Caution

Review failed

The head commit changed during the review from e36c039 to aef4ddf.

📝 Walkthrough

Walkthrough

Adds a Chromatic delay, replaces direct throws with crafted Error objects and synthetic stacks, migrates Storybook typings from StoryAnnotations to Meta/StoryObj and simplifies decorator signatures, and updates an expected error name in a Storybook play assertion.

Changes

Cohort / File(s) Summary
Chromatic configuration
code/addons/docs/src/blocks/blocks/Stories.stories.tsx
Added chromatic: { delay: 2000 } to meta.parameters.
Custom error stack / error throw changes
code/addons/docs/template/stories/docspage/error.stories.ts, code/core/src/manager/components/sidebar/Refs.stories.tsx
Replaced direct throw new Error(...) with constructing an Error, assigning a synthetic/dedent-crafted stack, then throwing it; added/adjusted ESLint directives where present.
Storybook typing & default export refactor
code/core/src/manager/components/sidebar/Refs.stories.tsx
Migrated typing from StoryAnnotations to Meta/StoryObj; introduced meta constant with satisfies Meta<typeof Ref> and export default meta; re-typed story exports; simplified decorator parameter typings by removing explicit any.
Play assertion update
code/core/template/stories/mount-in-play.stories.ts
Changed expected error name in play assertion from MountMustBeDestructuredError to SB_PREVIEW_API_0012.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to synthetic stack assignment and formatting (dedent usage) in both error files — verify stacks are accurate for tooling and tests.
  • Verify TypeScript satisfies Meta<typeof Ref> and StoryObj usages compile and preserve story behavior.
  • Confirm decorator signature simplifications still allow proper type inference and runtime behavior.
  • Check the updated play assertion string for test compatibility with existing test harness.

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: 0

🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/Refs.stories.tsx (1)

299-331: Parameterize StoryObj to retain typing.

Right now these stories fall back to StoryObj<any>. Hooking them to typeof Ref keeps arg/globals validation intact without changing runtime.

Apply this diff to tighten the typing:

 export default meta;

+type Story = StoryObj<typeof Ref>;
+
 export const Optimized = () => (
   <Ref
@@
 );
-export const ErroredWithErrorOpen: StoryObj = {
+export const ErroredWithErrorOpen: Story = {
@@
-};
-export const ErroredMobileWithErrorOpen: StoryObj = {
+};
+export const ErroredMobileWithErrorOpen: Story = {
@@
-};
-export const ErroredWithIndicatorOpen: StoryObj = {
+};
+export const ErroredWithIndicatorOpen: Story = {
@@
-};
-export const ErroredMobileWithIndicatorOpen: StoryObj = {
+};
+export const ErroredMobileWithIndicatorOpen: Story = {
@@
 };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0e575e and 9cff188.

📒 Files selected for processing (3)
  • code/addons/docs/src/blocks/blocks/Stories.stories.tsx (1 hunks)
  • code/addons/docs/template/stories/docspage/error.stories.ts (2 hunks)
  • code/core/src/manager/components/sidebar/Refs.stories.tsx (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/addons/docs/src/blocks/blocks/Stories.stories.tsx
  • code/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
**/*.{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/addons/docs/src/blocks/blocks/Stories.stories.tsx
  • code/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
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/addons/docs/src/blocks/blocks/Stories.stories.tsx
  • code/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
{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/addons/docs/src/blocks/blocks/Stories.stories.tsx
  • code/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
🧠 Learnings (7)
📓 Common learnings
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
📚 Learning: 2025-11-05T09:36:55.944Z
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.944Z
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.

Applied to files:

  • code/addons/docs/template/stories/docspage/error.stories.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:

  • code/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
📚 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/addons/docs/template/stories/docspage/error.stories.ts
  • code/core/src/manager/components/sidebar/Refs.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/core/src/manager/components/sidebar/Refs.stories.tsx
📚 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/core/src/manager/components/sidebar/Refs.stories.tsx
📚 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/manager/components/sidebar/Refs.stories.tsx
🧬 Code graph analysis (1)
code/core/src/manager/components/sidebar/Refs.stories.tsx (3)
code/core/src/manager/components/sidebar/Refs.tsx (1)
  • Ref (75-170)
code/core/src/manager-api/root.tsx (1)
  • ManagerContext (79-79)
code/core/src/manager/components/sidebar/IconSymbols.tsx (1)
  • IconSymbols (28-109)
⏰ 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). (4)
  • GitHub Check: Danger JS
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
  • GitHub Check: get-parameters
🔇 Additional comments (5)
code/addons/docs/template/stories/docspage/error.stories.ts (1)

13-22: Deterministic stack for Chromatic snapshots.

Crafting the error once and seeding a fixed stack keeps the captured output stable between runs—nice alignment with our snapshot goals.

code/addons/docs/src/blocks/blocks/Stories.stories.tsx (1)

10-12: Chromatic delay matches flake mitigation.

Adding the 2 s wait should let the story settle before capture and cut down the intermittent failures we’ve been seeing.

code/core/src/manager/components/sidebar/Refs.stories.tsx (3)

1-8: Lint guard and utilities look aligned.

Importing Meta/StoryObj alongside dedent and silencing the local rule keeps this file consistent with the CSF3 pattern and avoids the false positive.


27-43: Typed meta definition looks good.

Switching to a named meta that satisfies Meta<typeof Ref> plus the decorator cleanup tightens type checking without touching runtime behavior.


56-65: Synthetic stack keeps snapshots stable.

Using dedent to seed the stack mirrors the manager UI expectation and removes the nondeterministic frames from this error state story.

@storybook-app-bot
Copy link

storybook-app-bot bot commented Nov 12, 2025

Package Benchmarks

Commit: aef4ddf, ran on 15 November 2025 at 16:12:10 UTC

No significant changes detected, all good. 👏

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cff188 and 15cd7cb.

📒 Files selected for processing (1)
  • code/core/template/stories/mount-in-play.stories.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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.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: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
📚 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/core/template/stories/mount-in-play.stories.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:

  • code/core/template/stories/mount-in-play.stories.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: normal
  • GitHub Check: Core Unit Tests, windows-latest

@yannbf yannbf merged commit 66f72e6 into next Nov 17, 2025
63 checks passed
@yannbf yannbf deleted the yann/reduce-flake-stories branch November 17, 2025 10:09
@github-actions github-actions bot mentioned this pull request Nov 18, 2025
17 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 20, 2025
8 tasks
@coderabbitai coderabbitai bot mentioned this pull request Dec 15, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants