Skip to content

Commit

Permalink
fix: Don't recurse into non-enumerable members when freezing tree. Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
mweststrate authored May 5, 2020
2 parents 00caea1 + d64c2bd commit 497d1a0
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 9 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"javascript.validate.enable": false,
"typescript.tsdk": "node_modules/typescript/lib"
"typescript.tsdk": "node_modules/typescript/lib",
"jest.enableInlineErrorMessages": true
}
25 changes: 23 additions & 2 deletions __tests__/frozen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -191,13 +190,35 @@ 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)

m.delete("a")
})
})
})

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)
})
})
}
7 changes: 5 additions & 2 deletions src/core/finalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
11 changes: 7 additions & 4 deletions src/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,14 @@ export const ownKeys: (target: AnyObject) => PropertyKey[] =

export function each<T extends Objectish>(
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))
}
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 497d1a0

Please # to comment.