fix: Avoid exponential loops when a large number of components are un… #755
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改调整了 Changes
Suggested reviewers
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
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. tests/useWatch.test.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. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
src/useForm.ts
Outdated
| // Avoid exponential loops when a large number of components are unloaded | ||
| setTimeout(() => { | ||
| this.notifyWatch([namePath]); | ||
| }) | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
延迟方式仍存 O(N²) 隐患，建议合并为单次去抖触发
当前实现在每个 Field 卸载时都 setTimeout 一次，notifyWatch 仍会被调用 n 次；notifyWatch → getFieldsValue 会遍历全部剩余字段，整体复杂度依旧可能退化为 O(n²)。可以考虑为整个卸载阶段只触发一次异步通知，例如增加一个“待刷”标记并在下一个宏/微任务中统一执行：
+// 顶部成员
+private pendingNotify: boolean = false;
@@
- // Avoid exponential loops when a large number of components are unloaded
- setTimeout(() => {
- this.notifyWatch([namePath]);
- })
+ // 避免多次重复调用，统一去抖
+ if (!this.pendingNotify) {
+ this.pendingNotify = true;
+ Promise.resolve().then(() => {
+ this.pendingNotify = false;
+ this.notifyWatch(); // 合并调用，不再逐个 namePath
+ });
+ }这样无论一次批量卸载多少 Field，只会有一次 notifyWatch 调用，可彻底消除指数级循环风险。
[建议级别: 必需优化]
🤖 Prompt for AI Agents
In src/useForm.ts around lines 685 to 689, the current implementation calls
setTimeout for each Field unload, causing notifyWatch to be invoked multiple
times and leading to O(n²) complexity. To fix this, implement a debounce
mechanism by adding a flag to mark pending notifications and schedule a single
notifyWatch call in the next macro or microtask, ensuring that no matter how
many Fields unload in a batch, notifyWatch is only called once, eliminating the
exponential loop risk.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/useWatch.test.tsx (2)
123-128: 建议改用微任务冲刷或waitFor以提升测试稳定性此处通过
timeout()触发一个宏任务，再包裹在act中等待状态更新。内部实现只是Promise.resolve().then()（微任务）推迟了notifyWatch，宏任务等待不仅多余且会放慢整体测试速度，也可能在某些环境下导致间歇性超时。- await act(async () => { - await timeout(); - }); + await act(async () => { + // 仅需冲刷微任务即可等待 notifyWatch 调度完成 + await Promise.resolve(); + });或使用 RTL 的
waitFor：await waitFor(() => expect(container.querySelector<HTMLDivElement>('.values')?.textContent).toEqual('') );这样既缩短测试时间，又避免依赖固定延时造成的脆弱性。
165-170: 重复代码可抽取为公共辅助函数与上一段相同的延时等待逻辑再次出现，建议提取为如
await flushNotify();的帮助方法，以减少重复并保持两处行为一致。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/useForm.ts(1 hunks)tests/useWatch.test.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/useForm.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/useWatch.test.tsx (1)
tests/common/timeout.ts (1)
timeout(3-7)
I have a demo ready for you https://codesandbox.io/p/sandbox/rt8qcf
当有useWatch存在时，有大量的Field组件被卸载时，会导致react在commit阶段调用componentWillUnmount, 导致调用registerField的返回函数（在useWatch中是叫cancelRegister），然后调用notifyWatch，notifyWatch又会去调用this.getFieldsValue()， this.getFieldsValue()会去循环所有挂载的组件，就会导致循环指数增长，如果有50个组件就会有 (49+1)49/2 = 1225 次循环，如果有1000个组件就会有 （999+1） 999 / 2 = 499500 次循环，如果给notifyWatch增加promise.then或者setTimeout就可以解决这个问题，延迟执行后，已经卸载的组件就不会再出现在fieldEntities中，减少大量循环，且该commit对功能无影响
When a large number of Field components are unmounted, it will cause react to call componentWillUnmount during the commit phase, resulting in the call of the return function of registerField (called cancelRegister in useWatch). Then call 'notifyWatch', and 'notifyWatch' will call 'this.getFieldsValue()'. 'this.getFieldsValue()' will loop through all mounted components, which will cause the loop to grow exponentially. If there are 50 components, there will be (49+1)49/2 = 1225 loops. If there are 1000 components, there will be (999+1) 999/2 = 499,500 loops. If promise.then or setTimeout is added to notifyWatch, this problem can be solved. After the delayed execution, the uninstalled components will no longer appear in fieldEntities, reducing a large number of loops, and this commit has no impact on the functionality
Summary by CodeRabbit
Summary by CodeRabbit