Skip to content

Conversation

@Sidnioulz
Copy link
Member

@Sidnioulz Sidnioulz commented Dec 17, 2025

Closes #33352

What I did

Accounted for React elements passed to the render prop for addon tools. If the prop is not a function, I'm now using cloneElement instead of directly rendering it as a function. This fixes the bug as far as I can tell.

Checklist for Contributors

Testing

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

ø

Manual testing

Caution

I could not repro on our own SB instance so I ended up having to code a fix in the built SB manager in the repro repo's node_modules.

  1. Fork https://github.com/US-CBP/design-system/tree/develop/packages/web-components
  2. Run the SB instance in packages/web-components
  3. Go to any page, press F5 and notice the crash
  4. Replace the Storybook instance it contains with the code on this branch
  5. Rinse and repeat, it should no longer crash

Documentation

ø

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 pull request has been released as version 0.0.0-pr-33381-sha-f3410369. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-33381-sha-f3410369
Triggered by @yannbf
Repository storybookjs/storybook
Branch sidnioulz/issue-33352
Commit f3410369
Datetime Wed Dec 17 11:48:20 UTC 2025 (1765972100)
Workflow run 20301772924

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=33381

Summary by CodeRabbit

  • Bug Fixes
    • Fixed panel addon rendering so the active state is applied consistently across different render types.
    • Improved addon rendering support to handle both element and function renderers reliably, preventing potential hook-related issues.

✏️ Tip: You can customize this high-level summary in your review settings.

@Sidnioulz Sidnioulz added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Dec 17, 2025
@nx-cloud
Copy link

nx-cloud bot commented Dec 17, 2025

View your CI Pipeline Execution ↗ for commit 0e95f1e

Command Status Duration Result
nx run-many -t check --parallel=15 ❌ Failed 34s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-19 10:05:02 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

📝 Walkthrough

Walkthrough

Added a helper in Panel.tsx that normalizes addon render props so they can be React elements or functions; PreRenderAddons now uses this helper to ensure the active prop is applied consistently.

Changes

Cohort / File(s) Summary
Addon render prop normalization
code/core/src/manager/components/panel/Panel.tsx
Added renderChild helper to support addon render props provided as React elements or functions; replaced direct v.render({ active: true }) calls in PreRenderAddons with renderChild(v.render); updated React imports to include cloneElement, createElement, isValidElement, and useMemo; added explanatory comments about hooks and active handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file, focused change.
  • Review focus: correctness of renderChild branching (element vs function), prop merging (ensuring active is passed without overriding existing props), and any impacts on lifecycle/hooks usage.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f341036 and 0e95f1e.

📒 Files selected for processing (1)
  • code/core/src/manager/components/panel/Panel.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/manager/components/panel/Panel.tsx
⏰ 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). (3)
  • GitHub Check: normal
  • GitHub Check: nx
  • GitHub Check: Core Unit Tests, windows-latest

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

@storybook-app-bot
Copy link

storybook-app-bot bot commented Dec 17, 2025

Package Benchmarks

Commit: 0e95f1e, ran on 19 December 2025 at 10:05:38 UTC

No significant changes detected, all good. 👏

Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

I would've used a ternary return statement, but generally LGTM.

@Sidnioulz
Copy link
Member Author

I would've used a ternary return statement, but generally LGTM.

Generally agreed. Here, this allowed me to scope down the ts-expect-error to the exact problematic code, so I don't risk shadowing future errors elsewhere.

@Sidnioulz
Copy link
Member Author

I would've used a ternary return statement, but generally LGTM.

Feel free to merge this whenever you like, @ghengeveld :)

@Sidnioulz Sidnioulz force-pushed the sidnioulz/issue-33352 branch from f341036 to 9f3f9ff Compare December 19, 2025 09:53
@Sidnioulz Sidnioulz force-pushed the sidnioulz/issue-33352 branch from 9f3f9ff to 0e95f1e Compare December 19, 2025 09:54
@ghengeveld ghengeveld merged commit 14c61ea into next Dec 22, 2025
69 checks passed
@ghengeveld ghengeveld deleted the sidnioulz/issue-33352 branch December 22, 2025 14:41
ndelangen pushed a commit that referenced this pull request Dec 29, 2025
UI: Fix React error 300 on some addons
(cherry picked from commit 14c61ea)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Storybook 10 broken on reload or direct access to pages with Path parameter

4 participants