Reconciling multiple stores with the same object leads to unexpected behaviour and unwanted side effects

[JaielZeus]

Describe the bug

When using reconcile to set a store object and reusing the object given to reconcile to set another store object, the code breaks in an unexpected manner. Reusing the same object as a base for reconciliation on two different stores leads to some entanglement that is unexpected.

• In my example code in the playground I have a baseList object which serves as the data sources
• There is also a storedList and a derivedList store object, having different use cases to accomplish.
• In the storedList store object there will be ListEntrys put in as { value: number; } objects.
• Saved in the derivedList store object are the last 6 entries of the storedList
• Every click on the "Add" button increments a counter current and puts a new list entry into baseList
• Then the baseList is deep copied
• This deep copy is used to reconciliate the storedList store object
• That same deep copy is used to reconciliate the derivedList store object

After the reconciliation of derivedList, the storedList store object is mutated as well, which is unexpected to me.

I have found, and commented it in the code, 2 possible fixes to this constellation. It seems the reusage of the same underlying object leads to this behaviour when utilizing reconcile.

I have read reconcile is gonna get a rework in the 2.0 version. Will this still be a bug in the new version?

Your Example Website or App

https://playground.solidjs.com/anonymous/41a51eb9-de0c-4293-a35c-2ce85c6be36b

Steps to Reproduce the Bug or Issue

• Go to the playground and use the example.
• Click on the "Add" button
• Notice the console log output.

Expected behavior

The console should put out an array with integers in order, like [1,2,3,4,...] but it puts out wrong values for the first 6 elements after 6 clicks.

Screenshots or Videos

Expected output (after 1 of the 2 fixes in comments applied):

Wrong output:

Code

Platform

• Browser

Additional context

No response

[ryansolid]

Because Stores work by mutation and we don't clone everything we are going to hit stuff like this. It would incredibly unperformant to do that as default for all the other use cases you don't want to do this. And you lose all ability to do any sort of shallow equality checks if you are cloning all the time. Like you could never even diff properly a second time because it wouldn't be the same object that was in the source you were diffing against previously even if it hasn't changed. It is basically just untenable. Immutable internals are another approach but also a waste for the general case so I've needed to figure out if there is a middle ground.

Which I have for 2.0 I think. I've been working on a version of reconcile that doesn't mutate. There is a detachment from the reactivity of stores and the underlying object so they are swappable in this reconcile situation. That being said stores are still built off mutation so this fact might only help you if all you ever do is reconcile. Works great for like redux or X-State adapter.

If you are modifying things given to stores well they will be modified. It is fundamental. The same object in multiple locations is the same object from a store perspective and should be trackable no matter the path you take to get to it. Which differs from immutable data stores found in something like Redux. So there is only so much that can be done here.

[mizulu]

derived state
https://playground.solidjs.com/anonymous/26d2b9a9-9c06-46b6-8f02-ba120a8034df

[JaielZeus]

@ryansolid I see and understand this is a user produced issue because of the udnerlying design. We cannot really touch the underlying object of a store with another peace of code (like reconcile in a different store) that mutates it. It was jsut a source of confusion to me for a bit to pin it down.

@mizulu thanks for a potential solution, but the code was written to point something out and the use case is different from using a derived state like you did and a reconciliation where you do not want to change the reference to something but just mutate it in-place

[mizulu]

@JaielZeus, I have put that just in case, as an alternative to derive state, instead of syncing stores.
I understand your confusion and concern.

I also looked a little more into the issue.

basically an object that is used(shared) in 2 different stores will change in both stores.
when mutated by any store setter. ( this is how javascript objects works too, it is not solid specific)

   let o1 = {}
   let store1 = {a:o1}
   let store2 = {b:o1}
   o1.value = "test" 
   console.log( JSON.stringify(store1) ) // {a:{value:"test}}
   console.log( JSON.stringify(store1) ) // {b:{value:"test}}

---

same thing with solid stores

  let objA = { value: "A" };
  let objB = { value: "B" };
  let objC = { value: "C"  };

  let [storeA, setStoreA] = createStore([objA, objB]);
  let [storeB, setStoreB] = createStore([objA, objB]);

  setStoreB(reconcile([objC,objC]));

After the reconcile above both storeA and storeB will have [{value:"C"}, {value:"C"}]

This is because the reconcile mutate the raw objects objA and objB

Which is why in your example you see unexpected values, basically your "derived list"
store, will hold references to the first 6 raw objects from the "main list"(storeList)
and when the derived list is updated the first 6 elements in the "main list" always get updated too

So this explain they why

[mizulu]

Now I have looked in the docs of reconcile
https://docs.solidjs.com/reference/store-utilities/reconcile#reconcile

and then I wondered why in this example, both objA and objB are mutated in the array instead of replaced ?

  let [storeB, setStoreB] = createStore([objA, objB]);
  setStoreB(reconcile([objC,objC]));

When merge is false, referential checks are performed where possible to determine equality, and items that are not referentially equal are replaced.

Another "issue" we can see is that even if we do the following ( unlike the case above, we simply re order)
technically referential identity check is possible

but the reconcile will still mutate objA into objB shape first
and we end up with [{value:"B"} , {value:"B"}] and not [{value:"B"} , {value:"A"}]

  let [storeB, setStoreB] = createStore([objA, objB]);
  setStoreB(reconcile([objB,objA]));

I don't know if reconcile should have just replaced the object at 0 with objB and object at 1 with objA

