Merge pull request #415 from jsdevtom/feat/setting-length-array-makes…

…-delete

feat: Prefer using `remove` patches over changing the `length` property when shortening arrays

__tests__/patch.js

@@ -275,9 +275,10 @@ describe("arrays - modify and shrink", () => {
 			d.x[0] = 4
 			d.x.length = 2
 		},
+		[{op: "replace", path: ["x", 0], value: 4}, {op: "remove", path: ["x", 2]}],
 		[
-			{op: "replace", path: ["x", 0], value: 4},
-			{op: "replace", path: ["x", "length"], value: 2}
+			{op: "replace", path: ["x", 0], value: 1},
+			{op: "add", path: ["x", 2], value: 3}
 		]
 	)
 })
@@ -316,7 +317,7 @@ describe("arrays - truncate", () => {
 		d => {
 			d.x.length -= 2
 		},
-		[{op: "replace", path: ["x", "length"], value: 1}],
+		[{op: "remove", path: ["x", 2]}, {op: "remove", path: ["x", 1]}],
 		[
 			{op: "add", path: ["x", 1], value: 2},
 			{op: "add", path: ["x", 2], value: 3}
@@ -331,7 +332,7 @@ describe("arrays - pop twice", () => {
 			d.x.pop()
 			d.x.pop()
 		},
-		[{op: "replace", path: ["x", "length"], value: 1}]
+		[{op: "remove", path: ["x", 2]}, {op: "remove", path: ["x", 1]}]
 	)
 })
 
@@ -345,7 +346,7 @@ describe("arrays - push multiple", () => {
 			{op: "add", path: ["x", 3], value: 4},
 			{op: "add", path: ["x", 4], value: 5}
 		],
-		[{op: "replace", path: ["x", "length"], value: 3}]
+		[{op: "remove", path: ["x", 4]}, {op: "remove", path: ["x", 3]}]
 	)
 })
 
@@ -387,6 +388,28 @@ describe("arrays - splice (shrink)", () => {
 	)
 })
 
+describe("arrays - splice should should result in remove op.", () => {
+	runPatchTest(
+		[1, 2],
+		d => {
+			d.splice(1, 1)
+		},
+		[{op: "remove", path: [1]}],
+		[{op: "add", path: [1], value: 2}]
+	)
+})
+
+describe("arrays - NESTED splice should should result in remove op.", () => {
+	runPatchTest(
+		{a: {b: {c: [1, 2]}}},
+		d => {
+			d.a.b.c.splice(1, 1)
+		},
+		[{op: "remove", path: ["a", "b", "c", 1]}],
+		[{op: "add", path: ["a", "b", "c", 1], value: 2}]
+	)
+})
+
 describe("simple replacement", () => {
 	runPatchTest({x: 3}, _d => 4, [{op: "replace", path: [], value: 4}])
 })

src/patches.js

@@ -47,7 +47,6 @@ function generateArrayPatches(state, basePath, patches, inversePatches) {
 		}
 	}
 
-	const useRemove = end != base.length
 	const replaceCount = patches.length
 
 	// Process added indices.
@@ -58,20 +57,9 @@ function generateArrayPatches(state, basePath, patches, inversePatches) {
 			path,
 			value: copy[i]
 		}
-		if (useRemove) {
-			inversePatches.push({
-				op: "remove",
-				path
-			})
-		}
-	}
-
-	// One "replace" patch reverses all non-splicing "add" patches.
-	if (!useRemove) {
 		inversePatches.push({
-			op: "replace",
-			path: basePath.concat(["length"]),
-			value: base.length
+			op: "remove",
+			path
 		})
 	}
 }