Repository: immerjs/immer (js)

Issue:
Clarify that the first result from the producer is not immutable unless a change is made
## 🚀 Feature Proposal

As expected, when the producer is first applied to an object (or array) and a change is made, an immutable object results. However, if no such change is made, the result is mutable.

## Motivation

Docs are unclear on this matter. I suggest that it would be better to make the result immutable regardless. But if that doesn't work, it would be good for the docs to be clear on this matter.

With the current implementation, I'm forced to make an arbitrary change to the object to make sure that it is immutable immediately.

## Example

```
    it("Simple object becomes immutable as the produce() call made a change", () => {
        const c = {id: 3};
        const immutable = produce(c, draft => {
            draft.id = 4;
        });
        expect(c.id).eql(3);
        expect(immutable.id).eql(4);
        expect(() => (immutable as any).id = 5).to.throw("");
    });

    it("Simple object is not made immutable as the produce() call made no change", () => {
        const c = {id: 3};
        const immutable = produce(c, draft => {
        });
        (immutable as any).id = 4;
        expect(immutable.id).eql(4);
    });
```


Pull request description:
BREAKING CHANGE: In development mode, any new value stored in a tree will be deeply frozen
* [x] Fix #412 #363  #260 #230 
* [x] probably also fix #313, #229 as we will have now side effects during the execution of recipes anyway
* [x] document the side effect with autofreeze enabled!

Patch:
diff --git a/readme.md b/readme.md
index 92a4499..3e2b32a 100644
--- a/readme.md
+++ b/readme.md
@@ -548,6 +548,8 @@ isDraft(nextState) // => false
 
 Immer automatically freezes any state trees that are modified using `produce`. This protects against accidental modifications of the state tree outside of a producer. This comes with a performance impact, so it is recommended to disable this option in production. It is by default enabled. By default, it is turned on during local development and turned off in production. Use `setAutoFreeze(true / false)` to explicitly turn this feature on or off.
 
+_⚠️ If auto freezing is enabled, recipes are not entirely side-effect free: Any plain object or array that ends up in the produced result, will be frozen, even when these objects were not frozen before the start of the producer! ⚠️_
+
 ## Immer on older JavaScript environments?
 
 By default `produce` tries to use proxies for optimal performance. However, on older JavaScript engines `Proxy` is not available. For example, when running Microsoft Internet Explorer or React Native (< v0.59) on Android. In such cases, Immer will fallback to an ES5 compatible implementation which works identical, but is a bit slower.
diff --git a/src/common.js b/src/common.js
index 6abb39e..65f5e15 100644
--- a/src/common.js
+++ b/src/common.js
@@ -120,3 +120,10 @@ export function clone(obj) {
 	for (const key in obj) cloned[key] = clone(obj[key])
 	return cloned
 }
+
+export function deepFreeze(obj) {
+	if (!isDraftable(obj) || isDraft(obj) || Object.isFrozen(obj)) return
+	Object.freeze(obj)
+	if (Array.isArray(obj)) obj.forEach(deepFreeze)
+	else for (const key in obj) deepFreeze(obj[key])
+}
diff --git a/src/immer.js b/src/immer.js
index 2159bbc..2727f7c 100644
--- a/src/immer.js
+++ b/src/immer.js
@@ -11,7 +11,8 @@ import {
 	isEnumerable,
 	shallowCopy,
 	DRAFT_STATE,
-	NOTHING
+	NOTHING,
+	deepFreeze
 } from "./common"
 import {ImmerScope} from "./scope"
 
