feat: Introduce strictShallowCopy, See #941, hrsh7th-use-strict-shall…

…ow-copy

__tests__/base.js

@@ -9,7 +9,6 @@ import {
 } from "../src/immer"
 import {each, shallowCopy, DRAFT_STATE} from "../src/internal"
 import deepFreeze from "deep-freeze"
-import cloneDeep from "lodash.clonedeep"
 import * as lodash from "lodash"
 
 jest.setTimeout(1000)
@@ -22,17 +21,24 @@ test("immer should have no dependencies", () => {
 	expect(require("../package.json").dependencies).toBeUndefined()
 })
 
-runBaseTest("proxy (no freeze)", false)
-runBaseTest("proxy (autofreeze)", true)
-runBaseTest("proxy (patch listener)", false, true)
-runBaseTest("proxy (autofreeze)(patch listener)", true, true)
+for (const autoFreeze of [true, false]) {
+	for (const useStrictShallowCopy of [true, false]) {
+		for (const useListener of [true, false]) {
+			const name = `${autoFreeze ? "auto-freeze=true" : "auto-freeze=false"}:${
+				useStrictShallowCopy ? "shallow-copy=true" : "shallow-copy=false"
+			}:${useListener ? "use-listener=true" : "use-listener=false"}`
+			runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener)
+		}
+	}
+}
 
 class Foo {}
 
-function runBaseTest(name, autoFreeze, useListener) {
+function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) {
 	const listener = useListener ? function() {} : undefined
 	const {produce, produceWithPatches} = createPatchedImmer({
-		autoFreeze
+		autoFreeze,
+		useStrictShallowCopy
 	})
 
 	// When `useListener` is true, append a function to the arguments of every
@@ -890,10 +896,16 @@ function runBaseTest(name, autoFreeze, useListener) {
 				expect(s[test]).toBeTruthy()
 				s.foo = true
 			})
-			expect(nextState).toEqual({
-				[test]: true,
-				foo: true
-			})
+			if (useStrictShallowCopy) {
+				expect(nextState).toEqual({
+					[test]: true,
+					foo: true
+				})
+			} else {
+				expect(nextState).toEqual({
+					foo: true
+				})
+			}
 		})
 
 		if (!global.USES_BUILD)
@@ -909,16 +921,26 @@ function runBaseTest(name, autoFreeze, useListener) {
 					value: 1,
 					enumerable: false
 				})
