overwrapping proxies

[bigmistqke]

Hi luisherranz

First of all thank u for the nice library.
Is the smoothest state management dx in react imo. Dangerously smooth!

I came across a bit of an unpleasantry in dx with what looks like stale state, caused by unwanted nesting of the proxy-state.

Say I have the following store

const store = deepsignal({ renderedNode: undefined, nodes:  [{id: 1, x: 0, y: 0}, ...])

if I would do

store.renderedNode = store.nodes[0];

store.renderedNode would become a nested proxy.
accessing and mutating this proxy would cause inconsistent state, I assume because the outer-proxy consumes all the mutations.

It is possible to prevent this bug by doing

store.$renderedNode.value = store.nodes[0];

But I wonder if it would be possible to prevent this overwrapping/unwanted nesting automatically, p.ex by checking for a symbol on the proxy.

I think solidjs does something similar with their createStore.

[bigmistqke]

I think solidjs does something similar with their createStore.

upon second thought, solid does this:

• if you set a proxy-node with a proxy, it 'unwraps' the proxy (returning the values instead of the signals) and creates signals for each sibling again
• you can make references like store.$renderedNode.value = store.nodes[0]; with shallow merging. it would be something like setStore({renderedNode: store.nodes[0]}) in solid.

this whole shallow-merging is something I dislike the most from solid's store api, so I wouldn't recommend repeating it.

instead I would propose the opposite API:

store.renderedNode = store.nodes[0]

makes a reference to store.nodes, so that whenever u do p.ex store.renderedNodes.x = 1 it also changes store.nodes[0].x since they are referring to the same signal.

when doing

store.renderedNode = unwrap(store.nodes[0])

it would unwrap store.nodes[0] (afaik unwrap does not exist yet in deepsignal, but it would be a great addition.. I guess it's the same as store.$nodes[0].value) into its value, effectively copying the signal-tree. then you have two separate signals, so store.renderedNodes.x = 1 would not change store.nodes[0]

[luisherranz]

Good catch.

I've just opened a PR to fix it: #20

[luisherranz]

Released as [email protected].

@bigmistqke could you please test it out and see if everything works as expected now?

[bigmistqke]

Mmm, it crashes my application.

It still somehow overwraps in the application state, but am not yet able to reproduce it in a simpler test.
The escape hatch of store.$selectedNode.value = node also makes the app crash. Unsure what the cause there is, because it's not overwrapping.

Will keep u in the loop if i find out more information.

this is how the error looks when doing store.$selectedNode.value = node

this is how it looks when doing store.selectedNode = node

with set referring to

[bigmistqke]

Ok maybe false alarm. Could have been some caching issue. nope, error returned somehow.

In chrome we get the following error:

with d2 referring to

[bigmistqke]

Ok so I believe the crashing has somehow something to do with useComputed.
I have a useComputed that returns <div/>s related to the selected Node and somehow that causes a crash (before it was working just fine).

Still not capable to create a minimal reproduction. When I test it out with less complex state it's all working fine.

[luisherranz]

If you can reproduce it and share a Stackblitz with me, that would be really helpful. Thanks.

[bigmistqke]

I would like to, but can not seem to get deepsignal to work with stackblitz/codesandbox:

see
stackblitz
codesandbox

[bigmistqke]

Okay I think I found the culprit.

I had added a deepSignal in App.tsx to play around with the update and be able to create minimal reproductions.
If I remove that one everything works as expected.

The weird thing is it even broke computed in @preact/signals-react when I used it without deepsignal

this crashes

import { computed } from '@preact/signals-react';
import { deepSignal } from 'deepsignal';

const store = deepSignal({});
const rows = computed(() => [''].map(() => <></>);

const App = () => {
  return rows;
}

this does not crash

import { computed } from '@preact/signals-react';

const rows = computed(() => [''].map(() => <></>);

const App = () => {
  return rows;
}

My other store is in a seperate file and that one seems to be working fine.
I believe it even doesn't overwrap now, as long as the deepSignal in App.tsx is removed.
With it does overwrap somehow, hence the inconsistency.

It's an unrelated issue to the update, as it also happens when I revert the package to 1.3.0
Oof, that was a tough one.

[luisherranz]

I had added a deepSignal in App.tsx to play around with the update and be able to create minimal reproductions. If I remove that one everything works as expected.

For React, you should use:

import { deepSignal } from 'deepsignal/react';

Explanation.

I would like to, but can not seem to get deepsignal to work with stackblitz/codesandbox:

Maybe there's a problem with imports without modules. It works fine here:

• https://stackblitz.com/edit/vitejs-vite-zoh464?file=src%2Fmain.jsx
• https://codesandbox.io/s/deepsignal-demo-react-fupt1x?file=/src/index.js

[bigmistqke]

For React, you should use:

import { deepSignal } from 'deepsignal/react';

o no lol. you are right, was a typo, in the store I properly import it.
The errors go away when I use deepsignal/react... what a trip.
I guess the different @preact/signals import mess up some of preact-signal's internals.
Tricky one to debug, maybe could have been prevented by having it as separate packages (like @preact/signals do), but mb that boat has sailed with @deepsignal being another unrelated package too (bit unfortunate naming).

Thanks for the support!
I won't be able to work on the project for the next days, but will keep you in the loop if I find something else.

[luisherranz]

I would like to, but can not seem to get deepsignal to work with stackblitz/codesandbox:

I can't spot any differences between @preact/signals and deepsignal in terms of exports, package.json configuration and so on, so I don't know what to fix.

I won't be able to work on the project for the next days, but will keep you in the loop if I find something else.

Please, do. Thanks!