Conversation
A reference repeated in two sibling branches is not a circular reference,
but refSet never released anything once the walk left a branch. The second
visit found it in the set, warned, and returned false for deeply-equal input:
const shared = [];
isEqual({ errors: shared, warnings: shared }, { errors: [], warnings: [] });
// false, and "Warning: There may be circular references"
A cycle is a value reachable from itself, so the set has to hold the current
path rather than the whole history. Entries are now released as the walk
unwinds. Genuine cycles are still detected, because a self-referencing value
is still its own ancestor when it is reached again.
The tests gain a resetWarned() in beforeEach: warning is warningOnce, so a
message emitted by an earlier case is suppressed in every later one, and
asserting that nothing warned would otherwise prove nothing.
Closes react-component#816
|
@savkaoleg is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughChangesisEqual 循环引用处理
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The ancestor-path cleanup and accompanying cycle/reference tests align with the intended behavior, with no remaining merge-blocking issue identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. 小兔检查 refSet， Comment |
1 participant
Closes #816.
The problem
isEqualreturnsfalse, and logsWarning: There may be circular references, for two objects that are deeply equal — whenever one of them holds the same reference in two different keys:Neither object contains a cycle.
Why
refSetrecords every value the walk visits and never releases anything when the walk leaves a branch. A cycle is a value reachable from itself — a value that is its own ancestor on the current path — so the set needs to hold the path, not the history. As written, a reference legitimately reached twice in two sibling branches is indistinguishable from a cycle: the second visit finds it in the set, warns, and returnsfalse.The change
refSet.delete(a)in afinally, so entries are released as the walk unwinds. Genuine cycles are still caught, because a self-referencing value is still on the path when it is reached again.Why it shows up in practice
rc-field-form'sFieldkeeps one shared constant in two places:and
triggerMetaEventcompares the previous meta with the next throughisEqual. Once one field has validated — itserrorsbecoming a fresh[]— the comparison hits exactly the case above. The visible result in any antd app is this warning in the dev console after a programmaticform.setFieldValue(...), pointing at application code that holds no circular data, plus anonMetaChangethat fires when nothing changed.Tests
Four cases added to
src/test/isEqual.test.ts:Every test already in that file passes unchanged, including
should not equal 6, which is the existing cyclic case.false, warnstrue, silentfalse, warnstrue, silenttrue, silenttrue, silenta.self = a)false, warnsfalse, warnsfalse, warnsfalse, warnsSummary by CodeRabbit