Skip to content

Conversation

@cactuser-Lu
Copy link
Contributor

@cactuser-Lu cactuser-Lu commented Dec 4, 2025

fix: ant-design/ant-design#55970

Summary by CodeRabbit

发行说明

  • 错误修复
    • 改进了事件监听器清理机制,确保缓存资源得到正确释放,防止内存泄漏。

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 4, 2025

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

A member of the Team first needs to authorize it.

@gemini-code-assist
Copy link

Summary of Changes

Hello @cactuser-Lu, 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!

This pull request resolves a DOM residue issue by enhancing the cleanup mechanism within the useDomMotionEvents hook. The primary goal is to improve memory management and prevent potential bugs that could arise from lingering DOM references after a component has unmounted, ensuring a cleaner state and more robust application performance.

Highlights

  • DOM Residue Fix: Introduced an explicit nullification of the cacheElementRef.current within the useEffect cleanup function in useDomMotionEvents.ts. This change ensures that cached DOM element references are properly cleared when the component unmounts or the effect cleans up, preventing potential memory leaks or stale DOM references.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

概述

useDomMotionEvents 钩子的清理效果中,移除动作事件监听器后,缓存的元素引用现已通过将 cacheElementRef.current 设为 null 来显式清除。

变更

内聚组件 / 文件 变更摘要
缓存引用清理
src/hooks/useDomMotionEvents.ts
在清理效果中添加显式引用清除逻辑,将 cacheElementRef.current 设为 null,确保完整卸载时不存在悬挂引用

预估代码审查工作量

🎯 2 (简单) | ⏱️ ~8 分钟

  • 注意事项:验证此清理逻辑是否完全解决了原始问题中模态框未完全卸载的现象
  • 检查是否存在其他可能的缓存引用需要类似清理

诗句

🐰 缓存清空,引用释放,
内存归还,组件卸载。
模态窗口,今朝彻底,
不留踪迹,页面清爽!
✨🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确总结了主要变更:解决了DOM残留问题,这正是修复内容的核心。
Linked Issues check ✅ Passed PR通过清除缓存的元素引用来解决Modal未完全卸载的DOM残留问题,符合Issue #55970的目标。
Out of Scope Changes check ✅ Passed 修改仅涉及清除缓存元素引用,直接针对DOM残留问题,无超出范围的变更。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses a potential memory leak in the useDomMotionEvents hook. By setting cacheElementRef.current to null within the useEffect cleanup function, it ensures that the reference to the DOM element is properly released upon component unmount. This is an important fix that prevents detached DOM nodes from being retained in memory, improving the application's performance and stability, particularly in scenarios with dynamic component lifecycles. The implementation is clean and effective.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/hooks/useDomMotionEvents.ts (1)

36-40: 在卸载清理中将 cacheElementRef 置空是正确的方向

这一行在调用 removeMotionEvents(cacheElementRef.current) 之后把 cacheElementRef.current 设为 null,可以确保:

  • 卸载后不再持有旧的 DOM 引用,避免潜在的泄漏;
  • 当 Activity 等场景下重用同一个 DOM 节点重新激活时,cacheElementRef.currentnullpatchMotionEvents 不会因为 “element === cacheElementRef.current” 而跳过重新绑定动画/过渡事件,有助于修复 Modal 未正确卸载/动画未结束的残留问题。

从类型一致性的角度,可以考虑顺手把 ref 和辅助函数签名调整为显式支持 null,避免 TS 上的细微不一致(运行时行为本身是安全的):

const cacheElementRef = useRef<HTMLElement | null>(null);

function removeMotionEvents(element: HTMLElement | null) {
  if (element) {
    element.removeEventListener(transitionEndName, onInternalMotionEnd);
    element.removeEventListener(animationEndName, onInternalMotionEnd);
  }
}

这样与 cacheElementRef.current = null 更匹配,也让调用处不需要依赖宽松的类型检查。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fc6ca2 and 030ec18.

📒 Files selected for processing (1)
  • src/hooks/useDomMotionEvents.ts (1 hunks)

@yoyo837
Copy link
Member

yoyo837 commented Dec 4, 2025

这个好写用例吗?看看能不能补一个

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.48%. Comparing base (8fc6ca2) to head (030ec18).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #70   +/-   ##
=======================================
  Coverage   98.47%   98.48%           
=======================================
  Files          11       11           
  Lines         394      395    +1     
  Branches      108      107    -1     
=======================================
+ Hits          388      389    +1     
  Misses          6        6           

☔ View full report in Codecov by Sentry.
📢 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.

@afc163 afc163 merged commit 1e83f4b into react-component:master Dec 4, 2025
7 of 8 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.

Modal 组件与 Activity 组件结合使用后未正常卸载

3 participants