CLIInitiate: Simplify the CLI init port flag detection#33546
Conversation
…ing port handling and command execution.
|
View your CI Pipeline Execution ↗ for commit 629696c
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughReplaces a separate flags array with a unified Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as create-storybook CLI
participant Initiate as initiate.ts
participant Core as storybook/internal/core-server
participant Spawn as child_process.spawn
participant Storybook as Storybook Dev Server
CLI->>Initiate: invoke start (storybookCommand, options)
Initiate->>Core: getServerPort(default 6006)
Core-->>Initiate: availablePort
Initiate->>Initiate: parts = storybookCommand.split(' ')
Initiate->>Initiate: conditionally insert `--`, `--silent`, `-p <port>`, `--initial-path=/onboarding`, `--quiet`
Initiate->>Spawn: spawn(command=parts[0], args=parts[1..], stdio=inherit)
Spawn->>Storybook: launch dev server on chosen port
Storybook-->>CLI: runtime output (via inherited stdio)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{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:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
📚 Learning: 2025-11-05T09:38:47.712ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
🧬 Code graph analysis (1)code/lib/create-storybook/src/initiate.ts (2)
⏰ 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)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting Comment |
yannbf
left a comment
yannbf
left a comment
There was a problem hiding this comment.
LGTM, as long as you test the canary and make sure things work as intended. Because the change made affects every single project and the storybookCommand logic, I'd advise to at least try on 3 different projects, one being Angular.
…e same argument twice) Add a wrapper preventing adding any sbFLags when this is not supported (angular was broken)
…s/storybook into norbert/improve-cli-init-port
…'t actually include the `-p` part, it just has `npm run storybook` in it, so the duplicate `-p` argument is unavoidable
![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
🤖 Fix all issues with AI agents
In `@code/lib/create-storybook/src/initiate.ts`:
- Around line 216-218: Remove the debug console.log call that prints args;
replace it with the project logger or remove entirely: remove the line
"console.log({ args })" inside the block where "const [command, ...args] =
[...parts]" is defined, and if logging is required use the storybook logger
(import logger from 'storybook/internal/node-logger') and call
logger.info/logger.debug with a clear message referencing command and args
(e.g., logger.debug({ command, args })); ensure you add the import if you use
logger.
- Around line 198-210: The port flag is being added as a single string token
(parts.push(`-p ${availablePort}`)) which will be passed as one argument to the
spawned process; change the logic that builds the parts array so the port flag
and its value are pushed as two separate arguments (e.g., push the flag token
and the port value separately) where availablePort is used, ensuring the spawned
CLI receives '-p' and the port number as distinct arguments; keep the existing
conditions around supportsOnboarding and shouldOnboard and leave other pushes
(like '--initial-path=/onboarding' and '--quiet') unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/lib/create-storybook/src/initiate.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/lib/create-storybook/src/initiate.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/lib/create-storybook/src/initiate.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/lib/create-storybook/src/initiate.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/lib/create-storybook/src/initiate.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/lib/create-storybook/src/initiate.ts
🧬 Code graph analysis (1)
code/lib/create-storybook/src/initiate.ts (2)
code/core/src/core-server/index.ts (1)
getServerPort(34-34)code/lib/create-storybook/src/services/FeatureCompatibilityService.ts (1)
supportsOnboarding(29-33)
⏰ 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 (1)
code/lib/create-storybook/src/initiate.ts (1)
179-183: LGTM!The parts-based approach for building the command is cleaner than maintaining a separate flags array. The
--silentflag is correctly pushed as a single argument.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@yannbf unfortunately the double I think we do not actually want to configure/initialize the project any differently based on if the port was taken. So I'd say considering the double I've tested angular, and actually found a bug, and fixed it (not related to the port flag) |
2 participants
What I did
We discovered that the previous fix does functionally work, but it's still adding the
-p-flag twice.This is supported by commander (the underlaying library used for storybook CLI, so functionally we're okay.
This PR is meant to cleanup so the
-pargument is replaced properly.When using
pnpmthe command is shown to the user, and we want it to be displayed correctly.This is wrong:
In my canary testing is discovered that a sbFlag was added to angular empty init, which isn't supported, so I added a clear guard against this happening again in the future.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
I'm releasing another canary on this PR, and testing with
pnpmDocumentation
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-33546-sha-66ad959a. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-33546-sha-66ad959a sandboxor in an existing project withnpx storybook@0.0.0-pr-33546-sha-66ad959a upgrade.More information
0.0.0-pr-33546-sha-66ad959anorbert/improve-cli-init-port66ad959a1768475351)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=33546Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.