feat: the reactive set supports the difference, intersection, isDisjointFrom, isSubsetOf, isSupersetOf, and symmetricDifference methods
#11399
Conversation
Size ReportBundles
Usages
|
|
Could you please add a unit test? |
这里很多的api，好像都不支持
好像 |
…tFrom, isSubsetOf, isSupersetOf, and symmetricDifference` methods
linzhe141
changed the title
reactive set supports the difference, intersection, isDisjointFrom, isSubsetOf, isSupersetOf, and symmetricDifference methods
| @@ -173,6 +173,24 @@ function createForEach(isReadonly: boolean, isShallow: boolean) { | |||
| } | |||
| } | |||
|
|
|||
| function createSetProtoMethod(method: TriggerOpTypes) { | |||
| return function (this: SetTypes, value: unknown, _isShallow = false) { | |||
There was a problem hiding this comment.
Is anything passing the _isShallow flag? Is it actually needed?
There was a problem hiding this comment.
This is really not necessary
| if (!hadKey) { | ||
| // @ts-expect-error | ||
| result = target[method](value) | ||
| trigger(target, method, value, value) |
There was a problem hiding this comment.
Shouldn't this be tracking rather than triggering?
These methods don't currently track correctly inside reactive effects. e.g.:
The unit tests aren't really testing this, as the effect isn't wrapped around the method call.
I haven't confirmed this, but I suspect these methods could reuse TrackOpTypes.ITERATE.
| } | ||
| const target = toRaw(this) | ||
| const proto = getProto(target) | ||
| const hadKey = proto.has.call(target, value) |
There was a problem hiding this comment.
I don't understand what this is for. The name value here is a bit misleading, as it refers to the other set. Why do we need to check whether one set is inside the other? This seems to break the methods if one set contains the other:
edison1105
added
✨ feature request
There was a problem hiding this comment.
I think there's a lot of redundancy in this implementation that can be removed by taking a similar approach to createIterableMethod. Specifically:
- The
methodargument forcreateSetProtoMethodshould just be a string, rather than aTriggerOpTypes. - I don't think we need to add any new values to
TriggerOpTypesfor this, they're just string method names. - Much like for
iteratorMethods, I think these can all be added in a loop, rather than duplicating all the code for each method.
The refactoring in #12152 reduced the amount of duplication in createInstrumentations. Once the latest main is merged into this branch it shouldn't be necessary to repeat the logic 4 times
…fork into feat-set-support-intersection
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-core
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
close #11398
It requires Node v22