Calling unwrap on a Solid store returns extra metadata

[zaydek]

Describe the bug

Calling unwrap on a Solid store returns an object that is decorated with extraneous metadata that is unimportant for the end-programmer.

Here's a minimal repro with a screenshot attached to demonstrate:

If possible, I'd like the returned object to not include the symbol or have a second argument flag, e.g. deep: boolean, for unwrap API so that metadata is cleared. This is simply a quality of life feature but it would make debugging Solid stores easier without the need to call JSON.parse(JSON.stringify(state)) or equivalent.

Your Example Website or App

https://playground.solidjs.com/?version=1.4.1#NobwRAdghgtgpmAXGGUCWEwBowBcCeADgsrgM4Ae2YZA9gK4BOAxiWGjIbY7gAQi9GcCABM4jXgF9eAM0a0YvADo1aAGzQiAtACsyAegDucAEYqA3EogcuPfr2ZCouOAGU0Ac2hqps+YpU6DW09CysrG24+AUc4ZzdcbjgsXnoIQ0YoQl85BWVVYN0DMkShFXCIZloIEt5gEviUsjhcV1x4gF1eAF4HJxc2pIAKECteWVpaRHyTKEYVLCtJAEoKqpr1OAA6NVoPIbSMrKGGl2XViCt9a-1ZNOZcNGreAGEGCBdGIeX+K5vxhzVWrAKppXBNFpvMFdXqxeLuLxQNRDAAMy0sEBut3G61qGFi8A+PV43x6AD5eM1cFCPkNQbSfgBqXgARnRf2uykx-3GQlwTAgJI52IBAB4TPRcIlBQRiN0VBKpdUVLxqi8NMwANbdED4oSE3CSMnCgGmkD03DfSQm02i-SK6XG7mc8bs523a3urlYwTCMRfUndCmimmfXj6MkpES0Zj0A1bDwtACiajgBoAQvgAJIiIYqLKEFTLACEbrAkg6QA

Steps to Reproduce the Bug or Issue

1. Open this playground
2. Open the debugger or console and inspect the loggged object

Expected behavior

As a user I am expecting to receive back the state, clear of metadata or have a second argument flag provided by the unwrap API if for some reason this metadata is important.

Screenshots or Videos

No response

Platform

All

Additional context

No response

[ryansolid]

Currently that is just the way it works. Mostly for performance. I've intended to review this again when Weakmap performance improved. But generally speaking we can't remove this data the way things work as it is the same reference. All we could do is deep clone. Generally these properties are unique symbols and not iterable which is sufficient for most things. I will be doing some performance testing for 1.5 and we can review its impact.

[zaydek]

OK thank you. Honestly I'm having a lot more success modeling state using signals and if I need to instance them per component I think I can just nest them to a component and use a context. In my experiments using stores I found rapidly debugging state a pain point which is when this issue cropped up.

My primary concern is that unwrapped stores are going to appear noisy and potentially hurt user adoption. As a user I just feel uncomfortable using JSON.parse(JSON.stringify) in development to debug state (as I should be). Deep cloning can always be implemented in user-land but I'm sure whatever you have in mind will be faster than JSON.parse(JSON.stringify).

[ryansolid]

This has come up before when the properties were iterable. Since then there hasn't been any reports since most things that serialize will skip those properties. For the most part you can ignore those extra symbols, but I agree debugging proxies in general is a bit of a pain. Stores aren't too bad because you can just look at the target, but prop proxies that come from mergeProps and splitProps are especially awkward because of the indirection (they don't reflect their properties on the proxy target). I know there is some ability to affect at least chromium dev tools display of proxies which might ultimately be our solution for dev tools. That being said Stores generally are too valuable to avoid.

[zaydek]

Thanks for the reply. I'm looking forward to playing with stores again in the future if the developer experience can be slightly enhanced. Seeing as I find proxies somewhat of a hassle in the first place I think I'll continue playing with modeling state just using signals.

This isn't 100% relevant but I wanted to add here as context as to my leaning on signals:

The reason I like signals so much is that once you get past the accessor syntax, state can be modeled incrementally without the need for refactoring or implementing complex types. The downside with signals is that if you want to author reusable components (so that instances of a component don't share identical state) you need some mechanism to propagate signals to child components either via prop-drilling or contexts. Still working out how to solve for that but maybe this helps.

[zaydek]

Alright this is working great for me, definitely the debugging experience I was expecting. ☺️

function deepClone(ref: any) {
	if (typeof ref !== "object") {
		return ref
	} else {
		const ref2 = Array.isArray(ref) ? [] : {} as Record<string, any>
		for (const key in ref) {
			if (typeof ref === "object") {
				ref2[key] = deepClone(ref[key])
			} else {
				ref2[key] = ref[key]
			}
		}
		return ref2
	}
}

[ryansolid]

I have to admit I don't understand the complaint still. Debugging is one place where having access seems good. You can go back and forth between the target and the proxy and see what's going on. With a Weakmap you wouldn't be able to do that.

All that being said Weakmaps are still a performance hit. Not as bad as I thought they'd be only a couple points in the JS Framework Benchmark and the majority of the cost in clear rows benchmark. So still inclined to leave as is for 1.5. Maybe I will move this to the discussions to see if we can stir up more conversation. But otherwise I think this issue is stale for now.

[guillaumebrunerie]

A concrete situation where the current implementation can be pretty bad for performance is frozen objects.

If you store a large deeply frozen object in a store and then unwrap it, Solid will actually deeply clone the whole object because metadata cannot be stored on a frozen object the way Solid does it. Using WeakMaps instead would allow Solid to store and retrieve the whole object as is without any deep cloning.

It is also pretty unexpected to get a different reference when the object is frozen, and it breaks performance improvements that rely on reference equality (for instance WeakMap memoization outside of Solid).

[ryansolid]

Hmm.. frozen objects are a bit awkward in general anyway since Stores are mutated. The unwrapping happens on addition not on write since we need it to be internally referentially equivalent once present. I suspect this would be a problem regardless given that.

[guillaumebrunerie]

since Stores are mutated

Right, so my use case is slightly complex, but basically I'm implementing undo/redo inside a Solid store.
In order to do that, I'm using a snapshot function strongly inspired by Valtio (https://valtio.pmnd.rs/docs/api/advanced/snapshot), which is a proxy-based way to efficiently take deeply immutable snapshots of a mutable object (it uses setter traps to know which parts changed and shallow clone only those, while reusing previous snapshots for the parts that didn’t change). Then I'm storing those snapshots into a undoStack array inside the store. So the snapshots themselves are never meant to be mutated, but they can be added, removed, moved around (when pushing a new snapshot on the stack), etc.
Then I have a function that "applies" a snapshot onto a mutable state (to actually perform undo), but it only works efficiently if references are preserved, as it can then skip identical parts. Now, given that I have my own snapshot function, I could easily remove the Object.freeze call and then everything worked as expected, but it was pretty unexpected that I needed to not freeze snapshots in order for them to be stored as-is.

[angleKH]

This unwrap problem also exists for createMutable (noted in solidjs docs as "useful for integrating external systems or serving as a compatibility layer with frameworks"), where the extra properties can end up interfering with external systems. I've run into issues with serializing/deserializing unwrapped items and have had to rely on basic signals to add reactivity.