Repository: immerjs/immer (js)

Issue:
When ImmerJS processes objects containing Symbol keys, the Symbols become Proxies.
## 🙋‍♂ Question

Hello everyone,

I recently started using ImmerJS and encountered a challenging issue. In my data structure, there is a property with a key as Symbol, which is only used for querying purposes and will not be modified when using ImmerJS. Specifically, I try to query and modify certain data through `v[S1].id`.

However, when outputting the result in the console, I noticed that although the corresponding value was successfully modified, strangely my Symbol key `S1` was converted into a Proxy object. This has left me feeling very confused and frustrated.

Could you please let me know if this is a known bug or behavior caused by some boundary conditions? If anyone can provide an explanation or point to relevant documentation/discussion links, I would greatly appreciate it. Thank you all!

![image](https://github.com/immerjs/immer/assets/32004925/7546a9fc-a394-425c-a8c7-52cb3a730e27)


## Link to repro

~~Please provide a [CodeSandbox demo](https://codesandbox.io/s/immer-sandbox-6wijw) to clarify your question if possible. Typically, questions without minimal codesandbox demo won't be answered.~~

## Environment

We only accept questions against the latest Immer version.

- **Immer version:**
- [x] Occurs with `setUseProxies(true)`
- [ ] Occurs with `setUseProxies(false)` (ES5 only)

version: `"immer": "^10.0.2"`


Pull request description:
Fix: Fix handling of Symbol and non-enumerable properties in finalization / freeze. Fixes #1096, #1087, #1091

Patch:
diff --git a/src/core/finalize.ts b/src/core/finalize.ts
index 7c44631..6ee69ce 100644
--- a/src/core/finalize.ts
+++ b/src/core/finalize.ts
@@ -58,11 +58,8 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
 	const state: ImmerState = value[DRAFT_STATE]
 	// A plain object, might need freezing, might contain drafts
 	if (!state) {
-		each(
-			value,
-			(key, childValue) =>
-				finalizeProperty(rootScope, state, value, key, childValue, path),
-			true // See #590, don't recurse into non-enumerable of non drafted objects
+		each(value, (key, childValue) =>
+			finalizeProperty(rootScope, state, value, key, childValue, path)
 		)
 		return value
 	}
@@ -148,8 +145,14 @@ function finalizeProperty(
 			return
 		}
 		finalize(rootScope, childValue)
-		// immer deep freezes plain objects, so if there is no parent state, we freeze as well
-		if (!parentState || !parentState.scope_.parent_)
+		// Immer deep freezes plain objects, so if there is no parent state, we freeze as well
+		// Per #590, we never freeze symbolic properties. Just to make sure don't accidentally interfere
+		// with other frameworks.
+		if (
+			(!parentState || !parentState.scope_.parent_) &&
+			typeof prop !== "symbol" &&
+			Object.prototype.propertyIsEnumerable.call(targetObject, prop)
+		)
 			maybeFreeze(rootScope, childValue)
 	}
 }
diff --git a/src/utils/common.ts b/src/utils/common.ts
index 1ebb5b5..fb4a768 100644
--- a/src/utils/common.ts
+++ b/src/utils/common.ts
@@ -60,15 +60,19 @@ export function original(value: Drafted<any>): any {
 	return value[DRAFT_STATE].base_
 }
 
+/**
+ * Each iterates a map, set or array.
+ * Or, if any other kind of of object all it's own properties.
+ * Regardless whether they are enumerable or symbols
+ */
 export function each<T extends Objectish>(
 	obj: T,
-	iter: (key: string | number, value: any, source: T) => void,
-	enumerableOnly?: boolean
+	iter: (key: string | number, value: any, source: T) => void
 ): void
 export function each(obj: any, iter: any) {
 	if (getArchtype(obj) === ArchType.Object) {
-		Object.entries(obj).forEach(([key, value]) => {
-			iter(key, value, obj)
+		Reflect.ownKeys(obj).forEach(key => {
+			iter(key, obj[key], obj)
 		})
 	} else {
 		obj.forEach((entry: any, index: any) => iter(index, entry, obj))
@@ -191,7 +195,10 @@ export function freeze<T>(obj: any, deep: boolean = false): T {
 		obj.set = obj.add = obj.clear = obj.delete = dontMutateFrozenCollections as any
 	}
 	Object.freeze(obj)
-	if (deep) each(obj, (_key, value) => freeze(value, true), true)
+	if (deep)
+		// See #590, don't recurse into non-enumerable / Symbol properties when freezing
+		// So use Object.entries (only string-like, enumerables) instead of each()
+		Object.entries(obj).forEach(([key, value]) => freeze(value, true))
 	return obj
 }
 


Test patch:
diff --git a/__tests__/base.js b/__tests__/base.js
index 3e900bb..1c0059a 100644
--- a/__tests__/base.js
+++ b/__tests__/base.js
@@ -2362,6 +2362,22 @@ function testObjectTypes(produce) {
 			[customSymbol]: 3
 		})
 	})
+
+	describe("#1096 / #1087 / proxies", () => {
+		const sym = Symbol()
+
+		let state = {
+			id: 1,
+			[sym]: {x: 2}
+		}
+
+		state = produce(state, draft => {
+			draft.id = 2
+			draft[sym]
+		})
+
+		expect(state[sym].x).toBe(2)
+	})
 }
 
 function testLiteralTypes(produce) {
diff --git a/__tests__/frozen.js b/__tests__/frozen.js
index b0080f5..f130f3c 100644
--- a/__tests__/frozen.js
+++ b/__tests__/frozen.js
@@ -219,6 +219,7 @@ function runTests(name) {
 				state2.ref.state.x++
 			}).not.toThrow()
 			expect(state2.ref.state.x).toBe(2)
+			expect(component.state.x).toBe(2)
 		})
 
 		it("never freezes symbolic fields #590", () => {