@@ -87,8 +88,10 @@ export class Immer {
 			return this.processResult(result, scope)
 		} else {
 			result = recipe(base)
-			if (result === undefined) return base
-			return result !== NOTHING ? result : undefined
+			if (result === NOTHING) return undefined
+			if (result === undefined) result = base
+			this.maybeFreeze(result, true)
+			return result
 		}
 	}
 	produceWithPatches(arg1, arg2, arg3) {
@@ -170,6 +173,7 @@ export class Immer {
 			if (isDraftable(result)) {
 				// Finalize the result in case it contains (or is) a subset of the draft.
 				result = this.finalize(result, null, scope)
+				this.maybeFreeze(result)
 			}
 			if (scope.patches) {
 				scope.patches.push({
@@ -209,6 +213,7 @@ export class Immer {
 			return draft
 		}
 		if (!state.modified) {
+			this.maybeFreeze(state.base, true)
 			return state.base
 		}
 		if (!state.finalized) {
@@ -299,6 +304,7 @@ export class Immer {
 			// Search new objects for unfinalized drafts. Frozen objects should never contain drafts.
 			else if (isDraftable(value) && !Object.isFrozen(value)) {
 				each(value, finalizeProperty)
+				this.maybeFreeze(value)
 			}
 
 			if (isDraftProp && this.onAssign) {
@@ -309,4 +315,10 @@ export class Immer {
 		each(root, finalizeProperty)
 		return root
 	}
+	maybeFreeze(value, deep = false) {
+		if (this.autoFreeze && !isDraft(value)) {
+			if (deep) deepFreeze(value)
+			else Object.freeze(value)
+		}
+	}
 }


Test patch:
diff --git a/__tests__/frozen.js b/__tests__/frozen.js
index 2c69043..e1c3df8 100644
--- a/__tests__/frozen.js
+++ b/__tests__/frozen.js
@@ -50,72 +50,75 @@ function runTests(name, useProxies) {
 				expect(isFrozen(next)).toBeTruthy()
 				expect(isFrozen(next.a)).toBeTruthy()
 			})
-		})
 
-		describe("the result is never auto-frozen when", () => {
-			it("the producer is a no-op", () => {
-				const base = {}
-				const next = produce(base, () => {})
-				expect(next).toBe(base)
-				expect(isFrozen(next)).toBeFalsy()
+			it("a new object replaces the entire draft", () => {
+				const obj = {a: {b: {}}}
+				const next = produce({}, () => obj)
+				expect(next).toBe(obj)
+				expect(isFrozen(next)).toBeTruthy()
+				expect(isFrozen(next.a)).toBeTruthy()
+				expect(isFrozen(next.a.b)).toBeTruthy()
 			})
 
-			it("the root draft is returned", () => {
+			it("a new object is added to the root draft", () => {
 				const base = {}
-				const next = produce(base, draft => draft)
-				expect(next).toBe(base)
-				expect(isFrozen(next)).toBeFalsy()
+				const next = produce(base, draft => {
+					draft.a = {b: []}
+				})
+				expect(next).not.toBe(base)
+				expect(isFrozen(next)).toBeTruthy()
+				expect(isFrozen(next.a)).toBeTruthy()
+				expect(isFrozen(next.b)).toBeTruthy()
+			})
+
+			it("a new object is added to a nested draft", () => {
+				const base = {a: {}}
+				const next = produce(base, draft => {
+					draft.a.b = {c: {}}
+				})
+				expect(next).not.toBe(base)
+				expect(isFrozen(next)).toBeTruthy()
+				expect(isFrozen(next.a)).toBeTruthy()
+				expect(isFrozen(next.a.b)).toBeTruthy()
+				expect(isFrozen(next.a.b.c)).toBeTruthy()
 			})
 
 			it("a nested draft is returned", () => {
 				const base = {a: {}}
 				const next = produce(base, draft => draft.a)
 				expect(next).toBe(base.a)
-				expect(isFrozen(next)).toBeFalsy()
+				expect(isFrozen(next)).toBeTruthy()
 			})
 
 			it("the base state is returned", () => {
 				const base = {}
 				const next = produce(base, () => base)
 				expect(next).toBe(base)
-				expect(isFrozen(next)).toBeFalsy()
-			})
-
-			it("a new object replaces a primitive base", () => {
-				const obj = {}
-				const next = produce(null, () => obj)
-				expect(next).toBe(obj)
-				expect(isFrozen(next)).toBeFalsy()
+				expect(isFrozen(next)).toBeTruthy()
 			})
 
-			it("a new object replaces the entire draft", () => {
-				const obj = {a: {b: {}}}
-				const next = produce({}, () => obj)
-				expect(next).toBe(obj)
-				expect(isFrozen(next)).toBeFalsy()
-				expect(isFrozen(next.a)).toBeFalsy()
-				expect(isFrozen(next.a.b)).toBeFalsy()
+			it("the producer is a no-op", () => {
+				const base = {a: {}}
+				const next = produce(base, () => {})
+				expect(next).toBe(base)
+				expect(isFrozen(next)).toBeTruthy()
+				expect(isFrozen(next.a)).toBeTruthy()
 			})
 
-			it("a new object is added to the root draft", () => {
-				const base = {}
-				const next = produce(base, draft => {
-					draft.a = {}
-				})
-				expect(next).not.toBe(base)
+			it("the root draft is returned", () => {
+				const base = {a: {}}
+				const next = produce(base, draft => draft)
+				expect(next).toBe(base)
 				expect(isFrozen(next)).toBeTruthy()
-				expect(isFrozen(next.a)).toBeFalsy()
+				expect(isFrozen(next.a)).toBeTruthy()
 			})
 
-			it("a new object is added to a nested draft", () => {
-				const base = {a: {}}
-				const next = produce(base, draft => {
-					draft.a.b = {}
-				})
-				expect(next).not.toBe(base)
+			it("a new object replaces a primitive base", () => {
+				const obj = {a: {}}
+				const next = produce(null, () => obj)
+				expect(next).toBe(obj)
 				expect(isFrozen(next)).toBeTruthy()
 				expect(isFrozen(next.a)).toBeTruthy()
-				expect(isFrozen(next.a.b)).toBeFalsy()
 			})
 		})