Skip to content

Core: Fix stepping back through story interactions panel#32793

Merged
yannbf merged 3 commits into
storybookjs:nextfrom
ia319:bug/32630-instrumenter-debug-state-reset
Oct 28, 2025
Merged

Core: Fix stepping back through story interactions panel#32793
yannbf merged 3 commits into
storybookjs:nextfrom
ia319:bug/32630-instrumenter-debug-state-reset

Conversation

@ia319
Copy link
Copy Markdown
Member

@ia319 ia319 commented Oct 22, 2025

Closes #32630

What I did

Change:

I've updated the renderPhaseChanged handler to correctly pass the isDebugging variable to resetState when the newPhase is 'preparing'.

What is the cause of the bug?

  • On the first "Back" click (from isDebugging: false): The renderPhaseChanged handler fires with newPhase: 'playing'. This hits the else if (newPhase === 'playing') block (L227)

which correctly passes the isDebugging: true state, and the debugger works.

  • On the second "Back" click (from isDebugging: true):

    • Clicking "Back" (when isDebugging: true) calls the start function, which emits FORCE_REMOUNT with { isDebugging: true }.
    • This remount triggers the renderPhaseChanged handler, but this time with newPhase: 'preparing'.
    • The handler correctly enters the if (newPhase === 'preparing' && isDebugging) block (L225).
    • The Bug: Inside this block, resetState is called without passing the isDebugging variable.
    • This causes resetState to use its default value of isDebugging = false (from L112).
    • This isDebugging: false state incorrectly terminates the debug session. When the intercept function (L469), checks this flag and sees false
    • It stops returning the Promise that pauses execution. Instead, it immediately calls invoke, causing the play function to execute all remaining steps without stopping.

Recurrence log

Screenshot 2025-10-22 173257

PS: The problem will occur as long as start is called twice (or any time start is called while isDebugging is already true), such as clicking a specific step in the log (which also calls start).

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

In the sandbox, the relevant interactions are fine

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

  • Bug Fixes
    • Fixed an issue where debugging state was not properly preserved during state transitions in the instrumenter.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 22, 2025

📝 Walkthrough

Walkthrough

The renderPhaseChanged function in the instrumenter is modified to pass the isDebugging parameter to resetState when transitioning to 'preparing' state, previously omitted. This ensures consistent state initialization behavior across different transition paths.

Changes

Cohort / File(s) Summary
State Management Parameter Alignment
code/core/src/instrumenter/instrumenter.ts
Added isDebugging argument to resetState call in 'preparing' transition path to maintain consistency with 'playing' path behavior and preserve debug state during reset

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 a0e3884 and 91d99a8.

📒 Files selected for processing (1)
  • code/core/src/instrumenter/instrumenter.ts (1 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 (use yarn lint:js:cmd <file>)

Files:

  • code/core/src/instrumenter/instrumenter.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/core/src/instrumenter/instrumenter.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/core/src/instrumenter/instrumenter.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

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

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/instrumenter/instrumenter.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). (1)
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/core/src/instrumenter/instrumenter.ts (1)

224-226: LGTM! Bug fix correctly preserves debugging state.

The fix ensures isDebugging is passed to resetState when transitioning to the 'preparing' phase during debugging. Previously, omitting this parameter caused resetState to default isDebugging to false (line 112), which terminated the debug session and prevented the stepper from pausing execution. This change aligns the 'preparing' branch with the 'playing' branch behavior (line 228) and directly addresses the root cause described in #32630.


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

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Oct 22, 2025

View your CI Pipeline Execution ↗ for commit 1d4ca46

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

☁️ Nx Cloud last updated this comment at 2025-10-28 08:48:03 UTC

@yannbf yannbf self-assigned this Oct 27, 2025
@yannbf
Copy link
Copy Markdown
Member

yannbf commented Oct 27, 2025

Hey @ia319 thanks a lot for investigating this! While testing it locally by:

  1. Running yarn storybook:ui under the /code directory in the repo:
  2. Accessing this story:
    http://localhost:6006/?path=/story/addons-onboarding-features-intentsurvey--submitting&globals=sb_theme:default
  3. Interacting with the panel to go back a few times

It seems like the issue still exists, unfortunately:
2025-10-27 12 10 23

@ia319
Copy link
Copy Markdown
Member Author

ia319 commented Oct 27, 2025

@yannbf
I have no issues running it locally. I noticed that when I switched to this bug branch, it worked fine in the sandbox, but Storybook UI exhibited the same problem you pointed out. After deleting all node_modules and merging the latest commits from next, it runs fine again—so it was probably a caching issue.

2025-10-27.21-06-07.mp4

@yannbf yannbf changed the title fix(core): preserve isDebugging when reset state during prepare phase (#32630) Core: Fix stepping back through story interactions panel Oct 27, 2025
const { isDebugging } = this.getState(storyId);
if (newPhase === 'preparing' && isDebugging) {
return resetState({ storyId, renderPhase: newPhase });
return resetState({ storyId, renderPhase: newPhase, isDebugging });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @ghengeveld can you check whether there was a reason not to pass isDebugging here? Otherwise both if statements should probably be merged into something like this:

if (newPhase === 'playing' || newPhase === 'preparing' && isDebugging) {
    return resetState({ storyId, renderPhase: newPhase, isDebugging });
}

@yannbf
Copy link
Copy Markdown
Member

yannbf commented Oct 27, 2025

@yannbf I have no issues running it locally. I noticed that when I switched to this bug branch, it worked fine in the sandbox, but Storybook UI exhibited the same problem you pointed out. After deleting all node_modules and merging the latest commits from next, it runs fine again—so it was probably a caching issue.

You're so right! Probably an Nx cache issue. I checked it again, also in a sandbox, and it works well. Thank you so much! I'll just double check with @ghengeveld before merging this.

@yannbf yannbf merged commit 1c0424f into storybookjs:next Oct 28, 2025
54 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 28, 2025
9 tasks
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.

[Bug]: When stepping back through interactions two times, the stepper is blocked and the story hard reloads

2 participants