-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: isFragment not support React 19 #604
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #604 +/- ##
==========================================
+ Coverage 89.80% 89.90% +0.10%
==========================================
Files 40 41 +1
Lines 981 991 +10
Branches 329 334 +5
==========================================
+ Hits 881 891 +10
Misses 97 97
Partials 3 3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (5)
src/React/isFragment.ts (1)
8-19
: 实现逻辑严谨,建议增加单元测试函数实现考虑了各种边界情况,包括:
- 空值检查
- 类型检查
- React 18 和 19 的兼容性
建议为这个关键函数添加完整的单元测试用例,确保在不同场景下的正确性。
需要我帮助生成相应的单元测试代码吗?
tests/toArray-19.test.tsx (2)
5-23
: 模块模拟设置完善,建议补充注释模块模拟的设置很完整,覆盖了所有必要的 React 19 相关模块。建议添加简短的注释说明每个模拟的用途。
建议添加如下注释:
jest.mock('react', () => { + // 使用 React 19 进行测试以验证新版本兼容性 const react19 = jest.requireActual('react-19'); return react19; });
33-56
: 测试用例设计合理,建议扩展测试场景当前测试用例很好地验证了嵌套 Fragment 的处理,但建议增加以下测试场景:
- 空 Fragment 的处理
- 多层深度嵌套的情况
- 混合使用 Fragment 和其他组件的场景
- 错误处理场景
需要我帮助编写这些额外的测试用例吗?
src/ref.ts (2)
Line range hint
40-49
: 建议补充更详细的文档说明代码逻辑正确地适配了 React 19 的新特性,但建议添加以下说明:
- 为什么不再需要
forwardRef
props.propertyIsEnumerable('ref')
检查的具体原理- 与 React 18 的区别
建议添加如下注释:
// React 19 no need `forwardRef` anymore. So just pass if is a React element. + // In React 19, ref is handled differently: + // 1. All components can receive refs without forwardRef + // 2. ref is now enumerable in props instead of being a special property + // 3. This change maintains compatibility with both React 18 and 19
Line range hint
82-84
: 建议增加防御性编程当前实现虽然正确,但建议添加空值检查以提高代码健壮性。
建议修改为:
function isReactElement(node: React.ReactNode) { + if (node == null) return false; return isValidElement(node) && !isFragment(node); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Children/toArray.ts
(1 hunks)src/React/isFragment.ts
(1 hunks)src/ref.ts
(1 hunks)tests/toArray-19.test.tsx
(1 hunks)
🔇 Additional comments (4)
src/React/isFragment.ts (1)
1-3
: 符号常量定义清晰且具有前瞻性
使用 Symbol.for
定义常量是个好的做法,这样可以确保跨版本的符号一致性。同时考虑到了 React 18 和 19 的兼容性,显示了良好的前瞻性思维。
src/Children/toArray.ts (1)
1-1
: 导入路径修改合理
将 isFragment
从 react-is
改为本地实现是个好的改进:
- 减少了外部依赖
- 提供了更好的版本兼容性控制
- 便于后续维护和更新
src/ref.ts (2)
Line range hint 91-108
: 文档和实现都很完善
代码注释清晰地解释了 React 19 中 ref 的新行为,实现也正确地处理了向后兼容性。特别赞赏添加了源代码参考链接,这对后续维护很有帮助。
3-3
: 导入更改符合预期
将 isFragment
从 react-is
改为本地实现是正确的改动,这样可以更好地控制 Fragment 的检测逻辑,确保同时支持 React 18 和 19。
Also applies to: 5-5
✅ Verification successful
从 react-is
移除 isFragment
导入是安全的修改
代码分析显示:
- 已经实现了本地版本的
isFragment
,支持 React 18 和 19 的兼容性检查 - 所有使用
isFragment
的地方(src/ref.ts
和src/Children/toArray.ts
)都已经迁移到使用新的本地实现 - 新实现通过检查
$$typeof
和type
属性正确处理了两个 React 版本的 Fragment 类型
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证是否还有其他文件使用 react-is 的 isFragment
rg "import.*isFragment.*from 'react-is'"
Length of output: 48
Script:
#!/bin/bash
# 1. 检查是否有其他文件从 react-is 导入功能
rg "from 'react-is'" -A 2
# 2. 查找所有使用 isFragment 的地方
rg "isFragment\(" -A 2
# 3. 检查新的本地 isFragment 实现
fd "isFragment.ts" --exec cat {}
Length of output: 1306
React 19 对 Fragment 的定义变了,导致
react-is
不能同时支持两个版本,rc-util
里自行吃掉这个。fix ant-design/ant-design#51918
Summary by CodeRabbit
新特性
isFragment
函数,用于判断对象是否为 React Fragment。toArray-19.test.tsx
,验证toArray
函数对 React Fragment 的处理。Bug 修复
文档