createMutable stores do not always behave like regular JS objects that are observed for changes

[fabiospampinato]

In my opinion people expect deeply proxied objects that can intercept reads and writes to work identically to JS objects, with basically the only difference being that the proxy is aware of every read and write that goes through the proxied object.

Stores created with createMutable break that assumption in some ways that I think can cause bugs, and in ways that are not documented, in case the present behavior is intentional, so I think the following issues should be addressed:

• [ ] Setting a property to undefined deletes the property. That's just not what plain objects do. Repro: https://playground.solidjs.com/anonymous/b3976972-e6bb-42ef-8481-dc07bb203009
• [ ] A bunch of extra properties are attached to proxied objects. I don't think any detectable edits to the object that the user didn't explicitly perform should be performed, in particular in this case we can use WeakMaps instead. Repro: https://playground.solidjs.com/anonymous/577681f3-d496-4354-97f2-51bff69025f9
• [ ] Some interceptable writes are not intercepted, kinda creating a hole in the "observer membrane". Ideally this should get fixed to make createMutable work more predictably and reliably. In particular the defineProperty proxy trap is not implemented, so Object.defineProperty is not intercepted. Repro: https://playground.solidjs.com/anonymous/aea88d65-fea3-4cb8-a53a-56402667d0af

[ryansolid]

Yeah these are all good points. The first should be addressed if reasonable to do.

The second I wasn't willing to mess with performance. I did benchmark it again in the summer and it was definitely slower. This seems to bother people some so might get pushed this way regardless.

The 3rd is really an extension of the core issue here, and part of why I don't like createMutable in general. Reactivity should be special. The moment we stop treating it as such, to invisibly infiltrate existing code we lose something. Type system might be a saviour here but its hard from a JS perspective. I dislike createMutable because you are right. It should act like a plain JavaScript object given the API. And that just isn't great. I think when createMutable finds its proper home outside of core that's when we should do the third.

On that point, I am conscious of the fact that createStore might be too heavy-handed and we should look at that at the same time make it more permissive to use. It's quite possible that if we can make it composable produce should be the default API.

[fabiospampinato]

I did benchmark it again in the summer and it was definitely slower.

Do you have a link to the benchmark? I'd like to run it with my implementation also. In my little benchmark fwiw Solid's store is significantly slower.

[ryansolid]

JS Framework Benchmark. I didn't go micro into things as it was detectable there. I don't doubt there are ways to improve store performance but straight swapping the symbols for Weakmap was noticeable there.

[AFatNiBBa]

I don't think there is a need to use a weak map, you just need to exclude Symbol(solid-proxy) from the result of ProxyHandler.ownKeys(), ProxyHandler.getOwnPropertyDescriptor(), etc...

[fabiospampinato]

@AFatNiBBa potentially that would still be detectable because you can bail out of Solid's proxy. Possibly it's good enough, but the WeakMap approach doesn't leak this detail.

[AFatNiBBa]

In this case you could put the hidden properties on the ProxyHandler:

const handler = { get(t, k) { return k === "c" ? this.data : t[k]; } }; // Example of `ProxyHandler`

function makeProxy(obj) {
    const instanceOfhandler = Object.setPrototypeOf({ data: "something" }, handler); // You could use classes here
    return new Proxy(obj, instanceOfhandler);
}

const proxy = makeProxy({ a: 1, b: 2 });
console.log(proxy.b, proxy.c) // → 2 "something"

(I use a prototype in order to not re-instantiate all the handler every time)

[fabiospampinato]

@AFatNiBBa that's problematic for 2 reasons:

1. You need to create N objects containing traps instead of 1, wasting memory for nothing.
2. If the proxy is unwrapped and we pass it again to the createStore function we want to return the proxy we already made, but since the symbol is attached to the object containing traps for the previous proxy we had made we just can't reach it.

The WeakMap approach doesn't have these problems.

[AFatNiBBa]

In this case there might be another solution: Consider this base class Identity which has a constructor that returns whatever first argument you pass to it

class Identity {
    constructor(obj) {
        return obj;
    }
}

const obj = {};
console.log(obj === new Identity(obj)); // → true

We can use this base class to execute a constructor (This includes the class body) on any object

const a = {};
const b = new (class extends Identity { a = 32 })(a);
console.log(a === b, b.a); // → true 32

Why would we want that? Because in a class body we can define private fields! Native private fields are not accessible from within JavaScript (You would need to interact with the V8 internals or to use the DevTools protocol), so they won't affect the user object in any perceivable way. Consider this full example:

class Identity { constructor(obj) { return obj; } }
class Derived extends Identity {
    #a = "initial"
    static get(obj) { return obj.#a; }
    static set(obj, v) { obj.#a = v; }
}

const obj = {};
console.log(obj); // → {}
new Derived(obj);
console.log(obj); // → { #a: "initial" }
console.log(Derived.get(obj)); // → "initial"
Derived.set(obj, "new string");
console.log(Derived.get(obj)); // → "new string"

This would allow you to put the properties directly inside the object without having to worry about them showing up in places they're not supposed to. I also created a very light package to do this called "private-field" (NOT "private-fieldS")

import { createPrivateField } from "private-field";

const proxySymbol = createPrivateField();

function makeProxy(obj) {
    if (proxySymbol.has(obj)) return proxySymbol.get(obj);
    const out = new Proxy(obj, { /* Whatever */ });

    proxySymbol.define(obj);    // (You need to define the private field before or it will throw)
    proxySymbol.set(obj, out);

    proxySymbol.define(out);    // ← Keep this line
    proxySymbol.set(out, out);  // ← And this line in mind
    return out;
}

const obj = {};
const proxy1 = makeProxy(obj);
const proxy2 = makeProxy(obj);
const proxy3 = makeProxy(proxy1);
console.log(proxy1 === proxy2 && proxy2 === proxy3); // → true

In this example there are two strange lines of code that define the private field on the proxy too, these are needed if you want your proxy to have the same field as its target because private fields are so isolated from everything else that they do not interact with proxy traps, thus allowing you to define them on the proxy itself

Conclusion

The only problem I see with this solution is that when the Identity constructor gets called it could allocate the new this object (Even if it's not needed) before returning obj, but this isn't a big deal based on this test:

class UNIQUE_NAME { constructor(obj) { return obj; } }
debugger; // Create a memory snapshot
new UNIQUE_NAME({});
debugger; // Create a memory snapshot, search `UNIQUE_NAME` in the objects that have been created between the two snapshots

There will be no UNIQUE_NAME object in the heap, so even if the object was actually created it would've been immediately collected

[fabiospampinato]

Hold on, did you just figure out how to add private properties to plain objects? 🤯

It's definitely a very cool solution 🤔 worth measuring if it would be faster than just using a WeakMap, and what the memory consumption looks like.