From d64c2bd38b15584c41ec88a56c92e81652b224e0 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 5 May 2020 16:02:31 +0100 Subject: [PATCH] fix: Don't recurse into non-enumerable members when freezing tree. Fixes #590 --- .vscode/settings.json | 3 ++- __tests__/frozen.js | 25 +++++++++++++++++++++++-- src/core/finalize.ts | 7 +++++-- src/utils/common.ts | 11 +++++++---- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 002e270e..6f4faab4 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,5 @@ { "javascript.validate.enable": false, - "typescript.tsdk": "node_modules/typescript/lib" + "typescript.tsdk": "node_modules/typescript/lib", + "jest.enableInlineErrorMessages": true } diff --git a/__tests__/frozen.js b/__tests__/frozen.js index 7d4d3e31..34d0c3ba 100644 --- a/__tests__/frozen.js +++ b/__tests__/frozen.js @@ -23,7 +23,6 @@ function runTests(name, useProxies) { const base = {arr: [1], obj: {a: 1}} const next = produce(base, draft => { draft.arr.push(1) - debugger }) expect(isFrozen(base)).toBeFalsy() expect(isFrozen(base.arr)).toBeFalsy() @@ -191,7 +190,6 @@ function runTests(name, useProxies) { // https://github.com/immerjs/immer/issues/472 produce(frozen, draft => { - // if (useProxies) debugger ;["b", "c"].forEach(other => { const m = draft.map.get(other) @@ -199,5 +197,28 @@ function runTests(name, useProxies) { }) }) }) + + it("never freezes non-enumerable fields #590", () => { + const component = {} + Object.defineProperty(component, "state", { + value: {x: 1}, + enumerable: false, + writable: true, + configurable: true + }) + + const state = { + x: 1 + } + + const state2 = produce(state, draft => { + draft.ref = component + }) + + expect(() => { + state2.ref.state.x++ + }).not.toThrow() + expect(state2.ref.state.x).toBe(2) + }) }) } diff --git a/src/core/finalize.ts b/src/core/finalize.ts index 093752bc..f4ee8b77 100644 --- a/src/core/finalize.ts +++ b/src/core/finalize.ts @@ -64,8 +64,11 @@ 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) + each( + value, + (key, childValue) => + finalizeProperty(rootScope, state, value, key, childValue, path), + true // See #590, don't recurse into non-enumarable of non drafted objects ) return value } diff --git a/src/utils/common.ts b/src/utils/common.ts index c0954ff2..53a9fe74 100644 --- a/src/utils/common.ts +++ b/src/utils/common.ts @@ -67,11 +67,14 @@ export const ownKeys: (target: AnyObject) => PropertyKey[] = export function each( obj: T, - iter: (key: string | number, value: any, source: T) => void + iter: (key: string | number, value: any, source: T) => void, + enumerableOnly?: boolean ): void -export function each(obj: any, iter: any) { +export function each(obj: any, iter: any, enumerableOnly = false) { if (getArchtype(obj) === ArchtypeObject) { - ownKeys(obj).forEach(key => iter(key, obj[key], obj)) + ;(enumerableOnly ? Object.keys : ownKeys)(obj).forEach(key => + iter(key, obj[key], obj) + ) } else { obj.forEach((entry: any, index: any) => iter(index, entry, obj)) } @@ -178,7 +181,7 @@ export function freeze(obj: any, deep: boolean): void { obj.set = obj.add = obj.clear = obj.delete = dontMutateFrozenCollections as any } Object.freeze(obj) - if (deep) each(obj, (_, value) => freeze(value, true)) + if (deep) each(obj, (key, value) => freeze(value, true), true) } function dontMutateFrozenCollections() {