fix(core): add reentrancy guard to focus patch

[ia319]

Closes #32603

What I did

• add a reentrancy protection mechanism to the patched getter
• use a Set to track active elements within the current call stack and setTimeout to defer cleanup, preventing recursive focus calls.

Checklist for Contributors

Testing

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

• stories
• unit tests
  There is an error:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯

 FAIL   @storybook/cli  test/default/cli.test.cjs > help command
AssertionError: expected '(node:2813) [DEP0190] DeprecationWarn…' to be '' // Object.is equality

- Expected
+ Received

+ (node:2813) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.                                           
+ (Use `node --trace-deprecation ...` to show where the warning was created)
+

 ❯ test/default/cli.test.cjs:21:24
     19|   const stdoutString = cleanLog(stdout.toString());
     20| 
     21|   expect(stderrString).toBe('');
       |                        ^
     22|   expect(stdoutString).toContain('init');
     23|   expect(stdoutString).toContain('Initialize Storybook into your project');


 Test Files  1 failed | 473 passed | 17 skipped (491)
      Tests  1 failed | 4450 passed | 45 skipped | 2 todo (4498)
Type Errors  no errors
   Start at  14:33:03
   Duration  427.61s (transform 442.96s, setup 3128.87s, collect 1683.45s, tests 369.15s, environment 15.06s, prepare 46724.53s, typecheck 18.95s)

• integration tests
• end-to-end tests

Manual testing

Created a sandbox and added stories for the Cally, Chakra, and React Aria Components libraries.
After testing the pages, everything worked as expected with no errors.
Sandbox link:https://github.com/ia319/storybook-react-vite-focus-sandbox

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 canary-release-pr.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

• Bug Fixes
  • Fixed a rare recursive-focus issue that could cause UI freezes or crashes.
  • Improved stability of programmatic focus during rapid focus changes.
  • Prevented focus-related lockups to enhance keyboard navigation and accessibility.
  • Reduced risk of stack overflows in complex focus chains for smoother behavior in dialogs, modals, and interactive components.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a runtime safeguard in code/core/src/test/preview.ts that patches HTMLElement.prototype.focus with a getter-based wrapper using originalFocus/currentFocus, a focusingElements Set, and a patchedFocus flag; returns the original focus when reentry is detected and defers Set cleanup to the next tick.

Changes

Cohort / File(s)	Summary
Focus recursion guard code/core/src/test/preview.ts	Adds a guarded patch for HTMLElement.prototype.focus that preserves originalFocus, exposes currentFocus, tracks active elements in a focusingElements Set to detect reentrancy, uses a defensive getter to avoid infinite recursion, employs a patchedFocus flag to ensure single installation, and schedules Set cleanup with setTimeout(..., 0).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Test as Test code
  participant Elem as HTMLElement instance
  participant Patch as preview.ts patch
  participant Orig as originalFocus

  Note over Patch: installs guarded getter on HTMLElement.prototype.focus (guarded by patchedFocus)

  Test->>Elem: call focus()
  activate Patch
  Patch->>Patch: check focusingElements.has(Elem)?
  alt Not in Set
    Patch->>Patch: focusingElements.add(Elem)
    Patch->>Orig: invoke originalFocus.call(Elem, ...)
    Note right of Patch: schedule focusingElements.delete(Elem) on next tick (setTimeout 0)
    Orig-->>Patch: return
    Patch-->>Test: return result
  else In Set (reentrant)
    Patch-->>Test: return originalFocus (avoid re-entry)
  end
  deactivate Patch

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay attention to correctness of .call/binding when invoking originalFocus.
• Verify patch installation/removal semantics and that patchedFocus prevents double-patching.
• Check edge cases around asynchronous cleanup and potential memory retention in focusingElements.

✨ 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 91a97f7 and d991888.

📒 Files selected for processing (1)

• code/core/src/test/preview.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• code/core/src/test/preview.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

---

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

[coderabbitai]

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 2e08667 and 8b67a74.

📒 Files selected for processing (1)

• code/core/src/test/preview.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx,js,jsx}

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

Adhere to ESLint and Prettier rules across all JS/TS source files

Files:

• code/core/src/test/preview.ts

**/*.{ts,tsx}

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

Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode

Files:

• code/core/src/test/preview.ts

🔇 Additional comments (2)

code/core/src/test/preview.ts (2)

126-142: LGTM! Reentrancy guard correctly prevents recursive focus calls.

The getter logic properly implements the reentrancy protection:

• Detects recursive calls by checking Set membership (lines 128-131)
• Returns originalFocus to break the cycle when recursion is detected
• Defers cleanup via setTimeout(..., 0) (line 138) to ensure the protection marker persists throughout the entire synchronous call chain
• The arrow function correctly captures the lexical this from the getter context

This approach aligns with the PR objective of fixing the "Maximum call stack size exceeded" error in the Cally date picker.

---

111-112: LGTM! Correctly preserves the original focus method.

Capturing the original focus reference before any patching is essential for the reentrancy guard to function correctly. The comment clearly documents this requirement.

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 4f8575b

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

---

☁️ Nx Cloud last updated this comment at 2025-10-29 03:59:15 UTC