[mizulu]

one more way to fix this, is keying the object ( id: ? )

which will also fix this case

  let objA = {value:"A"}
  let objB = {value:"B"}
  let [storeB, setStoreB] = createStore([objA, objB]);
  console.log(storeB) // [{value"A"}, {value:"B"}]
  setStoreB(reconcile([objB,objA]));
  console.log(storeB) // [{value"B"}, {value:"B"}]

  let objA = {id:1,value:"A"}
  let objB = {id:2,value:"B"}
  let [storeB, setStoreB] = createStore([objA, objB]);
  console.log(storeB) // [{id:1,value"A"}, {id:2,value:"B"}]
  setStoreB(reconcile([objB,objA]));
  console.log(storeB) // [{id:2,value"B"}, {id:1,value:"A"}]

and the original issue case
https://playground.solidjs.com/anonymous/1301d15c-62aa-4b59-81b8-d25015abc30f

baseList.push({ value: current, id: current }); // <-  FIX: add id

[JaielZeus]

@mizulu as you say in the example I provided derivedList is holding the first few elements from storedList as reference but it is still a bit puzzling why it is the first x and not the last x elements.

Is there some memory leak going on by chance with reconcile? Are there multiple baseList deep copies being held in memory?

Also a peculiar thing is if you do const list = deepCopyBaseList.slice(-3).reverse(), then only the first element is mutated in the storedList:
[19,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]

[mizulu]

@JaielZeus

let say we limit from -6 to -3 for simplification
https://playground.solidjs.com/anonymous/ca213cb5-a2a7-4f8d-81b2-9759a154135a

after adding 3 elements this is how the stores will look like
(o1 , o2, etc are referentially equals, they are the same objects )

storedList
o1, o2, o3

derivedList
o1, o2, o3

on the 4th add()

storedList
o1, o2, o3, o4

but derived list is still as below, because it will always reconcile 3 elements into derivedList
it keep the same object o1,o2,o3 here after, and just mutate the,

derivedList
o1, o2, o3

(the last 3 elements now are o2,o3,o4) and so

1. o1 is mutated to o2 value
2. o2 is mutated to o3 value
3. o3 is mutated to o4 value

derivedList become from 1,2,3 into 2,3,4

and because we modify the o1,o2,o3
storedList become from 1,2,3,4 to 2,3,4,4

on the 5th add()

the next add reconcile derivedList again and just mutate o1,o2,o3
(the last 3 elements now are o3,o4,o5) and so
storedList will transform from 1,2,3,4,5 into 3,4,5,4,5

---

regarding the .reverse() case https://playground.solidjs.com/anonymous/ba13d914-e082-40e3-983d-de522450f6a6
it follows the same idea, as described for the case above. but here only the first element is shared

step 1

storedList
o1

derivedList
o1

on step 2

you take the deep clone
c1, c2

when you reconcile c1,c2 into storedList only the c2 is added as the 2nd element
c1 is just used to shape o1

storedList
o1[1], c2[2]

when you try to reconcile the reverse c2,c1 into derivedList
o1, takes the shape of c2.
and now the 2nd element is actually c1 which is not shared with storedList

derivedList
o1[2], c1[1]

so the stored ending up only sharing the first element o1,

[JaielZeus]

@mizulu ok got it thank you.I wasnt sure but that was my initial gut feeling too because of the output.

The other thing is: isn't o1, o2 etc after every click and reconciliation a reference to an object from the deep copied baseList array? Doesn't that mean the whole array is held in memory because 1 element is bein referenced by the stores? So after 3 clicks we have o1 from first deep copy, o2 from second deep copy and o3 from third deep copy. So 3 deep copies being held in memory?

[mizulu]

👍 I hope my explanation was helpful and not to hard to follow.

@JaielZeus

So 3 deep copies being held in memory

generally the answer is no.
the garbage collection will cleanup any object that does not have a hard reference

you can have a 1 million objects in an array
take one object put it in the store, and when the GC is ran, 9,999,999 objects will be freed including the array that held them.

If something is held in memory, this will be considered a memory leak.
and the could happen if solid internally help a reference to the whole array for no good reason after the reconciliation
but I don't think that is the case.

I am still curious to hear from @ryansolid about some of the points from #2486 (comment)
might be better to open another issue maybe because there is much going on here

[JaielZeus]

@mizulu not saying you are wrong and I want to believe you that this is always the case but I doubt for example chrome garbage collects those arrays for some reason. I had some issues when experimenting with weak maps. There in chrome it should, theoretically, remove a reference key and it's value when the underlying object that is used as a key has been freed and nothing but the weakmap is holding the reference of it as a key. Unfortunately I found out that chrome apparently garbage collects sporadically whenever it feels like it.

But back to the "memory leak" I will keep in mind that in general the whole array is not kept in memory. It makes sense. Thankss for clarifying everything that I struggled with to accept/understand :) (and your explanation was concise and to the point)

[mizulu]

found out that chrome apparently garbage collects sporadically whenever it feels like it.
that is probably true, the engine will decide when to preform the GC
when it feels like it.

if you are investigating suspected memory leaks
the dev tools may provide the tooling, that can help in identifying

it also provide a way to Collect Garbage

I will keep in mind that in general the whole array is not kept in memory

that is a good assumption to make, once the add() call is done
anything declared there, that is no longer reachable
should be handled at later time by the GC

leaks by the engine itself, are even less likely but may still be possible