Skip to content

Commit

Permalink
improve handling for Proxies / enumerable properties when freezing / …
Browse files Browse the repository at this point in the history
…finalizing
  • Loading branch information
mweststrate committed Mar 9, 2024
1 parent a8b7fde commit 1f46654
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 12 deletions.
16 changes: 16 additions & 0 deletions __tests__/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions __tests__/frozen.js
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
17 changes: 10 additions & 7 deletions src/core/finalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}
Expand Down
17 changes: 12 additions & 5 deletions src/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 1f46654

Please sign in to comment.