Bug: useActor from @xstate/solid mutates context

[DeylEnergy]

XState version

XState version 5

Description

Hi guys. Have been tinkering with XState and Solid recently. Found a surprising issue. Seems like diffing and setting updated store values in createImmutable implementation mutates the context within machine.

In my example, on next event first item within current (which is an object) gets removed and is put into the other array (done). next performed 3 times totally (until current is empty). Then prev event performed 3 times. I expect to see initial items within current array, but all of them are {value: 'Stack #2'}

Expected result

Objects within arrays in context shouldn't mutate

Actual result

Objects within arrays in XState context gets mutated when used with useActor

Reproduction

https://codesandbox.io/p/devbox/runtime-thunder-qy36ws?embed=1&file=%2Fsrc%2FApp.tsx

Additional context

No response

[davidkpiano]

Strange - thank you for the report. Would you be able to dive in and see where the issue is, and maybe what a potential fix can be?

cc. @GoldingAustin

[DeylEnergy]

Would you be able to dive in and see where the issue is, and maybe what a potential fix can be?

Sure. So far, found out that deep-cloning the snapshot (using deepClone function) on every state change solves issue. However, I believe it causes a lot of unnecessary work. I'll try to explore further and report back.

[GoldingAustin]

Thanks, @DeylEnergy, for the detailed bug report and codesandbox! I was able to pinpoint the source of the issue and implement a fix. When createImmutable updates an array, it diffs with the previous array indexes and inserts values into new indexes. When it inserts into those new indexes, we need to clone each (object type) value as it references the context object stored in the machine, leading to all the mutations due to how SolidJS stores work.

[DeylEnergy]

@GoldingAustin thank you for coming up with PR. Strangely, your fix passes all tests, but it doesn't work in a browser environment. Which is quite weird. Copied everything from @xstate/solid package into single file useActor into new codesandbox

I dove into the issue myself, and found out that there is one more place (in updateStore function) where we give away references to solid stores. deepCloning there solved my issue. Reflected the change in the new codesandbox example.

 if (!isWrappable(next) || !isWrappable(prev)) {
       // toString cannot be set in solid stores
       if (path[path.length - 1] !== 'toString') {
-        set(...path, () => next);
+        set(...path, () => deepClone(next));
       }
       return;
}

A bit aside: codesandbox seems to be unstable these days, it acts strangely out of blue, so if there is an issue, then local check would be more reliable. Thanks.

[GoldingAustin]

@DeylEnergy Thanks for testing! I checked out your codesandbox. Your version of updateStore wasn't quite in line with the changes in my PR. Moving the newIndex update loop before the diff loop was the key thing missing. This ensures the object/array is cloned before being inserted into the Solid store. When it happens after the diff loop, the original context reference is mutated.