-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
CLI: Add create-story command #33399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: yann/refactor-story-generation
Are you sure you want to change the base?
CLI: Add create-story command #33399
Conversation
|
View your CI Pipeline Execution ↗ for commit 80bd496
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThis PR adds a file-system tree selection prompt API and UI, integrates it into automigrate and a new CLI command Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (generate-stories)
participant FileTree as FileSystemTreeSelect
participant Storybook as Storybook Config
participant Analyzer as Component Analyzer
participant StoryGen as Story Generator
participant FS as File System
User->>CLI: run generate-stories (interactive or glob)
alt interactive
CLI->>FileTree: request tree (root, filters, glob)
FileTree->>FS: read tree
FS-->>FileTree: directory tree
FileTree->>User: present tree UI
User-->>FileTree: select file(s)
FileTree-->>CLI: selected paths
else glob
CLI->>FS: resolve glob pattern
FS-->>CLI: matching paths
end
CLI->>Storybook: load config (configDir)
Storybook-->>CLI: config & metadata
CLI->>Analyzer: analyze selected files
Analyzer->>FS: read source files
FS-->>Analyzer: source code
Analyzer-->>CLI: ComponentInfo[] (exports, complexity)
CLI->>StoryGen: generate stories for components
loop per component
StoryGen->>FS: check for existing story
FS-->>StoryGen: exists?
alt write required
StoryGen->>FS: write story file
FS-->>StoryGen: written
else skip
StoryGen-->>CLI: skipped
end
end
StoryGen-->>CLI: results (generated, skipped, failed)
CLI->>User: report results
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 ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{js,jsx,ts,tsx,json,md,html,css,scss}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,jsx,json,html,ts,tsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-22T22:03:40.109ZApplied to files:
⏰ 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)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
code/lib/cli-storybook/src/generate-stories.ts (2)
247-249: Remove redundant dynamic imports.Lines 248-249 dynamically import
readFile,basename, andextnameinside the function, butreadFileis already imported at the top of the file (line 2), andbasenameandextnameare not imported but could be. Dynamic imports here add unnecessary overhead and complexity.🔎 Proposed fix
Update the top-level imports:
import { existsSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; -import { dirname, join } from 'node:path'; +import { basename, dirname, extname, join } from 'node:path';Then remove the dynamic imports from the function:
async function analyzeComponentFile(filePath: string): Promise<ComponentInfo[] | null> { - const { readFile } = await import('node:fs/promises'); - const { basename, extname } = await import('node:path'); - try { const content = await readFile(filePath, 'utf-8');
258-316: Consider adding JSDoc comments for the component analysis logic.The regex-based parsing approach is pragmatic for a first implementation, though it may not catch all edge cases (e.g., multiline exports, exports with decorators, re-exported symbols). The fallback logic based on filename convention is a good touch.
Consider documenting known limitations of the regex-based approach in a comment, or adding a JSDoc comment explaining the analysis strategy and its trade-offs.
code/core/src/core-server/utils/get-new-story-file.ts (1)
44-46: Use path-aware comparison instead of string startsWith.The
startsWithcheck is string-based and vulnerable to issues on Windows with mixed separators (e.g.,C:\\projectvsC:/project), case-insensitive filesystems like macOS and Windows, and prefix matching errors (e.g.,/projectincorrectly matching/project-other). Replace withisAbsolute()+relative()+ checking for..to safely detect if a path is within the project root:let relativeDir: string; if (isAbsolute(dir)) { const relativePath = relative(projectRoot, dir); relativeDir = relativePath.startsWith('..') ? dir : relativePath; } else { relativeDir = dir; }This requires adding
isAbsoluteto the existing import from'node:path'.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
code/core/package.json(2 hunks)code/core/src/core-server/utils/get-new-story-file.ts(3 hunks)code/core/src/node-logger/index.ts(1 hunks)code/core/src/node-logger/prompts/prompt-functions.ts(3 hunks)code/core/src/node-logger/prompts/prompt-provider-base.ts(2 hunks)code/core/src/node-logger/prompts/prompt-provider-clack.ts(3 hunks)code/lib/cli-storybook/src/automigrate/index.ts(1 hunks)code/lib/cli-storybook/src/bin/run.ts(2 hunks)code/lib/cli-storybook/src/generate-stories.ts(1 hunks)code/lib/cli-storybook/test/default/cli.test.cjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/lib/cli-storybook/src/automigrate/index.tscode/core/package.jsoncode/core/src/node-logger/prompts/prompt-provider-clack.tscode/core/src/node-logger/prompts/prompt-functions.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/node-logger/index.tscode/lib/cli-storybook/src/bin/run.tscode/core/src/node-logger/prompts/prompt-provider-base.tscode/lib/cli-storybook/src/generate-stories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/lib/cli-storybook/src/automigrate/index.tscode/core/src/node-logger/prompts/prompt-provider-clack.tscode/core/src/node-logger/prompts/prompt-functions.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/node-logger/index.tscode/lib/cli-storybook/src/bin/run.tscode/core/src/node-logger/prompts/prompt-provider-base.tscode/lib/cli-storybook/src/generate-stories.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/lib/cli-storybook/src/automigrate/index.tscode/core/src/node-logger/prompts/prompt-provider-clack.tscode/core/src/node-logger/prompts/prompt-functions.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/node-logger/index.tscode/lib/cli-storybook/src/bin/run.tscode/core/src/node-logger/prompts/prompt-provider-base.tscode/lib/cli-storybook/src/generate-stories.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/lib/cli-storybook/src/automigrate/index.tscode/core/src/node-logger/prompts/prompt-provider-clack.tscode/core/src/node-logger/prompts/prompt-functions.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/node-logger/index.tscode/lib/cli-storybook/src/bin/run.tscode/core/src/node-logger/prompts/prompt-provider-base.tscode/lib/cli-storybook/src/generate-stories.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/lib/cli-storybook/src/automigrate/index.tscode/core/package.jsoncode/core/src/node-logger/prompts/prompt-provider-clack.tscode/core/src/node-logger/prompts/prompt-functions.tscode/core/src/core-server/utils/get-new-story-file.tscode/core/src/node-logger/index.tscode/lib/cli-storybook/src/bin/run.tscode/core/src/node-logger/prompts/prompt-provider-base.tscode/lib/cli-storybook/src/generate-stories.ts
🧠 Learnings (5)
📓 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/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/node-logger/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/lib/cli-storybook/src/generate-stories.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/lib/cli-storybook/src/generate-stories.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/lib/cli-storybook/src/generate-stories.ts
🧬 Code graph analysis (5)
code/lib/cli-storybook/src/automigrate/index.ts (3)
code/core/src/node-logger/index.ts (1)
prompt(7-7)code/lib/cli-storybook/src/automigrate/index.test.ts (1)
prompt(59-61)code/lib/cli-storybook/src/automigrate/fixes/remove-essentials.ts (1)
prompt(125-127)
code/core/src/node-logger/prompts/prompt-provider-clack.ts (1)
code/core/src/node-logger/prompts/prompt-provider-base.ts (2)
FileSystemTreeSelectPromptOptions(42-57)PromptOptions(59-61)
code/lib/cli-storybook/src/bin/run.ts (2)
code/core/src/core-server/withTelemetry.ts (1)
withTelemetry(155-211)code/lib/cli-storybook/src/generate-stories.ts (1)
generateStories(89-211)
code/core/src/node-logger/prompts/prompt-provider-base.ts (2)
code/core/src/node-logger/index.ts (1)
FileSystemTreeSelectPromptOptions(12-12)code/core/src/node-logger/prompts/prompt-functions.ts (3)
FileSystemTreeSelectPromptOptions(27-27)BasePromptOptions(22-22)PromptOptions(28-28)
code/lib/cli-storybook/src/generate-stories.ts (1)
code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (1)
JsPackageManagerFactory(29-201)
⏰ 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: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (17)
code/lib/cli-storybook/test/default/cli.test.cjs (1)
57-58: LGTM!Test assertions for the new
generate-storiescommand follow the established pattern and correctly verify the command's presence in the help output.code/core/src/node-logger/index.ts (1)
9-13: LGTM!The new
FileSystemTreeSelectPromptOptionstype export is properly grouped with related prompt types and follows the established export pattern.code/lib/cli-storybook/src/bin/run.ts (1)
306-345: LGTM with minor observations.The new
generate-storiescommand follows the established CLI pattern well. The implementation correctly:
- Uses
withTelemetrywrapper (even if temporarily disabled)- Handles errors via
handleCommandFailure- Parses options with sensible defaults
- Reports success/failure with appropriate exit codes
Two observations for awareness:
- The
@ts-expect-erroron line 317 indicates the telemetry event type needs to be added if this command becomes permanent.- Telemetry is explicitly disabled (
disableTelemetry: true), which is reasonable for an experimental command.code/core/src/node-logger/prompts/prompt-functions.ts (1)
113-118: LGTM!The
fileSystemTreeSelectwrapper function follows the established pattern of other prompt wrappers in this file, properly delegating to the prompt provider.code/core/src/node-logger/prompts/prompt-provider-base.ts (1)
107-110: LGTM!The abstract method signature is well-defined and consistent with the options interface.
code/core/src/node-logger/prompts/prompt-provider-clack.ts (2)
22-57: LGTM!The
filterTreeByGlobhelper function is well-implemented:
- Correctly handles both files (leaf nodes) and directories (with children)
- Properly converts absolute paths to relative paths for glob matching
- Recursively prunes directories that have no matching children
- Uses picomatch with the correct
cwdoption
132-163: Implementation looks good with one consideration.The
fileSystemTreeSelectmethod is well-implemented, following the patterns of other prompt methods in this class. It properly:
- Provides sensible default filtering (excludes
node_modules,.git,dist)- Integrates glob filtering on top of the file tree
- Handles cancellation and logging consistently
One consideration: The method builds the entire file tree before filtering by glob (lines 143-152). For large directories, this could be slow. However, the
maxDepthoption mitigates this, and the current approach maintains clean separation of concerns.code/core/package.json (2)
226-226: @hipster/sb-utils cannot be verified in public npm registry.The
@hipster/sb-utilsdependency version0.0.8-canary.15.285ba09.0cannot be found in the public npm registry. This package is likely private or internal to your organization. Before merging, confirm that:
- The package is correctly configured in your private registry settings
- The canary version is intentional for your development workflow
- Team members have appropriate access to install private dependencies
266-266: Remove or clarify the clack-tree-select dependency reference.The package "clack-tree-select@^1.0.4" does not exist on the npm registry. Verify whether this is a misspelled package name, an internal/private package, or a planned future dependency. If this is a new feature in development, document it separately or confirm the correct package name (e.g., @clack/prompts provides tree selection as part of its roadmap, but no standalone tree-select package from the Clack project is currently available).
Likely an incorrect or invalid review comment.
code/lib/cli-storybook/src/generate-stories.ts (7)
1-13: LGTM!The imports are well-organized and follow the coding guidelines by using the server-side logger from
storybook/internal/node-loggerfor Node.js code.
15-72: LGTM!The filtering logic is well-structured with clear criteria for identifying "easy to Storybook" components. The error handling and logging are appropriate.
74-87: LGTM!The type definitions are clear and well-structured.
186-210: LGTM!The result reporting is clear and user-friendly, and the error handling properly extracts error messages and returns consistent result structures.
228-245: LGTM!The component extraction logic properly handles errors and aggregates results from multiple files.
319-332: LGTM!The story file existence check covers the common file extensions and naming conventions.
334-388: LGTM!The story generation loop has comprehensive error handling and provides clear feedback through logging. The logic correctly handles existing story files and the
forceflag.code/core/src/core-server/utils/get-new-story-file.ts (1)
95-95: Efficient reuse of computed paths.Using the precomputed
projectRootandrelativeDirinstead of callinggetProjectRoot()repeatedly and usingdirdirectly is more efficient and ensures consistency with the path normalization logic introduced at lines 44-46.Also applies to: 133-135
| export interface FileSystemTreeSelectPromptOptions extends BasePromptOptions { | ||
| /** The root directory to start browsing from */ | ||
| root?: string; | ||
| /** Whether to include files in the selection (default: true) */ | ||
| includeFiles?: boolean; | ||
| /** Whether to include hidden files and directories (default: false) */ | ||
| includeHidden?: boolean; | ||
| /** Maximum depth to traverse (default: Infinity) */ | ||
| maxDepth?: number; | ||
| /** Optional filter function to exclude certain paths */ | ||
| filter?: (path: string) => boolean; | ||
| /** Glob pattern(s) to filter files (based on picomatch) */ | ||
| glob?: string | string[]; | ||
| /** Whether to allow multiple selections (default: false) */ | ||
| multiple?: boolean; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default value mismatch between documentation and implementation.
The JSDoc comment on line 55 states multiple defaults to false, but the implementation in prompt-provider-clack.ts (line 158) defaults it to true:
// prompt-provider-base.ts
/** Whether to allow multiple selections (default: false) */
multiple?: boolean;
// prompt-provider-clack.ts line 158
multiple: options.multiple ?? true,Consider aligning the documentation with the actual default, or vice versa.
🔎 Proposed fix
Either update the documentation:
- /** Whether to allow multiple selections (default: false) */
+ /** Whether to allow multiple selections (default: true) */
multiple?: boolean;Or update the implementation in prompt-provider-clack.ts:
- multiple: options.multiple ?? true,
+ multiple: options.multiple ?? false,🤖 Prompt for AI Agents
In code/core/src/node-logger/prompts/prompt-provider-base.ts around lines 42-57:
the JSDoc for `multiple` says the default is false but the implementation in
prompt-provider-clack.ts sets it to true; choose one behavior and make them
consistent. Either update this JSDoc to state default true, or change the
implementation at prompt-provider-clack.ts line ~158 to use `options.multiple ??
false` (and run tests/linters to ensure no other callers rely on the true
default).
| const files = await prompt.fileSystemTreeSelect({ | ||
| message: 'Select a file to migrate', | ||
| root: process.cwd(), | ||
| includeFiles: true, | ||
| includeHidden: false, | ||
| glob: ['**/*.tsx', '**/*.json'], | ||
| maxDepth: 3, | ||
| filter: (path) => { | ||
| return !path.includes('node_modules') && !path.includes('.git'); | ||
| }, | ||
| }); | ||
| console.log({ files }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code appears to be debug/incomplete and blocks the --list flag.
Several issues with this code segment:
-
Violates logging guidelines: Uses
console.logdirectly instead of the server-side logger. As per coding guidelines, useloggerfromstorybook/internal/node-logger. -
Blocks
--listmode: The prompt runs before theif (list)check (line 141), forcing users to interact with the file selection prompt even when they just want to list available migrations. -
Unused result: The
filesvariable is logged but never used in the migration logic, suggesting this is incomplete or debug code that shouldn't be committed.
🔎 Suggested fix: Move or remove the debug code
If this is intended functionality, move it after the list check and integrate the result:
}: AutofixOptions): Promise<{
fixResults: Record<string, FixStatus>;
preCheckFailure?: PreCheckFailure;
} | null> => {
- const files = await prompt.fileSystemTreeSelect({
- message: 'Select a file to migrate',
- root: process.cwd(),
- includeFiles: true,
- includeHidden: false,
- glob: ['**/*.tsx', '**/*.json'],
- maxDepth: 3,
- filter: (path) => {
- return !path.includes('node_modules') && !path.includes('.git');
- },
- });
- console.log({ files });
if (list) {
logAvailableMigrations();
return null;
}
+
+ // If file selection is needed, add it here after the list check
+ // and use `logger.debug` instead of console.log📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const files = await prompt.fileSystemTreeSelect({ | |
| message: 'Select a file to migrate', | |
| root: process.cwd(), | |
| includeFiles: true, | |
| includeHidden: false, | |
| glob: ['**/*.tsx', '**/*.json'], | |
| maxDepth: 3, | |
| filter: (path) => { | |
| return !path.includes('node_modules') && !path.includes('.git'); | |
| }, | |
| }); | |
| console.log({ files }); | |
| if (list) { | |
| logAvailableMigrations(); | |
| return null; | |
| } | |
| // If file selection is needed, add it here after the list check | |
| // and use `logger.debug` instead of console.log |
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/automigrate/index.ts around lines 129-140, the
interactive prompt and console.log are debug/incomplete: move or remove the
prompt so it does not run before the `if (list)` check (prevent blocking
--list), replace console.log with the server-side logger (import from
storybook/internal/node-logger) and either integrate the `files` result into the
migration flow or drop it if unused; ensure the prompt only runs when not
listing (e.g., after handling `if (list)`) and that any logging uses
logger.info/error rather than console.log.
| // Filter components interactively if requested | ||
| const selectedComponents = components; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove dead code or implement the missing filtering logic.
The comment indicates that components should be filtered interactively if requested, but the code just assigns components to selectedComponents without any filtering. This appears to be either dead code or incomplete implementation.
🔎 Suggested fix
If the interactive filtering feature is not yet implemented, remove the unused variable and use components directly on line 184:
- // Filter components interactively if requested
- const selectedComponents = components;
-
// Generate stories for selected components
logger.debug('Generating stories for selected components...');
- const results = await generateStoriesForComponents(selectedComponents, options, force);
+ const results = await generateStoriesForComponents(components, options, force);Alternatively, if interactive component filtering is planned, implement it here with a prompt similar to the file selection at line 116.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Filter components interactively if requested | |
| const selectedComponents = components; | |
| // Generate stories for selected components | |
| logger.debug('Generating stories for selected components...'); | |
| const results = await generateStoriesForComponents(components, options, force); |
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/generate-stories.ts around lines 179-180, the
comment promises interactive filtering but the code only assigns const
selectedComponents = components, leaving dead/incomplete logic; either remove
selectedComponents and use components directly where referenced, or implement
interactive filtering here (reuse the file-selection/prompt pattern used earlier
at ~line 116) to prompt the user, let them pick one or many components, and set
selectedComponents to the result. Ensure any unused variable is removed to avoid
dead code and update downstream references accordingly.
| async function filterOutBarrelFiles(files: string[]) { | ||
| const filteredFiles = []; | ||
| for (const file of files) { | ||
| if (file.includes('index')) { | ||
| const content = await readFile(file, 'utf-8'); | ||
| if (!content.includes('export * from')) { | ||
| filteredFiles.push(file); | ||
| } | ||
| } else { | ||
| filteredFiles.push(file); | ||
| } | ||
| } | ||
| return filteredFiles; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve barrel file detection logic.
The current implementation has two issues:
-
Line 216:
file.includes('index')is too broad and will match any file with "index" anywhere in the path (e.g.,/indexed/Component.tsx,reindexed.ts). Use a more precise check likebasename(file).startsWith('index'). -
Line 218: Only checks for
'export * from'but doesn't catch'export { ... } from'pattern, which is also a common barrel file pattern.
🔎 Proposed fix
+import { basename } from 'node:path';
+
async function filterOutBarrelFiles(files: string[]) {
const filteredFiles = [];
for (const file of files) {
- if (file.includes('index')) {
+ const fileName = basename(file);
+ if (fileName.startsWith('index.') || fileName === 'index') {
const content = await readFile(file, 'utf-8');
- if (!content.includes('export * from')) {
+ // Check for common barrel file patterns
+ const isBarrelFile = /export\s+(?:\*|{[^}]*})\s+from\s+['"]/.test(content);
+ if (!isBarrelFile) {
filteredFiles.push(file);
}
} else {
filteredFiles.push(file);
}
}
return filteredFiles;
}🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/generate-stories.ts around lines 213 to 226, the
barrel-file detection is too broad and misses named-export barrels; replace the
current path check and export test by using
path.basename(file).startsWith('index') to only match actual index files, and
when an index file is detected read its contents and test for common barrel
export patterns (e.g. include both 'export * from' and patterns for named
exports such as an export { ... } from — use a simple regex or check for 'export
{' plus ' from') so index files that re-export members are excluded; otherwise
keep the file. Ensure to import/require path.basename if not already present.
…kjs/storybook into yann/generate-story-cli-command
Closes #
What I did
This pull request introduces a new interactive file system tree select prompt to the CLI, allowing users to visually browse and select files or directories, with advanced filtering options like glob patterns and max depth. It also adds the necessary dependencies and integrates this feature into the automigration workflow.
The changes are grouped as follows:
New File System Tree Select Prompt Feature:
fileSystemTreeSelectprompt to the CLI, allowing users to select files/directories interactively with options for filtering, glob patterns, and depth control (code/core/src/node-logger/prompts/prompt-provider-base.ts,code/core/src/node-logger/prompts/prompt-functions.ts,code/core/src/node-logger/index.ts,code/core/src/node-logger/prompts/prompt-provider-clack.ts). [1] [2] [3] [4] [5] [6] [7] [8] [9]code/lib/cli-storybook/src/automigrate/index.ts).Dependency Additions:
clack-tree-selectand its dependencies for tree view prompts, and@hipster/sb-utilsfor utility functions (code/core/package.json). [1] [2]File Path Handling Improvements:
code/core/src/core-server/utils/get-new-story-file.ts). [1] [2] [3]Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
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
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.