+				// Non-enumerable primitive property that won't modified.
+				Object.defineProperty(baseState, "baz", {
+					value: 1,
+					enumerable: false
+				})
+
+				// When using Proxy, even non-enumerable keys will be copied if it's changed.
+				const canReferNonEnumerableProperty = useStrictShallowCopy
 				const nextState = produce(baseState, s => {
-					expect(s.foo).toBeTruthy()
-					expect(isEnumerable(s, "foo")).toBeFalsy()
-					s.bar++
-					expect(isEnumerable(s, "foo")).toBeFalsy()
-					s.foo.a++
-					expect(isEnumerable(s, "foo")).toBeFalsy()
-				})
-				expect(nextState.foo).toBeTruthy()
-				expect(isEnumerable(nextState, "foo")).toBeFalsy()
+					if (canReferNonEnumerableProperty) expect(s.foo).toBeTruthy()
+					if (useStrictShallowCopy) expect(isEnumerable(s, "foo")).toBeFalsy()
+					if (canReferNonEnumerableProperty) s.bar++
+					if (useStrictShallowCopy) expect(isEnumerable(s, "foo")).toBeFalsy()
+					if (canReferNonEnumerableProperty) s.foo.a++
+					if (useStrictShallowCopy) expect(isEnumerable(s, "foo")).toBeFalsy()
+				})
+				if (canReferNonEnumerableProperty) expect(nextState.foo).toBeTruthy()
+				if (useStrictShallowCopy)
+					expect(isEnumerable(nextState, "foo")).toBeFalsy()
+				if (useStrictShallowCopy) expect(nextState.baz).toBeTruthy()
 			})
 
 		it("can work with own computed props", () => {

__tests__/map-set.js

@@ -7,7 +7,7 @@ import {
 	immerable,
 	enablePatches
 } from "../src/immer"
-import {each, shallowCopy, isEnumerable, DRAFT_STATE} from "../src/common"
+import {each, shallowCopy, isEnumerable, DRAFT_STATE} from "../src/utils/common"
 
 enablePatches()
 

__tests__/not-strict-copy.ts

@@ -0,0 +1,37 @@
+import {produce, setUseStrictShallowCopy} from "../src/immer"
+
+describe("setUseStrictShallowCopy(true)", () => {
+	test("keep descriptors", () => {
+		setUseStrictShallowCopy(true)
+
+		const base: Record<string, unknown> = {}
+		Object.defineProperty(base, "foo", {
+			value: "foo",
+			writable: false,
+			configurable: false
+		})
+		const copy = produce(base, (draft: any) => {
+			draft.bar = "bar"
+		})
+		expect(Object.getOwnPropertyDescriptor(copy, "foo")).toStrictEqual(
+			Object.getOwnPropertyDescriptor(base, "foo")
+		)
+	})
+})
+
+describe("setUseStrictShallowCopy(false)", () => {
+	test("ignore descriptors", () => {
+		setUseStrictShallowCopy(false)
+
+		const base: Record<string, unknown> = {}
+		Object.defineProperty(base, "foo", {
+			value: "foo",
+			writable: false,
+			configurable: false
+		})
+		const copy = produce(base, (draft: any) => {
+			draft.bar = "bar"
+		})
+		expect(Object.getOwnPropertyDescriptor(copy, "foo")).toBeUndefined()
+	})
+})

package.json

@@ -34,7 +34,7 @@
   "sideEffects": false,
   "scripts": {
     "test": "jest && yarn test:build && yarn test:flow",
-    "test:perf": "cd __performance_tests__ && babel-node add-data.js && babel-node todo.js && babel-node incremental.js",
+    "test:perf": "cd __performance_tests__ && babel-node add-data.js && babel-node todo.js && babel-node incremental.js && babel-node large-obj.js",
     "test:flow": "yarn flow check __tests__/flow",
     "test:build": "yarn build && NODE_ENV='production' yarn jest --config jest.config.build.js",
     "watch": "jest --watch",

src/core/current.ts

@@ -28,10 +28,14 @@ function currentImpl(value: any): any {
 		if (!state.modified_) return state.base_
 		// Optimization: avoid generating new drafts during copying
 		state.finalized_ = true
-		copy = copyHelper(value, archType)
+		copy = copyHelper(
+			value,
+			archType,
+			state.scope_.immer_.useStrictShallowCopy_
+		)
 		state.finalized_ = false
 	} else {
-		copy = copyHelper(value, archType)
+		copy = copyHelper(value, archType, true)
 	}
 
 	each(copy, (key, childValue) => {
@@ -42,7 +46,7 @@ function currentImpl(value: any): any {
 	return archType === ArchType.Set ? new Set(copy) : copy
 }
 
-function copyHelper(value: any, archType: number): any {
+function copyHelper(value: any, archType: number, strict: boolean): any {
 	// creates a shallow copy, even if it is a map or set
 	switch (archType) {
 		case ArchType.Map:
@@ -51,5 +55,5 @@ function copyHelper(value: any, archType: number): any {
 			// Set will be cloned as array temporarily, so that we can replace individual items
 			return Array.from(value)
 	}
-	return shallowCopy(value)
+	return shallowCopy(value, strict)
 }

src/core/immerClass.ts

@@ -35,10 +35,13 @@ interface ProducersFns {
 
 export class Immer implements ProducersFns {
 	autoFreeze_: boolean = true
+	useStrictShallowCopy_: boolean = false
 
-	constructor(config?: {autoFreeze?: boolean}) {
+	constructor(config?: {autoFreeze?: boolean; useStrictShallowCopy?: boolean}) {
 		if (typeof config?.autoFreeze === "boolean")
 			this.setAutoFreeze(config!.autoFreeze)
+		if (typeof config?.useStrictShallowCopy === "boolean")
+			this.setUseStrictShallowCopy(config!.useStrictShallowCopy)
 	}
 
 	/**
@@ -158,6 +161,15 @@ export class Immer implements ProducersFns {
 		this.autoFreeze_ = value
 	}
 
+	/**
+	 * Pass true to enable strict shallow copy.
+	 *
+	 * By default, immer does not copy the object descriptors such as getter, setter and non-enumrable properties.
+	 */
+	setUseStrictShallowCopy(value: boolean) {
+		this.useStrictShallowCopy_ = value
+	}
+
 	applyPatches<T extends Objectish>(base: T, patches: Patch[]): T {
 		// If a patch replaces the entire state, take that replacement as base
 		// before applying patches

src/core/proxy.ts

@@ -17,6 +17,7 @@ import {
 	createProxy,
 	ArchType
 } from "../internal"
+import {ImmerScope} from "./scope"
 
 interface ProxyBaseState extends ImmerBaseState {
 	assigned_: {
@@ -269,8 +270,15 @@ export function markChanged(state: ImmerState) {
 	}
 }
 
-export function prepareCopy(state: {base_: any; copy_: any}) {
+export function prepareCopy(state: {
+	base_: any
+	copy_: any
+	scope_: ImmerScope
+}) {
 	if (!state.copy_) {
-		state.copy_ = shallowCopy(state.base_)
+		state.copy_ = shallowCopy(
+			state.base_,
+			state.scope_.immer_.useStrictShallowCopy_
+		)
 	}
 }

src/immer.ts

@@ -58,6 +58,13 @@ export const produceWithPatches: IProduceWithPatches = immer.produceWithPatches.
  */
 export const setAutoFreeze = immer.setAutoFreeze.bind(immer)
 
+/**
+ * Pass true to enable strict shallow copy.
+ *
+ * By default, immer does not copy the object descriptors such as getter, setter and non-enumrable properties.
+ */
+export const setUseStrictShallowCopy = immer.setUseStrictShallowCopy.bind(immer)
+
 /**
  * Apply an array of Immer patches to the first argument.
  *

src/types/index.js.flow

@@ -83,6 +83,13 @@ declare export function setAutoFreeze(autoFreeze: boolean): void
  */
 declare export function setUseProxies(useProxies: boolean): void
 
+/**
+ * Pass false to disable strict shallow copy.
+ *
+ * By default, immer does not copy the object descriptors such as getter, setter and non-enumrable properties.
+ */
+declare export function setUseStrictShallowCopy(useStrictShallowCopy: boolean): void
+
 declare export function applyPatches<S>(state: S, patches: Patch[]): S
 
 declare export function original<S>(value: S): S

src/utils/common.ts

@@ -139,8 +139,19 @@ export function latest(state: ImmerState): any {
 }
 
 /*#__PURE__*/
-export function shallowCopy(base: any) {
+export function shallowCopy(base: any, strict: boolean) {
 	if (Array.isArray(base)) return Array.prototype.slice.call(base)
+
+	if (!strict && isPlainObject(base)) {
+		const keys = Object.keys(base)
+		const obj: any = Object.create(Object.getPrototypeOf(base))
+		for (let i = 0; i < keys.length; i++) {
+			const key = keys[i]
+			obj[key] = base[key]
+		}
+		return obj
+	}
+
 	const descriptors = getOwnPropertyDescriptors(base)
 	delete descriptors[DRAFT_STATE as any]
 	let keys = ownKeys(descriptors)

website/docs/api.md

@@ -30,6 +30,7 @@ title: API overview
 | `produceWithPatches` | Works the same as `produce`, but instead of just returning the produced object, it returns a tuple, consisting of `[result, patches, inversePatches]`. | [Patches](./patches.mdx) |
 | `setAutoFreeze` | Enables / disables automatic freezing of the trees produces. By default enabled. | [Freezing](./freezing.mdx) |
 | `setUseProxies` | Can be used to disable or force the use of `Proxy` objects. Useful when filing bug reports. |  |
+| `setUseStrictShallowCopy` | Can be used to enable strict shallow copy. If enable, immer copies non-enumerable properties as much as possible. | [Classes](./complex-objects.md) |
 
 ## Importing immer
 

website/docs/complex-objects.md

@@ -7,6 +7,8 @@ title: Classes
 <div data-ea-publisher="immerjs" data-ea-type="image" className="horizontal bordered"></div>
 </center>
 
+By default, Immer does not strictly handle Plain object's non-eumerable properties such as getters/setters for performance reason. If you want this behavior to be strict, you can opt-in with `useStrictShallowCopy(true)`.
+
 Plain objects (objects without a prototype), arrays, `Map`s and `Set`s are always drafted by Immer. Every other object must use the `immerable` symbol to mark itself as compatible with Immer. When one of these objects is mutated within a producer, its prototype is preserved between copies.
 
 ```js
@@ -59,12 +61,12 @@ console.log(clock2 instanceof Clock) // true
 The semantics on how classes are drafted are as follows:
 
 1. A draft of a class is a fresh object but with the same prototype as the original object.
-2. When creating a draft, Immer will copy all _own_ properties from the base to the draft.This includes non-enumerable and symbolic properties.
-3. _Own_ getters will be invoked during the copy process, just like `Object.assign` would.
-4. Inherited getters and methods will remain as is and be inherited by the draft.
-5. Immer will not invoke constructor functions.
-6. The final instance will be constructed with the same mechanism as the draft was created.
-7. Only getters that have a setter as well will be writable in the draft, as otherwise the value can't be copied back.
+1. When creating a draft, Immer will copy all _own_ properties from the base to the draft.This includes non-enumerable and symbolic properties (if `useStrictShallowCopy(true)` was called).
+1. _Own_ getters will be invoked during the copy process, just like `Object.assign` would.
+1. Inherited getters and methods will remain as is and be inherited by the draft.
+1. Immer will not invoke constructor functions.
+1. The final instance will be constructed with the same mechanism as the draft was created.
+1. Only getters that have a setter as well will be writable in the draft, as otherwise the value can't be copied back.
 
 Because Immer will dereference own getters of objects into normal properties, it is possible to use objects that use getter/setter traps on their fields, like MobX and Vue do.