Skip to content

Commit

Permalink
fix: Fixed #620: Symbolic properties were not drafted or finalized co…
Browse files Browse the repository at this point in the history
…rrectly
  • Loading branch information
mweststrate committed Jun 19, 2020
1 parent 7d6b57b commit 91915cf
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 6 deletions.
33 changes: 33 additions & 0 deletions __tests__/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,39 @@ function testObjectTypes(produce) {
expect(next.y).toBe(2)
expect(state.x).toBe(0)
})

describe("#620", () => {
const customSymbol = Symbol("customSymbol")

class TestClass {
[immerable] = true;
[customSymbol] = 1
}

/* Create class instance */
let test0 = new TestClass()

/* First produce. This works */
let test1 = produce(test0, draft => {
expect(draft[customSymbol]).toBe(1)
draft[customSymbol] = 2
})
expect(test1).toEqual({
[immerable]: true,
[customSymbol]: 2
})

/* Second produce. This does NOT work. See console error */
/* With version 6.0.9, this works though */
let test2 = produce(test1, draft => {
expect(draft[customSymbol]).toBe(2)
draft[customSymbol] = 3
})
expect(test2).toEqual({
[immerable]: true,
[customSymbol]: 3
})
})
}

function testLiteralTypes(produce) {
Expand Down
15 changes: 10 additions & 5 deletions src/plugins/es5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
getCurrentScope,
die,
markChanged,
objectTraps
objectTraps,
ownKeys
} from "../internal"

type ES5State = ES5ArrayState | ES5ObjectState
Expand Down Expand Up @@ -49,7 +50,9 @@ export function enableES5() {
// Descriptors we want to skip:
if (isArray) delete descriptors.length
delete descriptors[DRAFT_STATE as any]
for (let key in descriptors) {
const keys = ownKeys(descriptors)
for (let i = 0; i < keys.length; i++) {
const key: any = keys[i]
descriptors[key] = proxyProperty(
key,
isArray || !!descriptors[key].enumerable
Expand Down Expand Up @@ -204,9 +207,10 @@ export function enableES5() {

// Search for added keys and changed keys. Start at the back, because
// non-numeric keys are ordered by time of definition on the object.
const keys = Object.keys(draft_)
const keys = ownKeys(draft_)
for (let i = keys.length - 1; i >= 0; i--) {
const key = keys[i]
const key: any = keys[i]
if (key === DRAFT_STATE) continue
const baseValue = base_[key]
// The `undefined` check is a fast path for pre-existing keys.
if (baseValue === undefined && !has(base_, key)) {
Expand All @@ -225,7 +229,8 @@ export function enableES5() {

// At this point, no keys were added or changed.
// Compare key count to determine if keys were deleted.
return keys.length !== Object.keys(base_).length
const baseIsDraft = !!base_[DRAFT_STATE as any]
return keys.length !== ownKeys(base_).length + (baseIsDraft ? 0 : 1) // + 1 to correct for DRAFT_STATE
}

function hasArrayChanges(state: ES5ArrayState) {
Expand Down
4 changes: 3 additions & 1 deletion src/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ export function shallowCopy(base: any) {
if (Array.isArray(base)) return base.slice()
const descriptors = Object.getOwnPropertyDescriptors(base)
delete descriptors[DRAFT_STATE as any]
for (let key in descriptors) {
let keys = ownKeys(descriptors)
for (let i = 0; i < keys.length; i++) {
const key: any = keys[i]
const desc = descriptors[key]
if (desc.writable === false) {
desc.writable = true
Expand Down

0 comments on commit 91915cf

Please sign in to comment.