-
-
Notifications
You must be signed in to change notification settings - Fork 278
fix: batch remove laggy #757
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改引入了表单批量更新机制,涵盖了核心表单组件、上下文、类型定义以及相关的 React 组件。新增了批量更新组件 Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Form
participant BatchUpdate
participant FormStore
participant Watcher
App->>Form: 渲染表单
Form->>BatchUpdate: 挂载并获取 ref
Form->>FormStore: setBatchUpdate(batchUpdate)
FormStore->>Form: 注册 setBatchUpdate
Note over FormStore: 注册/注销 Field 时
FormStore->>FormStore: batchNotifyWatch(namePath)
FormStore->>BatchUpdate: batch(key, callback)
BatchUpdate->>FormStore: 批量执行 callback
FormStore->>Watcher: notifyWatch(namePath)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/examples/debug.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. src/FieldContext.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. src/useForm.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
看了下,还是很卡啊 |
只做了 unmount,没问题再切 mount |
试了,目前来看没问题,但是 getFieldsValue() 为啥会 setValue,耗费性能 |
因为 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #757 +/- ##
==========================================
+ Coverage 99.44% 99.45% +0.01%
==========================================
Files 18 19 +1
Lines 1260 1296 +36
Branches 298 315 +17
==========================================
+ Hits 1253 1289 +36
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 1
♻️ Duplicate comments (2)
src/BatchUpdate.tsx (1)
9-32
: 批处理实现逻辑正确,但需考虑复杂性实现逻辑合理:
- 使用useState收集批处理任务
- 通过useLayoutEffect确保正确的执行时序
- 使用useImperativeHandle暴露batch方法
从之前的评论来看,开发者考虑过更简单的方案如防抖或Promise.resolve。当前方案的优势是保持React的时序控制,但确实增加了复杂性。
src/useForm.ts (1)
668-668
: 正确替换为批量通知调用。将
notifyWatch
调用替换为batchNotifyWatch
调用是正确的优化:
- 第668行:在字段注册时使用批量通知
- 第708行:在字段注销时使用批量通知
这样可以避免在字段注册/注销过程中的频繁通知,符合性能优化的目标。
Also applies to: 708-708
🧹 Nitpick comments (1)
src/BatchUpdate.tsx (1)
24-27
: 扩展运算符性能考虑在批处理任务较多时,频繁使用扩展运算符
...ori
可能会影响性能。如果批处理键值对数量很大,可以考虑使用其他方式更新状态。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/demo/debug.md
(1 hunks)docs/examples/debug.tsx
(1 hunks)src/BatchUpdate.tsx
(1 hunks)src/FieldContext.ts
(1 hunks)src/Form.tsx
(5 hunks)src/interface.ts
(2 hunks)src/useForm.ts
(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/interface.ts (1)
src/BatchUpdate.tsx (1)
BatchTask
(3-3)
src/Form.tsx (1)
src/BatchUpdate.tsx (2)
BatchUpdateRef
(5-7)BatchTask
(3-3)
src/useForm.ts (2)
src/interface.ts (1)
InternalNamePath
(6-6)src/BatchUpdate.tsx (1)
BatchTask
(3-3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WIP
🔇 Additional comments (14)
docs/demo/debug.md (1)
1-4
: 文档结构合理简洁的文档格式,正确引用了调试示例代码。
src/interface.ts (2)
4-4
: 类型导入正确正确导入了BatchTask类型,支持新的批处理机制。
237-237
: 接口扩展合理setBatchUpdate方法的添加与现有的InternalHooks接口风格一致,类型签名正确。
src/FieldContext.ts (1)
45-45
: 上下文更新正确setBatchUpdate方法的添加保持了与其他方法一致的模式,使用warningFunc作为占位符是正确的做法。
docs/examples/debug.tsx (3)
4-7
: 监听组件实现简洁WatchCom组件正确使用Form.useWatch监听字段变化,实现简洁有效。
13-20
: 数据生成优化得当使用React.useMemo正确缓存大量数据生成,避免不必要的重复计算。500个字段的数量足以展现性能问题。
35-47
: 性能测试场景有效条件渲染大量表单字段的切换场景能够有效重现性能延迟问题,为批处理优化提供了良好的测试用例。
src/Form.tsx (4)
17-17
: 导入语句正确添加了批量更新相关类型。新增的导入语句正确引入了
BatchUpdate
组件及其相关类型定义,为后续的批量更新机制提供了必要的类型支持。
74-75
: 正确获取了批量更新设置钩子。从内部钩子中解构出
setBatchUpdate
方法,这与useForm.ts
中新增的钩子方法保持一致。
199-199
: BatchUpdate 组件正确集成到渲染树中。将
BatchUpdate
组件添加到ListContext.Provider
内部是正确的,确保了批量更新组件能够正确接收到 ref 回调。
123-157
: 批量更新逻辑实现需要添加错误处理。批量更新的整体设计思路正确,通过
BatchUpdate
子组件避免 Form 自身重新渲染。但是存在一个潜在的运行时错误风险:在第156行调用
setBatchUpdate(batchUpdate)
时,batchUpdate
函数内部会立即调用tryFlushBatch()
,但此时batchUpdateRef.current
可能还未设置,导致任务无法正确批量执行。建议添加错误处理和空值检查:
const batchUpdate: BatchTask = (key, callback) => { batchUpdateTasksRef.current.push([key, callback]); tryFlushBatch(); };可以优化为:
const batchUpdate: BatchTask = (key, callback) => { batchUpdateTasksRef.current.push([key, callback]); + // 延迟到下一个 tick 执行,确保 ref 已设置 + React.startTransition(() => { tryFlushBatch(); + }); };Likely an incorrect or invalid review comment.
src/useForm.ts (3)
41-41
: 正确导入了批量任务类型。导入
BatchTask
类型为批量更新机制提供了必要的类型支持。
123-123
: 内部钩子正确暴露了批量更新设置方法。在
getInternalHooks
方法中添加setBatchUpdate
钩子,与Form.tsx
中的使用保持一致。
219-226
: 批量通知逻辑实现正确且高效。
batchNotifyWatch
方法的实现很好地解决了频繁通知的性能问题:
- 使用数组累积需要通知的
namePath
- 通过批量机制在合适的时机统一执行通知
- 执行后清空累积列表,避免重复通知
这种设计符合 PR 目标中提到的性能优化需求。
private batchUpdate: BatchTask; | ||
|
||
private setBatchUpdate = (batchUpdate: BatchTask) => { | ||
this.batchUpdate = batchUpdate; | ||
}; | ||
|
||
// Batch call the task, only last will be called | ||
private batch = (key: string, callback: VoidFunction) => { | ||
this.batchUpdate(key, callback); | ||
}; |
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.
批量更新实现需要添加空值检查。
batch
方法直接调用 this.batchUpdate(key, callback)
,但 batchUpdate
在初始化时可能为 undefined
,这会导致运行时错误。
建议添加空值检查:
// Batch call the task, only last will be called
private batch = (key: string, callback: VoidFunction) => {
+ if (!this.batchUpdate) {
+ // 如果批量更新函数尚未设置,直接执行回调
+ callback();
+ return;
+ }
this.batchUpdate(key, callback);
};
📝 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.
private batchUpdate: BatchTask; | |
private setBatchUpdate = (batchUpdate: BatchTask) => { | |
this.batchUpdate = batchUpdate; | |
}; | |
// Batch call the task, only last will be called | |
private batch = (key: string, callback: VoidFunction) => { | |
this.batchUpdate(key, callback); | |
}; | |
private batchUpdate: BatchTask; | |
private setBatchUpdate = (batchUpdate: BatchTask) => { | |
this.batchUpdate = batchUpdate; | |
}; | |
// Batch call the task, only last will be called | |
private batch = (key: string, callback: VoidFunction) => { | |
if (!this.batchUpdate) { | |
// 如果批量更新函数尚未设置,直接执行回调 | |
callback(); | |
return; | |
} | |
this.batchUpdate(key, callback); | |
}; |
🤖 Prompt for AI Agents
In src/useForm.ts around lines 229 to 238, the batch method calls
this.batchUpdate without checking if it is defined, which can cause runtime
errors if batchUpdate is undefined. Add a null or undefined check before calling
this.batchUpdate in the batch method to ensure it is a valid function before
invocation.
在 Form 组件中注册一个
useLayoutEffect
用于跟随 React 生命周期保证时序。没问题的话就继续修。close #755
Summary by CodeRabbit
新功能
文档