Skip to content

Conversation

@valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Nov 24, 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 pull request has been released as version 0.0.0-pr-33151-sha-8e7a3312. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-33151-sha-8e7a3312
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/update-clack
Commit 8e7a3312
Datetime Tue Nov 25 13:14:31 UTC 2025 (1764076471)
Workflow run 19670763766

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=33151

Summary by CodeRabbit

  • New Features

    • Spinner operations now support explicit error and cancel handlers for improved task control and feedback.
  • Chores

    • Updated dependencies to latest versions.
    • Enhanced logging verbosity during CLI initialization.

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

@valentinpalkovic valentinpalkovic self-assigned this Nov 24, 2025
@valentinpalkovic valentinpalkovic added maintenance User-facing maintenance tasks ci:normal labels Nov 24, 2025
@nx-cloud
Copy link

nx-cloud bot commented Nov 24, 2025

View your CI Pipeline Execution ↗ for commit 8e7a331

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

☁️ Nx Cloud last updated this comment at 2025-11-25 14:33:19 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

📝 Walkthrough

Walkthrough

This PR updates the @clack/prompts dependency from 1.0.0-alpha.6 to 1.0.0-alpha.7 and refactors the spinner and task API to introduce new cancel(message?) and error(message?) methods to the SpinnerInstance interface, replacing direct stop() calls in error and cancellation scenarios across the node-logger system and consumers.

Changes

Cohort / File(s) Summary
Dependency update
code/core/package.json
Updated devDependency @clack/prompts from 1.0.0-alpha.6 to 1.0.0-alpha.7
SpinnerInstance interface
code/core/src/node-logger/prompts/prompt-provider-base.ts
Added cancel(message?: string) and error(message?: string) methods to the SpinnerInstance interface alongside existing start, stop, and message methods
SpinnerInstance implementations
code/core/src/node-logger/prompts/prompt-functions.ts, prompt-provider-clack.ts
Implemented new cancel and error handlers on SpinnerInstance with state reset logic; replaced logger.log with log in interactive path; added shouldLog guards for error path in non-interactive path
Task API updates
code/core/src/node-logger/tasks.ts
Replaced task.stop() calls with task.cancel() in abort scenarios and task.error() in error-handling paths within executeTaskWithSpinner
Consumer updates
code/lib/create-storybook/src/scaffold-new-project.ts
Changed spinner error-handling from spinner.stop() to spinner.error() in scaffoldNewProject error path
Debug logging configuration
scripts/tasks/sandbox-parts.ts
Changed init CLI step log level from conditional (debug-based) to unconditional loglevel: 'debug'

Sequence Diagram

sequenceDiagram
    actor User
    participant Task
    participant Spinner
    participant Logger
    
    rect rgb(220, 240, 220)
    Note over Task,Logger: New: Error Handling Path
    User->>Task: error state
    Task->>Spinner: error(message)
    Spinner->>Logger: log error via spinnerInstance.error
    Spinner->>Spinner: reset activeSpinner
    Spinner->>Spinner: restore console patch
    end
    
    rect rgb(220, 240, 220)
    Note over Task,Logger: New: Cancellation Path
    User->>Task: abort signal
    Task->>Spinner: cancel(message)
    Spinner->>Logger: log cancel via spinnerInstance.cancel
    Spinner->>Spinner: reset activeSpinner
    Spinner->>Spinner: restore console patch
    end
    
    rect rgb(240, 240, 220)
    Note over Task,Logger: Existing: Success Path (unchanged)
    User->>Task: complete
    Task->>Spinner: stop()
    Spinner->>Spinner: cleanup
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Attention areas:
    • Verify cancel() and error() implementations are consistent across both prompt-provider-clack.ts and prompt-functions.ts
    • Ensure shouldLog('error') guards are applied consistently in non-interactive spinner paths
    • Confirm all task API call sites have been updated from stop() to the appropriate cancel() or error() method
    • Review the debug logging unconditional change in sandbox-parts.ts to confirm intent (differs from main refactor pattern)

Possibly related PRs

  • CLI: Implement design feedback #32984 — Also modifies node-logger prompt/spinner API files (prompt-functions.ts, prompt-provider-base.ts, prompt-provider-clack.ts, and tasks.ts) with overlapping changes to the spinner and task system.
✨ 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 1e80e8d and 8e7a331.

