-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Core: 10.1 features WIP #32810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Core: 10.1 features WIP #32810
Conversation
Fixes screen reader announcing incorrect selected state on non-selected tabs. Added aria-selected and tabIndex attributes to TabButton components to properly communicate tab state to assistive technologies like Windows Narrator. Fixes #32654
fix(a11y): add aria-selected attribute to tab buttons
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
…r/code-snippets # Conflicts: # code/core/src/core-server/dev-server.ts
React: Improve error handling of component manifest generation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
code/renderers/react/src/enrichCsf.ts (2)
10-14: Type mismatch on early return. Return the previous value.PresetPropertyFn must return the property type; returning undefined can break typing.
Apply this diff:
export const enrichCsf: PresetPropertyFn<'experimental_enrichCsf'> = async (input, options) => { const features = await options.presets.apply('features'); - if (!features.experimentalCodeExamples) { - return; - } + if (!features.experimentalCodeExamples) { + return input; + }
90-100: Spread order bug: existing source overrides new snippet.Place the spread first, then set code so the generated snippet wins.
Apply this diff:
t.objectProperty( t.identifier('source'), t.objectExpression([ - t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)), t.spreadElement( t.optionalMemberExpression( docsParameter, t.identifier('source'), false, true ) ), + t.objectProperty(t.identifier('code'), t.stringLiteral(snippet)), ]) ),code/renderers/react/src/componentManifest/generator.test.ts (1)
96-105: Ensure proper test cleanup (beforeEach return is ignored).Move cleanup to afterEach and restore spies; Vitest ignores returns from beforeEach.
Apply:
-import { beforeEach, expect, test, vi } from 'vitest'; +import { beforeEach, afterEach, expect, test, vi } from 'vitest';And at the end of the file (after line 204):
beforeEach(() => { vi.spyOn(process, 'cwd').mockReturnValue('/app'); vol.fromJSON( { // ... file contents ... }, '/app' ); - return () => vol.reset(); }); + +afterEach(() => { + vol.reset(); + vi.restoreAllMocks(); +});
🧹 Nitpick comments (1)
code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
413-503: LGTM! Correct args spread transformation.The function properly expands
{...args}into concrete attributes while:
- Preserving existing attributes and preventing duplicates
- Maintaining correct insertion order (at the first spread location)
- Recursively processing nested JSX
- Handling both elements and fragments
The comma operator usage on line 478 (
((changed = true), toJsxChildren(...))) is functionally correct but could be written more explicitly for clarity.Consider this more explicit alternative for line 478:
- const children = - sawArgsSpread && newChildren.length === 0 && merged.children - ? ((changed = true), toJsxChildren(merged.children)) - : newChildren; + const children = + sawArgsSpread && newChildren.length === 0 && merged.children + ? (changed = true, toJsxChildren(merged.children)) + : newChildren;Note: The comma operator is valid but may be less familiar to some reviewers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks)code/core/src/csf-tools/CsfFile.ts(7 hunks)code/core/src/csf-tools/enrichCsf.test.ts(21 hunks)code/core/src/types/modules/core-common.ts(5 hunks)code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsx(1 hunks)code/renderers/react/src/componentManifest/generateCodeSnippet.ts(1 hunks)code/renderers/react/src/componentManifest/generator.test.ts(1 hunks)code/renderers/react/src/componentManifest/generator.ts(1 hunks)code/renderers/react/src/componentManifest/utils.ts(1 hunks)code/renderers/react/src/enrichCsf.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 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/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generateCodeSnippet.tscode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/csf-tools/enrichCsf.test.ts
**/*.{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/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generateCodeSnippet.tscode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/csf-tools/enrichCsf.test.ts
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/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generateCodeSnippet.tscode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/csf-tools/enrichCsf.test.ts
{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/renderers/react/src/enrichCsf.tscode/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/generator.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/generateCodeSnippet.tscode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/CsfFile.tscode/core/src/csf-tools/enrichCsf.test.ts
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/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.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/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.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/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
🧠 Learnings (26)
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/core/src/types/modules/core-common.tscode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsxcode/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
PR: storybookjs/storybook#32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/types/modules/core-common.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/types/modules/core-common.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
Applied to files:
code/core/src/types/modules/core-common.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/core/src/types/modules/core-common.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/core/src/types/modules/core-common.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
PR: storybookjs/storybook#32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/renderers/react/src/componentManifest/generateCodeSnippet.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should be placed in beforeEach blocks
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the spy: true option
Applied to files:
code/renderers/react/src/componentManifest/generator.test.tscode/core/src/csf-tools/enrichCsf.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/renderers/react/src/componentManifest/generator.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions
Applied to files:
code/core/src/csf-tools/enrichCsf.test.ts
🧬 Code graph analysis (8)
code/renderers/react/src/enrichCsf.ts (5)
code/core/src/csf-tools/enrichCsf.ts (1)
enrichCsf(144-150)code/renderers/react/src/preset.ts (1)
enrichCsf(13-13)code/core/src/types/modules/core-common.ts (1)
PresetPropertyFn(650-655)code/core/src/common/utils/formatter.ts (1)
getPrettier(1-6)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-225)
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsx (2)
code/core/src/csf-tools/CsfFile.ts (1)
loadCsf(1060-1064)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-225)
code/core/src/types/modules/core-common.ts (2)
code/core/src/core-server/index.ts (1)
StoryIndexGenerator(11-11)code/core/src/csf-tools/CsfFile.ts (1)
CsfFile(281-1042)
code/renderers/react/src/componentManifest/generator.ts (6)
code/core/src/types/modules/core-common.ts (2)
ComponentManifest(348-358)ComponentManifestGenerator(365-367)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
DocObj(26-30)getMatchingDocgen(37-59)code/renderers/react/src/componentManifest/utils.ts (2)
groupBy(2-12)invariant(15-23)code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
getCodeSnippet(6-225)code/core/src/csf-tools/enrichCsf.ts (1)
extractDescription(158-178)code/renderers/react/src/componentManifest/jsdocTags.ts (1)
extractJSDocInfo(5-21)
code/renderers/react/src/componentManifest/generateCodeSnippet.ts (2)
code/core/src/csf-tools/CsfFile.ts (2)
CsfFile(281-1042)meta(964-966)code/renderers/react/src/componentManifest/utils.ts (1)
invariant(15-23)
code/renderers/react/src/componentManifest/generator.test.ts (1)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(20-135)
code/core/src/csf-tools/CsfFile.ts (1)
code/frameworks/nextjs/src/font/babel/helpers.ts (1)
ExportNamedDeclaration(148-171)
code/core/src/csf-tools/enrichCsf.test.ts (3)
code/core/src/csf-tools/enrichCsf.ts (1)
EnrichCsfOptions(6-10)code/core/src/csf-tools/CsfFile.ts (2)
loadCsf(1060-1064)formatCsf(1066-1076)code/renderers/react/src/enrichCsf.ts (1)
enrichCsf(10-112)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: merged
- GitHub Check: Publish canary version
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (18)
code/core/src/csf-tools/enrichCsf.test.ts (1)
14-23: LGTM! Clean async migration.The test helper and all test cases are correctly updated to handle the asynchronous enrichCsf flow. The title defaulting change to 'Unknown' aligns with the broader enrichment refactor.
code/renderers/react/src/componentManifest/utils.ts (1)
1-23: LGTM! Well-implemented utilities.Both
groupByandinvariantare clean, idiomatic implementations that follow common patterns for these utilities.code/core/src/types/modules/core-common.ts (2)
348-369: LGTM! Well-structured type definitions.The new manifest types are clean and comprehensive, providing a solid foundation for the component manifest generation feature.
111-112: LGTM! Type-safe preset overload.This overload simplifies type inference for preset consumers and aligns with the new experimental enrichment hooks.
code/core/src/csf-tools/CsfFile.ts (2)
290-301: LGTM! Enhanced path tracking for enrichment.The new fields enable precise AST path tracking required by the async enrichment workflow. The type narrowing of
_metaNodetoObjectExpressionimproves type safety.Also applies to: 307-309
379-390: LGTM! Component import specifier tracking.The enhanced logic correctly captures both named and default import specifiers, enabling downstream tools to match component definitions with their imports.
code/renderers/react/src/componentManifest/generateCodeSnippet.test.tsx (1)
1-569: LGTM! Comprehensive test coverage.The test suite thoroughly exercises code snippet generation across diverse CSF scenarios including CSF1/2/3/4 formats, custom renders, nested elements, and edge cases. The use of inline snapshots provides clear regression protection.
code/renderers/react/src/componentManifest/generator.ts (1)
20-135: LGTM! Robust manifest generator implementation.The generator handles errors gracefully at each step (missing files, parse failures, docgen issues) and properly aggregates metadata from multiple sources (CSF, JSDoc, React docgen). The error handling ensures partial failures don't break the entire manifest generation.
code/renderers/react/src/componentManifest/generateCodeSnippet.ts (10)
1-17: LGTM! Clean imports and error handling.The use of private CsfFile properties (
_storyDeclarationPath,_metaNode, etc.) is appropriate for internal manifest generation tooling, and the error handling with code frames provides helpful debugging context.
19-84: LGTM! Comprehensive story path normalization.The code correctly handles multiple CSF patterns including Template.bind(...), factory functions, CSF4 meta.story(), and TypeScript type assertions. The fallthrough logic for zero-argument calls is handled appropriately in the subsequent code.
86-148: LGTM! Well-structured args collection and validation.The code properly merges meta and story args (with story taking precedence), extracts render functions from multiple locations, and correctly separates valid JSX attribute names from invalid ones that need spread syntax.
150-206: LGTM! Correct function transformation logic.The code properly handles both arrow functions with direct JSX returns and functions with block statements. The transformation order (spreads then inlining) is correct, and the change tracking prevents unnecessary AST modifications.
208-225: LGTM! Appropriate fallback for non-function stories.The synthesized JSX element correctly handles object-only story configurations, and the invariant ensures componentName is available when needed for this code path.
227-274: LGTM! Well-structured helper functions.The utility functions are focused and handle their responsibilities correctly. The JSX attribute name validation regex (line 241) is appropriate for typical React usage, though it doesn't support full Unicode XML names—this is a reasonable practical limitation.
276-302: LGTM! Correct JSX attribute and children conversion.The conversion functions properly handle boolean shorthand (
fooinstead offoo={true}), string literals, expression containers, and various children node types.
304-331: LGTM! Comprehensive args member detection.The function correctly identifies both regular and optional chaining member access patterns (
args.foo,args["foo"],args?.foo,args?.["foo"]). The optional chaining on line 316's type check (t.isOptionalMemberExpression?.()) is defensive against older Babel versions.
333-411: LGTM! Correct recursive args inlining.The function properly inlines
args.fooreferences with their actual values throughout the JSX tree, handles both elements and fragments recursively, and correctly tracks whether any changes were made. The self-closing element logic is sound.
505-561: LGTM! Well-implemented AST utilities.Both
resolveBindIdentifierInitandpathForNodeare correctly implemented:
resolveBindIdentifierInitproperly searches the module scope for Template variable declarations, handling both plain and exported variablespathForNodeefficiently traverses the AST with early exit viap.stop()once the target is found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/core-server/build-static.ts (1)
168-188: Investigate the type cast necessity.The type cast on line 176 uses the
as unknown aspattern to bypass type checking when passingindexGeneratorto the manifest generator. This suggests a type incompatibility between the localStoryIndexGeneratorclass and the type expected byComponentManifestGenerator.While this works, it's a code smell that may indicate:
- Duplicate or divergent
StoryIndexGeneratortype definitions- The generator interface should accept a more general type
- A structural issue in the type system
Consider unifying the types or adjusting the
ComponentManifestGeneratorsignature to avoid the double-cast.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/core/src/core-server/build-static.ts(3 hunks)code/core/src/core-server/presets/favicon.test.ts(2 hunks)code/renderers/react/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- code/renderers/react/package.json
- code/core/src/core-server/presets/favicon.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/core/src/core-server/build-static.ts
**/*.{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/core/src/core-server/build-static.ts
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/core/src/core-server/build-static.ts
{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/core/src/core-server/build-static.ts
🧠 Learnings (3)
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/core/src/core-server/build-static.ts
🧬 Code graph analysis (1)
code/core/src/core-server/build-static.ts (3)
code/renderers/react/src/componentManifest/generator.ts (1)
componentManifestGenerator(20-135)code/renderers/react/src/preset.ts (1)
componentManifestGenerator(11-11)code/core/src/types/modules/core-common.ts (1)
ComponentManifestGenerator(365-367)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: merged
- GitHub Check: Publish canary version
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/core-server/build-static.ts (2)
1-1: LGTM! Imports support the new manifest generation feature.The added imports (
mkdir,writeFile, andComponentManifestGeneratortype) are appropriately used in the experimental component manifest generation code.Also applies to: 14-14
174-186: Good error handling for experimental feature.The try-catch appropriately logs errors without failing the build, which is suitable for an experimental feature. The use of
logger.error()correctly follows the coding guidelines for server-side code.Based on learnings
Feature branch for any 10.1 development.
Will be merged when 10.0 has been released and thus frees up the
nextbranch.🦋 Canary release
This pull request has been released as version
0.0.0-pr-32810-sha-caef1f25. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-32810-sha-caef1f2510.1-with-canary-releasecaef1f251761856434)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=32810