Skip to content

fix(undo): stop idle renders and flush pending commits on unmount - #968

Merged
thlpkee20-wq merged 1 commit into
RevoraOrg:masterfrom
Tonychris127:fix/941-undo-banner-lifecycle
Oct 22, 2026
Merged

thlpkee20-wq merged 1 commit into
RevoraOrg:masterfrom
Tonychris127:fix/941-undo-banner-lifecycle

Conversation

@Tonychris127

@Tonychris127 Tonychris127 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Overview

useUndoBanners previously updated its host every 100 ms even when empty and discarded pending commits when the host unmounted. The hook now runs one ticker only while actions are pending, skips unchanged banner snapshots, and commits remaining actions when their undo controls disappear on unmount.

Related Issue

Closes #941

Changes

  • Start the shared ticker on registration; stop it after expiry, undo, dismissal, bulk actions, or unmount.
  • Compare visible IDs/countdowns before scheduling a state update.
  • Detach and commit pending entries on cleanup; document the unmount behavior.
  • Add fake-timer tests for idle rendering, countdowns, timer restart, unmount, and StrictMode. Existing stacking, labels, durations, and undo behavior are preserved.

Verification Results

  • Seven new regression cases failed against unchanged master (c00dd8430ec77f45d36faa59f314dacda54d81ed) before the implementation change.
  • node node_modules/vitest/vitest.mjs run src/hooks/useUndoBanners.test.tsx src/hooks/useUndoKeyboard.test.tsx src/components/UndoBanner: 48 passed.
  • Focused hook coverage: 100% statements, branches, functions, and lines (18 hook tests).
  • npm run build: passed, with existing CSS/chunk-size warnings.
  • Changed-file ESLint and git diff --check: passed.
  • Full lint: 5 errors / 19 warnings, matching recorded verification of the exact base commit. tsc --noEmit reports existing syntax errors in TwoFactorSetup.test.tsx lines 100/107, also matching the base.
  • Full npm test -- --maxWorkers=2 with coverage: 4,033 passed / 149 failed / 3 pending on Windows/Node24. Compared with existing Linux/Node22 evidence for the exact base (4,026 passed / 148 failed / 3 pending), all 148 failed test identities and failed-to-load suites are unchanged. The one extra failure is ScheduledExportsPanel has no axe violations (waitFor timeout), also recorded in the earlier Windows run; its isolated rerun passes all 13 tests. The eight new hook tests pass. The repository-wide suite is not green.
Acceptance criterion Evidence
No idle renders/state updates Empty stack has no timer; render count and array identity remain stable through elapsed time and empty bulk actions
Countdown progresses while pending Fake-time assertions at 300 ms and expiry; only one ticker for multiple banners
Pending commits run once on unmount Unmount and StrictMode tests cover multiple pending entries and previously undone/dismissed/expired entries
Undo prevents commit Existing and new tests advance beyond expiry and unmount after undo
Regression fails before fix Seven failures on original hook; all pass after fix
Scoped behavior and CI Only hook and tests changed; CI's required production build passes locally; upstream CI awaits maintainer approval (action_required, run 39238376821)

@drips-wave

drips-wave Bot commented Oct 19, 2026

Copy link
Copy Markdown

@Tonychris127 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@thlpkee20-wq
thlpkee20-wq merged commit 4e93d96 into RevoraOrg:master Oct 22, 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.

perf(undo): useUndoBanners re-renders every 100 ms even with no banners and drops pending commits on unmount

2 participants