From b28d0b3cb16b022bc6689c2b958daf15ba9871d8 Mon Sep 17 00:00:00 2001
From: jdip <jdipaolo@gmail.com>
Date: Thu, 8 Sep 2022 19:50:29 -0400
Subject: [PATCH 1/2] Proposed fix for #819 to preserve Set insertion order

---
 __tests__/map-set.js | 34 ++++++++++++++++++++++++++++++++++
 src/core/finalize.ts | 20 +++++++++++++++-----
 src/utils/common.ts  |  1 -
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/__tests__/map-set.js b/__tests__/map-set.js
index 8bf726f9..e054c506 100644
--- a/__tests__/map-set.js
+++ b/__tests__/map-set.js
@@ -296,4 +296,38 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) {
 			expect(mapType1).toBe(mapType2)
 		})
 	})
+	describe("set issues " + name, () => {
+		test("#819.A - maintains order when adding", () => {
+			const objs = [
+				"a",
+				{
+					id: "b"
+				}
+			]
+
+			const set = new Set([objs[0]])
+			const newSet = produce(set, draft => {
+				draft.add(objs[1])
+			})
+
+			// passes
+			expect(Array.from(newSet)).toEqual([objs[0], objs[1]])
+		})
+
+		test("#819.B - maintains order when adding", () => {
+			const objs = [
+				{
+					id: "a"
+				},
+				"b"
+			]
+
+			const set = new Set([objs[0]])
+			const newSet = produce(set, draft => {
+				draft.add(objs[1])
+			})
+
+			expect(Array.from(newSet)).toEqual([objs[0], objs[1]])
+		})
+	})
 }
diff --git a/src/core/finalize.ts b/src/core/finalize.ts
index ad95b124..05c0a9ab 100644
--- a/src/core/finalize.ts
+++ b/src/core/finalize.ts
@@ -87,12 +87,19 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
 				: state.copy_
 		// Finalize all children of the copy
 		// For sets we clone before iterating, otherwise we can get in endless loop due to modifying during iteration, see #628
+		// To preserve insertion order in all cases we then clear the set
+		// And we let finalizeProperty know it needs to re-add non-draft children back to the target
 		// Although the original test case doesn't seem valid anyway, so if this in the way we can turn the next line
 		// back to each(result, ....)
-		each(
-			state.type_ === ProxyType.Set ? new Set(result) : result,
-			(key, childValue) =>
-				finalizeProperty(rootScope, state, result, key, childValue, path)
+		let resultEach = result
+		let isSet = false
+		if (state.type_ === ProxyType.Set) {
+			resultEach = new Set(result)
+			result.clear()
+			isSet = true
+		}
+		each(resultEach, (key, childValue) =>
+			finalizeProperty(rootScope, state, result, key, childValue, path, isSet)
 		)
 		// everything inside is frozen, we can freeze here
 		maybeFreeze(rootScope, result, false)
@@ -115,7 +122,8 @@ function finalizeProperty(
 	targetObject: any,
 	prop: string | number,
 	childValue: any,
-	rootPath?: PatchPath
+	rootPath?: PatchPath,
+	targetIsSet?: boolean
 ) {
 	if (__DEV__ && childValue === targetObject) die(5)
 	if (isDraft(childValue)) {
@@ -134,6 +142,8 @@ function finalizeProperty(
 		if (isDraft(res)) {
 			rootScope.canAutoFreeze_ = false
 		} else return
+	} else if (targetIsSet) {
+		targetObject.add(childValue)
 	}
 	// Search new objects for unfinalized drafts. Frozen objects should never contain drafts.
 	if (isDraftable(childValue) && !isFrozen(childValue)) {
diff --git a/src/utils/common.ts b/src/utils/common.ts
index dae5064e..6be8c72e 100644
--- a/src/utils/common.ts
+++ b/src/utils/common.ts
@@ -132,7 +132,6 @@ export function set(thing: any, propOrOldValue: PropertyKey, value: any) {
 	const t = getArchtype(thing)
 	if (t === Archtype.Map) thing.set(propOrOldValue, value)
 	else if (t === Archtype.Set) {
-		thing.delete(propOrOldValue)
 		thing.add(value)
 	} else thing[propOrOldValue] = value
 }

From ab864a675d1f4045411df0ba2bb8f21444a37d0b Mon Sep 17 00:00:00 2001
From: jdip <jdipaolo@gmail.com>
Date: Thu, 8 Sep 2022 20:28:26 -0400
Subject: [PATCH 2/2] Proposed fix for #819 (B)

We are already cloning and iterating over the Set. If we go a step further and clear it before iteration, and re-add all children during iteration, we can preserve insertion order.
---
 src/core/finalize.ts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/core/finalize.ts b/src/core/finalize.ts
index 05c0a9ab..5e621b45 100644
--- a/src/core/finalize.ts
+++ b/src/core/finalize.ts
@@ -89,8 +89,6 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
 		// For sets we clone before iterating, otherwise we can get in endless loop due to modifying during iteration, see #628
 		// To preserve insertion order in all cases we then clear the set
 		// And we let finalizeProperty know it needs to re-add non-draft children back to the target
-		// Although the original test case doesn't seem valid anyway, so if this in the way we can turn the next line
-		// back to each(result, ....)
 		let resultEach = result
 		let isSet = false
 		if (state.type_ === ProxyType.Set) {