From d91a6597e92570086b329ba5b197c18d211077db Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 11 Jan 2022 12:42:19 +0000
Subject: [PATCH] fix: incorrect patches for `delete` on arrays. Fixes #879

---
 __tests__/patch.js | 31 ++++++++++++++++++++++++++++++-
 src/core/proxy.ts  |  3 ++-
 src/plugins/es5.ts |  8 ++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/__tests__/patch.js b/__tests__/patch.js
index 1aa9a533..034f9468 100644
--- a/__tests__/patch.js
+++ b/__tests__/patch.js
@@ -15,7 +15,7 @@ jest.setTimeout(1000)
 
 const isProd = process.env.NODE_ENV === "production"
 
-function runPatchTest(base, producer, patches, inversePathes) {
+function runPatchTest(base, producer, patches, inversePathes, expectedResult) {
 	let resultProxies, resultEs5
 
 	function runPatchTestHelper() {
@@ -26,6 +26,11 @@ function runPatchTest(base, producer, patches, inversePathes) {
 			recordedInversePatches = i
 		})
 
+		if (expectedResult !== undefined)
+			test("produced the correct result", () => {
+				expect(res).toEqual(expectedResult)
+			})
+
 		test("produces the correct patches", () => {
 			expect(recordedPatches).toEqual(patches)
 			if (inversePathes) expect(recordedInversePatches).toEqual(inversePathes)
@@ -1389,3 +1394,27 @@ test("#888 patch to a primitive produces the primitive", () => {
 		expect(patches).toEqual([{op: "replace", path: [], value: true}])
 	}
 })
+
+describe("#879 delete item from array", () => {
+	runPatchTest(
+		[1, 2, 3],
+		draft => {
+			delete draft[1]
+		},
+		[{op: "replace", path: [1], value: undefined}],
+		[{op: "replace", path: [1], value: 2}],
+		[1, undefined, 3]
+	)
+})
+
+describe("#879 delete item from array - 2", () => {
+	runPatchTest(
+		[1, 2, 3],
+		draft => {
+			delete draft[2]
+		},
+		[{op: "replace", path: [2], value: undefined}],
+		[{op: "replace", path: [2], value: 3}],
+		[1, 2, undefined]
+	)
+})
diff --git a/src/core/proxy.ts b/src/core/proxy.ts
index 7aaaf4cd..7e6503a9 100644
--- a/src/core/proxy.ts
+++ b/src/core/proxy.ts
@@ -223,7 +223,8 @@ each(objectTraps, (key, fn) => {
 })
 arrayTraps.deleteProperty = function(state, prop) {
 	if (__DEV__ && isNaN(parseInt(prop as any))) die(13)
-	return objectTraps.deleteProperty!.call(this, state[0], prop)
+	// @ts-ignore
+	return arrayTraps.set!.call(this, state, prop, undefined)
 }
 arrayTraps.set = function(state, prop, value) {
 	if (__DEV__ && prop !== "length" && isNaN(parseInt(prop as any))) die(14)
diff --git a/src/plugins/es5.ts b/src/plugins/es5.ts
index 5cfa77f4..0e16b403 100644
--- a/src/plugins/es5.ts
+++ b/src/plugins/es5.ts
@@ -195,6 +195,9 @@ export function enableES5() {
 
 			for (let i = 0; i < min; i++) {
 				// Only untouched indices trigger recursion.
+				if (!draft_.hasOwnProperty(i)) {
+					assigned_[i] = true
+				}
 				if (assigned_[i] === undefined) markChangesRecursively(draft_[i])
 			}
 		}
@@ -241,12 +244,17 @@ export function enableES5() {
 		// So if there is no own descriptor on the last position, we know that items were removed and added
 		// N.B.: splice, unshift, etc only shift values around, but not prop descriptors, so we only have to check
 		// the last one
+		// last descriptor can be not a trap, if the array was extended
 		const descriptor = Object.getOwnPropertyDescriptor(
 			draft_,
 			draft_.length - 1
 		)
 		// descriptor can be null, but only for newly created sparse arrays, eg. new Array(10)
 		if (descriptor && !descriptor.get) return true
+		// if we miss a property, it has been deleted, so array probobaly changed
+		for (let i = 0; i < draft_.length; i++) {
+			if (!draft_.hasOwnProperty(i)) return true
+		}
 		// For all other cases, we don't have to compare, as they would have been picked up by the index setters
 		return false
 	}