remove React.findDOMNode #611
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
概述遍历在 变更
序列图sequenceDiagram
participant Caller
participant findDOMNode
participant Ref
Caller->>findDOMNode: 传入节点/引用
alt 引用有 current 属性
findDOMNode->>Ref: 获取 current
Ref-->>findDOMNode: 返回 DOM 节点
else 其他类型节点
findDOMNode-->>Caller: 返回相应的 DOM 节点或 null
end
诗歌
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/findDOMNode.test.tsx (2)
40-40: 调用findDOMNode(this.elementRef)时请确保传入的是正确的 ref 对象
此处直接将elementRef传给findDOMNode，若后续需要在elementRef中放置其他类型的节点或者逻辑，请注意和findDOMNode的类型签名保持一致。
44-44: 注意为<div>提供更多可读性或可访问性属性
可以考虑在<div>标签中添加一些标识符属性（例如data-testid）以便测试和调试时更好地定位元素。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Dom/findDOMNode.ts(1 hunks)tests/findDOMNode.test.tsx(2 hunks)
🔇 Additional comments (4)
src/Dom/findDOMNode.ts (2)
28-34: 使用联合类型后返回值可能为 null 的注意事项
函数返回值从 T 更新为 T | null。在调用处要注意判空逻辑，避免在返回值为 null 时出现潜在错误。
请确认所有函数调用方都能够正确处理返回值为 null 的情况，如果需要，我可以提供一段脚本来搜索所有调用点并检查是否进行判空处理。
40-41: 通过判断 current 属性获取 DOM 的实现已替换 React.findDOMNode
这段逻辑去除了对 React.findDOMNode 的依赖，改为直接使用 current 属性获取 DOM。实现简单且更符合 React 16+ 推荐的使用方式。
tests/findDOMNode.test.tsx (2)
38-38: 新增 elementRef 属性以替代旧的 findDOMNode 使用场景
通过在类组件内部定义 elementRef 来获取 DOM，大幅降低对 findDOMNode 的依赖。此做法符合官方最佳实践，值得推广。
57-57: 确认不会触发旧式警告或 React 内部提示
此处针对 console.error 的断言表明渲染此组件不再引发警告，证明移除 React.findDOMNode 后没有产生副作用。
| const domNode = getDOM(node); | ||
| if (domNode) { | ||
| return domNode as T; | ||
| } | ||
|
|
||
| if (node instanceof React.Component) { | ||
| return ReactDOM.findDOMNode?.(node) as unknown as T; | ||
| if (node && typeof node === 'object' && 'current' in node) { |
src/Dom/findDOMNode.ts
Outdated
| const domNode = getDOM(node); | ||
| if (domNode) { | ||
| return domNode as T; | ||
| } | ||
|
|
||
| if (node instanceof React.Component) { | ||
| return ReactDOM.findDOMNode?.(node) as unknown as T; | ||
| if (node && typeof node === 'object' && 'current' in node && isDOM(node.current)) { |
There was a problem hiding this comment.
这对于 nativeElement 还是没用的，应该是：
if (node && typeof node === 'object' && 'current' in node) {
const refDomNode = getDOM(node.current);
if (refDomNode) {
return refDomNode as T;
}
}There was a problem hiding this comment.
佬 我看 nativeElement 有一个测试用例了 我这边还需要再加一个吗
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/findDOMNode.test.tsx (1)
94-98: 建议增强未挂载引用的测试用例当前测试验证了基本场景，建议添加以下测试场景：
- 组件卸载后的行为
- 异步更新场景
- 多次挂载/卸载循环
建议添加如下测试代码：
it('should handle unmount scenarios', () => { const elementRef = React.createRef<HTMLDivElement>(); const { unmount } = render( <div ref={elementRef}>test</div> ); expect(findDOMNode(elementRef)).not.toBeNull(); unmount(); expect(findDOMNode(elementRef)).toBeNull(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Dom/findDOMNode.ts(1 hunks)tests/findDOMNode.test.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Dom/findDOMNode.ts
🔇 Additional comments (3)
tests/findDOMNode.test.tsx (3)
38-44: 代码改动符合最佳实践！使用
React.createRef替代findDOMNode是正确的改进方向，这样可以：
- 避免使用已废弃的
findDOMNodeAPI- 提供更好的类型安全性
- 符合 React 的声明式理念
57-57: 测试期望更新正确！由于现在使用了
React.createRef而不是findDOMNode，确实不应该触发任何警告，测试用例的期望值更新是合理的。
79-92: 测试用例设计合理！新增的测试用例很好地验证了：
- 使用
forwardRef转发引用- 正确获取 DOM 节点
- 符合 React 的最新实践
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #611 +/- ##
==========================================
+ Coverage 89.90% 90.15% +0.24%
==========================================
Files 41 36 -5
Lines 991 914 -77
Branches 320 300 -20
==========================================
- Hits 891 824 -67
+ Misses 97 88 -9
+ Partials 3 2 -1 ☔ View full report in Codecov by Sentry. |
| it('should return DOM node from ref.current', () => { | ||
| const TestComponent = React.forwardRef<HTMLDivElement>((_, ref) => { | ||
| return <div ref={ref}>test</div>; | ||
| }); |
There was a problem hiding this comment.
React 19 不需要 forwardRef 了，看看 https://codesandbox.io/p/sandbox/sparkling-forest-896qmn?file=%2Fsrc%2FApp.js%3A10%2C21 这种情况能不能兼容掉，避免还需要用户包一个 forwardRef。
原始问题：
remove React.findDOMNode
Summary by CodeRabbit
新特性
findDOMNode函数，支持新的类型{ current: T }，并可能返回null。elementRef，以增强组件对其 DOM 节点的识别能力。ref.current返回 DOM 节点的正确性。ref返回null的情况。错误修复
DOMWrapper类的测试期望，从期望错误更改为不期望错误。