fix(patches): fix patches for deletion of keys (#65)

README.md

@@ -115,18 +115,18 @@ Measure(ops/sec) to update 50K arrays and 1K objects, bigger is better([view sou
 ![Benchmark](benchmark.jpg)
 
 ```
-Naive handcrafted reducer - No Freeze x 4,486 ops/sec ±0.85% (98 runs sampled)
-Mutative - No Freeze x 6,338 ops/sec ±1.14% (93 runs sampled)
-Immer - No Freeze x 5.32 ops/sec ±0.43% (18 runs sampled)
+Naive handcrafted reducer - No Freeze x 4,469 ops/sec ±0.82% (99 runs sampled)
+Mutative - No Freeze x 6,015 ops/sec ±1.17% (94 runs sampled)
+Immer - No Freeze x 5.36 ops/sec ±0.28% (18 runs sampled)
 
-Mutative - Freeze x 969 ops/sec ±1.02% (98 runs sampled)
-Immer - Freeze x 383 ops/sec ±0.44% (94 runs sampled)
+Mutative - Freeze x 959 ops/sec ±0.90% (99 runs sampled)
+Immer - Freeze x 382 ops/sec ±0.53% (94 runs sampled)
 
-Mutative - Patches and No Freeze x 978 ops/sec ±0.22% (97 runs sampled)
-Immer - Patches and No Freeze x 5.32 ops/sec ±0.54% (18 runs sampled)
+Mutative - Patches and No Freeze x 970 ops/sec ±0.99% (96 runs sampled)
+Immer - Patches and No Freeze x 5.22 ops/sec ±0.80% (18 runs sampled)
 
-Mutative - Patches and Freeze x 507 ops/sec ±0.28% (97 runs sampled)
-Immer - Patches and Freeze x 279 ops/sec ±0.76% (91 runs sampled)
+Mutative - Patches and Freeze x 505 ops/sec ±0.85% (93 runs sampled)
+Immer - Patches and Freeze x 275 ops/sec ±0.49% (89 runs sampled)
 
 The fastest method is Mutative - No Freeze
 ```
@@ -137,7 +137,7 @@ Run `yarn benchmark` to measure performance.
 
 Immer relies on auto-freeze to be enabled, if auto-freeze is disabled, Immer will have a huge performance drop and Mutative will have a huge performance lead, especially with large data structures it will have a performance lead of more than 50x.
 
-So if you are using Immer, you will have to enable auto-freeze for performance. Mutative is disabled auto-freeze by default. With the default configuration of both, we can see the 16x performance gap between Mutative (`6,338 ops/sec`) and Immer (`383 ops/sec`).
+So if you are using Immer, you will have to enable auto-freeze for performance. Mutative is disabled auto-freeze by default. With the default configuration of both, we can see the 16x performance gap between Mutative (`6,015 ops/sec`) and Immer (`382 ops/sec`).
 
 Overall, Mutative has a huge performance lead over Immer in [more performance testing scenarios](https://github.com/unadlib/mutative/tree/main/test/performance). Run `yarn performance` to get all the performance results locally.
 

src/utils/draft.ts

@@ -1,5 +1,6 @@
 import { DraftType, Mark, ProxyDraft } from '../interface';
 import { dataTypes, PROXY_DRAFT } from '../constant';
+import { has } from './proto';
 
 export function latest<T = any>(proxyDraft: ProxyDraft): T {
   return proxyDraft.copy ?? proxyDraft.original;
@@ -45,15 +46,21 @@ export function getPath(
 ): (string | number | object)[] | null {
   if (Object.hasOwnProperty.call(target, 'key')) {
     // check if the parent is a draft and the original value is not equal to the current value
-    const proxyDraft = getProxyDraft(get(target.parent!.copy, target.key!));
+    const parentCopy = target.parent!.copy;
+    const proxyDraft = getProxyDraft(get(parentCopy, target.key!));
     if (proxyDraft !== null && proxyDraft?.original !== target.original) {
       return null;
     }
-    path.push(
-      target.parent!.type === DraftType.Set
-        ? Array.from(target.parent!.setMap!.keys()).indexOf(target.key)
-        : target.key
-    );
+    const isSet = target.parent!.type === DraftType.Set;
+    const key = isSet
+      ? Array.from(target.parent!.setMap!.keys()).indexOf(target.key)
+      : target.key;
+    // check if the key is still in the next state parent
+    if (
+      !((isSet && parentCopy.size > (key as number)) || has(parentCopy, key!))
+    )
+      return null;
+    path.push(key);
   }
   if (target.parent) {
     return getPath(target.parent, path);

test/__snapshots__/apply.test.ts.snap

@@ -1,5 +1,85 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`#64 - Invalid inversePatches when array length changes 1`] = `
+[
+  {
+    "op": "replace",
+    "path": [
+      0,
+    ],
+    "value": {
+      "id": 1,
+    },
+  },
+  {
+    "op": "replace",
+    "path": [
+      1,
+    ],
+    "value": {
+      "id": 2,
+    },
+  },
+  {
+    "op": "replace",
+    "path": [
+      2,
+    ],
+    "value": {
+      "id": 30,
+    },
+  },
+  {
+    "op": "replace",
+    "path": [
+      "length",
+    ],
+    "value": 3,
+  },
+]
+`;
+
+exports[`#64 - Invalid inversePatches when array length changes 2`] = `
+[
+  {
+    "op": "replace",
+    "path": [
+      0,
+    ],
+    "value": {
+      "id": 0,
+    },
+  },
+  {
+    "op": "replace",
+    "path": [
+      1,
+    ],
+    "value": {
+      "id": 1,
+    },
+  },
+  {
+    "op": "replace",
+    "path": [
+      2,
+    ],
+    "value": {
+      "id": 2,
+    },
+  },
+  {
+    "op": "add",
+    "path": [
+      3,
+    ],
+    "value": {
+      "id": 3,
+    },
+  },
+]
+`;
+
 exports[`#466 mapChangeBug 1`] = `
 [
   {
@@ -2593,6 +2673,56 @@ exports[`minimum amount of changes 2`] = `
 ]
 `;
 
+exports[`modify deep object 1`] = `
+{
+  "map": Map {
+    "set1" => Set {
+      {
+        "a": 2,
+      },
+      {
+        "b": 2,
+      },
+    },
+    "set2" => Set {
+      {
+        "c": 3,
+      },
+    },
+  },
+}
+`;
+
+exports[`modify deep object 2`] = `
+[
+  {
+    "op": "replace",
+    "path": [
+      "map",
+      "set1",
+      0,
+      "a",
+    ],
+    "value": 2,
+  },
+]
+`;
+
+exports[`modify deep object 3`] = `
+[
+  {
+    "op": "replace",
+    "path": [
+      "map",
+      "set1",
+      0,
+      "a",
+    ],
+    "value": 1,
+  },
+]
+`;
+
 exports[`nested change in array 1`] = `
 [
   {

test/apply.test.ts

@@ -8,7 +8,7 @@
 /* eslint-disable prefer-destructuring */
 /* eslint-disable no-param-reassign */
 /* eslint-disable @typescript-eslint/ban-ts-comment */
-import { create, apply, Patches } from '../src';
+import { create, apply, Patches, original } from '../src';
 import { deepClone } from '../src/utils';
 
 test('classic case', () => {
@@ -1347,3 +1347,67 @@ test('merge multiple patches', () => {
   ]);
   expect(errorPrevState).not.toEqual(arr);
 });
+
+test('#64 - Invalid inversePatches when array length changes', () => {
+  checkPatches(
+    [0, 1, 2, 3].map((id) => ({ id })),
+    (draft) => {
+      draft[3].id *= 10;
+      draft.splice(0, 1);
+    }
+  );
+});
+
+test('modify deep object', () => {
+  const a = { a: 1 };
+  const b = { b: 2 };
+  const c = { c: 3 };
+  const set1 = new Set([a, b]);
+  const set2 = new Set([c]);
+  // @ts-ignore
+  const map = new Map([
+    ['set1', set1],
+    ['set2', set2],
+  ]);
+  const base = { map };
+
+  function first(set: any) {
+    return Array.from(set.values())[0];
+  }
+
+  function second(set: any) {
+    return Array.from(set.values())[1];
+  }
+
+  const fn = (draft: any) => {
+    const v = first(draft.map.get('set1'));
+    expect(original(v)).toBe(a);
+    expect(v).toEqual(a);
+    expect(v).not.toBe(a);
+    // @ts-ignore
+    v.a++;
+  };
+
+  const [state, patches, inversePatches] = create(base, fn, {
+    enablePatches: true,
+  });
+
+  const prevState = apply(state, inversePatches);
+  expect(prevState).toEqual(base);
+  const nextState = apply(base, patches);
+  expect(nextState).toEqual(state);
+
+  expect(state).toMatchSnapshot();
+  expect(patches).toMatchSnapshot();
+  expect(inversePatches).toMatchSnapshot();
+
+  expect(a.a).toBe(1);
+  expect(base.map.get('set1')).toBe(set1);
+  expect(first(base.map.get('set1'))).toBe(a);
+  expect(state).not.toBe(base);
+  expect(state.map).not.toBe(base.map);
+  expect(state.map.get('set1')).not.toBe(base.map.get('set1'));
+  expect(second(base.map.get('set1'))).toBe(b);
+  expect(base.map.get('set2')).toBe(set2);
+  expect(first(state.map.get('set1'))).toEqual({ a: 2 });
+});