📒 Files selected for processing (1)
  • code/lib/create-storybook/src/scaffold-new-project.ts (1 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/scaffold-new-project.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/scaffold-new-project.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/scaffold-new-project.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/scaffold-new-project.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 (1)
code/lib/create-storybook/src/scaffold-new-project.ts (1)

172-190: Error-path spinner handling looks correct and consistent

Using spinner.error(...) in the executeCommand catch block while still throwing GenerateNewProjectOnInitError keeps the external behavior the same and more accurately reflects the failed state of the task. The change is cohesive with the surrounding logic and doesn’t introduce new edge cases.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 32e2bea and 0e5f1c5.

⛔ Files ignored due to path filters (1)
  • code/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • code/core/package.json (1 hunks)
  • code/core/src/node-logger/prompts/prompt-functions.ts (2 hunks)
  • code/core/src/node-logger/prompts/prompt-provider-base.ts (1 hunks)
  • code/core/src/node-logger/prompts/prompt-provider-clack.ts (1 hunks)
  • code/core/src/node-logger/tasks.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • code/core/src/node-logger/tasks.ts
  • code/core/package.json
🧰 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/core/src/node-logger/prompts/prompt-provider-base.ts
  • code/core/src/node-logger/prompts/prompt-functions.ts
  • code/core/src/node-logger/prompts/prompt-provider-clack.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/core/src/node-logger/prompts/prompt-provider-base.ts
  • code/core/src/node-logger/prompts/prompt-functions.ts
  • code/core/src/node-logger/prompts/prompt-provider-clack.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/core/src/node-logger/prompts/prompt-provider-base.ts
  • code/core/src/node-logger/prompts/prompt-functions.ts
  • code/core/src/node-logger/prompts/prompt-provider-clack.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/core/src/node-logger/prompts/prompt-provider-base.ts
  • code/core/src/node-logger/prompts/prompt-functions.ts
  • code/core/src/node-logger/prompts/prompt-provider-clack.ts
🧬 Code graph analysis (2)
code/core/src/node-logger/prompts/prompt-functions.ts (2)
code/core/src/node-logger/logger/logger.ts (1)
  • shouldLog (67-69)
code/core/src/node-logger/index.ts (1)
  • logger (49-74)
code/core/src/node-logger/prompts/prompt-provider-clack.ts (1)
code/core/src/node-logger/index.ts (1)
  • logTracker (8-8)
⏰ 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/core/src/node-logger/prompts/prompt-provider-base.ts (1)

48-49: LGTM! Clean API evolution.

The replacement of stop with cancel and the addition of a dedicated error method provides clearer semantics for different termination scenarios. The type signatures are consistent with the existing interface pattern.

code/core/src/node-logger/prompts/prompt-functions.ts (1)

125-138: LGTM! Proper state management and error handling.

Both cancel and error methods correctly:

  • Reset the activeSpinner state before restoring the console
  • Apply appropriate log level guards (info for cancel, error for error)
  • Delegate to the underlying spinner instance

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33ca730 and 9524438.

📒 Files selected for processing (1)
  • code/lib/create-storybook/src/scaffold-new-project.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/scaffold-new-project.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/scaffold-new-project.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/scaffold-new-project.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/scaffold-new-project.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 (1)
code/lib/create-storybook/src/scaffold-new-project.ts (1)

182-184: LGTM! Appropriate use of error() for failure cases.

Using spinner.error() for the failure path is semantically correct and provides better clarity than the previous stop() method.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
scripts/tasks/sandbox-parts.ts (1)

198-205: Unconditional loglevel: 'debug' may ignore the debug flag

Here loglevel is always set to 'debug', so the init step will run in debug mode even when the caller doesn't pass --debug (unless a template overrides loglevel via initOptions). Consider tying this to the debug flag or letting the CLI default handle it.

For example:

   await executeCLIStep(steps.init, {
     cwd,
     optionValues: {
-      loglevel: 'debug',
+      loglevel: debug ? 'debug' : 'info',
       yes: true,
       ...extra,
       ...(template.initOptions || {}),
     },

Please double‑check this against the current Storybook CLI loglevel/debug semantics to ensure we're not changing behavior unintentionally.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9524438 and 9876f85.

📒 Files selected for processing (1)
  • scripts/tasks/sandbox-parts.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,mjs}

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

Use camelCase for variable and function names

Files:

  • scripts/tasks/sandbox-parts.ts
**/*.{ts,tsx}

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

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

Files:

  • scripts/tasks/sandbox-parts.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:

  • scripts/tasks/sandbox-parts.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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
AGENTS.md (1)

1-47: Comprehensive and well-structured contributor guidelines—approved for merge.

The AGENTS.md file provides clear, actionable guidance covering project structure, build/test commands, coding conventions, testing workflows, and commit/PR practices. The Wallaby.js subsection (lines 23–35) is particularly valuable for developer ergonomics and troubleshooting.

Minor observations:

  • The testing section is strong, but consider cross-linking or referencing the recent updates to the spinner/task API (stop → cancel/error, see related changes in this PR) if those changes significantly impact test patterns or CLI scaffolding workflows. This would help contributors avoid surprises when they touch spinner-related code.
  • The commands shown (e.g., yarn --cwd code test) are correct; ensure they remain synchronized with any tooling changes.

Based on learnings, updating relevant documentation for significant changes is valued in this repository. If the spinner/task API changes represent significant API surface changes, consider whether a migration guide or changelog entry complements this file.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9876f85 and e02f7e6.

📒 Files selected for processing (2)
  • AGENTS.md (1 hunks)
  • code/lib/create-storybook/src/scaffold-new-project.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/lib/create-storybook/src/scaffold-new-project.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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 README.md : Update relevant README files for significant changes

Applied to files:

  • AGENTS.md
📚 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:

  • AGENTS.md
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.

Applied to files:

  • AGENTS.md
⏰ 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

@valentinpalkovic valentinpalkovic merged commit a3d8244 into next Nov 25, 2025
65 of 69 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/update-clack branch November 25, 2025 14:12
@github-actions github-actions bot mentioned this pull request Nov 25, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal maintenance User-facing maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants