Skip to content

feat: add autoUpdateOptions to avoid a Floating UI RangeError - #3530

Open
dennisridder wants to merge 1 commit into
shipshapecode:mainfrom
dennisridder:main
Open

dennisridder wants to merge 1 commit into
shipshapecode:mainfrom
dennisridder:main

Conversation

@dennisridder

@dennisridder dennisridder commented Sep 22, 2026

Copy link
Copy Markdown

layoutShift tracking can loop until the browser throws RangeError: Maximum call stack size exceeded. Passing { layoutShift: false } through autoUpdateOptions stops that, and the option is left off steps that never set it.

Summary by CodeRabbit

  • New Features

    • Added optional per-step configuration for Floating UI’s automatic positioning updates.
    • Step-level settings can override tour-wide defaults, with nested options merged automatically.
    • Supports disabling layout-shift monitoring where appropriate.
  • Documentation

    • Added guidance on configuring automatic updates, including a warning about potential infinite-loop errors under certain conditions.
  • Tests

    • Added coverage for option forwarding, default settings, step-level overrides, and nested configuration merging.

layoutShift tracking can loop until the browser throws RangeError: Maximum call stack size exceeded. Passing { layoutShift: false } through autoUpdateOptions stops that, and the option is left off steps that never set it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Sep 22, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the shipshapecode Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a422aa36-18de-46a4-b67c-697408ca56b6

📥 Commits

Reviewing files that changed from the base of the PR and between fb76ce9 and bf2bcb1.

📒 Files selected for processing (3)
  • shepherd.js/src/step.ts
  • shepherd.js/src/utils/floating-ui.ts
  • shepherd.js/test/unit/utils/floating-ui.spec.js

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds optional autoUpdateOptions to step configuration. Tour and step values are deep-merged, then passed to Floating UI’s autoUpdate function. Unit tests cover direct options, inherited defaults, overrides, and deep merging.

Changes

Floating UI auto-update configuration

Layer / File(s) Summary
Option contract and merging
shepherd.js/src/step.ts, shepherd.js/src/utils/floating-ui.ts, shepherd.js/test/unit/utils/floating-ui.spec.js
StepOptions accepts AutoUpdateOptions. Tour and step values are deep-merged when present.
Auto-update runtime passthrough
shepherd.js/src/utils/floating-ui.ts, shepherd.js/test/unit/utils/floating-ui.spec.js
setupTooltip passes the merged options to autoUpdate. Tests verify direct options, default options, and step-level overrides.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Step
  participant setupTooltip
  participant FloatingUI
  Step->>setupTooltip: provide merged autoUpdateOptions
  setupTooltip->>FloatingUI: call autoUpdate with target, tooltip, callback, and options
  FloatingUI-->>setupTooltip: return cleanup function
Loading

Suggested reviewers: chuckcarpenter

Merge Risk: ⚪ Minimal · up to bf2bc

The change adds configurable Floating UI auto-update behavior per step, including disabling layout-shift tracking, with no concrete merge-blocking impact identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change, adding autoUpdateOptions, and states its purpose of avoiding the Floating UI RangeError.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

shepherd.js/test/unit/utils/floating-ui.spec.js

(node:2) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignore-files
(Use node --trace-warnings ... to show where the warning was created)

Oops! Something went wrong! :(

ESLint: 10.9.1

A config object is using the "root" key, which is not supported in flat config system.

Flat configs always act as if they are the root config file, so this key can be safely removed.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant