chore: support Ref check update #594
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@zombieJ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough此次变更主要集中在 Changes
Possibly related PRs
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #594 +/- ##
==========================================
- Coverage 89.84% 89.78% -0.07%
==========================================
Files 40 40
Lines 975 979 +4
Branches 325 314 -11
==========================================
+ Hits 876 879 +3
- Misses 96 97 +1
Partials 3 3 ☔ View full report in Codecov by Sentry. |
![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
🧹 Outside diff range and nitpick comments (4)
src/ref.ts (1)
Line range hint
82-95: 建议补充 getNodeRef 的单元测试用例当前的注释说明了在 React 19 中 ref 处理的变化，但测试用例覆盖不够完整。建议添加以下场景的测试：
- props.ref 存在的情况
- 只有 node.ref 存在的情况
- 两者都不存在的情况
需要我帮您生成这些测试用例吗？
tests/ref-19.test.tsx (2)
42-73: 建议增强 useComposeRef 的测试覆盖当前测试用例验证了基本功能，但建议添加以下场景的测试：
- 传入空 ref 的情况
- 组件重渲染时 ref 的稳定性
- 错误处理场景
需要我帮您编写这些额外的测试用例吗？
16-24: 建议优化模块 mock 的实现当前对 react-dom 相关模块的 mock 实现比较分散，建议统一管理：
+ const mockReactDom19 = (moduleName: string) => { + return jest.mock(moduleName, () => { + return jest.requireActual(`${moduleName}-19`); + }); + }; + - jest.mock('react-dom/client', () => { - const reactDom19Client = jest.requireActual('react-dom-19/client'); - return reactDom19Client; - }); - - jest.mock('react-dom/test-utils', () => { - const reactDom19Test = jest.requireActual('react-dom-19/test-utils'); - return reactDom19Test; - }); + mockReactDom19('react-dom/client'); + mockReactDom19('react-dom/test-utils');tests/ref.test.tsx (1)
Line range hint
92-106: 需要更新测试用例以适配 React 19当前的改动注释掉了一个断言，但没有提供明确的原因说明。建议：
- 添加注释说明为什么需要注释掉这个断言
- 如果这个断言在 React 19 中不再适用，应该用新的断言替代
- 更新测试用例名称的原因也应该在注释中说明
建议添加如下注释：
it('function component1', () => { // Renamed to indicate React 19 specific behavior const holderRef = React.createRef<Holder>(); ... expect(supportRef(FC)).toBeFalsy(); - // expect(supportRef(holderRef.current.props.children)).toBeFalsy(); + // Note: This assertion is commented out because in React 19, + // function components handle refs differently through props.ref
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/ref.ts(2 hunks)tests/ref-19.test.tsx(3 hunks)tests/ref.test.tsx(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/ref.ts
[error] 42-42: Do not access Object.prototype method 'propertyIsEnumerable' from target object.
(lint/suspicious/noPrototypeBuiltins)
| // React 19 no need `forwardRef` anymore. So just pass if is a React element. | ||
| if ( | ||
| isReactElement(nodeOrComponent) && | ||
| (nodeOrComponent as any).props.propertyIsEnumerable('ref') | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
需要优化 ref 属性检查的实现方式
当前使用 propertyIsEnumerable 来检查 ref 属性可能存在潜在问题：
- 直接在对象上调用
propertyIsEnumerable不是最佳实践 - 如果
props是null或undefined会导致运行时错误
建议修改实现方式如下:
- if (
- isReactElement(nodeOrComponent) &&
- (nodeOrComponent as any).props.propertyIsEnumerable('ref')
- ) {
+ if (isReactElement(nodeOrComponent)) {
+ const { props } = nodeOrComponent;
+ if (props && Object.prototype.propertyIsEnumerable.call(props, 'ref')) {
return true;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // React 19 no need `forwardRef` anymore. So just pass if is a React element. | |
| if ( | |
| isReactElement(nodeOrComponent) && | |
| (nodeOrComponent as any).props.propertyIsEnumerable('ref') | |
| ) { | |
| return true; | |
| } | |
| // React 19 no need `forwardRef` anymore. So just pass if is a React element. | |
| if (isReactElement(nodeOrComponent)) { | |
| const { props } = nodeOrComponent; | |
| if (props && Object.prototype.propertyIsEnumerable.call(props, 'ref')) { | |
| return true; | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 42-42: Do not access Object.prototype method 'propertyIsEnumerable' from target object.
(lint/suspicious/noPrototypeBuiltins)
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/react-dom@19.0.0-rc-de68d2f4-20241204, npm/react@19.0.0-rc-de68d2f4-20241204 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
tests/ref-19.test.tsx (3)
3-3: 导入语句优化建议建议将
getNodeRef和useComposeRef的导入分开，以提高代码的可读性。-import { getNodeRef, useComposeRef } from '../src/ref'; +import { getNodeRef } from '../src/ref'; +import { useComposeRef } from '../src/ref';
16-24: 建议优化模拟模块的实现方式当前的模拟实现方式有重复代码。建议将共同的模拟逻辑抽象到一个辅助函数中。
+const mockReact19Module = (modulePath: string) => { + return jest.mock(modulePath, () => { + return jest.requireActual(`${modulePath}-19`); + }); +}; -jest.mock('react-dom/client', () => { - const reactDom19Client = jest.requireActual('react-dom-19/client'); - return reactDom19Client; -}); -jest.mock('react-dom/test-utils', () => { - const reactDom19Test = jest.requireActual('react-dom-19/test-utils'); - return reactDom19Test; -}); +mockReact19Module('react-dom/client'); +mockReact19Module('react-dom/test-utils');
47-78: useComposeRef 测试用例的改进建议测试用例实现得很好，但建议添加以下场景：
- 测试 ref 为 null 的情况
- 测试多个 ref 组合的情况
- 测试 ref 更新后的回调行为
建议添加以下测试场景：
it('useComposeRef with null ref', () => { const Demo = () => { const ref = useComposeRef(null, null); return <div ref={ref} />; }; render(<Demo />); }); it('useComposeRef with multiple refs', () => { const ref1 = React.createRef(); const ref2 = React.createRef(); const ref3 = React.createRef(); const Demo = () => { const ref = useComposeRef(ref1, ref2, ref3); return <div ref={ref} className="test" />; }; render(<Demo />); expect(ref1.current?.className).toBe('test'); expect(ref2.current?.className).toBe('test'); expect(ref3.current?.className).toBe('test'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
package.json(1 hunks)tests/ref-19.test.tsx(3 hunks)
🔇 Additional comments (1)
package.json (1)
70-71: 验证 React 19 稳定版本的兼容性
从发布候选版本更新到稳定版本是一个重要的变更。请确保：
- 所有功能都与 React 19 稳定版本兼容
- 已经进行了充分的测试验证
✅ Verification successful
Let me gather more information about the React usage in the codebase.
Let me try one more search to check for React 19 specific test files and configurations.
React 19 版本更新验证已完成
根据验证结果：
- 使用的 React 19.0.0 确实是当前 npm registry 中的最新稳定版本
- 代码库中已包含专门的 React 19 测试文件（tests/ref-19.test.tsx），并正确配置了相关依赖
- 测试文件显示已经适配了 React 19 的特性，包括引用（ref）相关的功能测试
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 npm registry 中最新的 React 19 版本
npm view react@19 version
# 检查是否有已知的重大问题
gh api graphql -f query='
{
repository(owner: "facebook", name: "react") {
issues(first: 5, states: OPEN, labels: ["Type: Bug", "Severity: High"]) {
nodes {
title
url
createdAt
}
}
}
}'
Length of output: 1170
Script:
#!/bin/bash
# Check React imports and usage patterns
rg -l "from ['\"](react|react-dom)" --type js --type jsx --type ts --type tsx
# Check for any React 19 specific features or APIs
rg "use(Transition|Deferred|Id|FormStatus|Optimistic)" --type js --type jsx --type ts --type tsx
# Look for test files that might be using React
fd -e test.js -e test.jsx -e test.ts -e test.tsx -e spec.js -e spec.jsx -e spec.ts -e spec.tsx
# Check package-lock.json for React dependencies
fd -e package-lock.json --exec cat {} | grep -A 5 -B 5 '"react":'
Length of output: 1027
Script:
#!/bin/bash
# Check content of React 19 specific test files
cat tests/ref-19.test.tsx
# Look for any React configuration files
fd -e .babelrc -e .babelrc.js -e babel.config.js -e jest.config.js -e vitest.config.js -e vitest.config.ts
# Check package.json for test scripts and configurations
cat package.json | jq '.scripts + .jest + .vitest'
Length of output: 2605
Summary by CodeRabbit
新功能
supportRef和getNodeRef功能。useComposeRef方法，允许合并多个引用。测试
useComposeRef的测试。supportRef和composeRef的测试用例，确保其正确性和稳定性。