fix(trigger): avoid render-based reset for interaction-level deduplication #601
Conversation
Walkthrough在 Changes
Sequence Diagram(s)（条件不满足：更改集中在单一组件内部的状态/回调同步处理与测试，未引入跨 3 个及以上独立组件的新控制流，故略去序列图。） Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @QDyanbing, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在修复触发器组件中的一个关键错误，该错误导致在受控模式下，事件去重状态无法正确重置，从而可能阻止 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 Pull Request 旨在修复一个与事件去重逻辑相关的问题。原先，用于交互级别去重的 lastTriggerRef 的重置依赖于组件的 re-render，这在受控模式下如果外部没有同步更新 open 状态，可能导致组件不 re-render，从而使去重状态在多次用户交互之间残留，引发后续 onOpenChange 事件被错误地忽略。修复方案将 lastTriggerRef 的清理时机从 render 周期中移除，改为在当前用户交互批次结束时通过 queueMicrotask 进行清理。这个改动是正确且有效的，从根本上解决了问题，确保了在各种场景下事件处理的正确性。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.tsx
🔇 Additional comments (1)
src/index.tsx (1)
375-375: 变更看起来不错！新增的
resetScheduledRef用于防止重复调度清理任务，这个设计是合理的。
src/index.tsx
Outdated
| const internalTriggerOpen = useEvent((nextOpen: boolean) => { | ||
| // `lastTriggerRef` is for interaction-level deduplication; do not reset it on render. | ||
| if (!resetScheduledRef.current) { | ||
| resetScheduledRef.current = true; | ||
| queueMicrotask(() => { | ||
| resetScheduledRef.current = false; | ||
| lastTriggerRef.current = []; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the code context around lines 377-385
wc -l src/index.tsxRepository: react-component/trigger
Length of output: 84
🏁 Script executed:
# Read the specific lines in context
sed -n '370,395p' src/index.tsxRepository: react-component/trigger
Length of output: 982
🏁 Script executed:
# Search for queueMicrotask usage in the file
rg -n "queueMicrotask" src/index.tsx -A 3 -B 3Repository: react-component/trigger
Length of output: 396
🌐 Web query:
queueMicrotask browser compatibility and support
💡 Result:
Short summary
- Native support: modern browsers and runtimes — Chrome 71+, Firefox 69+, Safari 12.1+, Edge (Chromium) 79+, Opera 58+, Node.js 11+. Also available in Web Workers. [1][2]
- Global usage (Can I use): ~95% of users covered; older browsers (IE, very old mobile/desktop versions) lack support. [2]
- Feature-detect: typeof queueMicrotask === 'function'. If missing, use a polyfill (core-js provides one) or fallback to Promise.resolve().then(...). [1][3]
Sources
- MDN Web Docs — queueMicrotask (Window / WorkerGlobalScope). [1]
- Can I use — queueMicrotask support table. [2]
- Polyfill note / core-js reference. [3]
🏁 Script executed:
# Check package.json for browser target configuration
cat package.json | head -50Repository: react-component/trigger
Length of output: 1356
🏁 Script executed:
# Search for any polyfill or compatibility handling for queueMicrotask
rg "queueMicrotask|Promise.*resolve.*then|polyfill" --type-list | head -20Repository: react-component/trigger
Length of output: 456
🏁 Script executed:
# Check if there's any Babel configuration that might handle polyfills
ls -la | grep -E "babel|tsconfig|\.browserlistrc"Repository: react-component/trigger
Length of output: 130
🏁 Script executed:
# Search for browser targets in tsconfig or similar
find . -maxdepth 2 -name "tsconfig*.json" -o -name ".babelrc*" -o -name "browserslist"Repository: react-component/trigger
Length of output: 82
🏁 Script executed:
# Check tsconfig.json
cat tsconfig.jsonRepository: react-component/trigger
Length of output: 441
🏁 Script executed:
# Search for polyfill or queueMicrotask usage more broadly
rg "queueMicrotask" -B 3 -A 3Repository: react-component/trigger
Length of output: 466
🏁 Script executed:
# Check if there's any polyfill setup
rg "polyfill|Promise.*then|core-js" --type ts --type tsxRepository: react-component/trigger
Length of output: 94
🏁 Script executed:
# Look for any global setup or index files that might handle compatibility
cat src/index.tsx | head -100Repository: react-component/trigger
Length of output: 3214
去重逻辑正确，出色地解决了受控模式下的问题！
使用 queueMicrotask 是解决交互级别去重的理想方案：
- ✅ 确保同一交互批次内的多次事件（如
mouseenter+pointerenter）正确共享去重状态 - ✅ 保证不同用户交互之间的去重状态被及时清理，避免误判
- ✅ 不依赖组件重渲染，完美解决受控模式下的去重问题
微任务在当前同步代码完成后、下一次事件循环前执行，这正好符合交互级别去重的时序需求。
浏览器兼容性注意： queueMicrotask 在现代浏览器中支持良好（Chrome 71+、Firefox 69+、Safari 12.1+、Edge 79+），覆盖约 95% 的用户。若需支持更旧的浏览器，需添加 polyfill（可使用 core-js 或 Promise.resolve().then() 作为备选）。当前项目 tsconfig 的 target 为 esnext，说明已假定目标用户使用现代浏览器。
🤖 Prompt for AI Agents
In src/index.tsx around lines 377-385, the current use of queueMicrotask may
break in older browsers; replace direct queueMicrotask calls with a small
cross-platform microtask helper (e.g., const scheduleMicrotask = (fn: () =>
void) => (typeof queueMicrotask === 'function' ? queueMicrotask(fn) :
Promise.resolve().then(fn))); then use scheduleMicrotask(() => {
resetScheduledRef.current = false; lastTriggerRef.current = []; }); and add the
minimal TypeScript typing for the helper so the code compiles without changing
target or adding large polyfills.
src/index.tsx
Outdated
|
|
||
| const lastTriggerRef = React.useRef<boolean[]>([]); | ||
| lastTriggerRef.current = []; | ||
| const resetScheduledRef = React.useRef(false); |
There was a problem hiding this comment.
这个直接回滚掉是不是就好了，也不用支持 React 16 了
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/open-change.test.tsx (2)
6-16: 考虑为循环次数添加注释或常量。
flush函数中硬编码的10次迭代是一个魔法数字。虽然对于当前的异步流程可能足够，但建议：
- 将其提取为命名常量（例如
FLUSH_ITERATIONS）- 或添加注释说明为何选择 10 次
🔎 可选的重构方案
+const FLUSH_ITERATIONS = 10; // Sufficient to flush nested timers and microtasks + const flush = async () => { - for (let i = 0; i < 10; i += 1) { + for (let i = 0; i < FLUSH_ITERATIONS; i += 1) { act(() => { jest.runAllTimers(); });
87-117: 测试逻辑正确，但建议增加多次交互的测试用例。当前测试验证了单次交互中的去重逻辑，符合预期。然而，根据链接的 issue #56259，原始 bug 表现为"Popover onChange 触发3次后失效"——即多次独立交互后第4次失效。
建议添加额外的测试用例来验证多次独立的 open/close 循环是否都能正常触发回调，例如：
🔎 建议的额外测试用例
在当前测试文件中添加以下测试：
it('should trigger callbacks consistently across multiple separate interactions', async () => { const onOpenChange = jest.fn(); const { container } = render( <Trigger action={['hover', 'focus']} popup={<strong>trigger</strong>} onOpenChange={onOpenChange} > <span className="target" /> </Trigger>, ); const target = container.querySelector('.target') as HTMLElement; // 模拟多次独立的打开/关闭交互 for (let i = 0; i < 5; i += 1) { act(() => { fireEvent.pointerEnter(target); fireEvent.focus(target); }); await flush(); act(() => { fireEvent.pointerLeave(target); fireEvent.blur(target); }); await flush(); } // 验证所有 10 次回调（5次打开 + 5次关闭）都被触发 expect(onOpenChange).toHaveBeenCalledTimes(10); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.tsxtests/open-change.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.tsx
🔇 Additional comments (3)
tests/open-change.test.tsx (3)
1-4: 导入语句正确。所有导入项均适用于此测试文件，使用了标准的测试库和组件。
18-85: 测试环境配置完善。测试套件的设置遵循了最佳实践：
- DOM 元素原型的模拟保持与其他测试一致（避免 jsdom 布局相关崩溃）
- 每个测试前正确重置矩形尺寸
- 假计时器的启用和清理管理得当
119-149: 关闭场景的测试逻辑正确。此测试正确验证了当
pointerLeave和blur同时发生时，关闭回调不会重复触发。测试结构与打开场景测试对称，使用defaultPopupVisible来设置初始打开状态是恰当的。与第一个测试用例一样，建议考虑添加多次独立交互的测试以完整覆盖原始 issue 中描述的场景。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #601 +/- ##
==========================================
+ Coverage 96.42% 97.26% +0.83%
==========================================
Files 17 17
Lines 950 949 -1
Branches 279 274 -5
==========================================
+ Hits 916 923 +7
+ Misses 34 26 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
问题说明
lastTriggerRef用于对同一次用户交互（pointer / mouse / focus 等）进行事件级去重，但其回收依赖组件 re-render。
在受控模式下，若外部未同步更新
open状态，组件可能不会 re-render，导致去重状态跨用户交互残留，后续
onOpenChange被错误吞掉。修复方案
调整
lastTriggerRef的回收时机，不再依赖 render，而是在当前用户交互批次结束时通过
queueMicrotask进行清理。Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.