-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
fix(reactivity): support Set composition methods #14058
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
WalkthroughAdds ES2024 Set composition methods (difference, intersection, symmetricDifference, union) and set-relationship predicates (isDisjointFrom, isSubsetOf, isSupersetOf) to instrumented reactive Sets and appends tests. Implementations delegate to the underlying raw Set via toRaw(); tests run only when the prototype methods exist at runtime. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Reactive as Instrumented Set
participant Raw as Raw Set (toRaw)
App->>Reactive: call e.g. difference(other)
activate Reactive
note right of Reactive `#E8F3FF`: check readonly? trigger ITERATE
Reactive->>Raw: toRaw(this).difference(other)
activate Raw
Raw-->>Raw: compute result (Set / boolean)
Raw-->>Reactive: return result
deactivate Raw
Reactive-->>App: return result
deactivate Reactive
note over Reactive,Raw `#F7FFF0`: same flow for intersection, symmetricDifference, union, isDisjointFrom, isSubsetOf, isSupersetOf
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 |
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
🧹 Nitpick comments (1)
packages/reactivity/__tests__/collections/Set.spec.ts (1)
463-563: Fix “suppoort” typo in test descriptionsSeveral new test titles read “suppoort”. Please correct them to “support” for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/reactivity/__tests__/collections/Set.spec.ts(1 hunks)packages/reactivity/src/collectionHandlers.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/reactivity/src/collectionHandlers.ts (2)
packages/reactivity/src/reactive.ts (2)
Target(18-24)toRaw(378-381)packages/shared/src/general.ts (1)
extend(24-24)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
🧹 Nitpick comments (1)
packages/reactivity/src/collectionHandlers.ts (1)
251-251: Consider computingwraponce at the start of this extend block.Since all four Set composition methods need the same wrapping logic, computing
wraponce at line 251 (right after the extend call) would reduce duplication:extend(instrumentations, { + const wrap = shallow ? toShallow : readonly ? toReadonly : toReactive + difference(this: SetType, other: SetLikeType): SetType {This is optional since the wrapping logic is lightweight, but it follows the pattern used elsewhere in the file (e.g., lines 116, 151).
Also applies to: 310-310
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/reactivity/src/collectionHandlers.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/reactivity/src/collectionHandlers.ts (3)
packages/reactivity/src/reactive.ts (2)
Target(18-24)toRaw(378-381)packages/shared/src/general.ts (1)
extend(24-24)packages/reactivity/src/dep.ts (3)
track(108-165)track(262-284)ITERATE_KEY(242-244)
🔇 Additional comments (2)
packages/reactivity/src/collectionHandlers.ts (2)
27-28: LGTM! Type aliases are well-defined.The type aliases appropriately represent reactive Sets and the Set-like interface required by ES2024 composition methods.
286-309: LGTM! Boolean predicates correctly track dependencies.The boolean-returning methods (isDisjointFrom, isSubsetOf, isSupersetOf) properly track the ITERATE dependency and don't need reactive wrapping since they return primitives. The implementation correctly registers reactivity dependencies on the receiver Set.
|
Refactor the code, referencing #11399 |
To support Set composition methods. These methods don't make any state change.
Related issue #11398
Summary by CodeRabbit
New Features
Tests