justin-schroeder / arrow-js

Reactivity without the framework
https://arrow-js.com
MIT License
2.32k stars 50 forks source link

Map, Set, WeakMap, WeakSet can't be wrapped successfully #88

Open HugoDF opened 10 months ago

HugoDF commented 10 months ago

If this PR gets merged -> https://github.com/justin-schroeder/arrow-js/pull/87, the reactivity will not wrap Map, Set, WeakMap, WeakSet (which means they'll be returned without any interception)

Without it being merged I don't think they work properly inside of reactive(), the following throws:

const data = reactive({
  map: new Map()
})
data.map.size

Error in the browser (Firefox): Uncaught TypeError: get size method called on incompatible Proxy

Output in a test with the above code:

TypeError: Method get Map.prototype.size called on incompatible receiver #<Map>
 ❯ Object.get src/reactive.ts:208:29
    206|       if (Reflect.has(depProps, p)) return Reflect.get(depProps, p)
    207|
    208|       const value = Reflect.get(...args)
       |                             ^
    209|       // For any existing dependency collectors that are active, add this
    210|       // property to their observed properties.
Clonkex commented 4 months ago

Can confirm, Set doesn't work as a reactive property 😢

justin-schroeder commented 4 months ago

They dont work inside reactive() — and honestly I’m leaning towards not implementing them either. Getting a reactive framework to work well in the ~2kb range is pretty tough and the smaller the api surface area the better. Proxying maps and sets would add some precious bytes to the pile — not ruling it out though. After the refactors ship if there is any room in the byte budget this would be something to look at doing.

Clonkex commented 4 months ago

That's actually ok with me so long as the documentation makes this very clear. I would prefer that it throws a sensible exception as well, but I don't know how much spare room you have for error checking.

keturn commented 2 months ago

ditto for TypedArray.

My current use case is fine with an immutable TypedArray—no need to react to changes to its individual elements—but it was an unpleasant surprise to discover the thing I retrieved from the state store was not usable.¹

I could work around it if I could unwrap the object from the proxy, but it doesn't look like it exposes its proxySource. Hmm… Reflect.getPrototypeOf(proxy) appears to do that, but that feels more like a lucky implementation detail than a well-defined API.

1: despite the fact that it went in to the store just fine.