fix: maximum updates Portal.tsx #29
Conversation
Fix for the following error:
error-boundary-callbacks.ts:80 Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
at getRootForUpdatedFiber (react-dom-client.development.js:3860:11)
at enqueueConcurrentHookUpdate (react-dom-client.development.js:3820:14)
at dispatchSetStateInternal (react-dom-client.development.js:8121:18)
at dispatchSetState (react-dom-client.development.js:8081:7)
at Portal.useEffect (Portal.js:63:5)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough在 Portal 的副作用中改用函数式更新并加入比较逻辑：先通过 getContainer/customizeContainer 计算 nextContainer，仅在 nextContainer 与当前不同的情况下才调用 setInnerContainer，且将 getContainer 添加为 effect 依赖。 Changes
Sequence Diagram(s)sequenceDiagram
participant React as React Component
participant Effect as useEffect
participant Getter as getContainer / customizeContainer
participant State as innerContainer state
rect rgba(52,152,219,0.08)
React->>Effect: render -> run effect
end
Effect->>Getter: 调用以计算 nextContainer
Getter-->>Effect: 返回 nextContainer
Note right of Effect: 计算 nextContainer（可能为 null）
Effect->>State: 读取 prevContainer
Note right of State: prevContainer = current state
Effect->>Effect: 比较 nextContainer 与 prevContainer
alt 不同
Effect->>State: setInnerContainer(nextContainer)
State-->>React: 触发 re-render
else 相同
Note over Effect,State: 跳过 setInnerContainer，避免 re-render
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5-15 分钟
诗歌
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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. Comment |
Summary of ChangesHello @James-Jager, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical React error, 'Maximum update depth exceeded,' by refining the state management within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a 'Maximum update depth exceeded' error in the Portal component by preventing unnecessary state updates. The change is correct and resolves the immediate issue. My review points out that the underlying cause is a useEffect hook missing a dependency array, which causes it to run on every render. I've suggested adding the dependency to improve efficiency and prevent similar issues in the future.
Missed dependency
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
==========================================
+ Coverage 93.27% 93.49% +0.21%
==========================================
Files 6 6
Lines 119 123 +4
Branches 44 45 +1
==========================================
+ Hits 111 115 +4
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a "Maximum update depth exceeded" error in the Portal component by preventing unnecessary state updates when the container hasn't changed. The fix introduces a comparison check using a functional state update and adds a dependency array to the useEffect hook.
Key Changes
- Modified
setInnerContainerto use a functional update with a comparison check to avoid cascading updates - Added dependency array
[getContainer]to ensure the effect re-runs when the container source changes - Prevents infinite update loops by returning the previous state when the container is unchanged
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setInnerContainer(customizeContainer ?? null); | ||
| }); | ||
| setInnerContainer((prev) => { | ||
| const nextContainer = customizeContainer ?? null; |
There was a problem hiding this comment.
The nullish coalescing operator ?? incorrectly converts false to null. When getPortalContainer returns false (which happens when getContainer === false), it should remain false to maintain its semantic meaning. The code distinguishes between false (explicitly disabled portal) and null (container not available).
This will cause issues because:
- The state type is
ContainerType | false, not includingnull - Line 138 checks
mergedContainer === falseto determine inline rendering - Converting
falsetonullbreaks this logic
Suggested fix:
setInnerContainer((prev) => {
const nextContainer = customizeContainer === undefined ? null : customizeContainer;
// Avoid cascading updates when the target container is unchanged
if (prev === nextContainer) {
return prev;
}
return nextContainer;
});| const nextContainer = customizeContainer ?? null; | |
| const nextContainer = customizeContainer === undefined ? null : customizeContainer; |
There was a problem hiding this comment.
The nullish coalescing operator (??) only converts null or undefined to the right-hand value. It does not convert false to null.
Current code: customizeContainer ?? null
false → stays false
null → stays null
Element → stays Element
undefined → becomes null
Suggested change: customizeContainer === undefined ? null : customizeContainer
false → stays false
null → stays null
Element → stays Element
undefined → becomes null
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Fix for the following error:
error-boundary-callbacks.ts:80 Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
at getRootForUpdatedFiber (react-dom-client.development.js:3860:11)
at enqueueConcurrentHookUpdate (react-dom-client.development.js:3820:14)
at dispatchSetStateInternal (react-dom-client.development.js:8121:18)
at dispatchSetState (react-dom-client.development.js:8081:7)
at Portal.useEffect (Portal.js:63:5)
Summary by CodeRabbit
发布说明