Addon-Docs: Scope slugger to docs render#34271
Merged
JReinhold merged 12 commits intoApr 27, 2026
Merged
Conversation
- create a docs-scoped slugger context - preserve slug deduping within a single docs page - keep heading anchors stable across docs navigation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a DocsSlugger type, factory, and React context; provides a memoized slugger from DocsContainer; Heading and Subheading consume the context for slug/id generation with fallback to the legacy singleton; refines DocsContext and global typings. Changes
Sequence Diagram(s)sequenceDiagram
participant DocsContainer as DocsContainer
participant Provider as DocsSluggerContext.Provider
participant Slugger as GithubSlugger
participant Heading as Heading/Subheading
DocsContainer->>Provider: useMemo(createDocsSlugger()) → provide slugger instance
Provider->>Slugger: holds DocsSlugger instance
Heading->>Provider: useContext(DocsSluggerContext)
alt context present
Heading->>Slugger: slug(children.toLowerCase()) → id
else no context
Heading->>Slugger: use legacy `slugs` singleton → slug → id
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/Docs.tsx (1)
27-27: Consider memoizing the slugger to preserve slug state across re-renders.The slugger is recreated on every render of
Docs. If this component re-renders (e.g., due to context or prop changes), headings would regenerate their IDs from a fresh slugger, potentially causing anchor instability within a single page view.♻️ Proposed fix using useMemo
+import { useMemo } from 'react'; + export function Docs<TRenderer extends Renderer = Renderer>({ context, docsParameter, }: DocsProps<TRenderer>) { const Container: ComponentType< PropsWithChildren<{ context: DocsContextProps<TRenderer>; theme: Theme }> > = docsParameter.container || DocsContainer; const Page = docsParameter.page || DocsPage; - const slugger = createDocsSlugger(); + const slugger = useMemo(() => createDocsSlugger(), []); return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Docs.tsx` at line 27, The slugger created by createDocsSlugger() is recreated on each render of the Docs component, causing unstable heading IDs; memoize it using React's useMemo so the slugger instance persists across re-renders (and optionally reset when the page changes by including a page identifier like location.pathname in the dependency array). Update the slugger declaration in the Docs component to use useMemo(() => createDocsSlugger(), [/* optional page key */]) and ensure useMemo is imported from React.
🤖 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/Docs.tsx`:
- Line 27: The slugger created by createDocsSlugger() is recreated on each
render of the Docs component, causing unstable heading IDs; memoize it using
React's useMemo so the slugger instance persists across re-renders (and
optionally reset when the page changes by including a page identifier like
location.pathname in the dependency array). Update the slugger declaration in
the Docs component to use useMemo(() => createDocsSlugger(), [/* optional page
key */]) and ensure useMemo is imported from React.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80719a5f-4b5c-4ef5-ab44-8ca1fe7506f1
📒 Files selected for processing (5)
code/addons/docs/src/blocks/blocks/Docs.tsxcode/addons/docs/src/blocks/blocks/DocsSluggerContext.tscode/addons/docs/src/blocks/blocks/Heading.tsxcode/addons/docs/src/blocks/blocks/Subheading.tsxcode/addons/docs/src/typings.d.ts
JReinhold
requested changes
Mar 25, 2026
Contributor
JReinhold
left a comment
JReinhold
left a comment
There was a problem hiding this comment.
Thanks for the PR! Added a few suggestions.
…docs-render' into bug/34198-scope-docs-slugger-to-docs-render
20 tasks
…docs-render' into bug/34198-scope-docs-slugger-to-docs-render
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/DocsContainer.tsx`:
- Line 29: DocsContainer currently calls createDocsSlugger() on every render
which recreates the GithubSlugger and breaks deduplication/causes context
identity churn; change the slugger instantiation to be stable (e.g. use useMemo
or useRef) and only recreate it when the docs page identity changes (use a
stable dependency such as the docs page identifier or the provided context), so
update the slugger creation (the slugger constant created from
createDocsSlugger) to use a memoized/ref value with a dependency like the page
id or context instead of recreating it every render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 556d3704-f817-4128-a46c-57d1bba18a85
📒 Files selected for processing (1)
code/addons/docs/src/blocks/blocks/DocsContainer.tsx
JReinhold
approved these changes
Apr 22, 2026
Contributor
JReinhold
left a comment
JReinhold
left a comment
There was a problem hiding this comment.
This looks good now! ❤️ There are some formatting errors and some type errors that is blocked in CI, but once those are fixed it should be ready to go. 🙏
Member
Author
🎉 This error doesn't seem like something I need to fix, right? Thanks for confirming |
Contributor
|
The type error is coming from this new code, so yeah I think you need to fix it. Sorry that I didn't make that clear. 😞 You can run |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 186 | 186 | 0 |
| Self size | 76 KB | 76 KB | 🎉 -48 B 🎉 |
| Dependency size | 32.78 MB | 32.89 MB | 🚨 +111 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.44 MB | 30.54 MB | 🚨 +106 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 190 | 190 | 0 |
| Self size | 15 KB | 15 KB | 🎉 -18 B 🎉 |
| Dependency size | 29.50 MB | 29.61 MB | 🚨 +111 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 535 | 535 | 0 |
| Self size | 650 KB | 650 KB | 0 B |
| Dependency size | 60.80 MB | 60.91 MB | 🚨 +111 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 273 | 273 | 0 |
| Self size | 23 KB | 23 KB | 0 B |
| Dependency size | 45.41 MB | 45.52 MB | 🚨 +111 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 16 KB | 16 KB | 0 B |
| Dependency size | 34.04 MB | 34.15 MB | 🚨 +111 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 166 | 166 | 0 |
| Self size | 18 KB | 18 KB | 🎉 -24 B 🎉 |
| Dependency size | 31.78 MB | 31.89 MB | 🚨 +111 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Member
Author
|
@JReinhold I made a small fix, and the checks have passed now. |
JReinhold
moved this from Empathy Queue (prioritized)
to In Progress
in Core Team Projects
github-project-automation
Bot
moved this from In Progress
to Done
in Core Team Projects
16 tasks
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
4 participants
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 #34198
What I did
I believe this is a bug; it could result in:
change:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
due to Issue #33944, before running, must first delete
code/node_modules/.cache.cd code && yarn storybook:uihttp://localhost:6006/?path=/docs/example-button--docsPrimaryand verify the URL hash is#primaryButton > DocsPrimaryagain and verify the URL hash is still#primaryDocumentation
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
Refactor
Chores