Addon Docs: Skip !autodocs stories when computing primary story
#32712
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new Changes
Sequence DiagramsequenceDiagram
participant Primary as Primary Component
participant usePrimaryStory as usePrimaryStory Hook
participant DocsContext as DocsContext
participant componentStories as componentStories()
Primary->>usePrimaryStory: Invoke hook
usePrimaryStory->>DocsContext: useContext(DocsContext)
usePrimaryStory->>componentStories: Get all stories from context
componentStories-->>usePrimaryStory: [Story1, Story2, Story3]
Note over usePrimaryStory: Filter: find first with<br/>'autodocs' tag
alt Story with autodocs found
usePrimaryStory-->>Primary: PreparedStory
Primary->>Primary: Render DocsStory
else No autodocs story
usePrimaryStory-->>Primary: undefined
Primary->>Primary: Return null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes involve new hook logic, component refactoring patterns that differ between Primary and Controls, comprehensive test coverage, and story configuration changes. While individual pieces follow clear patterns, each requires separate reasoning to verify correctness of the tag-based story selection and integration points. Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
7-10: Consider clarifying the doc comment.The comment states "filters out stories with the '!autodocs' tag", which is technically correct but could be more direct. Consider rephrasing to explicitly state what the hook returns.
Apply this diff to clarify the documentation:
/** - * A hook to get the primary story for the current component It filters out stories with the - * '!autodocs' tag. + * A hook to get the primary story for the current component. Returns the first story + * that includes the 'autodocs' tag (excluding stories tagged with '!autodocs'). */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/addons/docs/src/blocks/blocks/Controls.tsx(2 hunks)code/addons/docs/src/blocks/blocks/Primary.tsx(1 hunks)code/addons/docs/src/blocks/blocks/usePrimaryStory.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
code/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
code/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.ts
🧬 Code graph analysis (3)
code/addons/docs/src/blocks/blocks/Controls.tsx (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
usePrimaryStory(11-15)
code/addons/docs/src/blocks/blocks/Primary.tsx (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
usePrimaryStory(11-15)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
code/core/src/types/modules/story.ts (1)
PreparedStory(100-116)
🔇 Additional comments (2)
code/addons/docs/src/blocks/blocks/Primary.tsx (1)
7-13: LGTM! Clean refactor that centralizes story resolution.The component simplification looks good:
- Removes props-based complexity
- Delegates story selection to
usePrimaryStoryhook- Properly handles the case when no autodocs-tagged story exists
- Maintains the same rendering behavior for the primary story
This change aligns with the PR objective of fixing the !autodocs tag behavior for the first story.
code/addons/docs/src/blocks/blocks/Controls.tsx (1)
43-50: Approve Controls fallback logic
- Uses
usePrimaryStorywhenofprop is omitted (ensuring<Controls />in DocsPage and MDX renders correctly).- Gracefully returns
nullwhen no story is available.- Aligns with the Primary component’s approach and resolves the autodocs visibility issue.
| export const usePrimaryStory = (): PreparedStory | undefined => { | ||
| const context = useContext(DocsContext); | ||
| const stories = context.componentStories(); | ||
| return stories.find((story) => story.tags.includes('autodocs')); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
LGTM! The hook logic correctly addresses the bug.
The implementation correctly selects the first story with the 'autodocs' tag, which excludes stories marked with '!autodocs'. This should fix the issue where stories tagged with '!autodocs' were appearing as the primary preview when positioned first.
Verify the fix resolves the reported issue by testing against the reproduction case:
🏁 Script executed:
#!/bin/bash
# Description: Search for test files or stories that might cover the !autodocs tag behavior
# Look for existing tests related to autodocs tags
rg -n --type=ts --type=tsx -C3 '\!autodocs|autodocs.*tag'
# Find stories or test files that might need verification
fd -e story -e test.ts -e test.tsx | head -20Length of output: 1067
Add unit tests for '!autodocs' exclusion
No existing tests cover scenarios where a story is tagged !autodocs. Add tests (e.g., code/addons/docs/src/blocks/blocks/usePrimaryStory.test.ts) to verify that:
- Stories with the
'autodocs'tag are selected. - Stories with the
'!autodocs'tag are excluded.
There was a problem hiding this comment.
@ia319 hooks usually have their own test file. You should be able to mock the DocsContext and build tests that show
- The hook ignores
!autodocsstories - It takes the first story with
autodocs
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Sidnioulz
changed the title
|
View your CI Pipeline Execution ↗ for commit 796471b
☁️ Nx Cloud last updated this comment at |
|
After pull from upstream and rebase, I ran |
| export const usePrimaryStory = (): PreparedStory | undefined => { | ||
| const context = useContext(DocsContext); | ||
| const stories = context.componentStories(); | ||
| return stories.find((story) => story.tags.includes('autodocs')); | ||
| }; |
There was a problem hiding this comment.
@ia319 hooks usually have their own test file. You should be able to mock the DocsContext and build tests that show
- The hook ignores
!autodocsstories - It takes the first story with
autodocs
Some tests are flaky :) If you suspect a flaky test, feel free to ask us on |
Sidnioulz
changed the title
Sidnioulz
changed the title
Sidnioulz
changed the title
shilman
changed the title
!autodocs stories when computing primary story
There was a problem hiding this comment.
Hi @ia319,
We have one last change request for you. The Chromatic CI job shows that the Primary docs block now correctly filters out stories where autodocs is not set, but all the examples without toolbar are gone, because they all use code/addons/docs/src/blocks/examples/StoriesParameters.stories.tsx which does not have autodocs.
Could you please add autodocs to that example (as it's supposed to be used for toolbar testing), and add a few stories using code/addons/docs/src/blocks/examples/ButtonNoAutodocs.stories.tsx and code/addons/docs/src/blocks/examples/ButtonSomeAutodocs.stories.tsx to validate how the primary story loads?
The files to modify are
code/addons/docs/src/blocks/examples/StoriesParameters.stories.tsx: add autodocscode/addons/docs/src/blocks/blocks/Primary.stories.tsx: add stories using the examples above
Thanks!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/csf-tools/CsfFile.ts (1)
604-639: Missing parameter check for render object methods.When
renderis defined as an object method (e.g.,render() {}), the code skips theisArgsStorycheck at lines 611-615, relying on the defaultparameters.__isArgsStory = truefrom line 602. This could incorrectly mark no-parameter render methods as args stories.Current behavior:
render: () => {}→ correctly sets__isArgsStory: false(line 611-615)render: (args) => {}→ correctly sets__isArgsStory: true(line 611-615)render(args) {}→ correctly keeps__isArgsStory: true(default at line 602) ✓render() {}→ incorrectly keeps__isArgsStory: true(should be false) ✗Consider extending the check:
const key = p.key.name; if (t.isObjectMethod(p)) { + if (key === 'render') { + parameters.__isArgsStory = p.params.length > 0; + } self._storyAnnotations[exportName][key] = p; } else {Also update
isArgsStoryfunction (lines 118-149) to handle object methods:if (t.isFunctionDeclaration(storyFn)) { return storyFn.params.length > 0; } +if (t.isObjectMethod(storyFn)) { + return storyFn.params.length > 0; +} return false;
🧹 Nitpick comments (3)
code/core/src/csf-tools/CsfFile.test.ts (1)
1426-1458: Consider adding test coverage for no-args render method.The test validates
render(args) {}correctly expects__isArgsStory: true. However, there's no corresponding test forrender() {}(no parameters) to verify it correctly sets__isArgsStory: false, similar to the existing test at line 1358 for arrow function syntax.Add a test case like:
it('Object export with no-args render method', () => { expect( parse( dedent` export default { title: 'foo/bar' }; export const A = { render() {} } `, true ) ).toMatchInlineSnapshot(` meta: title: foo/bar stories: - id: foo-bar--a name: A parameters: __isArgsStory: false __id: foo-bar--a __stats: factory: false play: false render: true ... `); });code/core/src/components/components/syntaxhighlighter/syntaxhighlighter.tsx (1)
211-218: Guard against missingwindowwhen copying
globalWindowis pulled from@storybook/global. In non-browser environments (e.g., SSR builds or unit tests without JSDOM), that property can beundefined, which would makeglobalWindow.setTimeout(...)throw ifonClickever runs. A small null check (or falling back to the globalsetTimeout) would keep the function harmless in those environments while preserving current behavior in the browser.- globalWindow.setTimeout(() => setCopied(false), 1500); + (globalWindow ?? globalThis).setTimeout(() => setCopied(false), 1500);code/core/src/components/components/syntaxhighlighter/clipboard.ts (1)
5-11: Overly broad error catching masks unrelated failures.The empty catch block catches all errors from the top-level clipboard write, not just cross-origin access errors. If the top clipboard API fails for reasons other than cross-origin (e.g., permissions, API errors), this silently falls back without logging, potentially hiding real issues.
Consider catching only the expected error type or at minimum logging the caught error:
async function copyUsingClipboardAPI(text: string) { try { await globalWindow.top?.navigator.clipboard.writeText(text); - } catch { + } catch (error) { + // Fall back to current window if top window access fails (e.g., cross-origin) await globalWindow.navigator.clipboard.writeText(text); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (61)
CHANGELOG.prerelease.md(1 hunks)CONTRIBUTING.md(2 hunks)README.md(1 hunks)code/addons/a11y/manager.js(1 hunks)code/addons/a11y/package.json(1 hunks)code/addons/docs/package.json(1 hunks)code/addons/docs/src/blocks/blocks/Primary.stories.tsx(2 hunks)code/addons/docs/src/blocks/examples/StoriesParameters.stories.tsx(1 hunks)code/addons/links/package.json(1 hunks)code/addons/onboarding/package.json(1 hunks)code/addons/pseudo-states/package.json(1 hunks)code/addons/themes/package.json(1 hunks)code/addons/vitest/package.json(1 hunks)code/builders/builder-vite/package.json(1 hunks)code/builders/builder-webpack5/package.json(1 hunks)code/core/package.json(1 hunks)code/core/src/common/versions.ts(1 hunks)code/core/src/components/components/syntaxhighlighter/clipboard.ts(1 hunks)code/core/src/components/components/syntaxhighlighter/syntaxhighlighter.tsx(1 hunks)code/core/src/components/components/typography/elements/Code.tsx(1 hunks)code/core/src/components/index.ts(1 hunks)code/core/src/csf-tools/CsfFile.test.ts(3 hunks)code/core/src/csf-tools/CsfFile.ts(3 hunks)code/core/src/manager-api/version.ts(1 hunks)code/frameworks/angular/package.json(1 hunks)code/frameworks/ember/package.json(1 hunks)code/frameworks/html-vite/package.json(1 hunks)code/frameworks/nextjs-vite/package.json(2 hunks)code/frameworks/nextjs/package.json(2 hunks)code/frameworks/preact-vite/package.json(1 hunks)code/frameworks/react-native-web-vite/package.json(1 hunks)code/frameworks/react-vite/package.json(1 hunks)code/frameworks/react-webpack5/package.json(1 hunks)code/frameworks/server-webpack5/package.json(1 hunks)code/frameworks/svelte-vite/package.json(1 hunks)code/frameworks/sveltekit/package.json(1 hunks)code/frameworks/vue3-vite/package.json(1 hunks)code/frameworks/web-components-vite/package.json(1 hunks)code/lib/cli-sb/package.json(1 hunks)code/lib/cli-storybook/package.json(1 hunks)code/lib/cli-storybook/src/autoblock/block-major-version.ts(1 hunks)code/lib/codemod/package.json(1 hunks)code/lib/core-webpack/package.json(1 hunks)code/lib/create-storybook/package.json(1 hunks)code/lib/csf-plugin/package.json(1 hunks)code/lib/eslint-plugin/package.json(1 hunks)code/lib/react-dom-shim/package.json(1 hunks)code/package.json(1 hunks)code/presets/create-react-app/package.json(1 hunks)code/presets/react-webpack/package.json(1 hunks)code/presets/server-webpack/package.json(1 hunks)code/renderers/html/package.json(1 hunks)code/renderers/preact/package.json(1 hunks)code/renderers/react/package.json(1 hunks)code/renderers/server/package.json(1 hunks)code/renderers/svelte/package.json(1 hunks)code/renderers/vue3/package.json(1 hunks)code/renderers/web-components/package.json(1 hunks)docs/configure/integration/frameworks-feature-support.mdx(2 hunks)docs/versions/next.json(1 hunks)scripts/tasks/sandbox-parts.ts(1 hunks)
✅ Files skipped from review due to trivial changes (18)
- code/lib/react-dom-shim/package.json
- code/addons/docs/package.json
- code/frameworks/web-components-vite/package.json
- code/addons/themes/package.json
- code/builders/builder-vite/package.json
- code/renderers/server/package.json
- code/frameworks/server-webpack5/package.json
- code/frameworks/ember/package.json
- code/core/package.json
- code/builders/builder-webpack5/package.json
- CONTRIBUTING.md
- code/frameworks/react-webpack5/package.json
- code/lib/create-storybook/package.json
- code/addons/links/package.json
- code/presets/react-webpack/package.json
- code/addons/a11y/manager.js
- code/addons/pseudo-states/package.json
- code/lib/cli-sb/package.json
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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 (useyarn lint:js:cmd <file>)
Files:
code/lib/codemod/package.jsoncode/frameworks/nextjs-vite/package.jsoncode/lib/cli-storybook/src/autoblock/block-major-version.tscode/lib/core-webpack/package.jsoncode/renderers/html/package.jsoncode/frameworks/react-vite/package.jsoncode/core/src/manager-api/version.tscode/presets/server-webpack/package.jsoncode/addons/docs/src/blocks/examples/StoriesParameters.stories.tsxcode/frameworks/html-vite/package.jsoncode/renderers/react/package.jsoncode/frameworks/sveltekit/package.jsoncode/renderers/web-components/package.jsoncode/frameworks/svelte-vite/package.jsoncode/core/src/components/index.tscode/frameworks/angular/package.jsoncode/renderers/vue3/package.jsoncode/frameworks/vue3-vite/package.jsoncode/frameworks/react-native-web-vite/package.jsoncode/frameworks/preact-vite/package.jsoncode/renderers/svelte/package.jsoncode/frameworks/nextjs/package.jsoncode/addons/a11y/package.jsoncode/renderers/preact/package.jsoncode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/addons/vitest/package.jsoncode/core/src/csf-tools/CsfFile.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/components/components/syntaxhighlighter/clipboard.tsdocs/versions/next.jsoncode/package.jsoncode/core/src/common/versions.tscode/lib/eslint-plugin/package.jsoncode/core/src/components/components/typography/elements/Code.tsxscripts/tasks/sandbox-parts.tscode/addons/onboarding/package.jsoncode/lib/cli-storybook/package.jsoncode/core/src/components/components/syntaxhighlighter/syntaxhighlighter.tsxcode/presets/create-react-app/package.jsoncode/lib/csf-plugin/package.json
**/*.{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/lib/cli-storybook/src/autoblock/block-major-version.tscode/core/src/manager-api/version.tscode/addons/docs/src/blocks/examples/StoriesParameters.stories.tsxcode/core/src/components/index.tscode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/core/src/csf-tools/CsfFile.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/components/components/syntaxhighlighter/clipboard.tscode/core/src/common/versions.tscode/core/src/components/components/typography/elements/Code.tsxscripts/tasks/sandbox-parts.tscode/core/src/components/components/syntaxhighlighter/syntaxhighlighter.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/lib/cli-storybook/src/autoblock/block-major-version.tscode/core/src/manager-api/version.tscode/addons/docs/src/blocks/examples/StoriesParameters.stories.tsxcode/core/src/components/index.tscode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/core/src/csf-tools/CsfFile.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/components/components/syntaxhighlighter/clipboard.tscode/core/src/common/versions.tscode/core/src/components/components/typography/elements/Code.tsxcode/core/src/components/components/syntaxhighlighter/syntaxhighlighter.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/lib/cli-storybook/src/autoblock/block-major-version.tscode/core/src/manager-api/version.tscode/addons/docs/src/blocks/examples/StoriesParameters.stories.tsxcode/core/src/components/index.tscode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/core/src/csf-tools/CsfFile.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/components/components/syntaxhighlighter/clipboard.tscode/core/src/common/versions.tscode/core/src/components/components/typography/elements/Code.tsxscripts/tasks/sandbox-parts.tscode/core/src/components/components/syntaxhighlighter/syntaxhighlighter.tsx
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update documentation under
docs/for significant changes, including migration guides for breaking changes
Files:
docs/configure/integration/frameworks-feature-support.mdxdocs/versions/next.json
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/core/src/csf-tools/CsfFile.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/core/src/csf-tools/CsfFile.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/core/src/csf-tools/CsfFile.test.ts
**/README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update package or feature README files when making significant changes
Files:
README.md
scripts/**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In Node.js scripts, use
storybook/internal/node-loggerinstead ofconsole.*
Files:
scripts/tasks/sandbox-parts.ts
🧬 Code graph analysis (2)
code/core/src/csf-tools/CsfFile.test.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
parse(420-936)
code/core/src/components/components/syntaxhighlighter/clipboard.ts (1)
code/core/src/components/index.ts (1)
createCopyToClipboardFunction(40-40)
⏰ 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
🔇 Additional comments (28)
code/frameworks/react-vite/package.json (1)
3-3: Version bump to pre-release candidate looks good.The version change from
10.0.0-beta.13to10.0.0-rc.0is consistent with the PR's broader pre-release version updates across the Storybook ecosystem and aligns with the documentation addon improvements in this PR.code/frameworks/preact-vite/package.json (1)
3-3: Version bump is aligned with monorepo release coordination.The version update from
10.0.0-beta.13to10.0.0-rc.0follows proper semantic versioning conventions and aligns with the broader project-wide version bump mentioned in the PR context.code/frameworks/react-native-web-vite/package.json (1)
3-3: Version bump is correct and necessary.The version change from 10.0.0-beta.13 to 10.0.0-rc.0 aligns with Storybook's official v10.0.0-rc.0 pre-release. This is a standard part of the release process and maintains version consistency across the framework packages in the monorepo. The change follows semantic versioning and is consistent with the PR's inclusion of a broader rc.0 release.
code/lib/cli-storybook/package.json (1)
3-3: <!-- [scratchpad_start]
[task verify_package_coordination_1/1]
Verify beta → rc version migration consistency across all @storybook packages.[observations]
- All primary @storybook packages (addons, builders, core, frameworks, lib, presets, renderers) now at "10.0.0-rc.0"
- No remaining beta versions found anywhere in the coordinate packages
- Target file (code/lib/cli-storybook/package.json) correctly shows "10.0.0-rc.0"
- Coordination complete and consistent across entire monorepo
- Test storybook packages maintained at "0.0.0" (correct, not part of release)
- Scripts directory at "7.0.0-alpha.16" (separate versioning, unrelated)
[analysis]
- Version bump is properly coordinated across all affected packages
- Beta → rc migration is complete, no stragglers
- No inconsistencies or partial upgrades detected
- Original concern about package coordination is fully resolved
[pending]
None.[actions]
Generate final approval comment.[done]
Verification complete. All packages successfully migrated from beta to rc.0 with perfect coordination.
[scratchpad_start] -->Version bump coordinated correctly across all packages.
The migration from
10.0.0-beta.13to10.0.0-rc.0is complete and consistent. Verification confirms all 40+ @storybook packages in the monorepo have been updated to10.0.0-rc.0with no remaining beta versions.scripts/tasks/sandbox-parts.ts (1)
544-544: Verify that this dependency version change is intentional.This change updates the test-runner from a pinned canary build to the floating
@nexttag. This appears unrelated to the PR's stated objective of fixing!autodocstag behavior in the docs addon.Moving to a floating version tag reduces reproducibility, as different sandbox creation times will install different versions. This could introduce instability if breaking changes land in
@next.Please confirm:
- Is this change intentional and related to this PR?
- If intentional, is it acceptable to use a moving target for sandbox test environments?
code/core/src/csf-tools/CsfFile.test.ts (1)
1825-1980: Excellent test coverage for play and mount with object methods.These tests comprehensively validate that object method syntax works correctly for:
- Story-level play methods with proper
play-fntag addition- Meta-level play methods with tag inheritance
- Mount detection within object method parameters
The tests confirm the implementation handles object methods consistently with arrow functions and function declarations.
code/core/src/csf-tools/CsfFile.ts (2)
177-196: hasMount correctly extended to support object methods.The addition of
t.isObjectMethod(play)properly extends mount detection to handle play functions defined as object methods, maintaining consistency with arrow functions and function declarations. The parameter checking logic works uniformly across all three function types.
353-396: Meta annotation handling correctly supports object methods.Lines 355-356 properly distinguish between object methods and regular properties, storing the method node itself when encountering object method syntax (e.g.,
play({ context }) {}). This enables meta-level object method annotations and maintains consistency with the story-level handling introduced later in the file.code/lib/codemod/package.json (1)
3-3: LGTM: Version bump to RC.0Standard version bump from beta.13 to rc.0 as part of the release candidate cycle.
code/lib/eslint-plugin/package.json (1)
3-3: LGTM: Version bump to RC.0Standard version bump aligned with the broader RC release.
code/renderers/react/package.json (1)
3-3: LGTM: Version bump to RC.0Consistent version bump for the React renderer as part of the RC release.
code/renderers/web-components/package.json (1)
3-3: LGTM: Version bump to RC.0Standard version bump for the web-components renderer.
code/frameworks/svelte-vite/package.json (1)
3-3: LGTM: Version bump to RC.0Standard version bump for the Svelte-Vite framework.
code/presets/create-react-app/package.json (1)
3-3: LGTM: Version bump to RC.0Standard version bump for the create-react-app preset.
code/core/src/components/components/typography/elements/Code.tsx (1)
6-6: Verification complete—lazy module interface is correct.The lazy-syntaxhighlighter.tsx module properly exports
SyntaxHighlighteras a named export, and Code.tsx correctly imports it using matching syntax. The lazy module wraps the real component in Suspense and lazy(), preserving the component interface and the staticregisterLanguagemethod. The import path and export interface align correctly.docs/configure/integration/frameworks-feature-support.mdx (1)
37-37: Jest addon repository link verified as valid.The Jest addon repository exists and is public at https://github.com/storybookjs/addon-jest.
code/lib/csf-plugin/package.json (1)
3-3: LGTM! Version bump to RC release.The version update from beta.13 to rc.0 is consistent with the broader repository-wide RC release.
code/renderers/html/package.json (1)
3-3: LGTM! Version bump to RC release.Consistent version update as part of the RC release cycle.
code/lib/cli-storybook/src/autoblock/block-major-version.ts (1)
79-82: LGTM! Improved documentation link.The update from a version-specific migration guide to the general releases migration guide is more maintainable and provides better guidance for users encountering downgrade scenarios.
code/addons/onboarding/package.json (1)
3-3: LGTM! Version bump to RC release.Routine version update consistent with the RC release.
code/frameworks/angular/package.json (1)
3-3: LGTM! Version bump to RC release.Consistent version update as part of the repository-wide RC release.
code/frameworks/html-vite/package.json (1)
3-3: LGTM! Version bump to RC release.Routine version update aligned with the RC release.
code/core/src/manager-api/version.ts (1)
1-1: LGTM! Version constant updated.The version constant update correctly aligns with the package.json version bumps across the repository.
README.md (1)
127-127: LGTM! Jest addon repository reference updated.The link change reflects the jest addon being maintained in a separate repository, consistent with other external addons in the table.
code/core/src/components/index.ts (1)
40-40: LGTM: Clean refactoring.The export path change correctly reflects the extraction of clipboard functionality to a dedicated module while maintaining the public API.
code/core/src/components/components/syntaxhighlighter/clipboard.ts (1)
26-28: LGTM: Appropriate feature detection.The factory correctly returns the modern API when available and falls back to the workaround otherwise.
code/addons/docs/src/blocks/blocks/Primary.stories.tsx (2)
4-5: LGTM: Imports for new autodocs test cases.The imports correctly reference the new example story files for testing autodocs tag behavior.
65-83: LGTM: Well-structured test stories for autodocs functionality.The new
NoAutodocsExampleandSomeAutodocsExamplestories follow the established pattern and provide appropriate test coverage for the autodocs tag handling fix. The structure is consistent with existing stories and properly references the corresponding CSF files viarelativeCsfPaths.
| async function copyUsingWorkAround(text: string) { | ||
| const tmp = document.createElement('TEXTAREA') as HTMLTextAreaElement; | ||
| const focus = document.activeElement as HTMLTextAreaElement; | ||
|
|
||
| tmp.value = text; | ||
|
|
||
| document.body.appendChild(tmp); | ||
| tmp.select(); | ||
| document.execCommand('copy'); | ||
| document.body.removeChild(tmp); | ||
| focus.focus(); | ||
| } |
There was a problem hiding this comment.
Unsafe type cast and missing null check for focus restoration.
Two issues:
- Line 15:
document.activeElement as HTMLTextAreaElementis an unsafe cast—the active element could be any focusable element (button, input, div with tabindex, etc.). - Line 23:
focus.focus()will throw iffocusis null (e.g., whenactiveElementis the document or body).
Apply this diff to handle the null case safely:
async function copyUsingWorkAround(text: string) {
const tmp = document.createElement('TEXTAREA') as HTMLTextAreaElement;
- const focus = document.activeElement as HTMLTextAreaElement;
+ const focus = document.activeElement as HTMLElement | null;
tmp.value = text;
document.body.appendChild(tmp);
tmp.select();
document.execCommand('copy');
document.body.removeChild(tmp);
- focus.focus();
+ focus?.focus();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function copyUsingWorkAround(text: string) { | |
| const tmp = document.createElement('TEXTAREA') as HTMLTextAreaElement; | |
| const focus = document.activeElement as HTMLTextAreaElement; | |
| tmp.value = text; | |
| document.body.appendChild(tmp); | |
| tmp.select(); | |
| document.execCommand('copy'); | |
| document.body.removeChild(tmp); | |
| focus.focus(); | |
| } | |
| async function copyUsingWorkAround(text: string) { | |
| const tmp = document.createElement('TEXTAREA') as HTMLTextAreaElement; | |
| const focus = document.activeElement as HTMLElement | null; | |
| tmp.value = text; | |
| document.body.appendChild(tmp); | |
| tmp.select(); | |
| document.execCommand('copy'); | |
| document.body.removeChild(tmp); | |
| focus?.focus(); | |
| } |
docs/versions/next.json
Outdated
| @@ -1 +1 @@ | |||
| {"version":"10.0.0-beta.13","info":{"plain":"- CLI: CSF factories codemod - support annotations in npx context - [#32741](https://github.com/storybookjs/storybook/pull/32741), thanks @yannbf!\n- Move: Addon jest into it's own repository - [#32646](https://github.com/storybookjs/storybook/pull/32646), thanks @ndelangen!\n- Upgrade: Enhance ESM compatibility checks and banner generation - [#32694](https://github.com/storybookjs/storybook/pull/32694), thanks @ndelangen!"}} No newline at end of file | |||
| {"version":"10.0.0-rc.0","info":{"plain":"- A11Y: Bugfix missing `manager.js` entry-file - [#32780](https://github.com/storybookjs/storybook/pull/32780), thanks @ndelangen!\n- Addon Vitest: Support modifying mergeConfig on addon setup - [#32753](https://github.com/storybookjs/storybook/pull/32753), thanks @yannbf!\n- CLI: Change message in downgrade-blocker - [#32745](https://github.com/storybookjs/storybook/pull/32745), thanks @ndelangen!"}} No newline at end of file | |||
There was a problem hiding this comment.
Missing changelog entry for the primary bug fix.
The changelog doesn't mention the autodocs tag fix that this PR addresses (issue #32683). According to the PR title and description, this PR "Skip[s] !autodocs stories when computing primary story" and adds a usePrimaryStory hook. This fix should be documented in the changelog.
🤖 Prompt for AI Agents
In docs/versions/next.json around line 1, the changelog is missing the PR's
primary bugfix: add a new bullet describing the autodocs tag fix (issue #32683)
that "Skips `!autodocs` stories when computing primary story" and mentions the
added `usePrimaryStory` hook; update the "info.plain" string by inserting a new
line entry (matching existing bullet style and attribution) summarizing the fix
and linking to the PR/issue so the changelog reflects this change.
- add usePrimaryStory hook to centralize primary story selection - refactor Primary, Controls to use the hook instead of stories[0] - ensure !autodocs stories are correctly hidden in all doc blocks
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/docs/src/blocks/blocks/Primary.tsx (1)
7-12: Component no longer accepts props, but stories still passofargs — breaking change.The
Primarycomponent signature was changed toFC(accepting no props), andusePrimaryStory()does not accept or read any parameters. However, six stories inPrimary.stories.tsxcontinue to passofas args (DefaultWithOf,WithoutToolbarWithOf,DefaultOfStringMetaAttached,WithoutToolbarOfStringMetaAttached,NoAutodocsExample,SomeAutodocsExample). These args are now ignored; the component always uses the context's autodocs-tagged story instead, making these stories non-functional.Either update
Primaryto accept and handle theofprop (as other blocks likeControlsdo viacontext.resolveOf()), or update the stories to remove the unusedofargs.
♻️ Duplicate comments (1)
code/addons/docs/src/blocks/blocks/Primary.stories.tsx (1)
33-83: Stories passofargs but Primary no longer accepts props.Multiple stories (including the two new ones) define
ofin their args, but the Primary component signature was changed to accept no props in Primary.tsx (line 7). These args will be ignored, causing stories likeDefaultWithOf,NoAutodocsExample, andSomeAutodocsExampleto not behave as intended.Either:
- Restore the
ofprop support in Primary.tsx, or- Remove the
ofargs from these stories and update their names/descriptions to reflect that they use the auto-selected primary storyBased on the PR objectives, option 2 seems more aligned with the goal of centralizing primary story selection via
usePrimaryStory().
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx (1)
21-23: Consider type-safe mock context.The mock context only implements
componentStories. While sufficient for current tests, explicitly typing it asPartial<DocsContextProps>(which you do on line 25) helps catch issues ifusePrimaryStorylater accesses other context properties.Consider adding a type assertion in the factory:
-const createMockContext = (storyList: PreparedStory[]) => ({ - componentStories: vi.fn(() => storyList), -}); +const createMockContext = (storyList: PreparedStory[]): Partial<DocsContextProps> => ({ + componentStories: vi.fn(() => storyList), +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
code/addons/docs/src/blocks/blocks/Controls.tsx(2 hunks)code/addons/docs/src/blocks/blocks/Primary.stories.tsx(2 hunks)code/addons/docs/src/blocks/blocks/Primary.tsx(1 hunks)code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx(1 hunks)code/addons/docs/src/blocks/blocks/usePrimaryStory.ts(1 hunks)code/addons/docs/src/blocks/examples/StoriesParameters.stories.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- code/addons/docs/src/blocks/blocks/usePrimaryStory.ts
- code/addons/docs/src/blocks/examples/StoriesParameters.stories.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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 (useyarn lint:js:cmd <file>)
Files:
code/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.test.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/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.test.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/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Primary.stories.tsxcode/addons/docs/src/blocks/blocks/Primary.tsxcode/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx
🧬 Code graph analysis (3)
code/addons/docs/src/blocks/blocks/Controls.tsx (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
usePrimaryStory(11-15)
code/addons/docs/src/blocks/blocks/Primary.tsx (1)
code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
usePrimaryStory(11-15)
code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx (3)
code/core/src/csf-tools/CsfFile.ts (1)
stories(942-944)code/core/src/types/modules/story.ts (1)
PreparedStory(100-116)code/addons/docs/src/blocks/blocks/usePrimaryStory.ts (1)
usePrimaryStory(11-15)
⏰ 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
🔇 Additional comments (2)
code/addons/docs/src/blocks/blocks/Controls.tsx (1)
43-50: LGTM! Clean fallback logic for primary story resolution.The combined resolution strategy correctly prioritizes the explicit
ofprop when provided, then falls back to the hook-based primary story selection. The early return for missing stories is appropriate.code/addons/docs/src/blocks/blocks/usePrimaryStory.test.tsx (1)
30-66: Excellent test coverage for autodocs tag handling.The test suite comprehensively validates all edge cases:
- Filtering out !autodocs tagged stories
- Selecting the first autodocs-tagged story when multiple exist
- Handling missing autodocs tags
- Handling empty story lists
ia319
requested review from
Sidnioulz
and removed request for
jonniebigodes and
kylegach
4 participants
Closes #32683
What I did
Added usePrimaryStory hook to centralize primary story selection.
Refactored Primary.tsx to use usePrimaryStory instead of selecting stories[0].
Refactored Controls.tsx to call usePrimaryStory when no specific story is provided via of.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Translation:
Before modification:
Write a sample sandbox:
From the logs, we can see that the handling of
tagsincode/core/src/preview-api/modules/store/csf/prepareStory.tsis correct.The issue lies in
code/addons/docs/src/blocks/blocks/Primary.tsx,where
const primaryStory = context.componentStoriesFromCSFFile(csfFile)[0];always selects the first story directly.
After modification:
Primary.tsxandControls.tsxnow correctly retrieve the stories with theautodocstag.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
Refactor
Tests
New Features