Core: Improve story generation and args extraction#33395
Conversation
|
View your CI Pipeline Execution ↗ for commit 9822b1d
☁️ Nx Cloud last updated this comment at |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a centralized story-file generator and ArgTypes extraction pipeline: new generateStoryFile utility, React DocGen -> SBType mapping, dummy-prop generation, template improvements, and refactors create-new-story-channel and Manager UI to consume the new utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Manager as Manager UI
participant Channel as create-new-story-channel
participant Generator as generateStoryFile
participant Docgen as React DocGen / ArgTypes helpers
participant FS as File System
Manager->>Channel: CREATE_NEW_STORYFILE_REQUEST(payload)
Channel->>Generator: generateStoryFile(payload, options)
Generator->>Docgen: extractArgTypesFromDocgen? (when applicable)
Generator->>FS: existsSync(storyFilePath) (if check enabled)
alt file exists
Generator-->>Channel: { success: false, errorType: STORY_FILE_EXISTS, error }
else file missing or check disabled
Generator->>FS: writeFile(storyFilePath, content)
FS-->>Generator: write success
Generator-->>Channel: { success: true, storyId, kind, storyFilePath, exportedStoryName }
end
Channel-->>Manager: CREATE_NEW_STORYFILE_RESPONSE(result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
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: 3
🧹 Nitpick comments (9)
code/renderers/react/src/componentManifest/utils.ts (1)
93-94: Avoid explicitanytype annotation; prefer preserving type inference.The
cachedfunction is generic and should preserve the type ofresolveImport. Using: anydisables type safety for all consumers ofcachedResolveImport. If there's a type inference issue, consider using a type assertion or explicit generic parameter instead.🔎 Suggested fix to preserve type safety
-export const cachedResolveImport: any = cached(resolveImport, { name: 'resolveImport' }); +export const cachedResolveImport = cached(resolveImport, { name: 'resolveImport' });If type inference fails due to overloads or complex signatures, consider:
export const cachedResolveImport = cached<[string, resolve.SyncOpts], string>( resolveImport, { name: 'resolveImport' } );code/core/src/core-server/utils/get-new-story-file.test.ts (1)
129-163: Inline mock implementations should be moved tobeforeEachor use test-specific setup.Per coding guidelines, mock implementations should be defined in
beforeEachblocks rather than inline within test cases. Consider restructuring to use nesteddescribeblocks with dedicatedbeforeEachfor tests requiring specific mock returns.🔎 Suggested refactor pattern
+ describe('with placeholder props', () => { + beforeEach(() => { + vi.mocked(generateMockPropsFromDocgen).mockReturnValue({ + required: { + onClick: '__function__', + children: '__react_node__', + }, + } as any); + }); + - it('replaces __function__ and __react_node__ placeholders via AST and adds fn import', async () => { - vi.mocked(generateMockPropsFromDocgen).mockReturnValue({ - required: { - onClick: '__function__', - children: '__react_node__', - }, - } as any); + it('replaces __function__ and __react_node__ placeholders via AST and adds fn import', async () => { + // ... test body without mock setup + }); + });As per coding guidelines.
code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (1)
29-32: Multi-line JSON may misalign in the template output.
JSON.stringify(data.args, null, 2)produces multi-line output with 2-space indentation starting from column 0. When embedded in thededenttemplate at line 43, subsequent lines won't align with the surrounding code structure.For simple/flat args this is fine, but nested objects will produce:
export const Default = meta.story({ args: { "nested": { "key": "value" } } });🔎 Proposed fix to align nested JSON
const argsString = data.args && Object.keys(data.args).length > 0 - ? `{ args: ${JSON.stringify(data.args, null, 2)} }` + ? `{ args: ${JSON.stringify(data.args, null, 2).replace(/\n/g, '\n ')} }` : '{}';Alternatively, consider using a single-line format for smaller args objects, or a dedicated pretty-print utility that respects target indentation.
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
142-146: Error response assertion may be incomplete.The production code at line 50-56 sets
payload: undefinedfor non-STORY_FILE_EXISTSerrors. The test assertion doesn't include this field. While Vitest'stoHaveBeenCalledWithmay still pass (depending on matching behavior), explicitly assertingpayload: undefinedwould make the test more precise.🔎 Suggested assertion update
expect(createNewStoryFileEventListener).toHaveBeenCalledWith({ error: 'Failed to write file', id: 'components-page--default', + payload: undefined, success: false, });code/renderers/react/src/componentManifest/getComponentProps.test.ts (1)
42-43: Multiple@ts-expect-errorcomments indicate type gaps.The "fix this later" comments suggest the
reactDocgenresult type doesn't properly expose.datawhentype === 'success'. Consider using a discriminated union or type guard to eliminate these suppressions.Would you like me to help design a proper discriminated union type for the
reactDocgenreturn value to eliminate these type suppressions?Also applies to: 75-76, 108-109, 113-114, 193-194, 293-294
code/core/src/core-server/utils/get-new-story-file.ts (1)
58-60: Consider logging the caught error for debugging purposes.The empty catch block silently swallows errors. While the fallback to CSF3 is acceptable, logging the error at debug level would help diagnose issues.
🔎 Proposed improvement
- } catch { + } catch (error) { + logger.debug(`Could not determine CSF factory usage: ${error}`); // TODO: improve this later on, for now while CSF factories are experimental, just fallback to CSF3 }code/core/src/core-server/utils/generate-story.ts (1)
71-89: Potential race condition: check-then-write pattern.The file existence check at line 81 and the write at line 92 are not atomic. Between
existsSyncandwriteFile, another process could create the file. For story generation this is likely low-risk, but worth noting.Consider using
writeFilewith exclusive flag (wx) to atomically fail if the file exists, which would eliminate the race condition:// Alternative approach using exclusive flag await writeFile(storyFilePath, formattedContent, { encoding: 'utf-8', flag: 'wx' });Note: This would require adjusting error handling to catch
EEXISTerrors.code/renderers/react/src/componentManifest/getComponentProps.ts (2)
152-155: Redundant regex check for image detection.The third condition on line 155
/(image|img|photo|avatar|logo)/i.test(name)is redundant - it will always be true if either of the first two conditions (prefix/suffix checks) are true. The comment suggests "weak match only if no url keywords" but the logic doesn't enforce that.🔎 Suggested simplification
- const isImage = - /^(image|img|photo|avatar|logo)/.test(name) || - /(image|img|photo|avatar|logo)$/i.test(name) || - /(image|img|photo|avatar|logo)/i.test(name); + // Match if the prop name contains image-related keywords + const isImage = /(image|img|photo|avatar|logo)/i.test(name);If you intended different behavior (e.g., prioritizing exact prefix/suffix matches), the logic would need adjustment.
250-254: Tuple mock generation loses type information.For tuples, the current implementation converts all elements to strings regardless of their actual types. This could produce incorrect mock values for tuples like
[number, boolean].🔎 Proposed fix to preserve tuple element types
case 'tuple': if (elements && elements.length > 0) { - return elements.map((_: any) => generateMockValue({ name: 'string' })); + return elements.map((el: any) => generateMockValue(el)); } return [];
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
code/core/src/core-server/index.ts(1 hunks)code/core/src/core-server/server-channel/create-new-story-channel.test.ts(1 hunks)code/core/src/core-server/server-channel/create-new-story-channel.ts(3 hunks)code/core/src/core-server/utils/generate-story.ts(1 hunks)code/core/src/core-server/utils/get-mocked-props-for-args.test.ts(1 hunks)code/core/src/core-server/utils/get-mocked-props-for-args.ts(1 hunks)code/core/src/core-server/utils/get-new-story-file.test.ts(2 hunks)code/core/src/core-server/utils/get-new-story-file.ts(4 hunks)code/core/src/core-server/utils/new-story-templates/csf-factory-template.test.ts(1 hunks)code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts(2 hunks)code/core/src/core-server/utils/new-story-templates/javascript.ts(2 hunks)code/core/src/core-server/utils/new-story-templates/typescript.ts(3 hunks)code/renderers/react/src/componentManifest/getComponentProps.test.ts(1 hunks)code/renderers/react/src/componentManifest/getComponentProps.ts(1 hunks)code/renderers/react/src/componentManifest/utils.ts(1 hunks)code/renderers/react/src/preset.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/renderers/react/src/componentManifest/utils.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/renderers/react/src/componentManifest/utils.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/utils.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/renderers/react/src/componentManifest/utils.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/renderers/react/src/componentManifest/utils.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
🧠 Learnings (29)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/preset.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/server-channel/create-new-story-channel.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.test.tscode/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/core-server/index.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/preset.ts
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.tscode/core/src/core-server/utils/get-mocked-props-for-args.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-mocked-props-for-args.tscode/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/new-story-templates/csf-factory-template.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/renderers/react/src/componentManifest/getComponentProps.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Place all mocks at the top of the test file before any test cases
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 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/renderers/react/src/preset.ts
🧬 Code graph analysis (10)
code/renderers/react/src/componentManifest/utils.ts (1)
code/core/src/common/utils/interpret-files.ts (1)
resolveImport(25-58)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
stories(968-970)
code/core/src/core-server/utils/generate-story.ts (3)
code/core/src/core-server/index.ts (3)
GenerateStoryResult(13-13)GenerateStoryOptions(13-13)generateStoryFile(12-12)code/core/src/core-server/utils/get-new-story-file.ts (1)
getNewStoryFile(26-132)code/core/src/common/utils/formatter.ts (1)
formatFileContent(51-69)
code/renderers/react/src/componentManifest/getComponentProps.ts (2)
code/renderers/react/src/componentManifest/reactDocgen.ts (3)
getReactDocgen(238-268)parseWithReactDocgen(85-95)DocObj(26-30)code/renderers/react/src/componentManifest/utils.ts (1)
cachedReadFileSync(89-89)
code/renderers/react/src/componentManifest/getComponentProps.test.ts (2)
code/renderers/react/src/componentManifest/fixtures.ts (1)
fsMocks(3-108)code/renderers/react/src/componentManifest/getComponentProps.ts (2)
getComponentDocgen(21-63)generateMockProps(107-128)
code/core/src/core-server/utils/get-new-story-file.ts (3)
code/core/src/core-server/utils/get-mocked-props-for-args.ts (2)
ComponentDocgenData(29-31)generateMockPropsFromDocgen(238-257)code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (1)
getCsfFactoryTemplateForNewStoryFile(18-45)code/core/src/core-server/utils/new-story-templates/javascript.ts (1)
getJavaScriptTemplateForNewStoryFile(16-45)
code/core/src/core-server/utils/new-story-templates/csf-factory-template.test.ts (1)
code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (1)
getCsfFactoryTemplateForNewStoryFile(18-45)
code/core/src/core-server/server-channel/create-new-story-channel.ts (2)
code/core/src/core-server/index.ts (1)
generateStoryFile(12-12)code/core/src/core-server/utils/generate-story.ts (1)
generateStoryFile(64-108)
code/renderers/react/src/preset.ts (2)
code/core/src/core-server/index.ts (2)
ComponentDocgenPropType(16-16)ComponentDocgenData(15-15)code/renderers/react/src/componentManifest/getComponentProps.ts (1)
getComponentDocgen(21-63)
code/core/src/core-server/utils/new-story-templates/typescript.ts (1)
scripts/utils/tools.ts (1)
dedent(119-119)
🪛 Biome (2.1.2)
code/renderers/react/src/componentManifest/getComponentProps.ts
[error] 257-257: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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). (4)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (31)
code/core/src/core-server/utils/get-mocked-props-for-args.test.ts (1)
1-149: LGTM! Comprehensive test coverage for mock value generation.The test suite thoroughly covers:
- Primitive types (boolean, number, null, void, any, unknown)
- String token heuristics for semantic prop names
- React node handling with renderer-specific behavior
- Complex types (objects, unions, arrays, tuples)
- Edge cases for empty/missing docgen data
Tests are well-organized and follow vitest best practices.
code/core/src/core-server/utils/new-story-templates/javascript.ts (1)
24-32: Approve args generation logic; consider JSON serialization edge cases.The implementation correctly generates the story export with optional args. Note that
JSON.stringifycannot serialize functions orundefinedvalues - these will be omitted or converted tonull. This aligns with the__function__placeholder pattern used elsewhere in this PR, where special values are replaced via AST post-processing.Verify that all arg values passed to this template are JSON-serializable (primitives, objects, arrays) or use placeholder strings for non-serializable types.
code/core/src/core-server/utils/new-story-templates/csf-factory-template.test.ts (1)
27-68: LGTM! Good test coverage for named imports and custom preview paths.The new tests appropriately extend coverage for:
- Named import syntax with non-default exports
- Custom
previewImportPathconfigurationThe inline snapshots correctly reflect the expected template output based on the implementation in
csf-factory-template.ts.code/core/src/core-server/utils/new-story-templates/typescript.ts (1)
14-15: LGTM! Consistent args support for TypeScript template.The implementation mirrors the JavaScript template approach while maintaining TypeScript-specific type annotations (
: Story). The conditional logic for args inclusion is clean and follows the established pattern.Also applies to: 26-34
code/core/src/core-server/utils/get-new-story-file.test.ts (1)
165-195: Test correctly validates placeholder replacement without fn import.The test appropriately verifies that
__react_node__placeholders are replaced without adding unnecessaryfnimports when no function placeholders exist. Same inline mock pattern suggestion applies here.code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (2)
12-16: LGTM on the interface extension.The new optional properties
previewImportPathandargscleanly extend the data interface to support custom preview paths and dynamic args generation.
25-27: Template generation looks correct.The conditional preview import and args string integration are well-structured. The template correctly uses the dynamic
previewImportandargsStringvariables.Also applies to: 34-44
code/core/src/core-server/server-channel/create-new-story-channel.ts (2)
50-58: Error response structure looks correct.The conditional payload for
STORY_FILE_EXISTSerrors correctly includestypeandkind, while other errors returnundefinedpayload. The error message fallback to'Unknown error occurred'is appropriate.
27-66: Good refactor to centralized story generation.The shift from direct file operations to
generateStoryFileimproves code organization and error handling consistency. The structured result-based flow is cleaner than the previous approach.code/core/src/core-server/index.ts (1)
12-17: New public API exports look appropriate.The exports extend the core-server API surface with story generation utilities and docgen types. This follows the existing pattern of re-exporting from internal utils and aligns with the coding guideline to export functions that need to be tested.
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (2)
20-25: Mocks provide deterministic test fixtures for the new flow.The added mocks for
extractFrameworkPackageName,normalizeStories, andgetStoryIdalign with the centralizedgenerateStoryFileworkflow. ThenormalizeStoriesfallback pattern is a good defensive approach.
54-100: Test coverage for success path looks good.The test correctly verifies the emitted response structure including
storyId,storyFilePath, andexportedStoryNamefrom the successful story generation flow.code/renderers/react/src/componentManifest/getComponentProps.test.ts (3)
9-12: Good test setup pattern.The
beforeEachcorrectly initializes the in-memory filesystem and mocksprocess.cwd. This follows the coding guidelines for implementing mock behaviors inbeforeEachblocks.
15-195: Comprehensive coverage forgetComponentDocgen.The tests thoroughly cover various component patterns: named exports, default exports, multiple exports, non-existent components, utility files, file errors, and class components. Good edge case coverage.
742-1111: ThoroughgenerateMockPropstest coverage.The tests cover a wide range of prop types including primitives, special string patterns (color, date), React nodes, functions, unions, arrays, tuples, records, event types, and edge cases. The real-world
DetailedButtonexample at the end provides good integration-level validation.code/core/src/core-server/utils/get-new-story-file.ts (4)
1-24: LGTM on imports and module structure.The imports are well-organized, correctly using
storybook/internal/node-loggerfor server-side logging as per coding guidelines. The new dependencies (empathic/walk, csf-tools helpers) are appropriately imported for the added functionality.
62-79: Good defensive implementation with proper logging.The docgen integration correctly uses a try/catch block with debug logging for both success and failure cases. The approach of proceeding without args on failure is appropriate for graceful degradation.
156-173: Well-structured helper with safe defaults.The
checkForImportsMapfunction properly walks up the directory tree and handles errors gracefully by returningfalse. The use ofwalk.upwith thelastoption correctly bounds the traversal.
179-203: Clean AST-based placeholder replacement.The function efficiently short-circuits when no placeholders exist, uses Babel traverse for safe AST manipulation, and correctly injects the
fnimport only when needed. The logic is sound.code/core/src/core-server/utils/generate-story.ts (2)
11-19: Well-designed result type with clear discriminated union.The
GenerateStoryResultinterface provides a clean API with typed error codes, making it easy for consumers to handle different failure scenarios.
101-107: Good catch-all error handling.The error handling correctly catches any exception and returns a structured error result. Using
e?.messagewith a fallback handles edge cases where the error might not have a message property.code/renderers/react/src/componentManifest/getComponentProps.ts (1)
21-63: Solid implementation with good fallback logic.The function handles multiple component name matching strategies (actualName, displayName, exportName) and returns a consistent shape. Error handling logs at debug level as appropriate.
code/core/src/core-server/utils/get-mocked-props-for-args.ts (4)
6-22: Well-designed discriminated union type.The
ComponentDocgenPropTypeprovides a clean, exhaustive representation of prop types with proper discriminants (kind). The TODO comment for extending node renderer support is noted.
34-49: Clean tokenization implementation.The tokenization handles camelCase, snake_case, and kebab-case naming conventions properly. The
hasAnyhelper provides a clean set membership check.
74-108: Thoughtful scoring system for type inference.The weighted scoring with negative penalties for metadata-like tokens (e.g.,
imageWidth,imageCount) is a good approach to reduce false positives for image URL generation.
125-236: Comprehensive mock value generation with proper switch exhaustiveness.The function handles all
ComponentDocgenPropTypevariants. The recursive handling for objects and proper literal normalization are well implemented.code/renderers/react/src/preset.ts (5)
36-46: Correct type guard implementations.The type guards properly narrow the union types for elements, object signatures, and literals using appropriate discriminant checks.
48-138: Comprehensive type mapping with good React-specific handling.The
mapReactDocgenTypefunction covers React-specific types (JSX.Element, ComponentType, ReactNode, etc.) and maps them appropriately. The conservative fallback to{ kind: 'other', name }for unknown types is a safe default.
88-120: Good defensive handling for array element types.The check for unresolved custom types (lines 97-118) ensures that complex custom types don't produce incorrect mock data. Mapping them as
'other'allows consumers to handle them appropriately.
144-175: Clean renderer-to-core integration hook.The
getDocgenDatafunction provides a clean abstraction that allows core to extract docgen data without importing renderer packages directly. The early returns for missing data are appropriate.
219-228: Improved function signature with default parameter.The updated
resolvedReactsignature withexisting: Record<string, unknown> = {}is cleaner than checking for undefined internally.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 181 KB | 🚨 +181 KB 🚨 |
| Dependency size | 0 B | 2.98 MB | 🚨 +2.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 18 | 🚨 +18 🚨 |
| Self size | 0 B | 1.64 MB | 🚨 +1.64 MB 🚨 |
| Dependency size | 0 B | 9.25 MB | 🚨 +9.25 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 14 KB | 🚨 +14 KB 🚨 |
| Dependency size | 0 B | 5 KB | 🚨 +5 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-onboarding
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 331 KB | 🚨 +331 KB 🚨 |
| Dependency size | 0 B | 673 B | 🚨 +673 B 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook-addon-pseudo-states
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 21 KB | 🚨 +21 KB 🚨 |
| Dependency size | 0 B | 692 B | 🚨 +692 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-themes
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 378 KB | 🚨 +378 KB 🚨 |
| Dependency size | 0 B | 338 KB | 🚨 +338 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 17 | 🚨 +17 🚨 |
| Self size | 0 B | 125 KB | 🚨 +125 KB 🚨 |
| Dependency size | 0 B | 2.00 MB | 🚨 +2.00 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 75 KB | 🚨 +75 KB 🚨 |
| Dependency size | 0 B | 32.21 MB | 🚨 +32.21 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 49 | 🚨 +49 🚨 |
| Self size | 0 B | 20.19 MB | 🚨 +20.19 MB 🚨 |
| Dependency size | 0 B | 16.52 MB | 🚨 +16.52 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 118 KB | 🚨 +118 KB 🚨 |
| Dependency size | 0 B | 30.40 MB | 🚨 +30.40 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 196 | 🚨 +196 🚨 |
| Self size | 0 B | 15 KB | 🚨 +15 KB 🚨 |
| Dependency size | 0 B | 28.93 MB | 🚨 +28.93 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 22 KB | 🚨 +22 KB 🚨 |
| Dependency size | 0 B | 2.16 MB | 🚨 +2.16 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 538 | 🚨 +538 🚨 |
| Self size | 0 B | 646 KB | 🚨 +646 KB 🚨 |
| Dependency size | 0 B | 59.17 MB | 🚨 +59.17 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 127 | 🚨 +127 🚨 |
| Self size | 0 B | 1.12 MB | 🚨 +1.12 MB 🚨 |
| Dependency size | 0 B | 21.79 MB | 🚨 +21.79 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 13 KB | 🚨 +13 KB 🚨 |
| Dependency size | 0 B | 2.15 MB | 🚨 +2.15 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 159 | 🚨 +159 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 22.97 MB | 🚨 +22.97 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 117 | 🚨 +117 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 19.58 MB | 🚨 +19.58 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 278 | 🚨 +278 🚨 |
| Self size | 0 B | 24 KB | 🚨 +24 KB 🚨 |
| Dependency size | 0 B | 44.10 MB | 🚨 +44.10 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 204 | 🚨 +204 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 33.47 MB | 🚨 +33.47 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 24 | 🚨 +24 🚨 |
| Self size | 0 B | 55 KB | 🚨 +55 KB 🚨 |
| Dependency size | 0 B | 26.82 MB | 🚨 +26.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 25 | 🚨 +25 🚨 |
| Self size | 0 B | 56 KB | 🚨 +56 KB 🚨 |
| Dependency size | 0 B | 26.88 MB | 🚨 +26.88 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 114 | 🚨 +114 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 43.98 MB | 🚨 +43.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 21 | 🚨 +21 🚨 |
| Self size | 0 B | 19 KB | 🚨 +19 KB 🚨 |
| Dependency size | 0 B | 2.19 MB | 🚨 +2.19 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 183 | 🚨 +183 🚨 |
| Self size | 0 B | 775 KB | 🚨 +775 KB 🚨 |
| Dependency size | 0 B | 67.25 MB | 🚨 +67.25 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 176 | 🚨 +176 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 65.82 MB | 🚨 +65.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/core-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 11 KB | 🚨 +11 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 50 | 🚨 +50 🚨 |
| Self size | 0 B | 999 KB | 🚨 +999 KB 🚨 |
| Dependency size | 0 B | 36.71 MB | 🚨 +36.71 MB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/csf-plugin
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 9 | 🚨 +9 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.26 MB | 🚨 +1.26 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
eslint-plugin-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 131 KB | 🚨 +131 KB 🚨 |
| Dependency size | 0 B | 2.82 MB | 🚨 +2.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 791 B | 🚨 +791 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 68 | 🚨 +68 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 5.98 MB | 🚨 +5.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 170 | 🚨 +170 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 31.24 MB | 🚨 +31.24 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 10 | 🚨 +10 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.20 MB | 🚨 +1.20 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 29 KB | 🚨 +29 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 57 | 🚨 +57 🚨 |
| Self size | 0 B | 723 KB | 🚨 +723 KB 🚨 |
| Dependency size | 0 B | 12.92 MB | 🚨 +12.92 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 8 KB | 🚨 +8 KB 🚨 |
| Dependency size | 0 B | 716 KB | 🚨 +716 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 45 KB | 🚨 +45 KB 🚨 |
| Dependency size | 0 B | 230 KB | 🚨 +230 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 55 KB | 🚨 +55 KB 🚨 |
| Dependency size | 0 B | 211 KB | 🚨 +211 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 41 KB | 🚨 +41 KB 🚨 |
| Dependency size | 0 B | 47 KB | 🚨 +47 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
code/core/src/core-server/utils/generate-story.ts (2)
5-5: Remove unused importformatFileContent.The
formatFileContentfunction is imported but never used in this file. It appears formatting is already handled ingetNewStoryFile.🔎 Proposed fix
-import { formatFileContent, getStoryId } from 'storybook/internal/common'; +import { getStoryId } from 'storybook/internal/common';
100-105: Consider logging errors for observability.While returning a structured error result is appropriate, logging the error would help with debugging and observability. As per coding guidelines, use the server-side logger from
storybook/internal/node-loggerfor Node.js code.🔎 Proposed fix
+import { logger } from 'storybook/internal/node-logger'; + // ... at line 100 } catch (e: any) { + logger.error(`Failed to generate story file: ${e?.message || 'Unknown error'}`); return { success: false, error: e?.message || 'Unknown error occurred', errorType: 'UNKNOWN', }; }code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx (1)
161-186: Consider logging errors in the catch block.The empty catch block at line 185 silently swallows any errors that occur during arg types extraction or story saving. While this may be intentional to not block story creation on optional functionality, logging would help with debugging.
As per coding guidelines, use the client-side logger from
storybook/internal/client-loggerfor browser code.🔎 Proposed fix
+import { logger } from 'storybook/internal/client-logger'; + // ... at line 185 - } catch (e) {} + } catch (e) { + logger.debug('Failed to extract arg types or save story args:', e); + }code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts (1)
42-43: Consider improving type safety instead of@ts-expect-error.Multiple occurrences of
@ts-expect-errorwith "fix this later" comments suggest a typing issue with accessingreactDocgen.data. Consider adding a type guard or narrowing the discriminated union properly.🔎 Example type guard approach
// Helper to narrow the discriminated union function assertSuccess(reactDocgen: ReturnType<typeof getComponentDocgen>['reactDocgen']): asserts reactDocgen is { type: 'success'; data: /* DocgenData type */ } { if (reactDocgen.type !== 'success') { throw new Error('Expected success type'); } } // In test: const result = getComponentDocgen(componentFilePath); expect(result).not.toBeNull(); assertSuccess(result!.reactDocgen); expect(result.reactDocgen.data.displayName).toBe('Button'); // No @ts-expect-error neededcode/core/src/core-server/utils/get-new-story-file.ts (1)
59-61: Consider logging when CSF factory detection fails.The empty catch block silently ignores errors when detecting CSF factory preview. Adding a debug log would help with troubleshooting.
🔎 Proposed fix
- } catch { + } catch (error) { + logger.debug(`Could not detect CSF factory preview: ${error}`); // TODO: improve this later on, for now while CSF factories are experimental, just fallback to CSF3 }code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
125-127: Consider handling undefinednamein the default case.If
docgenType.nameis undefined (which TypeScript allows based on theTypeDescriptorinterface), the returned object will havename: undefined. Consider providing a fallback value for clarity.🔎 Proposed fix
default: - return { kind: 'other', name }; + return { kind: 'other', name: name ?? 'unknown' };
87-108: Array element type mapping may differ from standalone type mapping.The known types list excludes React-specific types like
ReactNode,JSX.Element, etc. This meansArray<ReactNode>gets mapped as{ kind: 'array', element: { kind: 'other', name: 'ReactNode' } }, whereas standaloneReactNodemaps to{ kind: 'node', renderer: 'react' }.If this inconsistency is intentional (per the "conservative" comment), consider adding a brief comment explaining why React types are excluded from the array element handling.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
code/core/src/core-server/utils/generate-story.ts(1 hunks)code/core/src/core-server/utils/get-new-story-file.test.ts(7 hunks)code/core/src/core-server/utils/get-new-story-file.ts(4 hunks)code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts(1 hunks)code/renderers/react/src/preset.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/renderers/react/src/preset.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
🧠 Learnings (26)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 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/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/core-server/utils/generate-story.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
🧬 Code graph analysis (4)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts (1)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (1)
getComponentDocgen(145-187)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)parseWithReactDocgen(85-95)code/renderers/react/src/componentManifest/utils.ts (1)
cachedReadFileSync(89-89)
code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx (1)
code/renderers/react/template/stories/js-argtypes.stories.jsx (1)
argTypes(41-41)
code/core/src/core-server/utils/generate-story.ts (1)
code/core/src/core-server/utils/get-new-story-file.ts (1)
getNewStoryFile(27-136)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (16)
code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx (1)
161-162: LGTM on the renderer-specific branching.The conditional logic to skip arg-types extraction for React (which now uses docgen-based flow) is well-structured. The comment clearly explains the reasoning.
code/core/src/core-server/utils/get-new-story-file.test.ts (4)
16-21: Good use of spy-based mocks.The mock setup correctly uses
vi.mock()with the{ spy: true }option as per coding guidelines. Grouping related mocks together improves readability.
24-33: Well-structured beforeEach for default mock behaviors.The default mock implementations in
beforeEachfollow the coding guidelines pattern. This ensures each test starts with a clean, predictable state.
148-180: Test for__function__placeholder replacement is comprehensive.The test validates both the import injection (
fnfromstorybook/test) and the placeholder replacement. The inline mock is acceptable here since it's test-specific configuration that differs from the default.
182-254: CSF factory test thoroughly exercises the integration.The test covers the CSF factory workflow including:
- Preview config detection
- Imports map checking
- Mock props generation with various types
- Import path resolution
The inline snapshot provides good regression protection.
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts (2)
9-12: Good test setup with memfs.The
beforeEachproperly mocksprocess.cwdand populates the in-memory filesystem. Usingvol.fromJSONwith fixtures provides a clean, isolated test environment.
197-739: Comprehensive prop type extraction test.The large inline snapshot thoroughly validates the docgen prop extraction for various TypeScript types including unions, tuples, objects, arrays, functions, and generics. While verbose, this provides excellent regression protection for the docgen integration.
code/core/src/core-server/utils/get-new-story-file.ts (4)
15-15: Good: Using server-side logger as per coding guidelines.The import of
loggerfromstorybook/internal/node-loggerfollows the coding guidelines for Node.js code.
63-81: Well-structured docgen integration with proper error handling.The docgen-based mock props generation:
- Uses debug logging for both success and failure paths
- Gracefully falls back to no args on error
- Only sets args when required props exist
This is a robust implementation.
160-177: Clean implementation ofcheckForImportsMap.The function properly walks up the directory tree using
empathic/walk, checks forpackage.jsonfiles, and returnstrueif animportsfield exists. The silent catch fallback is appropriate here since this is best-effort detection.
183-207: Efficient AST-based placeholder replacement.The implementation:
- Has early return optimization to skip parsing when no placeholders exist
- Uses babel traverse for reliable AST manipulation
- Properly injects the
fnimport only when neededThis is a clean approach for handling non-primitive arg placeholders.
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (5)
1-14: LGTM!Imports are well-organized with proper use of type-only imports. Logger usage from
storybook/internal/node-loggerfollows the coding guidelines for Node.js code.
16-24: LGTM!Type definitions are clear and appropriately scoped. The type aliases for react-docgen types improve readability throughout the module.
26-36: LGTM!Type guards are correctly implemented and provide safe type narrowing for the mapping function.
145-187: LGTM!The function is well-documented with JSDoc and has appropriate error handling. The logic for finding components by name with fallback to first component is clear.
One minor consideration: the OR chains at lines 168 and 180 would treat empty strings as falsy, falling through to the next option. This is likely fine since component names are typically non-empty, but worth noting.
189-214: LGTM!The extraction logic is clean and handles the case where props may lack
tsTypeinformation gracefully by skipping them. The null check at line 195 properly guards against missing docgen data.
yannbf
force-pushed
the
yann/refactor-story-generation
branch
from
11f3e03 to
0d925b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
code/core/src/core-server/utils/generate-story.ts (1)
77-77: Replaceprocess.cwd()withgetProjectRoot()for consistent path resolution.The relative path calculation uses
process.cwd(), butgetNewStoryFile()buildsstoryFilePathusinggetProjectRoot()(see relevant code snippet at line 102). This inconsistency can cause incorrect relative paths when the Node.js process is launched from a directory other than the project root.Import
getProjectRootfrom'storybook/internal/common'and use it here:import { formatFileContent, + getProjectRoot, getStoryId, } from 'storybook/internal/common';- const relativeStoryFilePath = relative(process.cwd(), storyFilePath); + const relativeStoryFilePath = relative(getProjectRoot(), storyFilePath);This issue was previously identified in past reviews.
🧹 Nitpick comments (6)
code/core/src/core-server/utils/get-new-story-file.ts (2)
52-61: Consider logging the error in the catch block.The empty catch block silently swallows errors when detecting CSF factory preview. While the fallback to CSF3 is reasonable, adding a debug log would help troubleshoot issues with preview config detection.
} catch { + logger.debug(`Could not detect CSF factory preview: ${error}`); // TODO: improve this later on, for now while CSF factories are experimental, just fallback to CSF3 }
160-177: Consider logging errors incheckForImportsMap.The function silently returns
falsefor any error, including JSON parse errors or permission issues. Whilefalseis a safe default, adding debug logging would help diagnose issues with imports map detection.} catch { + logger.debug(`Could not check for imports map: ${error}`); return false; }code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts (2)
42-43: Address@ts-expect-errorcomments with proper typing.Multiple occurrences of
@ts-expect-error fix this latersuggest the return type ofgetComponentDocgencould be improved. Consider defining a discriminated union type forreactDocgenso that whentype === 'success', thedataproperty is properly typed withdisplayName,props, etc.🔎 Suggested approach
Define a proper return type in
extractDocgenInfo.ts:type ReactDocgenSuccess = { type: 'success'; data: { displayName?: string; actualName?: string; exportName?: string; props?: Record<string, ReactDocgenPropDescriptor>; // ... other properties }; }; type ReactDocgenResult = ReactDocgenSuccess | { type: 'error'; error: { name: string; message: string } };Then update
getComponentDocgento use this type, which would eliminate the need for@ts-expect-errordirectives.Also applies to: 75-76, 108-109, 113-114, 193-194, 293-294
9-12: Consider addingafterEachcleanup for restored test isolation.While Vitest auto-restores spies between tests by default (with
restoreMocks: true), explicitly callingvi.restoreAllMocks()inafterEachmakes the cleanup intent clear and ensures isolation even if configuration changes.beforeEach(() => { vi.spyOn(process, 'cwd').mockReturnValue('/app'); vol.fromJSON(fsMocks, '/app'); }); + +afterEach(() => { + vi.restoreAllMocks(); + vol.reset(); +});code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
149-151: Type assertion oncachedReadFileSyncresult.The
as stringcast is required becausereadFileSyncwith'utf-8'encoding can still returnBufferin some edge cases per Node.js types. This is acceptable, but consider adding a runtime check if robustness is needed.const code = cachedReadFileSync(filePath, 'utf-8'); if (typeof code !== 'string') { logger.debug(`Unexpected buffer result for ${filePath}`); return null; }
202-211: Props withouttsTypeare silently skipped.When a prop has no
tsType(e.g., PropTypes-based components or incomplete parsing), it's skipped entirely. This is likely intentional for TypeScript-focused extraction, but consider logging or including these props with a fallback type for better debugging.if (!tsType) { + // Could optionally include with fallback: mapped.props![propName] = { required: false, type: { kind: 'unknown' } }; continue; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
code/core/src/core-server/utils/generate-story.ts(1 hunks)code/core/src/core-server/utils/get-new-story-file.test.ts(7 hunks)code/core/src/core-server/utils/get-new-story-file.ts(4 hunks)code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts(1 hunks)code/renderers/react/src/componentManifest/utils.ts(1 hunks)code/renderers/react/src/preset.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- code/renderers/react/src/preset.ts
- code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.tscode/renderers/react/src/componentManifest/utils.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts
🧠 Learnings (23)
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/generate-story.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/generate-story.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
🧬 Code graph analysis (3)
code/renderers/react/src/componentManifest/utils.ts (1)
code/core/src/common/utils/interpret-files.ts (1)
resolveImport(25-58)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)parseWithReactDocgen(85-95)code/renderers/react/src/componentManifest/utils.ts (1)
cachedReadFileSync(89-89)
code/core/src/core-server/utils/generate-story.ts (1)
code/core/src/core-server/utils/get-new-story-file.ts (1)
getNewStoryFile(27-136)
⏰ 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: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (7)
code/core/src/core-server/utils/get-new-story-file.test.ts (1)
16-33: LGTM! Mocking setup follows best practices.The mocks are correctly placed at the top with
spy: true, and default behaviors are properly configured inbeforeEach. This follows the spy mocking rules from coding guidelines.Based on learnings and coding guidelines.
code/core/src/core-server/utils/generate-story.ts (1)
11-68: LGTM! Well-designed interfaces and comprehensive documentation.The
GenerateStoryResultandGenerateStoryOptionsinterfaces provide a clean, type-safe API. The JSDoc comment is thorough with a practical example, making the function's behavior clear to consumers.code/core/src/core-server/utils/get-new-story-file.ts (2)
63-81: LGTM! Appropriate error handling for optional docgen props.The docgen-driven props generation includes proper debug logging and gracefully degrades when extraction fails. This allows the story generation to proceed without args rather than failing completely.
179-207: LGTM! Robust AST-based placeholder replacement.The function uses AST traversal to safely replace
__function__placeholders withfn()calls and automatically adds the required import. The early return optimization avoids unnecessary parsing when no placeholders are present.code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts (1)
1-739: Good test coverage forgetComponentDocgenfunction.The test suite comprehensively covers:
- Simple named exports and default exports
- Multiple component exports with selective extraction
- Error handling for non-existent files and missing components
- Class components (legacy React patterns)
- Complex prop type extraction including unions, tuples, objects, functions, and discriminated unions
The inline snapshot on lines 298-739 provides thorough validation of the docgen prop type mapping.
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
26-36: Well-implemented type guards for react-docgen type discrimination.The type guards
isElementsType,isObjectSignatureType, andisLiteralTypeproperly narrow the discriminated union types from react-docgen, enabling type-safe access to nested properties inmapReactDocgenType.
145-187: Solid implementation ofgetComponentDocgenwith proper error handling.The function:
- Uses cached file reading for performance
- Handles multiple matching strategies (actualName, displayName, exportName)
- Returns
nullconsistently for all error cases- Logs debug information for troubleshooting
The JSDoc documentation is thorough and explains the component resolution behavior well.
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 (2)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (2)
77-85: Incorrect field in request payload.The
alreadyContainsArgsfield at line 80 is added to theCREATE_NEW_STORYFILE_REQUESTpayload, butCreateNewStoryRequestPayload(defined increate-new-story.tslines 1-10) does not include this field. This field belongs only in the response payload.Suggested fix
mockChannel.emit(CREATE_NEW_STORYFILE_REQUEST, { id: 'components-page--default', payload: { - alreadyContainsArgs: false, componentFilePath: 'src/components/Page.jsx', componentExportName: 'Page', componentIsDefaultExport: true, + componentExportCount: 1, }, });Note:
componentExportCountis missing from this test payload but is required byCreateNewStoryRequestPayload.
130-139: Same issue: alreadyContainsArgs in request payload.Line 133 incorrectly includes
alreadyContainsArgsin the request payload. This field should only appear in the response.Suggested fix
mockChannel.emit(CREATE_NEW_STORYFILE_REQUEST, { id: 'components-page--default', payload: { - alreadyContainsArgs: false, componentFilePath: 'src/components/Page.jsx', componentExportName: 'Page', componentIsDefaultExport: true, componentExportCount: 1, }, } satisfies RequestData<CreateNewStoryRequestPayload>);
♻️ Duplicate comments (4)
code/core/src/core-server/utils/get-new-story-file.test.ts (1)
173-243: Inline mock implementations violate coding guidelines.Lines 177-194 contain inline mock implementations within the test case instead of a
beforeEachblock. This was flagged in a previous review. While the test logic is correct and comprehensive for CSF factory workflow validation, consider restructuring for consistency.Suggested restructuring approach
+ describe('CSF factory workflow', () => { + const configDir = join(__dirname, '.storybook'); + const previewConfigPath = join(configDir, 'preview.ts'); + + beforeEach(() => { + vi.mocked(findConfigFile).mockReturnValue( + previewConfigPath as unknown as ReturnType<typeof findConfigFile> + ); + vi.mocked(isCsfFactoryPreview).mockReturnValue(true); + vi.mocked(walk.up).mockReturnValue([configDir] as unknown as ReturnType<typeof walk.up>); + vi.mocked(existsSync).mockImplementation((path) => path.toString().endsWith('package.json')); + vi.mocked(readFile).mockImplementation(async (path) => { + const p = path.toString(); + if (p === previewConfigPath) return 'export default {};'; + if (p.endsWith('package.json')) return JSON.stringify({ imports: {} }); + return ''; + }); + }); + - it('should create a new story file (CSF factory)', async () => { - const configDir = join(__dirname, '.storybook'); - const previewConfigPath = join(configDir, 'preview.ts'); - - vi.mocked(findConfigFile).mockReturnValue(...); - // ... other mocks + it('should create a new story file (CSF factory)', async () => { + // test body without inline mocks + }); + });code/core/src/core-server/server-channel/create-new-story-channel.ts (1)
27-40: Non-null assertions indicate type narrowing issues.Lines 34-37 use non-null assertions (
!) onresult.storyId,result.storyFilePath,result.exportedStoryName, andresult.alreadyContainsArgs. This was previously flagged. TheGenerateStoryResulttype should be a discriminated union withsuccess: trueandsuccess: falsevariants to enable proper type narrowing.Based on the relevant code snippet from
generate-story.ts(lines 64-106), the function returns objects with different shapes for success/failure, but the type definition likely uses a single interface with optional properties.code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (1)
88-110: Array element allowlist missing React node types.The allowlist for known element types (lines 90-106) doesn't include React component types like
'JSX.Element','ComponentType','ReactElement', etc. that are mapped at the top level (lines 51-60). This meansArray<JSX.Element>would return{ name: 'array', value: { name: 'other', value: 'JSX.Element' } }instead of the expected node-typed array element.The past review marked this as "addressed," but the code still shows the incomplete allowlist.
#!/bin/bash # Verify if the allowlist was updated in a more recent commit rg -A 25 "case 'Array':" code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/generate-story.ts (1)
76-76: Replaceprocess.cwd()withgetProjectRoot()for consistent path resolution.The
storyFilePathfromgetNewStoryFile()is constructed usinggetProjectRoot(), but the relative path calculation here usesprocess.cwd(). This inconsistency can produce incorrect relative paths when the Node.js process runs from a directory other than the project root.🔎 Proposed fix
+import { getProjectRoot } from 'storybook/internal/common'; ... - const relativeStoryFilePath = relative(process.cwd(), storyFilePath); + const relativeStoryFilePath = relative(getProjectRoot(), storyFilePath);
🧹 Nitpick comments (1)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
145-149: Missing payload in error response assertion.The error response assertion doesn't include a
payloadfield, but the implementation atcreate-new-story-channel.tslines 51-57 shows that UNKNOWN errors emitpayload: undefined. This is correct behavior, but consider adding explicitpayload: undefinedto the assertion for clarity.Optional: Make assertion explicit
expect(createNewStoryFileEventListener).toHaveBeenCalledWith({ error: 'Failed to write file', id: 'components-page--default', + payload: undefined, success: false, });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
code/core/src/core-events/data/create-new-story.ts(1 hunks)code/core/src/core-server/index.ts(1 hunks)code/core/src/core-server/server-channel/create-new-story-channel.test.ts(4 hunks)code/core/src/core-server/server-channel/create-new-story-channel.ts(3 hunks)code/core/src/core-server/utils/generate-story.ts(1 hunks)code/core/src/core-server/utils/get-mocked-props-for-args.test.ts(1 hunks)code/core/src/core-server/utils/get-mocked-props-for-args.ts(1 hunks)code/core/src/core-server/utils/get-new-story-file.test.ts(7 hunks)code/core/src/core-server/utils/get-new-story-file.ts(4 hunks)code/core/src/csf/SBType.ts(3 hunks)code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts(1 hunks)code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts(1 hunks)code/renderers/react/src/preset.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- code/core/src/core-server/utils/get-mocked-props-for-args.test.ts
- code/core/src/core-server/index.ts
- code/renderers/react/src/preset.ts
- code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
- code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-events/data/create-new-story.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/csf/SBType.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-events/data/create-new-story.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/csf/SBType.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-events/data/create-new-story.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/csf/SBType.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-events/data/create-new-story.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/csf/SBType.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-events/data/create-new-story.tscode/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/generate-story.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/csf/SBType.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
🧠 Learnings (28)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-mocked-props-for-args.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-mocked-props-for-args.ts
🧬 Code graph analysis (7)
code/core/src/core-server/utils/get-new-story-file.test.ts (3)
code/core/src/common/utils/get-storybook-info.ts (1)
findConfigFile(105-109)code/core/src/core-server/utils/get-new-story-file.ts (1)
getNewStoryFile(27-146)code/core/src/csf-tools/ConfigFile.ts (1)
isCsfFactoryPreview(1177-1192)
code/core/src/core-server/utils/generate-story.ts (2)
code/core/src/core-events/data/create-new-story.ts (1)
CreateNewStoryRequestPayload(1-10)code/core/src/core-server/utils/get-new-story-file.ts (1)
getNewStoryFile(27-146)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (5)
code/core/src/csf/SBType.ts (1)
SBType(48-58)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)parseWithReactDocgen(85-95)code/renderers/react/src/componentManifest/utils.ts (1)
cachedReadFileSync(89-89)code/core/src/core-server/index.ts (1)
ComponentArgTypesData(14-14)code/core/src/core-server/utils/get-mocked-props-for-args.ts (1)
ComponentArgTypesData(8-10)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
stories(968-970)
code/core/src/core-server/server-channel/create-new-story-channel.ts (4)
code/core/src/core-server/index.ts (1)
generateStoryFile(12-12)code/core/src/core-server/utils/generate-story.ts (1)
generateStoryFile(65-107)code/core/src/core-events/data/create-new-story.ts (2)
CreateNewStoryResponsePayload(12-21)CreateNewStoryErrorPayload(23-26)code/core/src/telemetry/index.ts (1)
telemetry(31-72)
code/core/src/core-server/utils/get-new-story-file.ts (6)
code/core/src/common/utils/get-storybook-info.ts (1)
findConfigFile(105-109)code/core/src/csf-tools/ConfigFile.ts (3)
isCsfFactoryPreview(1177-1192)loadConfig(1150-1153)printConfig(1159-1161)code/core/src/core-server/utils/get-mocked-props-for-args.ts (1)
generateMockPropsFromArgTypes(16-46)code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (1)
getCsfFactoryTemplateForNewStoryFile(18-45)code/core/src/core-server/utils/new-story-templates/javascript.ts (1)
getJavaScriptTemplateForNewStoryFile(16-45)code/core/src/common/js-package-manager/JsPackageManager.ts (1)
packageJson(768-770)
code/core/src/core-server/utils/get-mocked-props-for-args.ts (1)
code/core/src/csf/SBType.ts (1)
SBType(48-58)
🪛 Biome (2.1.2)
code/core/src/core-server/utils/get-mocked-props-for-args.ts
[error] 230-230: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (21)
code/core/src/core-server/utils/get-new-story-file.test.ts (2)
1-29: Well-structured mock setup following coding guidelines.The test file correctly uses
vi.mock()with{ spy: true }for all dependencies and establishes default mock behaviors in thebeforeEachblock. This aligns with the project's Vitest mocking patterns.
144-171: Inline mock implementation in test case.The
presets.applymock forgetArgTypesDataat lines 158-162 is defined inline within the test case. Per coding guidelines, mock implementations should be inbeforeEachblocks. However, since this is a test-specific argTypes scenario and the mock is simple, this is acceptable as a practical trade-off for test readability.The test logic correctly verifies that the
__function__placeholder is replaced withfn()and the import is added.code/core/src/csf/SBType.ts (3)
6-8: Good addition of 'date' scalar type.Adding 'date' to the scalar type union is appropriate for representing Date values in component props.
35-42: SBLiteralType and SBTupleType extend type coverage.Both types follow the established pattern and are correctly added to the union.
SBLiteralTypewithvalue: unknownis appropriate for literal values, andSBTupleTypecorrectly referencesSBType[]for tuple elements.
14-18: SBNodeType correctly implements framework-specific node representation.The type appropriately captures framework-specific renderable nodes with the
name: 'node'discriminator andrenderer: 'react'metadata. Mock prop generation handles this type correctly throughgenerateMockValueFromSBType, which returns either the prop name or a default "Hello world" string. The TODO comment appropriately notes the need for extending renderer support to other frameworks.code/core/src/core-events/data/create-new-story.ts (1)
12-21: Well-documented payload extension.The
alreadyContainsArgsfield is appropriately added with a clear comment explaining its purpose. This enables the UI to inform users whether args were auto-generated from docgen data.code/core/src/core-server/server-channel/create-new-story-channel.ts (2)
47-59: Conditional error payload structure is appropriate.The error handling correctly distinguishes between
STORY_FILE_EXISTSand other errors, emitting the appropriate payload shape. However, line 55 uses a non-null assertion onresult.kindwhich should be addressed with proper type narrowing.
61-66: Telemetry captures both error type and message.The telemetry correctly captures
result.errorTypewhen available, falling back toresult.error. This provides useful diagnostic information.code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
15-27: Mock factory pattern is acceptable here.The mock implementations inside
vi.mock()are appropriate as they define the module replacement. ThegetStoryIdmock correctly returns a Promise with the expected shape.code/core/src/core-server/utils/get-new-story-file.ts (3)
170-187: LGTM!The
checkForImportsMapfunction correctly walks up the directory tree to detect if apackage.jsonwith animportsfield exists. The error handling appropriately returnsfalseas a safe default.
189-217: LGTM!The placeholder replacement logic is well-implemented with an early return optimization when no placeholders exist. The AST-based replacement ensures safe transformation, and the conditional import injection for
fnfrom'storybook/test'is correctly handled.
63-85: LGTM!The ArgTypes extraction and mock prop generation is well-structured with appropriate debug logging (following the coding guidelines for using
storybook/internal/node-logger) and graceful error handling that allows story generation to proceed without args when mock generation fails.code/core/src/core-server/utils/generate-story.ts (2)
11-30: LGTM!The
GenerateStoryResultandGenerateStoryOptionsinterfaces are well-defined with clear documentation. The optional fields and constrained error types ('STORY_FILE_EXISTS' | 'UNKNOWN') provide a clean API for consumers.
65-107: LGTM!The
generateStoryFilefunction has a clean control flow with proper error handling. The file existence check correctly returns before writing whencheckFileExistsis enabled, and all code paths return appropriateGenerateStoryResultobjects.code/core/src/core-server/utils/get-mocked-props-for-args.ts (4)
16-46: LGTM!The
generateMockPropsFromArgTypesfunction correctly handles both scalar type strings and SBType objects, properly partitioning props into required and optional groups based on theargType.type.requiredflag.
48-123: LGTM!The token-based heuristics for inferring semantic types from prop names are well-designed. The scoring system with penalties for metadata-like tokens (e.g.,
imageWidth,imageSize) helps avoid generating inappropriate mock values like image URLs for dimension props.
140-176: LGTM!The
generateMockValueFromSBTypefunction provides comprehensive coverage of all SBType variants. The string case intelligently applies context-aware heuristics (color, date, URL, email, phone) based on prop names, producing realistic mock values.
200-220: LGTM!The union handling sensibly prioritizes literal values (which provide concrete mock data), and the array handling takes a conservative approach by returning an empty array when the element type is unknown (
'other'), avoiding potentially invalid mock data.code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (3)
39-74: LGTM!The
mapReactDocgenToArgTypefunction provides comprehensive mapping from React docgen types to SBType. The handling of React node types (lines 51-60) correctly maps various JSX/React element types to{ name: 'node', type: 'react' }, and object signatures properly extract nested properties recursively.
146-188: LGTM!The
getComponentDocgenfunction is well-implemented with:
- Cached file reading for performance via
cachedReadFileSync- Proper component matching by
actualName,displayName, orexportName- Debug logging for errors following the coding guidelines for
storybook/internal/node-logger- Null-safe returns for missing components
190-215: LGTM!The
extractArgTypesFromDocgenfunction cleanly orchestrates docgen extraction and type mapping. Skipping props withouttsTypeis appropriate since those can't be meaningfully mapped. The returnedComponentArgTypesDatastructure integrates well with the mock prop generation utilities.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
code/core/src/core-server/utils/get-new-story-file.test.ts (1)
144-243: Inline mock implementations should be moved tobeforeEachblocks.Both new test cases contain inline mock implementations that violate the coding guidelines:
- Lines 158-162: Mock for
experimental_getArgTypesDatain the placeholder test- Lines 177-194 and 206-217: Multiple mocks in the CSF factory test
Consider wrapping each test in a nested
describeblock with its ownbeforeEachto set up the specific mocks.Example restructuring for the CSF factory test
+ describe('when using CSF factory', () => { + const configDir = join(__dirname, '.storybook'); + const previewConfigPath = join(configDir, 'preview.ts'); + + beforeEach(() => { + vi.mocked(findConfigFile).mockReturnValue( + previewConfigPath as unknown as ReturnType<typeof findConfigFile> + ); + vi.mocked(isCsfFactoryPreview).mockReturnValue(true); + vi.mocked(walk.up).mockReturnValue([configDir] as unknown as ReturnType<typeof walk.up>); + vi.mocked(existsSync).mockImplementation((path) => path.toString().endsWith('package.json')); + vi.mocked(readFile).mockImplementation(async (path) => { + const p = path.toString(); + if (p === previewConfigPath) { + return 'export default {};'; + } + if (p.endsWith('package.json')) { + return JSON.stringify({ imports: {} }); + } + return ''; + }); + }); + - it('should create a new story file (CSF factory)', async () => { - const configDir = join(__dirname, '.storybook'); - const previewConfigPath = join(configDir, 'preview.ts'); - - vi.mocked(findConfigFile).mockReturnValue(...); - // ... other inline mocks ... - + it('should create a new story file (CSF factory)', async () => { const { exportedStoryName, storyFileContent, storyFilePath } = await getNewStoryFile( // ... rest of test ); }); + });Based on coding guidelines: "Implement mock behaviors in
beforeEachblocks" and "Avoid inline mock implementations within test cases."code/core/src/core-server/utils/get-new-story-file.ts (1)
139-145: Bug:dirnameis the imported function, not the directory path.Line 143 returns the
dirnamefunction imported fromnode:path(line 3) instead of thedirvariable computed at line 42. This will cause incorrect behavior for any consumer expecting the story file's directory path.🔎 Proposed fix
return { storyFilePath, exportedStoryName, storyFileContent: formattedStoryFileContent, - dirname, + dirname: dir, alreadyContainsArgs: !!args, };
🧹 Nitpick comments (1)
code/renderers/react/src/preset.ts (1)
78-78: Remove redundant type cast.The
optionsparameter is already typed asGetArgTypesDataOptionsin the function signature, making the cast unnecessary.🔎 Proposed fix
- const { componentFilePath, componentExportName } = (options ?? {}) as GetArgTypesDataOptions; + const { componentFilePath, componentExportName } = options ?? {};
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/preset.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/utils/get-new-story-file.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/utils/get-new-story-file.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/get-new-story-file.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/get-new-story-file.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/get-new-story-file.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/core-server/utils/get-new-story-file.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/renderers/react/src/preset.tscode/core/src/core-server/utils/get-new-story-file.ts
🧠 Learnings (25)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 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/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/get-new-story-file.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
🧬 Code graph analysis (2)
code/renderers/react/src/preset.ts (2)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
GetArgTypesDataOptions(17-20)extractArgTypesFromDocgen(190-215)code/frameworks/angular/src/client/public-types.ts (1)
ArgTypes(16-16)
code/core/src/core-server/utils/get-new-story-file.ts (2)
code/core/src/csf-tools/ConfigFile.ts (2)
loadConfig(1150-1153)printConfig(1159-1161)code/core/src/core-server/utils/get-mocked-props-for-args.ts (1)
generateMockPropsFromArgTypes(16-46)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (11)
code/core/src/core-server/utils/get-new-story-file.ts (5)
1-25: LGTM! Imports are well-organized.The new imports for AST manipulation (
types,traverse,loadConfig,printConfig), mock prop generation, formatting, and directory traversal are properly scoped and follow the codebase patterns. Usingloggerfromstorybook/internal/node-loggeraligns with coding guidelines for server-side Node.js code.
63-85: Good defensive coding for experimental argTypes extraction.The try/catch ensures graceful degradation if the experimental preset fails, and
logger.debugprovides useful debugging output without polluting user-facing logs. The approach of only using required props for generated args is sensible.
87-109: LGTM! CSF factory path handling logic is sound.The conditional logic for computing
previewImportPathcorrectly handles:
- Checking for an imports map to decide whether to use relative paths
- Proper path normalization with extension stripping
- Adding
./prefix when needed for relative imports
170-187: LGTM! Robust imports map detection.The helper correctly walks up the directory tree, handles errors gracefully with a
falsefallback, and short-circuits on the first match. Usingwalk.upwith the project root boundary is appropriate.
189-217: Clean AST-based placeholder replacement.The implementation efficiently skips parsing when no placeholders exist, correctly traverses string literals to find
__function__placeholders, and properly injects thefnimport fromstorybook/testonly when needed. The approach is maintainable for adding future placeholder types.code/core/src/core-server/utils/get-new-story-file.test.ts (3)
1-29: LGTM! Mock setup follows coding guidelines.The mock declarations use the
{ spy: true }option as required, and thebeforeEachblock properly clears all mocks and sets up baseline implementations forgetProjectRoot,findConfigFile, andexistsSync. This provides good test isolation.
31-142: Existing tests remain well-structured.The TypeScript, pnp workaround, and JavaScript story file creation tests provide good baseline coverage with clear expectations via inline snapshots.
222-243: Test expectations are comprehensive.The CSF factory test validates the complete generated output including:
- The
#.storybook/previewimport path (when imports map exists)- The
fnimport fromstorybook/test- The args object with properly typed mock values
- The CSF factory
meta.story()patternThis provides good confidence in the integrated behavior of argTypes extraction, mock prop generation, and placeholder replacement.
code/renderers/react/src/preset.ts (3)
3-10: LGTM! Imports are properly structured.The new imports for
ArgTypesand the docgen extraction utilities are correctly typed and align with their usage in theexperimental_getArgTypesDatafunction below.
69-69: LGTM! Simplified unused error variable.Removing the unused error parameter is a clean improvement.
74-98: The function is correctly implemented without thePresetPropertytype wrapper. UnlikeaddonsandpreviewAnnotations,experimental_getArgTypesDatais not a registered preset property inStorybookConfigRaw. Only properties that are part of the Storybook configuration schema (likeexperimental_indexers,experimental_enrichCsf) should usePresetProperty<K>orPresetPropertyFn<K>. The function follows the correct pattern for utility exports from preset files that are accessed via the preset system but are not registered configuration properties.Likely an incorrect or invalid review comment.
| export async function experimental_getArgTypesData( | ||
| _input: unknown, | ||
| options: GetArgTypesDataOptions | ||
| ): Promise<ArgTypes | null> { | ||
| const { componentFilePath, componentExportName } = (options ?? {}) as GetArgTypesDataOptions; | ||
|
|
||
| if (!componentFilePath) { | ||
| return null; | ||
| } | ||
|
|
||
| const argTypesData = extractArgTypesFromDocgen({ componentFilePath, componentExportName }); | ||
| if (!argTypesData?.props) { | ||
| return null; | ||
| } | ||
|
|
||
| const argTypes: ArgTypes = {}; | ||
| for (const [propName, propInfo] of Object.entries(argTypesData.props)) { | ||
| argTypes[propName] = { | ||
| name: propName, | ||
| type: propInfo.required ? { ...propInfo.type, required: true } : propInfo.type, | ||
| }; | ||
| } | ||
|
|
||
| return argTypes; | ||
| } |
There was a problem hiding this comment.
Add error handling and consider JSDoc for the experimental API.
The function lacks error handling around extractArgTypesFromDocgen, which could throw errors and crash the preset loading process. Additionally, as a public experimental API, it would benefit from JSDoc documentation explaining its purpose, parameters, and return values.
🔎 Proposed improvements
Add error handling:
export async function experimental_getArgTypesData(
_input: unknown,
options: GetArgTypesDataOptions
): Promise<ArgTypes | null> {
- const { componentFilePath, componentExportName } = (options ?? {}) as GetArgTypesDataOptions;
+ const { componentFilePath, componentExportName } = options ?? {};
if (!componentFilePath) {
return null;
}
+ try {
const argTypesData = extractArgTypesFromDocgen({ componentFilePath, componentExportName });
if (!argTypesData?.props) {
return null;
}
const argTypes: ArgTypes = {};
for (const [propName, propInfo] of Object.entries(argTypesData.props)) {
argTypes[propName] = {
name: propName,
type: propInfo.required ? { ...propInfo.type, required: true } : propInfo.type,
};
}
return argTypes;
+ } catch {
+ return null;
+ }
}Add JSDoc documentation:
/**
* Experimental preset function that extracts ArgTypes from React component docgen data.
*
* @param _input - Unused preset accumulator parameter (required for preset compatibility)
* @param options - Configuration including componentFilePath and optional componentExportName
* @returns Promise resolving to ArgTypes object or null if extraction fails
* @experimental This API is experimental and may change in future releases
*/
export async function experimental_getArgTypesData(
_input: unknown,
options: GetArgTypesDataOptions
): Promise<ArgTypes | null> {
// ... implementation
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In code/renderers/react/src/preset.ts around lines 74 to 98, wrap the call to
extractArgTypesFromDocgen in a try/catch so any thrown errors are caught and
handled (log the error or return null) to avoid crashing preset loading, and add
a JSDoc block above the function describing its experimental status, parameters
(_input and options) and return value (Promise<ArgTypes | null>) as shown in the
review; ensure the catch returns null (or logs then returns null) and preserve
existing behavior when argTypesData or props are missing.
yannbf
force-pushed
the
yann/ghost-stories-research
branch
from
940ad62 to
79cd680
Compare
…js/storybook into yann/refactor-story-generation
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
code/renderers/react/src/preset.ts (1)
74-98: Add error handling and JSDoc documentation.This comment echoes the previous review feedback: the function lacks error handling around
extractArgTypesFromDocgen(which could throw and crash preset loading) and JSDoc documentation explaining its experimental status, parameters, and return value.
🧹 Nitpick comments (4)
code/renderers/react/src/preset.ts (1)
78-78: Remove unnecessary type assertion.The
optionsparameter is already typed asGetArgTypesDataOptions, making the type assertion redundant. The nullish coalescing operator alone is sufficient.🔎 Proposed simplification
- const { componentFilePath, componentExportName } = (options ?? {}) as GetArgTypesDataOptions; + const { componentFilePath, componentExportName } = options ?? {};code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
39-136: Add JSDoc documentation for the exported function.
mapReactDocgenToArgTypeis a public API function but lacks documentation explaining its purpose, parameters, return value, and the type mapping behavior. This would help maintainers understand the conversion logic between react-docgen types and Storybook's internal SBType format.🔎 Suggested JSDoc
/** * Maps a react-docgen TypeDescriptor to Storybook's internal SBType format. * * Handles primitives, React node types, objects, functions, unions, arrays, tuples, * literals, and fallback types. For arrays, unresolved custom element types are * conservatively mapped to 'other' to avoid false assumptions. * * @param docgenType - The react-docgen TypeDescriptor to map * @returns The corresponding SBType representation * @public */ export function mapReactDocgenToArgType(docgenType: ReactDocgenTsType): SBType { // ... implementation }
197-222: Add JSDoc documentation for the exported function.
extractArgTypesFromDocgenis a public API function exported and consumed by the React preset but lacks documentation. Adding JSDoc would clarify its purpose (extracting ArgTypes from docgen data), parameters, return value (null when extraction fails), and relationship togetComponentDocgen.🔎 Suggested JSDoc
/** * Extracts ArgTypes data from a React component file using react-docgen. * * Reads component metadata via getComponentDocgen and maps props with tsType information * to ComponentArgTypesData format, including the required flag and type mapping via * mapReactDocgenToArgType. Props without tsType are skipped. * * @param options - Configuration with componentFilePath and optional componentExportName * @returns ComponentArgTypesData with mapped props, or null if extraction fails * @public */ export const extractArgTypesFromDocgen = ({ componentFilePath, componentExportName, }: GetArgTypesDataOptions) => { // ... implementation };code/core/src/core-server/utils/get-dummy-props-for-args.test.ts (1)
126-126: Optional: Remove extra property not in SBType interface.The test data includes
type: 'react'which is not part of the SBType interface for node types. While this extra property is likely ignored by the implementation, it adds confusion about the expected data shape.🔎 Proposed fix
- children: { required: false, type: { name: 'node', type: 'react' } }, + children: { required: false, type: { name: 'node' } },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
code/core/src/core-server/index.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/core/src/core-server/utils/get-new-story-file.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/renderers/react/src/preset.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/core-server/index.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/renderers/react/src/preset.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/renderers/react/src/preset.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/renderers/react/src/preset.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/renderers/react/src/preset.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-dummy-props-for-args.tscode/core/src/core-server/utils/get-new-story-file.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-12-22T22:03:40.109Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.109Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-12-22T22:03:40.109Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.109Z
Learning: Applies to code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/node-logger` for server-side logging in Node.js code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-12-22T22:03:40.109Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.109Z
Learning: Applies to code/{renderers}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/client-logger` for client-side logging in browser code
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/utils/get-new-story-file.ts
🧬 Code graph analysis (4)
code/renderers/react/src/preset.ts (2)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (2)
GetArgTypesDataOptions(17-20)extractArgTypesFromDocgen(197-222)code/frameworks/angular/src/client/public-types.ts (1)
ArgTypes(16-16)
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts (2)
code/core/src/core-server/utils/get-dummy-props-for-args.ts (3)
generateDummyValueFromSBType(142-265)generateDummyPropsFromDocgen(267-286)ComponentArgTypesData(8-10)code/core/src/core-server/index.ts (1)
ComponentArgTypesData(14-14)
code/core/src/core-server/utils/get-dummy-props-for-args.ts (3)
code/core/src/csf/SBType.ts (1)
SBType(48-58)code/core/src/core-server/index.ts (1)
ComponentArgTypesData(14-14)code/renderers/react/template/stories/js-argtypes.stories.jsx (1)
argTypes(41-41)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (4)
code/core/src/csf/SBType.ts (1)
SBType(48-58)code/renderers/react/src/componentManifest/reactDocgen.ts (2)
getReactDocgen(238-268)parseWithReactDocgen(85-95)code/renderers/react/src/componentManifest/utils.ts (1)
cachedReadFileSync(89-89)code/core/src/core-server/utils/get-dummy-props-for-args.ts (1)
ComponentArgTypesData(8-10)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (7)
code/core/src/core-server/utils/get-new-story-file.ts (4)
68-88: LGTM: Solid error handling for experimental ArgTypes extraction.The try-catch block appropriately handles failures in the experimental feature, ensuring the story generation continues even if ArgTypes extraction fails. Debug logging provides good visibility for troubleshooting.
92-103: LGTM: Preview import path computation is correct.The logic correctly:
- Checks for package.json imports map to determine if a relative path is needed
- Computes the story file's absolute directory path
- Generates a relative path from story file to preview config
- Strips extensions and normalizes the path format
The conditional logic ensures compatibility with different project configurations.
173-190: LGTM: Directory traversal and imports map detection is correct.The function safely walks up the directory tree with appropriate bounds (stopping at project root) and defensively handles all errors by returning false. The approach is suitable for this use case.
196-220: LGTM: AST transformation correctly handles function placeholders.The function:
- Optimizes by early-returning when no placeholders are present
- Safely traverses only StringLiteral nodes (avoiding comments and other contexts)
- Conditionally adds the import only when replacements occur
- Uses Babel's AST utilities correctly
The implementation is both correct and efficient.
code/core/src/core-server/utils/get-dummy-props-for-args.ts (3)
50-125: LGTM: Token-based heuristics are well-designed.The implementation thoughtfully:
- Tokenizes various naming conventions (camelCase, snake_case, kebab-case)
- Applies weighted scoring for semantic meaning
- Includes negative tokens to avoid false positives (e.g., "imageWidth" won't generate an image URL)
The heuristic approach provides reasonable dummy values based on prop names.
142-265: LGTM: Comprehensive value generation with sensible defaults.The switch statement handles all SBType cases with appropriate fallbacks:
- Primitives return sensible default values
- Complex types (objects, unions, arrays) are handled recursively
- Edge cases (empty unions, unknown array elements) have conservative defaults
- React-specific types are appropriately identified and handled
The implementation is both thorough and defensive.
267-286: LGTM: Docgen prop generation is correctly implemented.The function properly:
- Handles null/undefined docgen data
- Iterates over props and generates appropriate dummy values
- Correctly checks
propInfo.requiredto categorize props- Returns structured required/optional buckets
| export function generateDummyPropsFromArgTypes(argTypes: ArgTypes) { | ||
| const required: Record<string, unknown> = {}; | ||
| const optional: Record<string, unknown> = {}; | ||
|
|
||
| for (const [propName, argType] of Object.entries(argTypes)) { | ||
| // Determine if prop is required | ||
| const isRequired = argType.type && typeof argType.type === 'object' && argType.type.required; | ||
|
|
||
| // Generate dummy value directly from SBType | ||
| let dummyValue: unknown; | ||
| if (typeof argType.type === 'string') { | ||
| // Handle scalar type strings - convert to SBType | ||
| const sbType: SBType = { name: argType.type }; | ||
| dummyValue = generateDummyValueFromSBType(sbType, propName); | ||
| } else if (argType.type && typeof argType.type === 'object') { | ||
| // Handle SBType objects directly | ||
| dummyValue = generateDummyValueFromSBType(argType.type, propName); | ||
| } else { | ||
| // Fallback as we don't know what the type is | ||
| dummyValue = undefined; | ||
| } | ||
|
|
||
| if (isRequired) { | ||
| required[propName] = dummyValue; | ||
| } else { | ||
| optional[propName] = dummyValue; | ||
| } | ||
| } | ||
|
|
||
| return { required, optional }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ArgTypes and ArgType definitions
rg -nP -A5 'type ArgTypes\s*=' --type=ts
rg -nP -A5 'interface ArgType\b' --type=ts
rg -nP -A10 'type ArgType\b' --type=tsRepository: storybookjs/storybook
Length of output: 552
🏁 Script executed:
#!/bin/bash
# Get the full ArgType interface definition
cat -n code/addons/docs/src/blocks/components/ArgsTable/types.ts | head -100
# Also search for ArgTypes definition in CSF or related files
rg -nP 'type ArgTypes' --type=ts -A3
rg -nP 'required' code/addons/docs/src/blocks/components/ArgsTable/types.ts -B2 -A2Repository: storybookjs/storybook
Length of output: 4497
🏁 Script executed:
#!/bin/bash
# Search for InputType and StrictInputType definitions
rg -nP 'type InputType|interface InputType' --type=ts -A5
rg -nP 'type StrictInputType|interface StrictInputType' --type=ts -A5
# Search for SBType definition
rg -nP 'type SBType|interface SBType' --type=ts -A10
# Look at the actual file being reviewed
cat -n code/core/src/core-server/utils/get-dummy-props-for-args.tsRepository: storybookjs/storybook
Length of output: 10936
Fix the required property check on line 24.
The current code checks argType.type.required, but SBType does not have a required property. This causes isRequired to always be falsy, resulting in all props being categorized as optional.
Line 24 should check argType.required directly, consistent with the pattern in generateDummyPropsFromDocgen (line 278) which correctly accesses propInfo.required from ComponentArgTypesInfo.
🤖 Prompt for AI Agents
In code/core/src/core-server/utils/get-dummy-props-for-args.ts around lines 18
to 48, the required check is wrong: it inspects argType.type.required (SBType
has no required), so isRequired is always falsy and all props become optional;
change the check to use argType.required (i.e., read required directly from the
ArgTypes entry) and remove the incorrect dependency on argType.type.required so
required props are classified into the required object as intended.
There was a problem hiding this comment.
Pull request overview
This pull request refactors story file generation in the core server to improve modularity and adds support for automatic args extraction from component docgen data. The changes introduce new utilities for generating story files with pre-populated props, particularly for React components.
Key Changes:
- Introduces a new
generateStoryFileutility that orchestrates story creation, file existence checks, and metadata extraction - Adds docgen-based args extraction for React components via
experimental_getArgTypesDatapreset API - Implements dummy/mock prop generation from ArgTypes and docgen data with heuristics for realistic values (URLs, emails, dates, etc.)
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
code/renderers/react/src/preset.ts |
Adds experimental_getArgTypesData function to extract ArgTypes from React component docgen |
code/renderers/react/src/componentManifest/utils.ts |
Changes cachedResolveImport type to any |
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts |
New utility for extracting and mapping React docgen data to Storybook ArgTypes format |
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.test.ts |
Comprehensive tests for docgen extraction covering various component types |
code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx |
Updates story creation flow to skip redundant args fetch when args already exist from docgen |
code/core/src/csf/SBType.ts |
Extends SBType with new types: date, node, literal, and tuple |
code/core/src/core-server/utils/new-story-templates/typescript.ts |
Updates TypeScript template to support pre-populated args in generated stories |
code/core/src/core-server/utils/new-story-templates/javascript.ts |
Updates JavaScript template to support pre-populated args in generated stories |
code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts |
Updates CSF factory template to support custom preview import paths and args |
code/core/src/core-server/utils/new-story-templates/csf-factory-template.test.ts |
Adds tests for CSF factory template variations |
code/core/src/core-server/utils/get-new-story-file.ts |
Refactors story file generation to integrate args extraction, placeholder replacement, and formatting |
code/core/src/core-server/utils/get-new-story-file.test.ts |
Updates tests with proper mocking and adds test for placeholder replacement |
code/core/src/core-server/utils/get-mocked-props-for-args.ts |
New utility for generating mock prop values from ArgTypes/docgen with heuristic-based value generation |
code/core/src/core-server/utils/get-mocked-props-for-args.test.ts |
Tests for mock prop generation (contains failing tests due to placeholder value mismatch) |
code/core/src/core-server/utils/get-dummy-props-for-args.ts |
Near-duplicate of get-mocked-props with different naming and placeholder constant |
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts |
Tests for dummy prop generation (contains failing tests due to placeholder value mismatch) |
code/core/src/core-server/utils/generate-story.ts |
New orchestration utility for story file generation with comprehensive JSDoc |
code/core/src/core-server/server-channel/create-new-story-channel.ts |
Simplifies channel handler by delegating to generateStoryFile utility |
code/core/src/core-server/server-channel/create-new-story-channel.test.ts |
Updates tests to include alreadyContainsArgs field and mock story ID extraction |
code/core/src/core-server/index.ts |
Exports new public APIs for story generation and docgen types |
code/core/src/core-events/data/create-new-story.ts |
Adds alreadyContainsArgs field to response payload |
| export async function experimental_getArgTypesData( | ||
| _input: unknown, | ||
| options: GetArgTypesDataOptions | ||
| ): Promise<ArgTypes | null> { |
There was a problem hiding this comment.
Missing documentation for the public exported function experimental_getArgTypesData. Since this is part of the preset API and marked as experimental, it should have comprehensive JSDoc comments explaining its purpose, parameters, return value, and usage.
| const mostLikelyType = getMostLikelyTypeFromTokens(tokens); | ||
|
|
||
| switch (mostLikelyType) { | ||
| case 'image': | ||
| return 'https://placehold.co/600x400?text=Storybook'; | ||
|
|
||
| case 'url': | ||
| return 'https://example.com'; | ||
|
|
||
| case 'email': | ||
| return 'storybook@example.com'; | ||
|
|
||
| case 'phone': | ||
| return '1234567890'; | ||
|
|
||
| default: | ||
| return name ?? 'Hello world'; | ||
| } |
There was a problem hiding this comment.
The switch statement for the 'string' case is missing a default return statement after the switch on mostLikelyType. Although the code currently returns name || 'Hello world' after the switch, this makes the control flow less clear. Consider adding explicit return statements in each case or ensuring the default path is more obvious.
| return propName ?? 'Hello world'; | ||
|
|
||
| case 'function': | ||
| return '__function__'; |
There was a problem hiding this comment.
The function placeholder value differs between the two similar files. In this file it's the string '__function__', while in get-dummy-props-for-args.ts it's STORYBOOK_FN_PLACEHOLDER = '[[STORYBOOK_FN_PLACEHOLDER]]'. This inconsistency could lead to confusion and bugs if both are used in different contexts.
| return '__function__'; | |
| return '[[STORYBOOK_FN_PLACEHOLDER]]'; |
| }); | ||
|
|
||
| it('generates functions as placeholders', () => { | ||
| expect(generateDummyValueFromSBType({ name: 'function' })).toBe('__function__'); |
There was a problem hiding this comment.
The test expects the function placeholder to be '__function__', but the actual value returned by generateDummyValueFromSBType for function types is STORYBOOK_FN_PLACEHOLDER, which equals '[[STORYBOOK_FN_PLACEHOLDER]]'. Update the test to use the correct expected value.
| import type { ArgTypes, SBType } from 'storybook/internal/csf'; | ||
|
|
||
| export type ComponentArgTypesInfo = { | ||
| required: boolean; | ||
| type: SBType; | ||
| }; | ||
|
|
||
| export type ComponentArgTypesData = { | ||
| props?: Record<string, ComponentArgTypesInfo>; | ||
| }; | ||
|
|
||
| /** | ||
| * Generate mock props using ArgTypes instead of ComponentArgTypesData This provides more accurate | ||
| * mock generation by leveraging ArgTypes structure | ||
| */ | ||
| export function generateMockPropsFromArgTypes(argTypes: ArgTypes) { | ||
| const required: Record<string, unknown> = {}; | ||
| const optional: Record<string, unknown> = {}; | ||
|
|
||
| for (const [propName, argType] of Object.entries(argTypes)) { | ||
| // Determine if prop is required | ||
| const isRequired = argType.type && typeof argType.type === 'object' && argType.type.required; | ||
|
|
||
| // Generate mock value directly from SBType | ||
| let mockValue: unknown; | ||
| if (typeof argType.type === 'string') { | ||
| // Handle scalar type strings - convert to SBType | ||
| const sbType: SBType = { name: argType.type }; | ||
| mockValue = generateMockValueFromSBType(sbType, propName); | ||
| } else if (argType.type && typeof argType.type === 'object') { | ||
| // Handle SBType objects directly | ||
| mockValue = generateMockValueFromSBType(argType.type, propName); | ||
| } else { | ||
| // Fallback as we don't know what the type is | ||
| mockValue = undefined; | ||
| } | ||
|
|
||
| if (isRequired) { | ||
| required[propName] = mockValue; | ||
| } else { | ||
| optional[propName] = mockValue; | ||
| } | ||
| } | ||
|
|
||
| return { required, optional }; | ||
| } | ||
|
|
||
| // Tokenize prop names so we can use them to determine the most likely type of the prop e.g. image, url, etc. | ||
| function tokenize(name: string): string[] { | ||
| if (!name) { | ||
| return []; | ||
| } | ||
|
|
||
| return name | ||
| .replace(/([a-z0-9])([A-Z])/g, '$1 $2') | ||
| .replace(/[_\-]+/g, ' ') | ||
| .split(' ') | ||
| .map((t) => t.toLowerCase()) | ||
| .filter(Boolean); | ||
| } | ||
|
|
||
| function hasAny(tokens: string[], set: Set<string>): boolean { | ||
| return tokens.some((t) => set.has(t)); | ||
| } | ||
|
|
||
| const URL_TOKENS = new Set(['url', 'href', 'link']); | ||
| const IMAGE_TOKENS = new Set(['image', 'img', 'photo', 'avatar', 'logo']); | ||
| const EMAIL_TOKENS = new Set(['email', 'e-mail', 'mail']); | ||
| const PHONE_TOKENS = new Set(['phone', 'tel', 'telephone', 'mobile', 'cell']); | ||
|
|
||
| const COLOR_TOKENS = new Set(['color', 'background', 'bg']); | ||
| const DATE_TOKENS = new Set(['date', 'at', 'time', 'timestamp']); | ||
|
|
||
| const NEGATIVE_IMAGE_TOKENS = new Set([ | ||
| 'size', | ||
| 'width', | ||
| 'height', | ||
| 'ratio', | ||
| 'count', | ||
| 'status', | ||
| 'loading', | ||
| 'config', | ||
| 'options', | ||
| 'variant', | ||
| ]); | ||
|
|
||
| type MostLikelyType = 'image' | 'url' | 'email' | 'phone'; | ||
|
|
||
| function getMostLikelyTypeFromTokens(tokens: string[]): MostLikelyType | null { | ||
| const score: Partial<Record<MostLikelyType, number>> = {}; | ||
|
|
||
| for (const token of tokens) { | ||
| if (IMAGE_TOKENS.has(token)) { | ||
| score.image = (score.image ?? 0) + 3; | ||
| } | ||
| if (URL_TOKENS.has(token)) { | ||
| score.url = (score.url ?? 0) + 2; | ||
| } | ||
| if (EMAIL_TOKENS.has(token)) { | ||
| score.email = (score.email ?? 0) + 3; | ||
| } | ||
| if (PHONE_TOKENS.has(token)) { | ||
| score.phone = (score.phone ?? 0) + 3; | ||
| } | ||
| } | ||
|
|
||
| // Penalize image metadata so we don't end up adding an image URL to a prop like imageWidth | ||
| if (hasAny(tokens, NEGATIVE_IMAGE_TOKENS)) { | ||
| score.image = (score.image ?? 0) - 4; | ||
| } | ||
|
|
||
| let best: MostLikelyType | null = null; | ||
| let bestScore = 0; | ||
|
|
||
| for (const [kind, value] of Object.entries(score) as [MostLikelyType, number][]) { | ||
| if (value > bestScore) { | ||
| bestScore = value; | ||
| best = kind; | ||
| } | ||
| } | ||
|
|
||
| return best; | ||
| } | ||
|
|
||
| function normalizeStringLiteral(value: unknown) { | ||
| if (typeof value !== 'string') { | ||
| return value; | ||
| } | ||
|
|
||
| if ( | ||
| (value.startsWith('"') && value.endsWith('"')) || | ||
| (value.startsWith("'") && value.endsWith("'")) | ||
| ) { | ||
| return value.slice(1, -1); | ||
| } | ||
|
|
||
| return value; | ||
| } | ||
|
|
||
| export function generateMockValueFromSBType(sbType: SBType, propName?: string): unknown { | ||
| switch (sbType.name) { | ||
| case 'boolean': | ||
| return true; | ||
|
|
||
| case 'number': | ||
| return 42; | ||
|
|
||
| case 'string': { | ||
| const name = propName ?? ''; | ||
| const tokens = tokenize(name); | ||
| if (hasAny(tokens, COLOR_TOKENS)) { | ||
| return '#ff0000'; | ||
| } | ||
|
|
||
| if (hasAny(tokens, DATE_TOKENS)) { | ||
| return '2025-01-01'; | ||
| } | ||
|
|
||
| const mostLikelyType = getMostLikelyTypeFromTokens(tokens); | ||
|
|
||
| switch (mostLikelyType) { | ||
| case 'image': | ||
| return 'https://placehold.co/600x400?text=Storybook'; | ||
|
|
||
| case 'url': | ||
| return 'https://example.com'; | ||
|
|
||
| case 'email': | ||
| return 'storybook@example.com'; | ||
|
|
||
| case 'phone': | ||
| return '1234567890'; | ||
| } | ||
|
|
||
| return name || 'Hello world'; | ||
| } | ||
|
|
||
| case 'date': | ||
| return new Date('2025-01-01'); | ||
|
|
||
| case 'node': | ||
| return propName ?? 'Hello world'; | ||
|
|
||
| case 'function': | ||
| return '__function__'; | ||
|
|
||
| case 'literal': | ||
| return normalizeStringLiteral(sbType.value); | ||
|
|
||
| case 'object': { | ||
| const result: Record<string, unknown> = {}; | ||
|
|
||
| for (const [key, valueType] of Object.entries(sbType.value)) { | ||
| result[key] = generateMockValueFromSBType(valueType, key); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| case 'union': { | ||
| if (!sbType.value?.length) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Look for literal types in the union | ||
| const literalType = sbType.value.find((t) => t.name === 'literal'); | ||
| if (literalType?.name === 'literal') { | ||
| return normalizeStringLiteral(literalType.value); | ||
| } | ||
|
|
||
| return generateMockValueFromSBType(sbType.value[0], propName); | ||
| } | ||
|
|
||
| case 'array': { | ||
| // If we don't know what the element is, be conservative and return empty. | ||
| if (sbType.value.name === 'other') { | ||
| return []; | ||
| } | ||
| return [generateMockValueFromSBType(sbType.value, propName)]; | ||
| } | ||
|
|
||
| case 'tuple': | ||
| return sbType.value.map((el) => generateMockValueFromSBType(el)); | ||
|
|
||
| case 'enum': | ||
| return sbType.value[0] ?? propName; | ||
|
|
||
| case 'intersection': | ||
| // For intersections, combine all object types | ||
| const objectTypes = sbType.value.filter((t) => t.name === 'object'); | ||
| if (objectTypes.length > 0) { | ||
| const result: Record<string, unknown> = {}; | ||
| objectTypes.forEach((objType) => { | ||
| if (objType.name === 'object') { | ||
| Object.entries(objType.value).forEach(([key, type]) => { | ||
| result[key] = generateMockValueFromSBType(type, key); | ||
| }); | ||
| } | ||
| }); | ||
| return result; | ||
| } | ||
| return {}; | ||
|
|
||
| case 'other': { | ||
| const value = sbType.value; | ||
| if (value?.startsWith('React') || value?.includes('Event') || value?.includes('Element')) { | ||
| return '__function__'; | ||
| } | ||
|
|
||
| if (value === 'null') { | ||
| return null; | ||
| } | ||
|
|
||
| if (value === 'void' || value === 'undefined') { | ||
| return undefined; | ||
| } | ||
|
|
||
| return value ?? propName; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function generateMockPropsFromDocgen(argTypesData: ComponentArgTypesData | null) { | ||
| const required: Record<string, unknown> = {}; | ||
| const optional: Record<string, unknown> = {}; | ||
|
|
||
| if (!argTypesData?.props) { | ||
| return { required, optional }; | ||
| } | ||
|
|
||
| for (const [propName, propInfo] of Object.entries(argTypesData.props)) { | ||
| const mockValue = generateMockValueFromSBType(propInfo.type, propName); | ||
|
|
||
| if (propInfo.required) { | ||
| required[propName] = mockValue; | ||
| } else { | ||
| optional[propName] = mockValue; | ||
| } | ||
| } | ||
|
|
||
| return { required, optional }; | ||
| } |
There was a problem hiding this comment.
There appear to be two nearly identical files: get-mocked-props-for-args.ts and get-dummy-props-for-args.ts. These files contain almost the same code with only minor naming differences (mock vs dummy, __function__ vs STORYBOOK_FN_PLACEHOLDER). This creates significant code duplication and maintenance burden. Consider consolidating these files into a single implementation.
| export const cachedFindUp = cached(find.up, { name: 'findUp' }); | ||
|
|
||
| export const cachedResolveImport = cached(resolveImport, { name: 'resolveImport' }); | ||
| export const cachedResolveImport: any = cached(resolveImport, { name: 'resolveImport' }); |
There was a problem hiding this comment.
The type annotation : any defeats TypeScript's type checking. Consider using a more specific type like typeof cached or the actual function signature that's expected.
| export const cachedResolveImport: any = cached(resolveImport, { name: 'resolveImport' }); | |
| export const cachedResolveImport: typeof resolveImport = cached(resolveImport, { | |
| name: 'resolveImport', | |
| }); |
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 (2)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (2)
15-27: Addspy: trueoption to mock declarations per coding guidelines.The mock for
storybook/internal/commonshould include thespy: trueoption to follow the project's Vitest mocking standards. This ensures that the original module's behavior is preserved while allowing selective overrides.As per coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."♻️ Proposed refactor
-vi.mock('storybook/internal/common', async (importOriginal) => { +vi.mock('storybook/internal/common', { spy: true }, async (importOriginal) => { const actual = await importOriginal<typeof import('storybook/internal/common')>(); return { ...actual, getProjectRoot: () => process.cwd(), extractFrameworkPackageName: (name: string) => name?.replace('@storybook/', '') || '', normalizeStories: actual.normalizeStories || ((stories) => stories), getStoryId: async () => ({ storyId: 'components-page--default', kind: 'components-page', }), }; });
35-41: Addspy: trueoption to filesystem mock per coding guidelines.The mock for
node:fs/promisesshould include thespy: trueoption to follow the project's Vitest mocking standards.As per coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."♻️ Proposed refactor
-vi.mock('node:fs/promises', async (importOriginal) => { +vi.mock('node:fs/promises', { spy: true }, async (importOriginal) => { const actual = await importOriginal<typeof import('node:fs/promises')>(); return { ...actual, writeFile: mockFs.writeFile, }; });
🤖 Fix all issues with AI agents
In @code/core/src/core-server/server-channel/create-new-story-channel.ts:
- Around line 34-36: Refactor the GenerateStoryResult type into a discriminated
union (success: true with required storyId, kind, storyFilePath,
exportedStoryName; success: false with optional fields and error info) in
generate-story.ts so TypeScript can narrow inside if (result.success) blocks;
then remove the non-null assertions on result.storyId, result.storyFilePath, and
result.exportedStoryName in create-new-story-channel (where result.success is
checked) and rely on the narrowed types instead.
In @code/core/src/core-server/utils/generate-story.ts:
- Line 77: The relative path calculation uses process.cwd() causing inconsistent
results because storyFilePath (from getNewStoryFile()) is built using
getProjectRoot(); import and call getProjectRoot() and use its return value
instead of process.cwd() when computing relativeStoryFilePath (i.e., replace the
second argument to the relative(...) call), ensuring relativeStoryFilePath is
computed relative to the project root used by getNewStoryFile().
🧹 Nitpick comments (1)
code/core/src/core-server/utils/get-new-story-file.ts (1)
172-189: Consider logging errors in checkForImportsMap for debugging.The catch-all error handler at line 186 silently returns
falsefor any failure. While this is acceptable for a feature-detection helper, consider logging errors at debug level to aid troubleshooting, especially for JSON parse errors or permission issues.💡 Optional improvement
} catch { + logger.debug('Failed to check for imports map in package.json'); return false; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
code/core/src/core-server/index.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/server-channel/create-new-story-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.tscode/core/src/core-server/index.tscode/core/src/core-server/utils/generate-story.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/node-logger` for server-side logging in Node.js code
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/server-channel/create-new-story-channel.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Export functions from modules if they need to be tested
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Sandboxes are generated outside the repository at `../storybook-sandboxes/` by default, not in `./sandbox` or `code/sandbox/`
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 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/core-server/index.tscode/core/src/core-server/server-channel/create-new-story-channel.test.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/core/src/core-server/index.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{renderers}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/client-logger` for client-side logging in browser code
Applied to files:
code/core/src/core-server/index.tscode/core/src/core-server/utils/get-new-story-file.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/server-channel/create-new-story-channel.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `vi.mock()` to mock file system, loggers, and other external dependencies in tests
Applied to files:
code/core/src/core-server/utils/get-new-story-file.ts
🧬 Code graph analysis (3)
code/core/src/core-server/utils/get-dummy-props-for-args.test.ts (2)
code/core/src/core-server/utils/get-dummy-props-for-args.ts (3)
generateDummyValueFromSBType(142-265)generateDummyPropsFromDocgen(267-286)ComponentArgTypesData(8-10)code/core/src/core-server/index.ts (1)
ComponentArgTypesData(14-14)
code/core/src/core-server/server-channel/create-new-story-channel.test.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
stories(967-969)
code/core/src/core-server/utils/get-new-story-file.ts (3)
code/core/src/csf-tools/ConfigFile.ts (3)
isCsfFactoryPreview(1177-1192)loadConfig(1150-1153)printConfig(1159-1161)code/core/src/core-server/utils/get-dummy-props-for-args.ts (2)
generateDummyPropsFromArgTypes(18-48)STORYBOOK_FN_PLACEHOLDER(12-12)code/core/src/common/utils/formatter.ts (1)
formatFileContent(51-69)
⏰ 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: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (8)
code/renderers/react/src/componentManifest/reactDocgen/extractDocgenInfo.ts (1)
1-222: LGTM! Well-structured DocGen extraction utility.The implementation provides comprehensive type mapping from React DocGen to SBType, with proper error handling and support for complex types including unions, arrays, tuples, and object signatures. The allowlist for Array element types (lines 88-113) now correctly includes React component types, addressing the previously identified inconsistency.
code/core/src/core-server/index.ts (1)
12-14: LGTM! Public API exports are correctly structured.The new exports for
generateStoryFile,GenerateStoryResult,GenerateStoryOptions, andComponentArgTypesDatafollow the established pattern and properly expose the story generation utilities for external consumption.code/core/src/core-server/utils/get-new-story-file.ts (4)
66-88: LGTM! Experimental ArgTypes generation with proper fallback.The experimental ArgTypes extraction is well-implemented with appropriate error handling and debug logging. The code gracefully falls back to no args if extraction fails, ensuring the feature doesn't break story generation.
191-219: LGTM! Efficient placeholder replacement with AST transformation.The function has a smart early exit (line 197) to avoid expensive babel parsing when no placeholders are present. The AST traversal correctly identifies and replaces
STORYBOOK_FN_PLACEHOLDERstring literals withfn()calls and conditionally adds the necessary import.
133-147: LGTM! Proper content processing pipeline.The story content is correctly processed through placeholder replacement (line 133) and formatting (line 140) before being returned. The previously identified bug with
dirnamehas been resolved - the return object now correctly includesdirname: dir.
18-18: The empathic/walk import is correct.The namespace import
import * as walk from 'empathic/walk'is valid for empathic v2.0.0. Thewalk.up()function exists and correctly returns an iterable of parent directories when called with the{ last }option, matching the usage at line 175.code/core/src/core-server/utils/generate-story.ts (2)
64-107: LGTM! Well-structured story generation API.The function provides a clean, result-based API with comprehensive JSDoc, proper TypeScript typing, and appropriate error handling. The
checkFileExistsdefault prevents accidental file overwrites, and the error categorization (STORY_FILE_EXISTS vs UNKNOWN) enables proper handling by callers.
11-29: LGTM! Well-designed type interfaces.The
GenerateStoryResultinterface provides a clean result type withsuccessas a discriminator, and optional fields for metadata and errors. TheGenerateStoryOptionsinterface is properly documented with JSDoc including the default value.
| storyId: result.storyId!, | ||
| storyFilePath: result.storyFilePath!, | ||
| exportedStoryName: result.exportedStoryName!, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor GenerateStoryResult to use a discriminated union for type safety.
The non-null assertions on storyId, storyFilePath, and exportedStoryName are necessary because GenerateStoryResult uses success: boolean instead of a discriminated union with literal types. This prevents TypeScript from narrowing the optional fields within the if (result.success) block.
♻️ Recommended refactor
In code/core/src/core-server/utils/generate-story.ts, restructure GenerateStoryResult as a discriminated union:
export type GenerateStoryResult =
| {
success: true;
storyId: string;
kind: string;
storyFilePath: string;
exportedStoryName: string;
}
| {
success: false;
kind?: string;
storyFilePath?: string;
error?: string;
errorType?: 'STORY_FILE_EXISTS' | 'UNKNOWN';
};This allows TypeScript to automatically narrow types inside if (result.success) blocks, eliminating the need for non-null assertions at lines 34-36 and line 54.
🤖 Prompt for AI Agents
In @code/core/src/core-server/server-channel/create-new-story-channel.ts around
lines 34 - 36, Refactor the GenerateStoryResult type into a discriminated union
(success: true with required storyId, kind, storyFilePath, exportedStoryName;
success: false with optional fields and error info) in generate-story.ts so
TypeScript can narrow inside if (result.success) blocks; then remove the
non-null assertions on result.storyId, result.storyFilePath, and
result.exportedStoryName in create-new-story-channel (where result.success is
checked) and rely on the narrowed types instead.
| } | ||
|
|
||
| if (hasAny(tokens, DATE_TOKENS)) { | ||
| return '2025-01-01'; |
There was a problem hiding this comment.
This is bad because you never know if the date will be past or future
| } | ||
|
|
||
| case 'date': | ||
| return new Date('2025-01-01'); |
| } | ||
| }; | ||
|
|
||
| export async function experimental_getArgTypesData( |
There was a problem hiding this comment.
I recommend renaming this to internal_getArgTypesData, it's a minor thing, but we've used experiment in placed we wanted users to try it.
Which is not the case here.
| } | ||
| }; | ||
|
|
||
| export async function experimental_getArgTypesData( |
There was a problem hiding this comment.
Normally preset preset properties adhere to a system where they can be chained.
So the pattern to adhere to is:
function<A>(input: A, o: Options): AThis experimental_getArgTypesData's input and output are not the same; making it unchainable.
If this is likely to be replaced with some actual system later, that is likely okay for now..
| memoStore = new WeakMap(); | ||
| }; | ||
|
|
||
| export const cachedReadFileSync = cached(readFileSync, { name: 'cachedReadFile' }); |
There was a problem hiding this comment.
I can see this is pre-existing code, so hard to do anything about it, but it's generally recommended to avoid using synchronous fs-APIs if it can be prevented.
Like I mentioned in our call, I was very much expecting the argType functions to be async, because:
- It needs to interact with the
fs - We'll want to write these (at some point) for all renderers, and the API will have to be the same for all (since core-server will consume it) Though I guess there's no harm in
awaiting a synchronous value?
Closes #
What I did
This PR refactors the story generation logic (not save story, but create story) so that it is more detached, does not depend on Storybook to be running and also requires data from docgen and not argtypes directly. Something along the lines of:
It introduces a new preset which receives a component file path and export name, and then processes that over docgen (each renderer has to implement their own) and returns argtypes from that.
Additionally, there's a new function to generate dummy data based on argtypes, which is much improved and provides more features in the story generation. It now also accounts for handlers and uses
fnfromstorybook/testand adds the necessary import for that.Here's an example of before/after the new capabilities
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
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
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.