Docs: Ensure CodePanel hooks are called within component #33162
Conversation
| currentStoryId, | ||
| }: { | ||
| active: boolean | undefined; | ||
| lastEvent: any | undefined; |
There was a problem hiding this comment.
I did try to type this, then I noticed that where it is emitted, format is not actually set....
|
View your CI Pipeline Execution ↗ for commit 4102998
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe code panel rendering logic in the manager component is refactored into a new CodePanel component, extracting state management, parameter retrieval, channel subscriptions, and theming logic from inline code into an isolated component structure. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
✨ Finishing touches
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: 0
🧹 Nitpick comments (2)
code/addons/docs/src/manager.tsx (2)
17-25: Consider defining a type forlastEventinstead ofany.The
lastEventprop is typed asany, which reduces type safety. Even if theformatfield is unreliable (as noted in your comment), defining a minimal interface would improve code clarity.+interface SnippetRenderedEvent { + source?: string; + format?: SyntaxHighlighterFormatTypes; +} + const CodePanel = ({ active, lastEvent, currentStoryId, }: { active: boolean | undefined; - lastEvent: any | undefined; + lastEvent: SnippetRenderedEvent | undefined; currentStoryId: string | undefined; }) => {
26-32: Fix naming inconsistency between state variable and setter.The state is named
codeSnippetbut the setter issetSourceCode. For consistency, rename the setter to match the state variable name.- const [codeSnippet, setSourceCode] = useState<{ + const [codeSnippet, setCodeSnippet] = useState<{Then update the setter calls on lines 40 and 48:
- setSourceCode({ + setCodeSnippet({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/docs/src/manager.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use camelCase for variable and function names
Files:
code/addons/docs/src/manager.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes
Files:
code/addons/docs/src/manager.tsx
**/*.{ts,tsx,js,jsx,json,html,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes
Files:
code/addons/docs/src/manager.tsx
code/**/!(*.test).{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
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/manager.tsx
🧠 Learnings (1)
📓 Common learnings
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.
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.
⏰ 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: get-parameters
🔇 Additional comments (6)
code/addons/docs/src/manager.tsx (6)
34-37: LGTM!The parameter retrieval with sensible defaults is correct.
39-44: LGTM!The effect correctly resets the code snippet when navigating to a different story, preventing stale code from displaying.
46-50: LGTM!The channel subscription correctly updates the code snippet state when new source code is rendered.
52-53: LGTM!The theme detection logic is correct for determining dark mode.
55-67: LGTM!The rendering logic correctly prioritizes the code sources and applies the appropriate theme.
89-96: LGTM! This change fixes the React hooks error.By extracting hook calls into the
CodePanelcomponent and returning it from the render function, you've correctly resolved the issue where hooks were being called outside of a React component. This now complies with Storybook's addon API requirements.
valentinpalkovic
changed the title
2 participants
Closes #33161
What I did
Ensure code panel hooks are called within a react component rather than the addon render function, to ensure compliance with storybook's own docs:
storybook/code/core/src/types/modules/addons.ts
Lines 371 to 377 in 809c130
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
✏️ Tip: You can customize this high-level summary in your review settings.