Skip to content

fix(CCarousel): clear cycle timeouts reliably and scope effects#467

Merged
mrholek merged 1 commit into
mainfrom
fix/carousel-effects
Jun 13, 2026
Merged

fix(CCarousel): clear cycle timeouts reliably and scope effects#467
mrholek merged 1 commit into
mainfrom
fix/carousel-effects

Conversation

@mrholek

@mrholek mrholek commented Jun 13, 2026

Copy link
Copy Markdown
Member

Problem

  1. Stacked timeouts with pause={false}. cycle() relied on _pause() to clear the previous timeout, but _pause is gated on the pause prop — with pause={false} every cycle() call (mouse leave, slide change) scheduled an additional timeout without clearing the previous one.
  2. No cleanup on unmount. The pending cycle timeout kept running after unmount and called setActive on an unmounted component.
  3. Unscoped effects. The items-count effect and the scroll listener effect had no dependency array, so the count was recomputed and the window scroll listener was removed/re-added on every render.

Changes

  • Extract clearCycleTimeout() and call it unconditionally at the start of cycle(); _pause keeps its prop-gated semantics for hover.
  • Clear the pending timeout in an unmount cleanup effect.
  • Scope the items-count effect to [children] and register the scroll listener once ([] — the handler only touches refs and state setters).

Tests

2 new regression tests using jest.getTimerCount(): repeated mouse-leave with pause={false} does not grow the pending timer count, and unmount releases the pending cycle timeout. Both fail against main, pass with the fix. Full suite: 127 suites / 350 tests green, no snapshot changes.

- always clear the pending cycle timeout before scheduling a new one, so
  timeouts no longer stack when pause is disabled
- clear the pending timeout on unmount
- run the items-count effect only when children change and register the
  scroll listener once instead of on every render
@mrholek mrholek merged commit 9987b14 into main Jun 13, 2026
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