Skip to content

fix(useDelayState): cancel pending updates on unmount - #806

Merged
yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/cancel-delay-state-on-unmount
Sep 17, 2026
Merged

yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/cancel-delay-state-on-unmount

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cancel a pending animation-frame or timeout update when useDelayState unmounts
  • reuse the hook's existing cancelPending path for cleanup
  • cover both supported delay modes with regression tests

Problem

useDelayState cancels pending work before scheduling a replacement, but it did not cancel that work on unmount. In React 19/Vitest environments this can leave the callback alive beyond jsdom teardown and surface errors such as window is not defined.

Fixes #805.

Verification — September 17, 2026

Rebased onto current master (c241125); signed/GitHub-Verified head: c3e4e08f2fb6b69da4e1caa84c65c22765d97f8f.

  • Full suite: 30 suites, 199 tests passed; 1 test skipped.
  • TypeScript, ESM/CJS/declaration build, focused ESLint (0 errors), Prettier and diff checks passed.
  • GitHub signatures and changed-file blobs were checked against the local branch.
  • Includes upstream fix: resolve pre-existing test failures on master #815, which fixes the prior pickAttrs and React-17 fallback test failures.
  • Upstream CI: ✅ test currently action_required. A maintainer must approve the fork workflow before it can run.
  • Vercel preview still requires upstream team authorization.

The cleanup retains [cancelPending]: useEvent returns a stable callback, so this preserves unmount cleanup and satisfies the hook dependency contract.

AI assistance disclosure

Codex assisted with implementation, conflict resolution, regression tests, and validation.

Summary by CodeRabbit

  • Bug 修复

    • 修复组件卸载时仍执行延迟状态更新的问题。
    • 组件卸载后会自动取消待处理的动画帧或定时器,避免意外更新。
  • 测试

    • 增加针对帧数和毫秒延迟场景的卸载清理测试。

@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

@nrps9909 is attempting to deploy a commit to the React Component Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 25da6cbb-dd06-4287-be3e-53340ba0d428

📥 Commits

Reviewing files that changed from the base of the PR and between d1045f4 and cb17e58.

📒 Files selected for processing (2)
  • src/hooks/useDelayState.ts
  • tests/useDelayState.test.tsx

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


Walkthrough

useDelayState 现在会在组件卸载时调用 cancelPending,取消待处理的 requestAnimationFrame 或定时器更新。新增参数化测试覆盖两种延迟方式,并验证卸载后定时器已清除。

Changes

延迟状态清理

Layer / File(s) Summary
卸载清理与参数化验证
src/hooks/useDelayState.ts, tests/useDelayState.test.tsx
组件卸载时调用 cancelPending。测试覆盖帧延迟和毫秒延迟,并确认卸载后所有定时器均已清除。

Estimated code review effort: 2 (简单) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cb17e

This localized change cancels pending delayed updates when the hook unmounts, with regression tests covering both delay modes. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed PR 实现了 Issue #805 的核心要求:组件卸载时调用现有的 cancelPending,取消 timeout 和 animation-frame 模式下的待处理更新,并增加对应回归测试。
Out of Scope Changes check ✅ Passed 代码变更和测试变更均直接服务于 Issue #805 及 PR 目标,未发现无关或超出范围的修改。
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 1 functions across 2 files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:在 useDelayState 卸载时取消待处理更新。标题简洁、明确,并与代码和测试变更一致。
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Pareder

Pareder commented Sep 15, 2026

Copy link
Copy Markdown

@zombieJ @yoyo837 Could you please review?

@yoyo837

yoyo837 commented Sep 15, 2026

Copy link
Copy Markdown
Member

CI failed.

@pushkedra

Copy link
Copy Markdown

On "CI failed" — the two failures in test / react component workflow are
tests/utils.test.ts › pickAttrs › forwards every React DOM event handler and
tests/hooks-17.test.tsx › useId › fallback of React 17 or lower. Neither touches useDelayState, and neither file is in this diff (2 files, +17/−0). A rebase and re-run would confirm they are pre-existing — I could not check that from outside, as the test workflow has not run on master recently.

One note on the change: the cleanup is useEffect(() => cancelPending, [cancelPending]), while the sibling copy in @rc-component/picker uses []. cancelPending comes from useEvent, so the two are equivalent today — but [] states the intent better: were that identity ever to change, the effect would re-run and its cleanup would cancel a live pending update.

We hit #805 independently on antd 6; numbers posted there. The fix matches the diagnosis.

@nrps9909
nrps9909 force-pushed the codex/cancel-delay-state-on-unmount branch from cb17e58 to c3e4e08 Compare September 17, 2026 08:16
@nrps9909

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (c241125); signed/GitHub-Verified head: c3e4e08f2fb6b69da4e1caa84c65c22765d97f8f.

  • Full suite: 30 suites, 199 tests passed; 1 test skipped.
  • TypeScript, ESM/CJS/declaration build, focused ESLint (0 errors), Prettier and diff checks passed.
  • GitHub signatures and changed-file blobs were checked against the local branch.
  • Includes upstream fix: resolve pre-existing test failures on master #815, which fixes the prior pickAttrs and React-17 fallback test failures.
  • Upstream CI: ✅ test currently action_required. A maintainer must approve the fork workflow before it can run.
  • Vercel preview still requires upstream team authorization.

The cleanup retains [cancelPending]: useEvent returns a stable callback, so this preserves unmount cleanup and satisfies the hook dependency contract.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.78%. Comparing base (c241125) to head (c3e4e08).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #806      +/-   ##
==========================================
+ Coverage   86.77%   86.78%   +0.01%     
==========================================
  Files          41       41              
  Lines        1096     1097       +1     
  Branches      397      382      -15     
==========================================
+ Hits          951      952       +1     
  Misses        143      143              
  Partials        2        2              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yoyo837
yoyo837 merged commit 335e3cf into react-component:master Sep 17, 2026
11 of 12 checks passed
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.

useDelayState throws error in vitest

4 participants