Skip to content

CLI: Fix passing flags for bun users during init#33166

Merged
valentinpalkovic merged 2 commits into
nextfrom
valentin/fix-flag-passing-for-bun
Nov 25, 2025
Merged

CLI: Fix passing flags for bun users during init#33166
valentinpalkovic merged 2 commits into
nextfrom
valentin/fix-flag-passing-for-bun

Conversation

@valentinpalkovic
Copy link
Copy Markdown
Contributor

@valentinpalkovic valentinpalkovic commented Nov 25, 2025

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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/core team 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
    • Extended package manager support to include Bun alongside NPM for proper command-line argument handling in Storybook initialization.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 25, 2025

📝 Walkthrough

Walkthrough

The pull request updates the import section of initiate.ts to include PackageManagerName from storybook/internal/common and modifies the condition in runStorybookDev that determines when to pass the extra dash (--) separator for command flags. The check expands from npm-only to include both npm and bun package managers via a new doesNeedExtraDash boolean.

Changes

Cohort / File(s) Change Summary
Import and command-flag logic update
code/lib/create-storybook/src/initiate.ts
Added imports for PackageManagerName and executeCommand from storybook/internal/common. Modified the condition for adding extra dash from packageManager.type === 'npm' && projectType !== ProjectType.ANGULAR to doesNeedExtraDash && projectType !== ProjectType.ANGULAR, where doesNeedExtraDash is true for npm and bun package managers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file with localized changes to imports and one conditional statement
  • Clear, explicit logic change (expanding package manager support from npm-only to npm and bun)
  • No complex control flow modifications or multi-file architectural changes

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3d8244 and ae8201c.

📒 Files selected for processing (1)
  • code/lib/create-storybook/src/initiate.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use camelCase for variable and function names

Files:

  • code/lib/create-storybook/src/initiate.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes

Files:

  • code/lib/create-storybook/src/initiate.ts
**/*.{ts,tsx,js,jsx,json,html,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes

Files:

  • code/lib/create-storybook/src/initiate.ts
code/**/!(*.test).{ts,tsx,js,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
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/create-storybook/src/initiate.ts
🧠 Learnings (8)
📓 Common learnings
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: 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.
📚 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/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
📚 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/create-storybook/src/initiate.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: The Storybook repository is a TypeScript/React monorepo with main codebase in 'code/' directory and tooling in 'scripts/' directory

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-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error

Applied to files:

  • code/lib/create-storybook/src/initiate.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error

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)

2-6: LGTM: Import changes support the bug fix.

The addition of PackageManagerName enables more explicit package manager type checking in the flag-passing logic below.


188-193: Type compatibility verified, but bun's need for -- separator requires explicit confirmation and test coverage.

The enum type compatibility is confirmed correct: PackageManagerName.NPM = 'npm' and PackageManagerName.BUN = 'bun' are string literals that validly compare against packageManager.type.

However:

  1. Bun's flag behavior is undocumented: Web search of official bun documentation did not reveal confirmation that bun requires the -- separator when passing flags to scripts—unlike npm where this is a known requirement.
  2. No test coverage: The change lacks tests to verify bun's behavior and ensure this doesn't break scripts in bun environments.

Confirm with bun documentation or testing that bun run <script> -- --flags is necessary, and add test coverage for this change.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Nov 25, 2025

View your CI Pipeline Execution ↗ for commit a9ee9bc

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 46s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-25 17:55:30 UTC

@valentinpalkovic valentinpalkovic merged commit 13d4773 into next Nov 25, 2025
63 of 66 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/fix-flag-passing-for-bun branch November 25, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants