CLI: Fix issue with running Storybook after being initialized #32929
Conversation
|
View your CI Pipeline Execution ↗ for commit f3d21dc
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe initiate.ts file has been refactored to extract Storybook dev server spawning logic into an internal helper function. The spawning mechanism was changed from a synchronous package manager call to using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
✨ Finishing touches
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: 2
🧹 Nitpick comments (1)
code/lib/create-storybook/src/initiate.ts (1)
592-592: Replaceconsole.logwith the logger.The code uses
console.logdirectly, which violates the coding guidelines for this file. Use the importedloggerinstead.As per coding guidelines, apply this diff:
- console.log(err); + logger.error(err);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/lib/create-storybook/src/initiate.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/lib/create-storybook/src/initiate.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/lib/create-storybook/src/initiate.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/lib/create-storybook/src/initiate.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/lib/create-storybook/src/initiate.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to scripts/**/*.{ts,js,mjs} : In Node.js scripts, use `storybook/internal/node-logger` instead of `console.*`
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
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`.
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.
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : In application code, use Storybook loggers instead of `console.*` (client code: `storybook/internal/client-logger`; server code: `storybook/internal/node-logger`)
Applied to files:
code/lib/create-storybook/src/initiate.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/lib/create-storybook/src/initiate.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to code/.storybook/** : Place internal UI Storybook configuration in `code/.storybook/` and maintain it there
Applied to files:
code/lib/create-storybook/src/initiate.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/lib/create-storybook/src/initiate.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/lib/create-storybook/src/initiate.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/lib/create-storybook/src/initiate.ts (2)
812-813: LGTM!The delegation to the new helper function is clean and properly awaited.
35-36: The original review comment is incorrect.The dependency verification shows
execais properly declared indevDependencieswith version^5.0.0. Theeslint-disable-next-line depend/ban-dependenciessuppression is an established pattern in Storybook for bundled development dependencies (consistent with howfast-printfis handled per the codebase conventions).Additionally, the fire-and-forget pattern for
execa.command()at line 858 is intentional and documented in the code comment explaining it spawns a subprocess in the user's project directory to avoid path issues.Likely an incorrect or invalid review comment.
| const supportsOnboarding = [ | ||
| ProjectType.REACT_SCRIPTS, | ||
| ProjectType.REACT, | ||
| ProjectType.WEBPACK_REACT, | ||
| ProjectType.REACT_PROJECT, | ||
| ProjectType.NEXTJS, | ||
| ProjectType.VUE3, | ||
| ProjectType.ANGULAR, | ||
| ].includes(projectType); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use the existing ONBOARDING_PROJECT_TYPES constant.
The supportsOnboarding check duplicates the ONBOARDING_PROJECT_TYPES constant defined at lines 66-75. This creates a maintenance burden if the list changes.
Apply this diff to use the existing constant:
- const supportsOnboarding = [
- ProjectType.REACT_SCRIPTS,
- ProjectType.REACT,
- ProjectType.WEBPACK_REACT,
- ProjectType.REACT_PROJECT,
- ProjectType.NEXTJS,
- ProjectType.VUE3,
- ProjectType.ANGULAR,
- ].includes(projectType);
+ const supportsOnboarding = ONBOARDING_PROJECT_TYPES.includes(projectType);📝 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 supportsOnboarding = [ | |
| ProjectType.REACT_SCRIPTS, | |
| ProjectType.REACT, | |
| ProjectType.WEBPACK_REACT, | |
| ProjectType.REACT_PROJECT, | |
| ProjectType.NEXTJS, | |
| ProjectType.VUE3, | |
| ProjectType.ANGULAR, | |
| ].includes(projectType); | |
| const supportsOnboarding = ONBOARDING_PROJECT_TYPES.includes(projectType); |
🤖 Prompt for AI Agents
In code/lib/create-storybook/src/initiate.ts around lines 830 to 838, the
supportsOnboarding variable re-declares the list of onboarding project types
instead of using the existing ONBOARDING_PROJECT_TYPES constant defined earlier
(lines 66-75); replace the inline array with
ONBOARDING_PROJECT_TYPES.includes(projectType) so the check reuses the canonical
constant (ensure the constant is in scope/imported if necessary).
| execa.command(`${storybookCommand} ${flags.join(' ')}`, { | ||
| stdio: 'inherit', | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Missing await on the execa.command call.
The execa.command() call at line 858 is not awaited, which means the function returns immediately without waiting for the Storybook dev server to start. This appears unintentional based on the async function signature and may cause issues if the caller expects the process to complete or if error handling is needed.
If the intent is to spawn a long-running dev server and return immediately (fire-and-forget), consider:
- Removing the
try-catchwrapper since errors won't be caught from an unawaited promise - Adding a comment explaining the fire-and-forget pattern
- Optionally handling the returned ChildProcess for cleanup
If the process should be awaited, apply this diff:
- execa.command(`${storybookCommand} ${flags.join(' ')}`, {
+ await execa.command(`${storybookCommand} ${flags.join(' ')}`, {
stdio: 'inherit',
});🏁 Script executed:
#!/bin/bash
# View the file context around lines 858-860 with surrounding code and try-catch wrapper
sed -n '820,900p' code/lib/create-storybook/src/initiate.tsLength of output: 1375
🏁 Script executed:
#!/bin/bash
# Search for similar execa patterns with fire-and-forget or .catch() usage
rg -n -B2 -A2 'execa\.(command|.*)\(' code/ --type ts --type tsx | head -100Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for execa patterns in TypeScript files to find similar patterns
rg -n 'execa\.' code/ -g '*.ts' -A3 | head -150Length of output: 626
🏁 Script executed:
#!/bin/bash
# View context around the awaited execa call for comparison
sed -n '180,210p' code/lib/create-storybook/src/scaffold-new-project.tsLength of output: 796
🏁 Script executed:
#!/bin/bash
# Search for .catch() patterns on execa or similar async calls
rg -n '\.catch\(' code/lib/create-storybook/ -g '*.ts' -B2 -A2Length of output: 744
Await the execa.command() call.
The review comment is correct. The initiate() function is called with await in code/lib/create-storybook/src/bin/run.ts:59 (.catch(() => process.exit(1))), which means the caller expects the returned promise to complete before handling errors. Without awaiting execa.command() at line 858, the function returns immediately and breaks the promise chain, making the try-catch ineffective.
Apply this fix:
- execa.command(`${storybookCommand} ${flags.join(' ')}`, {
+ await execa.command(`${storybookCommand} ${flags.join(' ')}`, {
stdio: 'inherit',
});📝 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.
| execa.command(`${storybookCommand} ${flags.join(' ')}`, { | |
| stdio: 'inherit', | |
| }); | |
| await execa.command(`${storybookCommand} ${flags.join(' ')}`, { | |
| stdio: 'inherit', | |
| }); |
🤖 Prompt for AI Agents
In code/lib/create-storybook/src/initiate.ts around lines 858 to 860, the
execa.command(...) call is not awaited so the function returns before the
spawned process completes, breaking the caller's promise chain and error
handling; update the code to either prepend await to execa.command(...) or
return the execa.command(...) promise so the initiate() function only resolves
after the command finishes, ensuring errors are propagated to the caller.
CLI: Fix issue with running Storybook after being initialized (cherry picked from commit c6a705d)
github-actions
bot
added
the
patch:done
Closes #
What I did
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 pull request has been released as version
0.0.0-pr-32929-sha-f3d21dc6. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-32929-sha-f3d21dc6yann/fix-init-issuef3d21dc61762246041)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=32929Summary by CodeRabbit
Release Notes