[@xstate/solid] Fix context mutation with new values nested in arrays (…

…#5100) * @xstate/solid Fix new array index value not being cloned before insertion into store - Fix #5099 Signed-off-by: Austin Golding <[email protected]> * @xstate/solid Fix new array index value not being cloned before insertion into store - Fix #5099 Signed-off-by: Austin Golding <[email protected]> * @xstate/solid Update new array index set to use placeholder value if wrappable to prevent mutating original object/array without needing to deep clone Signed-off-by: Austin Golding <[email protected]> --------- Signed-off-by: Austin Golding <[email protected]>
statelyai · Oct 17, 2024 · 519188a · 519188a
1 parent 42304da
commit 519188a
Show file tree

Hide file tree

Showing 3 changed files with 151 additions and 9 deletions.
diff --git a/.changeset/strong-keys-work.md b/.changeset/strong-keys-work.md
@@ -0,0 +1,5 @@
+---
+'@xstate/solid': patch
+---
+
+When setting new array indexes, if the value is an object/array, use placeholder empty value to prevent mutation of original machine context
diff --git a/packages/xstate-solid/src/createImmutable.ts b/packages/xstate-solid/src/createImmutable.ts
@@ -12,6 +12,14 @@ const resolvePath = (path: any[], obj = {}): unknown => {
   return current;
 };
 
+const getWrappablePlaceholder = (value: any) => {
+  if (!isWrappable(value)) return value;
+  if (Array.isArray(value)) {
+    return [];
+  }
+  return {};
+};
+
 const updateStore = <Path extends unknown[]>(
   nextStore: Store<any>,
   prevStore: Store<any>,
@@ -53,21 +61,23 @@ const updateStore = <Path extends unknown[]>(
       const smallestSize = Math.min(prev.length, next.length);
       const largestSize = Math.max(next.length, prev.length);
 
+      // Update new or now undefined indexes
+      if (newIndices !== 0) {
+        for (let newEnd = smallestSize; newEnd <= largestSize - 1; newEnd++) {
+          set(...path, newEnd, getWrappablePlaceholder(next[newEnd]));
+        }
+      }
+
       // Diff array
       for (let start = 0, end = largestSize - 1; start <= end; start++, end--) {
         diff(next[start], prev[start], [...path, start] as Path);
         if (start === end) break;
         diff(next[end], prev[end], [...path, end] as Path);
       }
 
-      // Update new or now undefined indexes
-      if (newIndices !== 0) {
-        for (let newEnd = smallestSize; newEnd <= largestSize - 1; newEnd++) {
-          set(...path, newEnd, next[newEnd]);
-        }
-        if (prev.length > next.length) {
-          set(...path, 'length', next.length);
-        }
+      // Update length if it has changed
+      if (prev.length !== next.length) {
+        set(...path, 'length', next.length);
       }
     } else {
       // Update new values

diff --git a/packages/xstate-solid/test/useActor.test.tsx b/packages/xstate-solid/test/useActor.test.tsx
@@ -8,7 +8,8 @@ import {
   mergeProps,
   on,
   onCleanup,
-  onMount
+  onMount,
+  For
 } from 'solid-js';
 import { fireEvent, render, screen, waitFor } from 'solid-testing-library';
 import {
@@ -685,6 +686,132 @@ describe('useActor', () => {
     expect(countEl.textContent).toEqual('1');
   });
 
+  it('Moving objects between arrays should not mutate context', () => {
+    const stackMachine = createMachine({
+      types: {} as {
+        context: { current: { value: string }[]; done: { value: string }[] };
+        events: { type: 'next' } | { type: 'prev' };
+      },
+      id: 'stack',
+      initial: 'active',
+      context: {
+        current: [
+          { value: 'Stack #1' },
+          { value: 'Stack #2' },
+          { value: 'Stack #3' }
+        ],
+        done: []
+      },
+      states: {
+        active: {
+          on: {
+            next: {
+              guard: ({ context }) => context.current.length > 0,
+              actions: assign(({ context }) => {
+                const [first, ...rest] = context.current;
+                return {
+                  current: rest,
+                  done: [...context.done, first]
+                };
+              })
+            },
+            prev: {
+              guard: ({ context }) => context.done.length > 0,
+              actions: assign(({ context }) => {
+                const rest = context.done.slice(0, -1);
+                const last = context.done.at(-1);
+                return {
+                  current: last ? [last, ...context.current] : context.current,
+                  done: rest
+                };
+              })
+            }
+          }
+        }
+      }
+    });
+
+    function App() {
+      const [snapshot, send] = useActor(stackMachine);
+
+      return (
+        <>
+          <div style={{ display: 'flex', gap: '24px' }}>
+            <div>
+              Current
+              <ul data-testid="current-list">
+                <For each={snapshot.context.current}>
+                  {(item) => {
+                    return <li>{item.value}</li>;
+                  }}
+                </For>
+              </ul>
+            </div>
+
+            <div>
+              Done
+              <ul data-testid="done-list">
+                <For each={snapshot.context.done}>
+                  {(item) => {
+                    return <li>{item.value}</li>;
+                  }}
+                </For>
+              </ul>
+            </div>
+          </div>
+          <div>
+            <button
+              data-testid="first-current-to-done-button"
+              onClick={() => send({ type: 'next' })}
+            >
+              Remove first
+            </button>{' '}
+            <button
+              data-testid="last-done-to-current-button"
+              onClick={() => send({ type: 'prev' })}
+            >
+              Return last
+            </button>
+          </div>
+        </>
+      );
+    }
+
+    render(() => <App />);
+
+    const firstToDoneButton = screen.getByTestId(
+      'first-current-to-done-button'
+    );
+    const lastToCurrentButton = screen.getByTestId(
+      'last-done-to-current-button'
+    );
+    const currentList = screen.getByTestId('current-list');
+    const doneList = screen.getByTestId('done-list');
+
+    expect(currentList.children.length).toBe(3);
+    expect(doneList.children.length).toBe(0);
+
+    fireEvent.click(firstToDoneButton);
+    fireEvent.click(firstToDoneButton);
+    fireEvent.click(firstToDoneButton);
+
+    expect(currentList.children.length).toBe(0);
+    expect(doneList.children.length).toBe(3);
+    expect(doneList.innerHTML).toBe(
+      '<li>Stack #1</li><li>Stack #2</li><li>Stack #3</li>'
+    );
+
+    fireEvent.click(lastToCurrentButton);
+    fireEvent.click(lastToCurrentButton);
+    fireEvent.click(lastToCurrentButton);
+
+    expect(currentList.children.length).toBe(3);
+    expect(doneList.children.length).toBe(0);
+    expect(currentList.innerHTML).toBe(
+      '<li>Stack #1</li><li>Stack #2</li><li>Stack #3</li>'
+    );
+  });
+
   it('should capture initial actions', () => {
     let count = 0;