Addon-docs: Restore docs.components overrides for doc blocks#34111
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JReinhold
changed the title
docs.components overrides for doc blocks
Contributor
There was a problem hiding this comment.
Pull request overview
Restores docs.components overrides for Storybook docs blocks when those blocks are imported in MDX (MDX2+) and when used internally by autodocs/docs pages, by introducing a shared wrapper that consults the active MDX components map.
Changes:
- Added
withMdxComponentOverridehelper (+ unit tests) to re-applydocs.componentsoverrides for imported doc blocks. - Wrapped the core docs blocks (Title/Subtitle/Story/Canvas/etc.) with the new helper so overrides apply consistently.
- Added a Docs2 “ComponentOverrides” CSF + MDX fixture for manual verification of override rendering.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| code/addons/docs/template/stories/docs2/component-overrides.stories.tsx | Adds a CSF autodocs fixture that sets parameters.docs.components with recognizable override markers. |
| code/addons/docs/template/stories/docs2/ComponentOverrides.mdx | Adds an MDX fixture importing blocks to verify overrides apply to imported blocks. |
| code/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsx | Introduces the shared helper that reads overrides via useMDXComponents(). |
| code/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsx | Adds unit coverage for default/override/self-reference fallback behavior. |
| code/addons/docs/src/blocks/blocks/Wrapper.tsx | Wraps Wrapper block with the override helper. |
| code/addons/docs/src/blocks/blocks/Unstyled.tsx | Wraps Unstyled block with the override helper. |
| code/addons/docs/src/blocks/blocks/Title.tsx | Wraps Title block with the override helper. |
| code/addons/docs/src/blocks/blocks/Subtitle.tsx | Wraps Subtitle block with the override helper. |
| code/addons/docs/src/blocks/blocks/Subheading.tsx | Wraps Subheading block with the override helper. |
| code/addons/docs/src/blocks/blocks/Story.tsx | Wraps Story block with the override helper. |
| code/addons/docs/src/blocks/blocks/Stories.tsx | Wraps Stories block with the override helper. |
| code/addons/docs/src/blocks/blocks/Source.tsx | Wraps Source block with the override helper (and simplifies unused catch binding). |
| code/addons/docs/src/blocks/blocks/Primary.tsx | Wraps Primary block with the override helper. |
| code/addons/docs/src/blocks/blocks/Meta.tsx | Wraps Meta block with the override helper (and simplifies unused catch binding). |
| code/addons/docs/src/blocks/blocks/Markdown.tsx | Wraps Markdown block with the override helper. |
| code/addons/docs/src/blocks/blocks/Heading.tsx | Wraps Heading block with the override helper. |
| code/addons/docs/src/blocks/blocks/DocsStory.tsx | Wraps DocsStory block with the override helper. |
| code/addons/docs/src/blocks/blocks/Description.tsx | Wraps Description block with the override helper. |
| code/addons/docs/src/blocks/blocks/Controls.tsx | Wraps Controls block with the override helper. |
| code/addons/docs/src/blocks/blocks/Canvas.tsx | Wraps Canvas block with the override helper. |
| code/addons/docs/src/blocks/blocks/ArgTypes.tsx | Wraps ArgTypes block with the override helper. |
You can also share your feedback on Copilot code review. Take the survey.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces MDX component override support to doc blocks through a new Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Block as Block Component
participant HOC as withMdxComponentOverride
participant MDXContext as MdxWrappedBlockContext
participant MDXProvider as useMDXComponents
App->>Block: Render Block (e.g., ArgTypes)
activate Block
Block->>HOC: WrappedBlock called
activate HOC
HOC->>MDXProvider: Get MDX component overrides
MDXProvider-->>HOC: Return overrides map
HOC->>MDXContext: Check if blockName wrapped
alt Blockname not in wrapped set
MDXContext-->>HOC: Not wrapped yet
alt Override exists for blockName
HOC->>MDXContext: Add blockName to wrapped set
HOC->>MDXProvider: Render Override component
MDXProvider-->>Block: Wrapped override rendered
else No override
HOC->>Block: Render original Impl
Block-->>App: Default block rendered
end
else Already wrapped
HOC->>Block: Render original Impl (prevent recursion)
Block-->>App: Fallback to default block
end
deactivate HOC
deactivate Block
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
📝 Coding Plan
Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
code/addons/docs/src/blocks/blocks/Unstyled.tsx (1)
5-9: Potential issue: user-providedclassNamewill be overwritten.The current prop spread order
{...props} className="sb-unstyled"means anyclassNamepassed by the user will be silently overwritten. If preserving user classes alongsidesb-unstyledis desired, consider merging them.♻️ Optional: Merge classNames if user classes should be preserved
-const UnstyledImpl: React.FC< - React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement> -> = (props) => <div {...props} className="sb-unstyled" />; +const UnstyledImpl: React.FC< + React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement> +> = ({ className, ...rest }) => ( + <div {...rest} className={className ? `${className} sb-unstyled` : 'sb-unstyled'} /> +);If the current behavior is intentional (always use exactly
sb-unstyled), this can be disregarded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Unstyled.tsx` around lines 5 - 9, The UnstyledImpl currently spreads props and then sets a fixed className, which silently overwrites any user-supplied className; update UnstyledImpl (the props handling in the component passed into withMdxComponentOverride) to merge props.className with "sb-unstyled" (e.g., build a combinedClassName from props.className and "sb-unstyled" and pass className={combinedClassName}) so user classes are preserved; if the intention is to always enforce only "sb-unstyled" then no change is needed.code/addons/docs/src/blocks/blocks/Markdown.tsx (1)
13-13: Minor inconsistency: Consider adding explicitFCtype annotation.Other block implementations in this PR use explicit
FC<Props>type annotations (e.g.,const CanvasImpl: FC<CanvasProps>), while this uses implicit typing. For consistency across the codebase:-const MarkdownImpl = (props: MarkdownProps) => { +const MarkdownImpl: FC<MarkdownProps> = (props) => {This would require importing
FCfrom React.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Markdown.tsx` at line 13, The MarkdownImpl component currently uses implicit typing; make it consistent with other blocks by adding an explicit React.FC annotation: change the declaration to use FC<MarkdownProps> and import FC from 'react' (ensure the import statement includes FC alongside React or adjust to named import). Update the component signature reference MarkdownImpl and the props type MarkdownProps accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/Title.tsx`:
- Line 55: withMdxComponentOverride is causing infinite recursion when an
override renders the public export (e.g., components.Title = props => <Title
{...props} />) because the runtime re-resolves components.Title each render; to
fix, add a re-entry escape hatch in withMdxComponentOverride.tsx: define a
module-level Symbol (e.g., MDX_WRAPPED_BLOCK) and set it on the WrappedBlock
export (WrappedBlock[MDX_WRAPPED_BLOCK] = true), then when resolving
components[name] inside withMdxComponentOverride check if the resolved Override
=== WrappedBlock OR if typeof Override === 'function' &&
Override[MDX_WRAPPED_BLOCK] is true and in that case return WrappedBlock
directly (skip MDX re-resolution/wrapping); ensure you reference
withMdxComponentOverride, WrappedBlock and Override in the change so overrides
that compose with the public block no longer recurse.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/Markdown.tsx`:
- Line 13: The MarkdownImpl component currently uses implicit typing; make it
consistent with other blocks by adding an explicit React.FC annotation: change
the declaration to use FC<MarkdownProps> and import FC from 'react' (ensure the
import statement includes FC alongside React or adjust to named import). Update
the component signature reference MarkdownImpl and the props type MarkdownProps
accordingly.
In `@code/addons/docs/src/blocks/blocks/Unstyled.tsx`:
- Around line 5-9: The UnstyledImpl currently spreads props and then sets a
fixed className, which silently overwrites any user-supplied className; update
UnstyledImpl (the props handling in the component passed into
withMdxComponentOverride) to merge props.className with "sb-unstyled" (e.g.,
build a combinedClassName from props.className and "sb-unstyled" and pass
className={combinedClassName}) so user classes are preserved; if the intention
is to always enforce only "sb-unstyled" then no change is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1363b366-0c6a-4ae4-8684-9496a673e584
📒 Files selected for processing (21)
code/addons/docs/src/blocks/blocks/ArgTypes.tsxcode/addons/docs/src/blocks/blocks/Canvas.tsxcode/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Description.tsxcode/addons/docs/src/blocks/blocks/DocsStory.tsxcode/addons/docs/src/blocks/blocks/Heading.tsxcode/addons/docs/src/blocks/blocks/Markdown.tsxcode/addons/docs/src/blocks/blocks/Meta.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/Source.tsxcode/addons/docs/src/blocks/blocks/Stories.tsxcode/addons/docs/src/blocks/blocks/Story.tsxcode/addons/docs/src/blocks/blocks/Subheading.tsxcode/addons/docs/src/blocks/blocks/Subtitle.tsxcode/addons/docs/src/blocks/blocks/Title.tsxcode/addons/docs/src/blocks/blocks/Unstyled.tsxcode/addons/docs/src/blocks/blocks/Wrapper.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsxcode/addons/docs/template/stories/docs2/ComponentOverrides.mdxcode/addons/docs/template/stories/docs2/component-overrides.stories.tsx
|
View your CI Pipeline Execution ↗ for commit 3bd8c33
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 76 KB | 76 KB | 🚨 +48 B 🚨 |
| Dependency size | 32.19 MB | 32.21 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 140 KB | 140 KB | 🎉 -54 B 🎉 |
| Dependency size | 30.40 MB | 30.42 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 189 | 189 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 28.91 MB | 28.92 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 650 KB | 650 KB | 0 B |
| Dependency size | 59.91 MB | 59.93 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 24 KB | 24 KB | 🎉 -12 B 🎉 |
| Dependency size | 44.53 MB | 44.55 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 197 | 197 | 0 |
| Self size | 16 KB | 16 KB | 🚨 +12 B 🚨 |
| Dependency size | 33.45 MB | 33.47 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 779 KB | 779 KB | 0 B |
| Dependency size | 67.38 MB | 67.40 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 65.91 MB | 65.93 MB | 🚨 +19 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 167 | 167 | 0 |
| Self size | 18 KB | 18 KB | 🚨 +24 B 🚨 |
| Dependency size | 31.40 MB | 31.42 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsx`:
- Around line 27-31: The conditional in withMdxComponentOverride incorrectly
rejects any function where Override[MDX_WRAPPED_BLOCK] is truthy, blocking valid
cross-block overrides (e.g., docs.components.Title = Subtitle); update the if
condition in withMdxComponentOverride to stop treating "is a wrapped component"
as a rejection—remove the (typeof Override === 'function' &&
Override[MDX_WRAPPED_BLOCK]) clause so only wrappedBlocks.has(blockName) and
Override === WrappedBlock prevent wrapping, and add a regression test that sets
components.Title = Subtitle to ensure cross-block overrides work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e753d417-4970-4112-870e-21a6433dbf9a
📒 Files selected for processing (5)
code/addons/docs/src/blocks/blocks/Meta.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsxcode/addons/docs/template/stories/docs2/ComponentOverrides.mdxcode/addons/docs/template/stories/docs2/component-overrides.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- code/addons/docs/template/stories/docs2/component-overrides.stories.tsx
- code/addons/docs/src/blocks/blocks/Meta.tsx
- code/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsx
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsx (1)
27-77: Add one regression test for the attached-MDX renderer path.These cases prove the HOC, but they do not exercise the bug path described in the PR: an attached MDX docs page resolving
parameters.docs.componentsfrom its attached CSF file. A regression in theMdxDocsRender/attached-story wiring would still pass this suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsx` around lines 27 - 77, Add a regression test that exercises the attached-MDX renderer path (not just the HOC) by mounting the MdxDocsRender (or whichever renderer is used for attached MDX pages) with a story that has parameters.docs.components coming from its attached CSF file; specifically, create a fake story/attached CSF that exports a TestBlock override via parameters.docs.components, render the attached MDX via MdxDocsRender (or the attached-story wiring path) and assert the override is used (and falls back correctly when it points to the wrapped block). Reference symbols: withMdxComponentOverride, MdxDocsRender, TestBlock, parameters.docs.components, attached CSF.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsx`:
- Around line 27-77: Add a regression test that exercises the attached-MDX
renderer path (not just the HOC) by mounting the MdxDocsRender (or whichever
renderer is used for attached MDX pages) with a story that has
parameters.docs.components coming from its attached CSF file; specifically,
create a fake story/attached CSF that exports a TestBlock override via
parameters.docs.components, render the attached MDX via MdxDocsRender (or the
attached-story wiring path) and assert the override is used (and falls back
correctly when it points to the wrapped block). Reference symbols:
withMdxComponentOverride, MdxDocsRender, TestBlock, parameters.docs.components,
attached CSF.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 612dbd4b-921e-4a4e-9049-430ebdd96805
📒 Files selected for processing (3)
code/addons/docs/src/blocks/blocks/Meta.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.test.tsxcode/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- code/addons/docs/src/blocks/blocks/withMdxComponentOverride.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/component-overrides.stories.tsx (1)
70-78: Consider simplifying the type assertion.The type assertion on line 75 works but is verbose. Since
TestBlockComponentsis narrower than whatMDXProvideraccepts, you could simplify by widening the type or using a more direct cast.♻️ Optional: Simplify type handling
type TestBlockComponents = { TestBlock: React.ComponentType<TestBlockProps>; }; -const renderTestBlock = (components: TestBlockComponents | undefined) => ( - <MDXProvider components={components as React.ComponentProps<typeof MDXProvider>['components']}> +const renderTestBlock = (components: Record<string, React.ComponentType<any>> | undefined) => ( + <MDXProvider components={components}> <TestBlock label="Hello" /> </MDXProvider> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/component-overrides.stories.tsx` around lines 70 - 78, The explicit cast on components is unnecessary—either widen TestBlockComponents to match MDXProvider's components prop or change renderTestBlock's parameter type to React.ComponentProps<typeof MDXProvider>['components'] | undefined so you can pass components directly to <MDXProvider> without the as-cast; update either the type alias TestBlockComponents to intersect/extend React.ComponentProps<typeof MDXProvider>['components'] (ensuring TestBlock is present) or change renderTestBlock(components: ...) to use the MDXProvider components type and remove the cast when rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/ComponentOverrides.mdx`:
- Around line 38-41: The MDX references nonexistent story exports
ComponentOverrideStories.Basic and ComponentOverrideStories.Secondary which will
break rendering; update the MDX to reference the actual exported story
identifiers from the stories file (e.g., UsesDefaultImplementation,
UsesMdxOverride, FallsBackWhenOverrideIsWrappedBlock,
FallsBackWhenOverrideComposesPublicBlock, AllowsDifferentWrappedBlockOverride)
or alternatively add Basic and Secondary exports to the stories module so
ComponentOverrideStories.Basic and .Secondary exist; ensure the <Story>,
<Canvas>, <Source>, and <DocsStory> tags use the corrected export names.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/component-overrides.stories.tsx`:
- Around line 70-78: The explicit cast on components is unnecessary—either widen
TestBlockComponents to match MDXProvider's components prop or change
renderTestBlock's parameter type to React.ComponentProps<typeof
MDXProvider>['components'] | undefined so you can pass components directly to
<MDXProvider> without the as-cast; update either the type alias
TestBlockComponents to intersect/extend React.ComponentProps<typeof
MDXProvider>['components'] (ensuring TestBlock is present) or change
renderTestBlock(components: ...) to use the MDXProvider components type and
remove the cast when rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2f5c8eb-02a1-4013-8ce4-3cd0fb340c25
📒 Files selected for processing (20)
code/.storybook/main.tscode/addons/docs/src/blocks/ComponentOverrides.mdxcode/addons/docs/src/blocks/blocks/ArgTypes.tsxcode/addons/docs/src/blocks/blocks/Canvas.tsxcode/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Description.tsxcode/addons/docs/src/blocks/blocks/DocsStory.tsxcode/addons/docs/src/blocks/blocks/Heading.tsxcode/addons/docs/src/blocks/blocks/Markdown.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/Source.tsxcode/addons/docs/src/blocks/blocks/Stories.tsxcode/addons/docs/src/blocks/blocks/Story.tsxcode/addons/docs/src/blocks/blocks/Subheading.tsxcode/addons/docs/src/blocks/blocks/Subtitle.tsxcode/addons/docs/src/blocks/blocks/Title.tsxcode/addons/docs/src/blocks/blocks/Unstyled.tsxcode/addons/docs/src/blocks/blocks/Wrapper.tsxcode/addons/docs/src/blocks/blocks/with-mdx-component-override.tsxcode/addons/docs/src/blocks/component-overrides.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- code/addons/docs/src/blocks/blocks/Heading.tsx
- code/addons/docs/src/blocks/blocks/Subheading.tsx
- code/addons/docs/src/blocks/blocks/Primary.tsx
- code/addons/docs/src/blocks/blocks/Source.tsx
- code/addons/docs/src/blocks/blocks/DocsStory.tsx
- code/addons/docs/src/blocks/blocks/Subtitle.tsx
- code/addons/docs/src/blocks/blocks/Unstyled.tsx
- code/addons/docs/src/blocks/blocks/Controls.tsx
- code/addons/docs/src/blocks/blocks/Story.tsx
- code/addons/docs/src/blocks/blocks/Title.tsx
- code/addons/docs/src/blocks/blocks/Description.tsx
- code/addons/docs/src/blocks/blocks/Canvas.tsx
Sidnioulz
approved these changes
Mar 12, 2026
Contributor
Sidnioulz
left a comment
Sidnioulz
left a comment
There was a problem hiding this comment.
LGTM overall, great idea. I think there's a bug that could trigger under specific conditions (if someone overused Wrapper or Unstyled), see comments. Approving the rest now :)
Comment on lines
+70
to
+73
| { | ||
| directory: '../addons/docs/src', | ||
| titlePrefix: 'addons/docs', | ||
| }, |
Contributor
There was a problem hiding this comment.
Isn't renaming gonna mess with Chromatic?
Contributor
Author
There was a problem hiding this comment.
yeah I had to accept a bunch of changes, but it was super confusing to add addons/docs/src while addons/docs/src/blocks was also present - not just from a config standpoint but also in the sidebar.
Comment on lines
+27
to
+29
| if (wrappedBlocks?.has(blockName) || Override === WrappedBlock) { | ||
| return <Block {...props} />; | ||
| } |
Contributor
There was a problem hiding this comment.
I dont understand why the first condition matters, or how the second condition can even occur. I think it would help maintainability if you separated those two checks and gave more detail, for each, on how they could occur.
Contributor
There was a problem hiding this comment.
Ok I think I get it now. Because we're not aliasing but swapping at runtime, the real implem inside an override must be preserved.
The presence of the React context lets you detect that we've already overridden once, so any remaining block found in children is likely to be called by the override.
Do we have any possible with our existing blocks to have this code pattern though?
<A>
<B>
<A>Foo</A>
</B>
</A>If we want to ensure we only skip overrides for A in the context of rendering A's own children, shouldn't you start from an empty set for each context provider, instead of appending to the parent set?
Contributor
Author
There was a problem hiding this comment.
I don't think there is any practical scenario where a problem like that could occur, so I'm not too worried.
Contributor
Author
There was a problem hiding this comment.
(in general, I agree that this pattern here to avoid infinite recursion is pretty confusing, which is why it's heavily commented)
…bookjs/storybook into fix/docs-block-component-overrides
This was referenced Apr 27, 2026
1 task
This was referenced May 3, 2026
1 task
This was referenced May 10, 2026
1 task
This was referenced May 16, 2026
1 task
This was referenced May 28, 2026
Closed
Closed
Closed
Closed
Closed
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
docs.componentsoverrides for Storybook doc blocks that are imported in MDX or used internally by autodocs/docs pageswithMdxComponentOverridehelper and applied it across the relevant docs blocksparameters.docs, which makes the documented MDX component override flow work there tooDocs2/ComponentOverridesCSF+MDX manual verification fixture that renders recognizable override markers for the overridden blocksChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
https://localhost:6006/?path=/docs/addons-docs-blocks-component-overrides--mdx.override:*markers for blocks likeTitle,Subtitle,Primary,Controls, andStoriesinstead of the default block UI.MDXentry forDocs2/ComponentOverrides.override:*markers, confirmingdocs.componentsnow applies to imported blocks likeCanvas,Story,Source, and friends.See deployed stories:https://635781f3500dd2c49e189caf-ahwwtndcxh.chromatic.com/?path=/docs/addons-docs-blocks-component-overrides--mdx
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
Documentation