setting element reference in `setStore` modifies the values in the array

[Reverier-Xu]

Describe the bug

I have a store just like this:

const [store, setStore] = createStore({
  items: [] as Item[],
  current: null as Item | null,
})

If I use setStore("current", items[index]) to change current into another element in array, the source array will be changed by this call, see the example below.

Your Example Website or App

https://stackblitz.com/edit/solidjs-issue-vgcaxz

Steps to Reproduce the Bug or Issue

1. Click the Item 1, and the current will change from null to item 1.
2. Click the Item 2, the current will change from item 1 to item 2, but it also changes the item1 element to item2.

Expected behavior

I expected the current will change without the array changes.

Screenshots or Videos

No response

Platform

• OS: Linux
• Browser: Edge
• Version: 123.0.2420.81

Additional context

No response

[Reverier-Xu]

A temp solution for users maybe using deep copy:

setStore('current', JSON.parse(JSON.stringify(items[index])))

But this looks ugly...

[elite174]

@Reverier-Xu try this setStore({current: item});

[danieltroger]

OH MY GOD THANK YOU FROM THE BOTTOM OF MY HEART FOR REPORTING THIS. Objects passed into setStore being mutated has cost the company where I'm working at hours of frustration and debugging but it's been happening so deeply nested in our stack that we haven't reached the breaking point of actually finding the issue. We're using reconcile and I initially thought that was the issue, good to see it's just with stores.

Also, a nicer temp solution than JSON.stringify is using structuredClone (that's how we temp fixed it): setStore('current', structuredClone(items[index]))

[SarguelUnda]

I would change the workflow à little to adress your problem by normalising the state à little:

const [store, setStore] = createStore(() => {
  items: [] as Item[],
  currentIdx: null as number | null,
})

const current = () => store.items[store.currentIdx]

Now you just setStore("currentIdx", index) and current update reactively. It has the advantage of having a single source of truth.

[SarguelUnda]

https://www.solidjs.com/docs/latest#getters

If you want everything in the same place you can also write it :

const [store, setStore] = createStore(() => {
  items: [] as Item[],
  currentIdx: null as number | null,
   get current() {
        return this.items[this.currentIdx]
   }
})

[danieltroger]

It's great that there's a workaround for @Reverier-Xu's case but I still think createStore should be fixed, because I don't think there's any possible workaround for my usecase except for structuredClone right now

[Reverier-Xu]

@Reverier-Xu try this setStore({current: item});

@elite174 That seems a better & elegant way for this case and do not require copy'hacks, I forked the example site and find it worked well, but this usage is not introduced in Solid Docs umm...

but @danieltroger's issue still happens, weird that.

[SarguelUnda]

@elite174 That seems a better & elegant way for this case and do not require copy'hacks, I forked the example site and find it worked well, but this usage is not introduced in Solid Docs umm...

It is there : https://docs.solidjs.com/concepts/stores#modifying-objects

When using store setters to modify objects, if a new value is an object, it will be shallow merged with the existing value.

and there : https://www.solidjs.com/docs/latest/api#updating-stores

Objects are always shallowly merged

I think there is still a little misunderstanding on store and it is mainly coming from typescript in my opinion. From doc:

With stores, you can now target nested properties and elements within these structures to create a dynamic tree of reactive data.

The dynamic part is important. What you try to achieve it seems is a store that "stop" being writable at the current level. Meaning you want current to only be a reference to an object in items. Store can't understand that because they track and merge all the available path when using setter. You have then two options to achieve your goal. Either implicitly update the reference with the

setStore({current: item}); pattern or even setStore(produce(s => s.current = item)});

or being explicit about the computed aspect of current with the getter option:

const [store, setStore] = createStore(() => {
  items: [] as Item[],
  currentIdx: null as number | null,
   get current() {
        return this.items[this.currentIdx]
   }
})

[elite174]

I wouldn't recommend using syntax from the example (setStore("current", items[index])) The point is that it's quite dangerous. I described an issue with it in my article here: https://vladislav-lipatov.medium.com/solidjs-pain-points-and-pitfalls-part-2-175aca906e98

[elite174]

Personally, I think it's better to use objects or produce

[ryansolid]

Yes.. this is due to shallow object merging. It was a call to be consistent with top level but it is an idiosyncrasy of this path API. So while it is confusing it is documented and by design. This dates back to when React was still on class components and it was very common to call setState with an object. Merging was expected. hope to move away from this API as the default in future versions of Solid. But it is what it is now. Closing as by design.

The answer is not to structured clone at least in the simple case. It's to use the object syntax to set. You can always go up a level and use an object and it will not merge.

Ie

// don't
setStore("current", something)

// do
setStore({ current: something });

// don't
setStore("a", "b", something);

// do
setStore("a", { b: something });

---

@danieltroger My guess though your issue is that reconcile or any state changes in stores do mutate. This way reactivity is transitive (same reference in multiple stores etc)... So if you keep external references to objects that have been passed into stores previously then yes you will in some cases find them changed. This also makes sense from the perspective that we close over variable references that don't change so something has to mutate along the way.

[danieltroger]

@ryansolid thanks for your explanation. I don't quite understand everything ("transitive reactivity", "close over variable references") but it seems plausible - we do pass an object down into the store, clone parts of it and add/remove things to the clone, pass that out (to react) and pass it back into the store.

1. Is it desired behavior then that stores mutate the object being passed into it, and not a bug, why? (Maybe mention this in the docs because I would not expect it)
2. Would calling unwrap on all values before passing them back up to "react world" stop this behavior from happening? Because structuredClone doesn't work on functions so it's not a "scalable" solution to this problem

[hawkerboy7]

Hi, I ran into this same issue 2 months ago see: https://github.com/solidjs/solid/issues/2063 And trying figuring it out was kind of like @danieltroger describes :)

The problem I've noticed is that switch from "data-type" null to object. The first object passed will be used as the object for future mutations.

Another way to "solve" it is to not switch from null to item so:

const [store, setStore] = createStore({
  items: [],
  current: {},
})

See my playground example: https://playground.solidjs.com/anonymous/89aecb87-da00-4fa8-bfc3-72794555d630. You can change the current: null to current: {}

However if (store.current) { } will now always be truthy. And if it where to get set to null at some point and re-set again we introduce the issue again.

So probably best to just follow what @elite174 and ryansolid wrote :)