From db9b6897659c8aeb8938abf0537761ee2e4b0370 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 11 Dec 2019 10:03:09 +0000 Subject: [PATCH 01/28] Added tests reproducing #472 and #466 --- __tests__/map-set.js | 176 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 __tests__/map-set.js diff --git a/__tests__/map-set.js b/__tests__/map-set.js new file mode 100644 index 000000000..3b0f2f347 --- /dev/null +++ b/__tests__/map-set.js @@ -0,0 +1,176 @@ +"use strict" +import {Immer, nothing, original, isDraft, immerable} from "../src/index" +import {each, shallowCopy, isEnumerable, DRAFT_STATE} from "../src/common" + +jest.setTimeout(1000) + +runBaseTest("proxy (no freeze)", true, false) +runBaseTest("proxy (autofreeze)", true, true) +runBaseTest("proxy (autofreeze)(patch listener)", true, true, true) +runBaseTest("es5 (no freeze)", false, false) +runBaseTest("es5 (autofreeze)", false, true) +runBaseTest("es5 (autofreeze)(patch listener)", false, true, true) + +function runBaseTest(name, useProxies, autoFreeze, useListener) { + const listener = useListener ? function() {} : undefined + const {produce, produceWithPatches} = createPatchedImmer({ + useProxies, + autoFreeze + }) + + // When `useListener` is true, append a function to the arguments of every + // uncurried `produce` call in every test. This makes tests easier to read. + function createPatchedImmer(options) { + const immer = new Immer(options) + + const {produce} = immer + immer.produce = function(...args) { + return typeof args[1] === "function" && args.length < 3 + ? produce(...args, listener) + : produce(...args) + } + + return immer + } + + describe("map issues " + name, () => { + test("#472 ", () => { + const project = produce(new Map(), draft => { + draft.set("bar1", {blocked: false}) + draft.set("bar2", {blocked: false}) + }) + + // Read before write -- no error + produce(project, draft => { + const bar1 = draft.get("bar1") + const bar2 = draft.get("bar2") + bar1.blocked = true + bar2.blocked = true + }) + + // Read/write interleaved -- error + produce(project, draft => { + const bar1 = draft.get("bar1") + bar1.blocked = true + const bar2 = draft.get("bar2") + bar2.blocked = true // TypeError: "blocked" is read-only + }) + }) + + test("#466 - setNoPatches", () => { + const obj = { + set: new Set() + } + + const result = produceWithPatches(obj, draft => { + draft.set.add("abc") + }) + expect(result).toEqual([ + {set: new Set(["abc"])}, + [{op: "add", path: ["set", 0], value: "abc"}], + [{op: "remove", path: ["set", 0], value: "abc"}] + ]) + }) + + test("#466 - mapChangeBug ", () => { + const obj = { + map: new Map([ + ["a", new Map([["b", true], ["c", true], ["d", true]])], + ["b", new Map([["a", true]])], + ["c", new Map([["a", true]])], + ["d", new Map([["a", true]])] + ]) + } + const result = produceWithPatches(obj, draft => { + const aMap = draft.map.get("a") + aMap.forEach((_, other) => { + const otherMap = draft.map.get(other) + otherMap.delete("a") + }) + }) + expect(result).toEqual([ + { + map: new Map([ + ["a", new Map([["b", true], ["c", true], ["d", true]])], + ["b", new Map()], + ["c", new Map()], + ["d", new Map()] + ]) + }, + [ + { + op: "remove", + path: ["map", "b", "a"] + }, + { + op: "remove", + path: ["map", "c", "a"] + }, + { + op: "remove", + path: ["map", "d", "a"] + } + ], + [ + { + op: "add", + path: ["map", "b", "a"], + value: true + }, + { + op: "add", + path: ["map", "c", "a"], + value: true + }, + { + op: "add", + path: ["map", "d", "a"], + value: true + } + ] + ]) + }) + + test("#466 - mapChangeBug2 ", () => { + const obj = { + map: new Map([ + ["a", new Map([["b", true], ["c", true], ["d", true]])], + ["b", new Map([["a", true]])], + ["c", new Map([["a", true]])], + ["d", new Map([["a", true]])] + ]) + } + const obj1 = produce(obj, draft => {}) + const result = produceWithPatches(obj1, draft => { + const aMap = draft.map.get("a") + aMap.forEach((_, other) => { + const otherMap = draft.map.get(other) + otherMap.delete("a") + }) + }) + expect(result).toEqual([ + { + map: new Map([ + ["a", new Map([["b", true], ["c", true], ["d", true]])], + ["b", new Map([])], + ["c", new Map([])], + ["d", new Map([])] + ]) + }, + [ + { + op: "remove", + path: ["map", "b", "a"] + } + ], + [ + { + op: "add", + path: ["map", "b", "a"], + value: true + } + ] + ]) + }) + }) +} From 521fc801d334634070e566472d8a699978ac4a3e Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 16 Dec 2019 10:28:00 +0000 Subject: [PATCH 02/28] Separated map / set logic --- src/es5.js | 198 +++-------------------------------------------- src/mapset.js | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+), 189 deletions(-) create mode 100644 src/mapset.js diff --git a/src/es5.js b/src/es5.js index 3eabedf17..6c9b7aca5 100644 --- a/src/es5.js +++ b/src/es5.js @@ -15,6 +15,7 @@ import { makeIterable, makeIterateSetValues } from "./common" +import {proxyMap, proxySet, hasMapChanges, hasSetChanges} from "./mapset" import {ImmerScope} from "./scope" export function willFinalize(scope, result, isReplaced) { @@ -74,7 +75,8 @@ function revoke() { this.revoked = true } -function latest(state) { +// TODO: remove export +export function latest(state) { return state.copy || state.base } @@ -113,14 +115,16 @@ function set(state, prop, value) { state.copy[prop] = value } -function markChanged(state) { +// TODO: kill export +export function markChanged(state) { if (!state.modified) { state.modified = true if (state.parent) markChanged(state.parent) } } -function prepareCopy(state) { +// TODO: kill export +export function prepareCopy(state) { if (!state.copy) state.copy = clonePotentialDraft(state.base) } @@ -158,162 +162,8 @@ function proxyProperty(draft, prop, enumerable) { Object.defineProperty(draft, prop, desc) } -function proxyMap(target) { - Object.defineProperties(target, mapTraps) - - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - proxyMethod(iterateMapValues) - ) - } -} - -const mapTraps = finalizeTraps({ - size: state => latest(state).size, - has: state => key => latest(state).has(key), - set: state => (key, value) => { - if (latest(state).get(key) !== value) { - prepareCopy(state) - markChanged(state) - state.assigned.set(key, true) - state.copy.set(key, value) - } - return state.draft - }, - delete: state => key => { - prepareCopy(state) - markChanged(state) - state.assigned.set(key, false) - state.copy.delete(key) - return false - }, - clear: state => () => { - if (!state.copy) { - prepareCopy(state) - } - markChanged(state) - state.assigned = new Map() - for (const key of latest(state).keys()) { - state.assigned.set(key, false) - } - return state.copy.clear() - }, - forEach: (state, key, reciever) => cb => { - latest(state).forEach((value, key, map) => { - cb(reciever.get(key), key, map) - }) - }, - get: state => key => { - const value = latest(state).get(key) - - if (state.finalizing || state.finalized || !isDraftable(value)) { - return value - } - - if (value !== state.base.get(key)) { - return value - } - const draft = createProxy(value, state) - prepareCopy(state) - state.copy.set(key, draft) - return draft - }, - keys: state => () => latest(state).keys(), - values: iterateMapValues, - entries: iterateMapValues -}) - -function proxySet(target) { - Object.defineProperties(target, setTraps) - - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - proxyMethod(iterateSetValues) - ) - } -} - -const iterateSetValues = makeIterateSetValues(createProxy) - -const setTraps = finalizeTraps({ - size: state => { - return latest(state).size - }, - add: state => value => { - if (!latest(state).has(value)) { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - state.copy.add(value) - } - return state.draft - }, - delete: state => value => { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.delete(value) - }, - has: state => key => { - return latest(state).has(key) - }, - clear: state => () => { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.clear() - }, - keys: iterateSetValues, - entries: iterateSetValues, - values: iterateSetValues, - forEach: state => (cb, thisArg) => { - const iterator = iterateSetValues(state)() - let result = iterator.next() - while (!result.done) { - cb.call(thisArg, result.value, result.value, state.draft) - result = iterator.next() - } - } -}) - -function finalizeTraps(traps) { - return Object.keys(traps).reduce(function(acc, key) { - const builder = key === "size" ? proxyAttr : proxyMethod - acc[key] = builder(traps[key], key) - return acc - }, {}) -} - -function proxyAttr(fn) { - return { - get() { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return fn(state) - } - } -} - -function proxyMethod(trap, key) { - return { - get() { - return function(...args) { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return trap(state, key, state.draft)(...args) - } - } - } -} - -function assertUnrevoked(state) { +// TODO: remove export +export function assertUnrevoked(state) { if (state.revoked === true) throw new Error( "Cannot use a proxy that has been revoked. Did you pass an object from inside an immer function to an async process? " + @@ -429,36 +279,6 @@ function hasArrayChanges(state) { return false } -function hasMapChanges(state) { - const {base, draft} = state - - if (base.size !== draft.size) return true - - // IE11 supports only forEach iteration - let hasChanges = false - draft.forEach(function(value, key) { - if (!hasChanges) { - hasChanges = isDraftable(value) ? value.modified : value !== base.get(key) - } - }) - return hasChanges -} - -function hasSetChanges(state) { - const {base, draft} = state - - if (base.size !== draft.size) return true - - // IE11 supports only forEach iteration - let hasChanges = false - draft.forEach(function(value, key) { - if (!hasChanges) { - hasChanges = isDraftable(value) ? value.modified : !base.has(key) - } - }) - return hasChanges -} - function createHiddenProperty(target, prop, value) { Object.defineProperty(target, prop, { value: value, diff --git a/src/mapset.js b/src/mapset.js new file mode 100644 index 000000000..5f2b8743c --- /dev/null +++ b/src/mapset.js @@ -0,0 +1,210 @@ +import { + each, + has, + is, + isDraft, + isDraftable, + isEnumerable, + isMap, + isSet, + hasSymbol, + shallowCopy, + DRAFT_STATE, + iterateMapValues, + makeIterable, + makeIterateSetValues +} from "./common" + +// TODO: kill: +import { + createProxy, + assertUnrevoked, + latest, + prepareCopy, + markChanged +} from "./es5" + +export function proxyMap(target) { + Object.defineProperties(target, mapTraps) + + if (hasSymbol) { + Object.defineProperty( + target, + Symbol.iterator, + proxyMethod(iterateMapValues) + ) + } +} + +const mapTraps = finalizeTraps({ + size: state => latest(state).size, + has: state => key => latest(state).has(key), + set: state => (key, value) => { + if (latest(state).get(key) !== value) { + prepareCopy(state) + markChanged(state) + state.assigned.set(key, true) + state.copy.set(key, value) + } + return state.draft + }, + delete: state => key => { + prepareCopy(state) + markChanged(state) + state.assigned.set(key, false) + state.copy.delete(key) + return false + }, + clear: state => () => { + if (!state.copy) { + prepareCopy(state) + } + markChanged(state) + state.assigned = new Map() + for (const key of latest(state).keys()) { + state.assigned.set(key, false) + } + return state.copy.clear() + }, + forEach: (state, key, reciever) => cb => { + latest(state).forEach((value, key, map) => { + cb(reciever.get(key), key, map) + }) + }, + get: state => key => { + const value = latest(state).get(key) + + if (state.finalizing || state.finalized || !isDraftable(value)) { + return value + } + + if (value !== state.base.get(key)) { + return value + } + const draft = createProxy(value, state) + prepareCopy(state) + state.copy.set(key, draft) + return draft + }, + keys: state => () => latest(state).keys(), + values: iterateMapValues, + entries: iterateMapValues +}) + +export function proxySet(target) { + Object.defineProperties(target, setTraps) + + if (hasSymbol) { + Object.defineProperty( + target, + Symbol.iterator, + proxyMethod(iterateSetValues) + ) + } +} + +const iterateSetValues = makeIterateSetValues(createProxy) + +const setTraps = finalizeTraps({ + size: state => { + return latest(state).size + }, + add: state => value => { + if (!latest(state).has(value)) { + markChanged(state) + if (!state.copy) { + prepareCopy(state) + } + state.copy.add(value) + } + return state.draft + }, + delete: state => value => { + markChanged(state) + if (!state.copy) { + prepareCopy(state) + } + return state.copy.delete(value) + }, + has: state => key => { + return latest(state).has(key) + }, + clear: state => () => { + markChanged(state) + if (!state.copy) { + prepareCopy(state) + } + return state.copy.clear() + }, + keys: iterateSetValues, + entries: iterateSetValues, + values: iterateSetValues, + forEach: state => (cb, thisArg) => { + const iterator = iterateSetValues(state)() + let result = iterator.next() + while (!result.done) { + cb.call(thisArg, result.value, result.value, state.draft) + result = iterator.next() + } + } +}) + +function finalizeTraps(traps) { + return Object.keys(traps).reduce(function(acc, key) { + const builder = key === "size" ? proxyAttr : proxyMethod + acc[key] = builder(traps[key], key) + return acc + }, {}) +} + +function proxyAttr(fn) { + return { + get() { + const state = this[DRAFT_STATE] + assertUnrevoked(state) + return fn(state) + } + } +} + +function proxyMethod(trap, key) { + return { + get() { + return function(...args) { + const state = this[DRAFT_STATE] + assertUnrevoked(state) + return trap(state, key, state.draft)(...args) + } + } + } +} + +export function hasMapChanges(state) { + const {base, draft} = state + + if (base.size !== draft.size) return true + + // IE11 supports only forEach iteration + let hasChanges = false + draft.forEach(function(value, key) { + if (!hasChanges) { + hasChanges = isDraftable(value) ? value.modified : value !== base.get(key) + } + }) + return hasChanges +} + +export function hasSetChanges(state) { + const {base, draft} = state + + if (base.size !== draft.size) return true + + // IE11 supports only forEach iteration + let hasChanges = false + draft.forEach(function(value, key) { + if (!hasChanges) { + hasChanges = isDraftable(value) ? value.modified : !base.has(key) + } + }) + return hasChanges +} From 9bcefb658c101b4da43e4bb9bd1fca973cd372af Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sat, 21 Dec 2019 14:41:43 +0000 Subject: [PATCH 03/28] map unification WIP --- __tests__/empty.ts | 28 ++++-- src/common.ts | 6 +- src/es5.ts | 171 +---------------------------------- src/immer.ts | 10 +- src/{mapset.js => mapset.ts} | 25 +++-- src/proxy.ts | 9 +- src/scope.ts | 9 +- 7 files changed, 61 insertions(+), 197 deletions(-) rename src/{mapset.js => mapset.ts} (86%) diff --git a/__tests__/empty.ts b/__tests__/empty.ts index f9f5f47a7..a0fb8438f 100644 --- a/__tests__/empty.ts +++ b/__tests__/empty.ts @@ -1,13 +1,29 @@ -import {produce, produceWithPatches} from "../src" +import {produce, produceWithPatches, setUseProxies} from "../src" test("empty stub test", () => { expect(true).toBe(true) }) -test("delete 7", () => { - debugger - const [_, p] = produceWithPatches(new Set(["y", 1]), d => { - d.delete("y") +describe("map set", () => { + test("can assign set value", () => { + setUseProxies(false) + + const baseState = new Map([["x", 1]]) + const nextState = produce(baseState, s => { + s.set("x", 2) + }) + expect(nextState).not.toBe(baseState) + expect(nextState.get("x")).toEqual(2) + }) + + test("can assign by key", () => { + setUseProxies(false) + + const baseState = new Map([["x", {a: 1}]]) + const nextState = produce(baseState, s => { + s.get("x")!.a++ + }) + expect(nextState).not.toBe(baseState) + expect(nextState.get("x")!.a).toEqual(2) }) - expect(p.length).toBe(1) }) diff --git a/src/common.ts b/src/common.ts index 0bc7ed9be..b8b0de80c 100644 --- a/src/common.ts +++ b/src/common.ts @@ -214,7 +214,7 @@ export function iterateMapValues(state, prop, receiver) { } } -export function makeIterateSetValues(createProxy) { +export function makeIterateSetValues() { function iterateSetValues(state, prop?) { const isEntries = prop === "entries" return () => { @@ -237,7 +237,7 @@ export function makeIterateSetValues(createProxy) { if (state.finalized || !isDraftable(value) || state.finalizing) { return value } - draft = createProxy(value, state) + draft = state.scope.immer.createProxy(value, state) state.drafts.set(key, draft) if (state.modified) { state.copy.add(draft) @@ -249,7 +249,7 @@ export function makeIterateSetValues(createProxy) { return iterateSetValues } -function latest(state: ImmerState): T { +export function latest(state: any): any { return state.copy || state.base } diff --git a/src/es5.ts b/src/es5.ts index 6a35daccb..e04f51b7c 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -13,18 +13,19 @@ import { DRAFT_STATE, iterateMapValues, makeIterable, - makeIterateSetValues + makeIterateSetValues, + latest } from "./common" import {proxyMap, proxySet, hasMapChanges, hasSetChanges} from "./mapset" import {ImmerScope} from "./scope" import {ImmerState} from "./types" -interface ES5Draft { +export interface ES5Draft { [DRAFT_STATE]: ES5State } // TODO: merge with ImmerState? -interface ES5State { +export interface ES5State { scope: ImmerScope modified: boolean finalizing: boolean @@ -100,11 +101,6 @@ function revoke(this: ES5State) { this.revoked = true } -// TODO: remove export -export function latest(state) { - return state.copy || state.base -} - // Access a property without creating an Immer draft. function peek(draft, prop) { const state = draft[DRAFT_STATE] @@ -187,164 +183,7 @@ function proxyProperty(draft, prop, enumerable) { Object.defineProperty(draft, prop, desc) } -function proxyMap(target) { - Object.defineProperties(target, mapTraps) - - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - // @ts-ignore - proxyMethod(iterateMapValues) //TODO: , Symbol.iterator) - ) - } -} - -const mapTraps = finalizeTraps({ - size: state => latest(state).size, - has: state => key => latest(state).has(key), - set: state => (key, value) => { - if (latest(state).get(key) !== value) { - prepareCopy(state) - markChanged(state) - state.assigned.set(key, true) - state.copy.set(key, value) - } - return state.draft - }, - delete: state => key => { - prepareCopy(state) - markChanged(state) - state.assigned.set(key, false) - state.copy.delete(key) - return false - }, - clear: state => () => { - if (!state.copy) { - prepareCopy(state) - } - markChanged(state) - state.assigned = new Map() - for (const key of latest(state).keys()) { - state.assigned.set(key, false) - } - return state.copy.clear() - }, - forEach: (state, key, reciever) => cb => { - latest(state).forEach((value, key, map) => { - cb(reciever.get(key), key, map) - }) - }, - get: state => key => { - const value = latest(state).get(key) - - if (state.finalizing || state.finalized || !isDraftable(value)) { - return value - } - - if (value !== state.base.get(key)) { - return value - } - const draft = createProxy(value, state) - prepareCopy(state) - state.copy.set(key, draft) - return draft - }, - keys: state => () => latest(state).keys(), - values: iterateMapValues, - entries: iterateMapValues -}) - -function proxySet(target) { - Object.defineProperties(target, setTraps) - - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - // @ts-ignore - proxyMethod(iterateSetValues) //TODO: , Symbol.iterator) - ) - } -} - -const iterateSetValues = makeIterateSetValues(createProxy) - -const setTraps = finalizeTraps({ - size: state => { - return latest(state).size - }, - add: state => value => { - if (!latest(state).has(value)) { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - state.copy.add(value) - } - return state.draft - }, - delete: state => value => { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.delete(value) - }, - has: state => key => { - return latest(state).has(key) - }, - clear: state => () => { - markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.clear() - }, - keys: iterateSetValues, - entries: iterateSetValues, - values: iterateSetValues, - forEach: state => (cb, thisArg) => { - const iterator = iterateSetValues(state)() - let result = iterator.next() - while (!result.done) { - cb.call(thisArg, result.value, result.value, state.draft) - result = iterator.next() - } - } -}) - -function finalizeTraps(traps) { - return Object.keys(traps).reduce(function(acc, key) { - const builder = key === "size" ? proxyAttr : proxyMethod - acc[key] = builder(traps[key], key) - return acc - }, {}) -} - -function proxyAttr(fn) { - return { - get() { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return fn(state) - } - } -} - -function proxyMethod(trap, key) { - return { - get() { - return function(this: ES5Draft, ...args) { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return trap(state, key, state.draft)(...args) - } - } - } -} - -function assertUnrevoked(state) { +export function assertUnrevoked(state) { if (state.revoked === true) throw new Error( "Cannot use a proxy that has been revoked. Did you pass an object from inside an immer function to an async process? " + diff --git a/src/immer.ts b/src/immer.ts index a6d94cf01..6e2e89000 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -51,7 +51,7 @@ export class Immer implements ProducersFns { ) => void onDelete?: (state: ImmerState, prop: string | number) => void onCopy?: (state: ImmerState) => void - createProxy!:(value: T) => T; + createProxy!:(value: T, parent: any) => T; willFinalize!: (scope: ImmerScope, thing: any, isReplaced: boolean) => void; constructor(config?: { @@ -116,8 +116,8 @@ export class Immer implements ProducersFns { // Only plain objects, arrays, and "immerable classes" are drafted. if (isDraftable(base)) { - const scope = ImmerScope.enter() - const proxy = this.createProxy(base) + const scope = ImmerScope.enter(this) + const proxy = this.createProxy(base, undefined) let hasError = true try { result = recipe(proxy) @@ -171,8 +171,8 @@ export class Immer implements ProducersFns { if (!isDraftable(base)) { throw new Error("First argument to `createDraft` must be a plain object, an array, or an immerable object") // prettier-ignore } - const scope = ImmerScope.enter() - const proxy = this.createProxy(base) + const scope = ImmerScope.enter(this) + const proxy = this.createProxy(base, undefined) proxy[DRAFT_STATE].isManual = true scope.leave() return proxy as any diff --git a/src/mapset.js b/src/mapset.ts similarity index 86% rename from src/mapset.js rename to src/mapset.ts index 5f2b8743c..c2fbfd57a 100644 --- a/src/mapset.js +++ b/src/mapset.ts @@ -12,16 +12,17 @@ import { DRAFT_STATE, iterateMapValues, makeIterable, - makeIterateSetValues + makeIterateSetValues, + latest } from "./common" // TODO: kill: import { - createProxy, assertUnrevoked, - latest, prepareCopy, - markChanged + markChanged, // Looks to be the correct implementation for maps as well + ES5Draft, + ES5State } from "./es5" export function proxyMap(target) { @@ -31,11 +32,13 @@ export function proxyMap(target) { Object.defineProperty( target, Symbol.iterator, + // @ts-ignore TODO fix proxyMethod(iterateMapValues) ) } } +// TODO: eliminate these, and put in a Map superclass const mapTraps = finalizeTraps({ size: state => latest(state).size, has: state => key => latest(state).has(key), @@ -66,6 +69,7 @@ const mapTraps = finalizeTraps({ } return state.copy.clear() }, + // @ts-ignore TODO: forEach: (state, key, reciever) => cb => { latest(state).forEach((value, key, map) => { cb(reciever.get(key), key, map) @@ -81,13 +85,15 @@ const mapTraps = finalizeTraps({ if (value !== state.base.get(key)) { return value } - const draft = createProxy(value, state) + const draft = state.scope.immer.createProxy(value, state) prepareCopy(state) state.copy.set(key, draft) return draft }, keys: state => () => latest(state).keys(), + // @ts-ignore TODO: values: iterateMapValues, + // @ts-ignore TODO: entries: iterateMapValues }) @@ -98,12 +104,13 @@ export function proxySet(target) { Object.defineProperty( target, Symbol.iterator, + // @ts-ignore TODO proxyMethod(iterateSetValues) ) } } -const iterateSetValues = makeIterateSetValues(createProxy) +const iterateSetValues = makeIterateSetValues() const setTraps = finalizeTraps({ size: state => { @@ -149,7 +156,7 @@ const setTraps = finalizeTraps({ } }) -function finalizeTraps(traps) { +function finalizeTraps(traps: {[prop: string]: (state: ES5State) => Function }) { return Object.keys(traps).reduce(function(acc, key) { const builder = key === "size" ? proxyAttr : proxyMethod acc[key] = builder(traps[key], key) @@ -170,7 +177,7 @@ function proxyAttr(fn) { function proxyMethod(trap, key) { return { get() { - return function(...args) { + return function(this: ES5Draft, ...args) { const state = this[DRAFT_STATE] assertUnrevoked(state) return trap(state, key, state.draft)(...args) @@ -186,6 +193,7 @@ export function hasMapChanges(state) { // IE11 supports only forEach iteration let hasChanges = false + // TODO: optimize: break on first difference draft.forEach(function(value, key) { if (!hasChanges) { hasChanges = isDraftable(value) ? value.modified : value !== base.get(key) @@ -201,6 +209,7 @@ export function hasSetChanges(state) { // IE11 supports only forEach iteration let hasChanges = false + // TODO: optimize: break on first diff draft.forEach(function(value, key) { if (!hasChanges) { hasChanges = isDraftable(value) ? value.modified : !base.has(key) diff --git a/src/proxy.ts b/src/proxy.ts index 7f4432b26..42edff4ba 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -16,7 +16,8 @@ import { assignSet, original, iterateMapValues, - makeIterateSetValues + makeIterateSetValues, + latest } from "./common" import {ImmerScope} from "./scope" @@ -310,7 +311,7 @@ const mapTraps = makeTrapsForGetters>({ [hasSymbol ? Symbol.iterator : "@@iterator"]: iterateMapValues }) -const iterateSetValues = makeIterateSetValues(createProxy) +const iterateSetValues = makeIterateSetValues() /** * Set drafts */ @@ -356,10 +357,6 @@ const setTraps = makeTrapsForGetters>({ * Helpers */ -// Retrieve the latest values of the draft. -function latest(state) { - return state.copy || state.base -} // Access a property without creating an Immer draft. function peek(draft, prop) { diff --git a/src/scope.ts b/src/scope.ts index 4436feae1..c734dde2a 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -1,5 +1,6 @@ import {DRAFT_STATE} from "./common" import {ImmerState, Patch, PatchListener} from "./types" +import { Immer } from "./immer" /** Each scope represents a `produce` call. */ export class ImmerScope { @@ -11,10 +12,12 @@ export class ImmerScope { drafts: any[] parent?: ImmerScope patchListener?: PatchListener + immer: Immer - constructor(parent?: ImmerScope) { + constructor(parent: ImmerScope | undefined, immer: Immer) { this.drafts = [] this.parent = parent + this.immer = immer // Whenever the modified draft contains a draft from another scope, we // need to prevent auto-freezing so the unowned draft can be finalized. @@ -45,8 +48,8 @@ export class ImmerScope { } } - static enter() { - const scope = new ImmerScope(ImmerScope.current) + static enter(immer: Immer) { + const scope = new ImmerScope(ImmerScope.current, immer) ImmerScope.current = scope return scope } From 735c4023785bdda98841d98f3b0977d3388322a3 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Fri, 3 Jan 2020 20:15:39 +0000 Subject: [PATCH 04/28] basic map tests succeeding --- __tests__/base.js | 20 ++-- __tests__/empty.ts | 114 ++++++++++++++++++++- __tests__/frozen.js | 2 +- __tests__/hooks.js | 12 +-- __tests__/map-set.js | 6 +- __tests__/patch.js | 14 +-- __tests__/readme.js | 2 +- src/common.ts | 8 +- src/es5.ts | 19 ++-- src/immer.ts | 15 ++- src/mapset.ts | 235 +++++++++++++++++++++++++++++++++---------- src/patches.ts | 2 +- src/proxy.ts | 43 ++++---- tsconfig.json | 2 + 14 files changed, 383 insertions(+), 111 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index eb10df2af..6a0a386c2 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -319,7 +319,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(nextState).toBe(base) }) - it("supports iteration", () => { + it.skip("supports iteration", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -341,7 +341,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(result.get("second").a).toEqual(2) }) - it("supports 'entries'", () => { + it.skip("supports 'entries'", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -363,7 +363,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(result.get("second").a).toEqual(2) }) - it("supports 'values'", () => { + it.skip("supports 'values'", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -490,7 +490,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { it("can use 'delete' to remove items", () => { const nextState = produce(baseState, s => { expect(s.aMap.has("jedi")).toBe(true) - s.aMap.delete("jedi") + expect(s.aMap.delete("jedi")).toBe(true) expect(s.aMap.has("jedi")).toBe(false) }) expect(nextState.aMap).not.toBe(baseState.aMap) @@ -535,9 +535,17 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(base.get("first").get("second").prop).toBe("test") expect(result.get("first").get("second").prop).toBe("test1") }) + + it("treats void deletes as no-op", () => { + const base = new Map([["x", 1]]) + const next = produce(base, d => { + expect(d.delete("y")).toBe(false) + }) + expect(next).toBe(base) + }) }) - describe("set drafts", () => { + describe.skip("set drafts", () => { it("supports iteration", () => { const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) const findById = (set, id) => { @@ -1592,7 +1600,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) }) - describe(`complex nesting map / set / object`, () => { + describe.skip(`complex nesting map / set / object`, () => { const a = {a: 1} const b = {b: 2} const c = {c: 3} diff --git a/__tests__/empty.ts b/__tests__/empty.ts index a0fb8438f..9b0fad1be 100644 --- a/__tests__/empty.ts +++ b/__tests__/empty.ts @@ -1,10 +1,11 @@ import {produce, produceWithPatches, setUseProxies} from "../src" +import {DRAFT_STATE} from "../src/common" test("empty stub test", () => { expect(true).toBe(true) }) -describe("map set", () => { +describe("map set - es5", () => { test("can assign set value", () => { setUseProxies(false) @@ -12,6 +13,7 @@ describe("map set", () => { const nextState = produce(baseState, s => { s.set("x", 2) }) + expect(baseState.get("x")).toEqual(1) expect(nextState).not.toBe(baseState) expect(nextState.get("x")).toEqual(2) }) @@ -23,7 +25,117 @@ describe("map set", () => { const nextState = produce(baseState, s => { s.get("x")!.a++ }) + expect(nextState.get("x")!.a).toEqual(2) + expect(baseState.get("x")!.a).toEqual(1) + expect(nextState).not.toBe(baseState) + }) +}) + +describe("map set - proxy", () => { + test("can assign set value", () => { + setUseProxies(true) + + const baseState = new Map([["x", 1]]) + const nextState = produce(baseState, s => { + s.set("x", 2) + }) + expect(baseState.get("x")).toEqual(1) expect(nextState).not.toBe(baseState) + expect(nextState.get("x")).toEqual(2) + }) + + test("can assign by key", () => { + setUseProxies(true) + + const baseState = new Map([["x", {a: 1}]]) + const nextState = produce(baseState, s => { + s.get("x")!.a++ + }) expect(nextState.get("x")!.a).toEqual(2) + expect(baseState.get("x")!.a).toEqual(1) + expect(nextState).not.toBe(baseState) + }) + + test("deep change bubbles up", () => { + setUseProxies(true) + + const baseState = createBaseState() + const nextState = produce(baseState, s => { + s.anObject.nested.yummie = false + }) + expect(nextState).not.toBe(baseState) + expect(nextState.anObject).not.toBe(baseState.anObject) + expect(baseState.anObject.nested.yummie).toBe(true) + expect(nextState.anObject.nested.yummie).toBe(false) + expect(nextState.anArray).toBe(baseState.anArray) + }) + + it("can assign by key", () => { + setUseProxies(true) + const baseState = createBaseState() + + const nextState = produce(baseState, s => { + // Map.prototype.set should return the Map itself + const res = s.aMap.set("force", true) + expect(res).toBe((s.aMap as any)[DRAFT_STATE].draft) + }) + expect(nextState).not.toBe(baseState) + expect(nextState.aMap).not.toBe(baseState.aMap) + expect(nextState.aMap.get("force")).toEqual(true) + }) + + it("can use 'delete' to remove items", () => { + const baseState = createBaseState() + + const nextState = produce(baseState, s => { + expect(s.aMap.has("jedi")).toBe(true) + expect(s.aMap.delete("jedi")).toBe(true) + expect(s.aMap.has("jedi")).toBe(false) + }) + expect(nextState.aMap).not.toBe(baseState.aMap) + expect(nextState.aMap.size).toBe(baseState.aMap.size - 1) + expect(baseState.aMap.has("jedi")).toBe(true) + expect(nextState.aMap.has("jedi")).toBe(false) + }) + + it("support 'has'", () => { + const baseState = createBaseState() + + const nextState = produce(baseState, s => { + expect(s.aMap.has("newKey")).toBe(false) + s.aMap.set("newKey", true) + expect(s.aMap.has("newKey")).toBe(true) + }) + expect(nextState).not.toBe(baseState) + expect(nextState.aMap).not.toBe(baseState.aMap) + expect(baseState.aMap.has("newKey")).toBe(false) + expect(nextState.aMap.has("newKey")).toBe(true) }) }) + +function createBaseState() { + const data = { + anInstance: new (class {})(), + anArray: [3, 2, {c: 3}, 1], + aMap: new Map([ + ["jedi", {name: "Luke", skill: 10}], + ["jediTotal", 42], + ["force", "these aren't the droids you're looking for"] + ] as any), + aSet: new Set([ + "Luke", + 42, + { + jedi: "Yoda" + } + ]), + aProp: "hi", + anObject: { + nested: { + yummie: true + }, + coffee: false + } + } + return data +} diff --git a/__tests__/frozen.js b/__tests__/frozen.js index e7b4ef4a8..bf764d2f3 100644 --- a/__tests__/frozen.js +++ b/__tests__/frozen.js @@ -156,7 +156,7 @@ function runTests(name, useProxies) { expect(produce(res, d => void d.set("a", 2))).not.toBe(res) }) - it("will freeze sets", () => { + it.skip("will freeze sets", () => { const base = new Set() const res = produce(base, draft => { base.add(1) diff --git a/__tests__/hooks.js b/__tests__/hooks.js index 1fe448f1a..dce2c52fc 100644 --- a/__tests__/hooks.js +++ b/__tests__/hooks.js @@ -120,7 +120,7 @@ function createHookTests(useProxies) { }) }) - describe("when draft is a Map", () => { + describe.skip("when draft is a Map", () => { test("assign", () => { const key1 = {prop: "val1"} const key2 = {prop: "val2"} @@ -159,7 +159,7 @@ function createHookTests(useProxies) { }) }) - describe("when draft is a Set", () => { + describe.skip("when draft is a Set", () => { test("assign", () => { produce({a: new Set([1, 2, 3])}, s => { s.a.add(4) @@ -241,7 +241,7 @@ function createHookTests(useProxies) { }) }) - describe("when draft is a Map -", () => { + describe.skip("when draft is a Map -", () => { test("delete", () => { const key1 = {prop: "val1"} const key2 = {prop: "val2"} @@ -268,7 +268,7 @@ function createHookTests(useProxies) { }) }) - describe("when draft is a Set -", () => { + describe.skip("when draft is a Set -", () => { test("delete", () => { produce({a: new Set([1])}, s => { s.a.delete(1) @@ -296,7 +296,7 @@ function createHookTests(useProxies) { expect(calls).toShallowEqual([base.a.b, base.a, base]) }) - it("is called in the right order for Maps", () => { + it.skip("is called in the right order for Maps", () => { const base = new Map([["a", new Map([["b", 0]])]]) produce(base, s => { s.get("a").delete("b") @@ -304,7 +304,7 @@ function createHookTests(useProxies) { expect(calls).toShallowEqual([base.get("a"), base]) }) - it("is called in the right order for Sets", () => { + it.skip("is called in the right order for Sets", () => { const item1 = {a: 0} const base = new Set([item1]) produce(base, s => { diff --git a/__tests__/map-set.js b/__tests__/map-set.js index 3b0f2f347..287d06135 100644 --- a/__tests__/map-set.js +++ b/__tests__/map-set.js @@ -57,7 +57,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) }) - test("#466 - setNoPatches", () => { + test.skip("#466 - setNoPatches", () => { const obj = { set: new Set() } @@ -72,7 +72,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ]) }) - test("#466 - mapChangeBug ", () => { + test.skip("#466 - mapChangeBug ", () => { const obj = { map: new Map([ ["a", new Map([["b", true], ["c", true], ["d", true]])], @@ -131,7 +131,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ]) }) - test("#466 - mapChangeBug2 ", () => { + test.skip("#466 - mapChangeBug2 ", () => { const obj = { map: new Map([ ["a", new Map([["b", true], ["c", true], ["d", true]])], diff --git a/__tests__/patch.js b/__tests__/patch.js index cdebb4b17..70ab1d0d6 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -247,7 +247,7 @@ describe("delete 5", () => { ) }) -describe("delete 6", () => { +describe.skip("delete 6", () => { runPatchTest( new Set(["x", 1]), d => { @@ -258,7 +258,7 @@ describe("delete 6", () => { ) }) -describe("delete 7", () => { +describe.skip("delete 7", () => { runPatchTest( {x: new Set(["y", 1])}, d => { @@ -581,7 +581,7 @@ describe("arrays - splice (shrink)", () => { ) }) -describe("sets - add - 1", () => { +describe.skip("sets - add - 1", () => { runPatchTest( new Set([1]), d => { @@ -592,7 +592,7 @@ describe("sets - add - 1", () => { ) }) -describe("sets - add, delete, add - 1", () => { +describe.skip("sets - add, delete, add - 1", () => { runPatchTest( new Set([1]), d => { @@ -605,7 +605,7 @@ describe("sets - add, delete, add - 1", () => { ) }) -describe("sets - add, delete, add - 2", () => { +describe.skip("sets - add, delete, add - 2", () => { runPatchTest( new Set([2, 1]), d => { @@ -618,7 +618,7 @@ describe("sets - add, delete, add - 2", () => { ) }) -describe("sets - mutate - 1", () => { +describe.skip("sets - mutate - 1", () => { const findById = (set, id) => { for (const item of set) { if (item.id === id) return item @@ -729,7 +729,7 @@ describe("same value replacement - 5", () => { ) }) -describe("same value replacement - 6", () => { +describe.skip("same value replacement - 6", () => { runPatchTest( new Set(["x", 3]), d => { diff --git a/__tests__/readme.js b/__tests__/readme.js index 8a1a9eb48..e08e73ae1 100644 --- a/__tests__/readme.js +++ b/__tests__/readme.js @@ -86,7 +86,7 @@ describe("readme example", () => { }) }) - it("can update set", () => { + it.skip("can update set", () => { const state = { title: "hello", tokenSet: new Set() diff --git a/src/common.ts b/src/common.ts index b8b0de80c..37c9c989b 100644 --- a/src/common.ts +++ b/src/common.ts @@ -146,8 +146,10 @@ export function each( export function each(obj, iter) { if (Array.isArray(obj) || isMap(obj) || isSet(obj)) { obj.forEach((entry, index) => iter(index, entry, obj)) - } else { + } else if (obj && typeof obj === "object") { ownKeys(obj).forEach(key => iter(key, obj[key], obj)) + } else { + throw new Error("Cannot iterate primitive " + obj) } } @@ -157,7 +159,9 @@ export function isEnumerable(base: {}, prop: PropertyKey): boolean { } export function has(thing: ObjectishNoSet, prop: PropertyKey): boolean { - return isMap(thing) + return !thing + ? false + : isMap(thing) ? thing.has(prop) : Object.prototype.hasOwnProperty.call(thing, prop) } diff --git a/src/es5.ts b/src/es5.ts index e04f51b7c..a02e28c99 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -62,12 +62,17 @@ export function willFinalize( } export function createProxy(base: T, parent: ES5State): ES5Draft { + const scope = parent ? parent.scope : ImmerScope.current! + if (isMap(base)) { + const draft = proxyMap(base, parent) as any // TODO: typefix + scope.drafts.push(draft) + return draft + } + const isArray = Array.isArray(base) const draft = clonePotentialDraft(base) - if (isMap(base)) { - proxyMap(draft) - } else if (isSet(base)) { + if (isSet(base)) { proxySet(draft) } else { each(draft, prop => { @@ -75,8 +80,6 @@ export function createProxy(base: T, parent: ES5State): ES5Draft { }) } - // See "proxy.js" for property documentation. - const scope = parent ? parent.scope : ImmerScope.current! const state: ES5State = { scope, modified: false, @@ -101,6 +104,7 @@ function revoke(this: ES5State) { this.revoked = true } +// TODO: type these methods // Access a property without creating an Immer draft. function peek(draft, prop) { const state = draft[DRAFT_STATE] @@ -125,7 +129,7 @@ function get(state, prop) { return value } -function set(state, prop, value) { +function set(state: ES5State, prop, value) { assertUnrevoked(state) state.assigned[prop] = true if (!state.modified) { @@ -136,7 +140,6 @@ function set(state, prop, value) { state.copy[prop] = value } -// TODO: kill export export function markChanged(state) { if (!state.modified) { state.modified = true @@ -145,7 +148,7 @@ export function markChanged(state) { } // TODO: kill export -export function prepareCopy(state) { +function prepareCopy(state) { if (!state.copy) state.copy = clonePotentialDraft(state.base) } diff --git a/src/immer.ts b/src/immer.ts index 6e2e89000..97c8cf2d1 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -1,5 +1,7 @@ +// TODO: destructure these, as the * as import makes it unclear which methods get installed on the Immer class import * as legacyProxy from "./es5" import * as modernProxy from "./proxy" + import {applyPatches, generatePatches} from "./patches" import { assign, @@ -15,7 +17,8 @@ import { shallowCopy, DRAFT_STATE, NOTHING, - freeze + freeze, + latest } from "./common" import {ImmerScope} from "./scope" import {ImmerState, IProduce, IProduceWithPatches, Objectish, PatchListener, Draft, Patch} from "./types" @@ -53,6 +56,7 @@ export class Immer implements ProducersFns { onCopy?: (state: ImmerState) => void createProxy!:(value: T, parent: any) => T; willFinalize!: (scope: ImmerScope, thing: any, isReplaced: boolean) => void; + markChanged!:(state: any) => void; // TODO: immerState? constructor(config?: { useProxies?: boolean @@ -334,9 +338,14 @@ export class Immer implements ProducersFns { finalizeTree(root: any, rootPath: string[] | null, scope: ImmerScope) { const state = root[DRAFT_STATE] if (state) { - if (!this.useProxies) { + // TODO: remove crap + // if (state.modified && !state.copy) { + // state.copy = shallowCopy(state.base, false) + // } + // else + if (!this.useProxies && !isMap(root)) { // Create the final copy, with added keys and without deleted keys. - state.copy = shallowCopy(state.draft, true) + state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? } root = state.copy } diff --git a/src/mapset.ts b/src/mapset.ts index c2fbfd57a..3756a3a8d 100644 --- a/src/mapset.ts +++ b/src/mapset.ts @@ -19,83 +19,212 @@ import { // TODO: kill: import { assertUnrevoked, - prepareCopy, - markChanged, // Looks to be the correct implementation for maps as well ES5Draft, - ES5State } from "./es5" +import { ImmerScope } from "./scope"; +import { Immer } from "./immer"; -export function proxyMap(target) { - Object.defineProperties(target, mapTraps) +// TODO: create own states +// TODO: clean up the maps and such from ES5 / Proxy states - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - // @ts-ignore TODO fix - proxyMethod(iterateMapValues) - ) +export interface MapState { + parent: any; // TODO: type + scope: ImmerScope; + modified: boolean; + finalizing: boolean; + finalized: boolean; + copy: Map | undefined; + assigned: Map | undefined; + base: Map; + revoke(): void; + draft: ES5Draft; +} + +function prepareCopy(state: MapState) { + if (!state.copy) { + state.assigned = new Map() + state.copy = new Map(state.base); } } -// TODO: eliminate these, and put in a Map superclass -const mapTraps = finalizeTraps({ - size: state => latest(state).size, - has: state => key => latest(state).has(key), - set: state => (key, value) => { +// Make sure DraftMap declarion doesn't die if Map is not avialable... +const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : function FakeMap() {} as any + +// TODO: fix types for drafts +export class DraftMap extends MapBase implements Map { + [DRAFT_STATE]: MapState + constructor(target, parent) { + super() + this[DRAFT_STATE] = { + parent, + scope: parent ? parent.scope : ImmerScope.current, + modified: false, + finalized: false, + finalizing: false, + copy: undefined, + assigned: undefined, + base: target, + draft: this as any, // TODO: fix typing + revoke() { + // TODO: make sure this marks the Map as revoked, and assert everywhere + } + }; + } + + get size(): number { + return latest(this[DRAFT_STATE]).size + } + + has(key: K): boolean { + return latest(this[DRAFT_STATE]).has(key) + } + + set(key: K, value: V): this { + const state = this[DRAFT_STATE]; if (latest(state).get(key) !== value) { prepareCopy(state) - markChanged(state) - state.assigned.set(key, true) - state.copy.set(key, value) + state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly + state.assigned!.set(key, true) + state.copy!.set(key, value) + state.assigned!.set(key, true); } - return state.draft - }, - delete: state => key => { - prepareCopy(state) - markChanged(state) - state.assigned.set(key, false) - state.copy.delete(key) - return false - }, - clear: state => () => { - if (!state.copy) { - prepareCopy(state) + return this + } + + delete(key: K): boolean { + if (!this.has(key)) { + return false; } - markChanged(state) + + const state = this[DRAFT_STATE]; + prepareCopy(state) + state.scope.immer.markChanged(state) + state.assigned!.set(key, false) + state.copy!.delete(key) + return true + } + + clear() { + const state = this[DRAFT_STATE]; + prepareCopy(state) + state.scope.immer.markChanged(state) state.assigned = new Map() for (const key of latest(state).keys()) { state.assigned.set(key, false) } - return state.copy.clear() - }, + return state.copy!.clear() + } + // @ts-ignore TODO: - forEach: (state, key, reciever) => cb => { - latest(state).forEach((value, key, map) => { - cb(reciever.get(key), key, map) + forEach(cb) { + const state = this[DRAFT_STATE]; + latest(state).forEach((_value, key, _map) => { + cb(this.get(key), key, this) }) - }, - get: state => key => { - const value = latest(state).get(key) + } + get(key: K): V /* TODO: Draft */ { + const state = this[DRAFT_STATE]; + const value = latest(state).get(key) if (state.finalizing || state.finalized || !isDraftable(value)) { return value } - if (value !== state.base.get(key)) { - return value + return value // either already drafted or reassigned } const draft = state.scope.immer.createProxy(value, state) prepareCopy(state) - state.copy.set(key, draft) + state.copy!.set(key, draft) return draft - }, - keys: state => () => latest(state).keys(), + } + + keys() { + return latest(this[DRAFT_STATE]).keys(); + } + + // TODO: values and entries iterators // @ts-ignore TODO: - values: iterateMapValues, + // values: iterateMapValues, // @ts-ignore TODO: - entries: iterateMapValues -}) + // entries: iterateMapValues +} + + +export function proxyMap(target, parent) { + if (target instanceof DraftMap) { + return target; // TODO: or copy? + } + return new DraftMap(target, parent) + // Object.defineProperties(target, mapTraps) + + // if (hasSymbol) { + // Object.defineProperty( + // target, + // Symbol.iterator, + // // @ts-ignore TODO fix + // proxyMethod(iterateMapValues) + // ) + // } +} + +// TODO: eliminate these, and put in a Map superclass +// const mapTraps = finalizeTraps({ +// size: state => latest(state).size, +// has: state => key => latest(state).has(key), +// set: state => (key, value) => { +// if (latest(state).get(key) !== value) { +// prepareCopy(state) +// markChanged(state) +// state.assigned.set(key, true) +// state.copy.set(key, value) +// } +// return state.draft +// }, +// delete: state => key => { +// prepareCopy(state) +// markChanged(state) +// state.assigned.set(key, false) +// state.copy.delete(key) +// return false +// }, +// clear: state => () => { +// if (!state.copy) { +// prepareCopy(state) +// } +// markChanged(state) +// state.assigned = new Map() +// for (const key of latest(state).keys()) { +// state.assigned.set(key, false) +// } +// return state.copy.clear() +// }, +// // @ts-ignore TODO: +// forEach: (state, key, reciever) => cb => { +// latest(state).forEach((value, key, map) => { +// cb(reciever.get(key), key, map) +// }) +// }, +// get: state => key => { +// const value = latest(state).get(key) + +// if (state.finalizing || state.finalized || !isDraftable(value)) { +// return value +// } + +// if (value !== state.base.get(key)) { +// return value +// } +// const draft = state.scope.immer.createProxy(value, state) +// prepareCopy(state) +// state.copy.set(key, draft) +// return draft +// }, +// keys: state => () => latest(state).keys(), +// // @ts-ignore TODO: +// values: iterateMapValues, +// // @ts-ignore TODO: +// entries: iterateMapValues +// }) export function proxySet(target) { Object.defineProperties(target, setTraps) @@ -118,7 +247,7 @@ const setTraps = finalizeTraps({ }, add: state => value => { if (!latest(state).has(value)) { - markChanged(state) + state.scope.immer.markChanged(state) if (!state.copy) { prepareCopy(state) } @@ -127,7 +256,7 @@ const setTraps = finalizeTraps({ return state.draft }, delete: state => value => { - markChanged(state) + state.scope.immer.markChanged(state) if (!state.copy) { prepareCopy(state) } @@ -137,7 +266,7 @@ const setTraps = finalizeTraps({ return latest(state).has(key) }, clear: state => () => { - markChanged(state) + state.scope.immer.markChanged(state) if (!state.copy) { prepareCopy(state) } @@ -156,7 +285,7 @@ const setTraps = finalizeTraps({ } }) -function finalizeTraps(traps: {[prop: string]: (state: ES5State) => Function }) { +function finalizeTraps(traps: {[prop: string]: (state: any) => Function }) { return Object.keys(traps).reduce(function(acc, key) { const builder = key === "size" ? proxyAttr : proxyMethod acc[key] = builder(traps[key], key) diff --git a/src/patches.ts b/src/patches.ts index 3a347d561..e19c76e03 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -86,7 +86,7 @@ function generatePatchesFromAssigned( inversePatches: Patch[] ) { const {base, copy} = state - each(state.assigned, (key, assignedValue) => { + if (state.assigned) each(state.assigned, (key, assignedValue) => { const origValue = get(base, key) const value = get(copy, key) const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" diff --git a/src/proxy.ts b/src/proxy.ts index 42edff4ba..b13682b3d 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -20,6 +20,7 @@ import { latest } from "./common" import {ImmerScope} from "./scope" +import { proxyMap } from "./mapset" // Do nothing before being finalized. export function willFinalize() {} @@ -57,6 +58,13 @@ export function createProxy( parent: ES6State ): ES6Draft { const scope = parent ? parent.scope : ImmerScope.current! + + if (isMap(base)) { + const draft = proxyMap(base, parent) as any // TODO: typefix + scope.drafts.push(draft); + return draft; + } + const state: ES6State = { // Track which produce call this is associated with. scope, @@ -92,12 +100,7 @@ export function createProxy( target = [state] as any traps = arrayTraps } - // Map drafts must support object keys, so we use Map objects to track changes. - else if (isMap(base)) { - traps = mapTraps - state.drafts = new Map() - state.assigned = new Map() - } + // Set drafts use a Map object to track which of its values are drafted. // And we don't need the "assigned" property, because Set objects have no keys. else if (isSet(base)) { @@ -158,6 +161,7 @@ const objectTraps: ProxyHandler = { ? is(baseValue, value) || value === state.drafts[prop] : is(baseValue, value) && prop in state.base if (isUnchanged) return true + prepareCopy(state) markChanged(state) } state.assigned[prop] = true @@ -168,6 +172,7 @@ const objectTraps: ProxyHandler = { // The `undefined` check is a fast path for pre-existing keys. if (peek(state.base, prop) !== undefined || prop in state.base) { state.assigned[prop] = false + prepareCopy(state) markChanged(state) } else if (state.assigned[prop]) { // if an originally not assigned property was deleted @@ -368,31 +373,31 @@ function peek(draft, prop) { return desc && desc.value } -function markChanged(state) { +// TODO: unify with ES5 version, by getting rid of the drafts vs copy distinction? +export function markChanged(state) { if (!state.modified) { state.modified = true - const {base, drafts, parent} = state - const copy = shallowCopy(base) - - if (isSet(base)) { - // Note: The `drafts` property is preserved for Set objects, since - // we need to keep track of which values are drafted. - assignSet(copy, drafts) - } else { - // Merge nested drafts into the copy. - if (isMap(base)) assignMap(copy, drafts) - else assign(copy, drafts) + if (!isMap(base) && !isSet(base)) { + // TODO: drop creating copies here? + const copy = state.copy = shallowCopy(base) + assign(copy, drafts) state.drafts = null } - state.copy = copy if (parent) { markChanged(parent) } } } +// TODO: unify with ES5 version +function prepareCopy(state) { + if (!state.copy) { + state.copy = shallowCopy(state.base) + } +} + /** Create traps that all use the `Reflect` API on the `latest(state)` */ function makeReflectTraps( names: (keyof typeof Reflect)[] diff --git a/tsconfig.json b/tsconfig.json index 3076ab622..c0cd58280 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,6 +2,8 @@ "include": ["src"], "compilerOptions": { "lib": ["es2015"], + // TODO: change to ES5 and make sure maps still work + "target": "ES6", "strict": true, "declaration": true, "importHelpers": false, From 08f4682afb8937cfdf8eb84047c52369cb4632bd Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sun, 5 Jan 2020 16:22:50 +0000 Subject: [PATCH 05/28] added set and iteror stuff --- src/common.ts | 2 + src/es5.ts | 60 +++++++++++-- src/immer.ts | 2 +- src/{mapset.ts => map.ts} | 176 +++----------------------------------- src/proxy.ts | 2 +- src/set.ts | 174 +++++++++++++++++++++++++++++++++++++ 6 files changed, 243 insertions(+), 173 deletions(-) rename src/{mapset.ts => map.ts} (51%) create mode 100644 src/set.ts diff --git a/src/common.ts b/src/common.ts index 37c9c989b..e08cee1ae 100644 --- a/src/common.ts +++ b/src/common.ts @@ -105,6 +105,7 @@ export const ownKeys: (target) => PropertyKey[] = ) as any) : Object.getOwnPropertyNames +// TODO: duplicate of clone? TODO: should be used by prepareCopy of map / set export function shallowCopy( base: T, invokeGetters?: boolean @@ -257,6 +258,7 @@ export function latest(state: any): any { return state.copy || state.base } +// TODO: duplicate of shallow clone export function clone(obj: T): T export function clone(obj) { if (!isDraftable(obj)) return obj diff --git a/src/es5.ts b/src/es5.ts index a02e28c99..13ab308b5 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -16,7 +16,9 @@ import { makeIterateSetValues, latest } from "./common" -import {proxyMap, proxySet, hasMapChanges, hasSetChanges} from "./mapset" +import {proxyMap, hasMapChanges} from "./map" +import {proxySet, hasSetChanges} from "./set" + import {ImmerScope} from "./scope" import {ImmerState} from "./types" @@ -68,17 +70,18 @@ export function createProxy(base: T, parent: ES5State): ES5Draft { scope.drafts.push(draft) return draft } + if (isSet(base)) { + const draft = proxySet(base, parent) as any // TODO: typefix + scope.drafts.push(draft) + return draft + } const isArray = Array.isArray(base) const draft = clonePotentialDraft(base) - if (isSet(base)) { - proxySet(draft) - } else { - each(draft, prop => { - proxyProperty(draft, prop, isArray || isEnumerable(base, prop)) - }) - } + each(draft, prop => { + proxyProperty(draft, prop, isArray || isEnumerable(base, prop)) + }) const state: ES5State = { scope, @@ -216,24 +219,63 @@ function markChangesSweep(drafts) { } } +// TODO: refactor this to work per object-type +// TODO: Set / Map shouldn't be ES specific function markChangesRecursively(object) { if (!object || typeof object !== "object") return const state = object[DRAFT_STATE] if (!state) return const {base, draft, assigned} = state - if (!Array.isArray(object)) { + if (isSet(object)) { + if (hasSetChanges(state)) { + markChanged(state) + object.forEach(v => { + markChangesRecursively(v) + }) + } + } else if (isMap(object)) { + // if (hasMapChanges(object)) { + object.forEach((value, key) => { + if (assigned && base.get(key) === undefined && !has(base, key)) { + // TODO: this code seems invalid for Maps! + assigned.set(key, true) + markChanged(state) + } else if (!assigned || !assigned.get(key)) { + // TODO: === false? + // Only untouched properties trigger recursion. + markChangesRecursively(draft.get(key)) + } + }) + // Look for removed keys. + // TODO: is this code needed? + // TODO: use each? + if (assigned) + base.forEach((value, key) => { + // The `undefined` check is a fast path for pre-existing keys. + if (draft.get(key) === undefined && !has(draft, key)) { + assigned.set(key, false) + markChanged(state) + } + }) + // } + } else if (!Array.isArray(object)) { // Look for added keys. + // TODO: use each? Object.keys(draft).forEach(key => { // The `undefined` check is a fast path for pre-existing keys. if (base[key] === undefined && !has(base, key)) { + // TODO: this code seems invalid for Maps! assigned[key] = true markChanged(state) } else if (!assigned[key]) { + // TODO: === false ? // Only untouched properties trigger recursion. markChangesRecursively(draft[key]) } }) // Look for removed keys. + // TODO: is this code needed? + // TODO: use each? Object.keys(base).forEach(key => { // The `undefined` check is a fast path for pre-existing keys. if (draft[key] === undefined && !has(draft, key)) { diff --git a/src/immer.ts b/src/immer.ts index 97c8cf2d1..8a0862926 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -343,7 +343,7 @@ export class Immer implements ProducersFns { // state.copy = shallowCopy(state.base, false) // } // else - if (!this.useProxies && !isMap(root)) { + if (!this.useProxies && !isMap(root) && !isSet(root)) { // Create the final copy, with added keys and without deleted keys. state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? } diff --git a/src/mapset.ts b/src/map.ts similarity index 51% rename from src/mapset.ts rename to src/map.ts index 3756a3a8d..1fdf97398 100644 --- a/src/mapset.ts +++ b/src/map.ts @@ -142,14 +142,24 @@ export class DraftMap extends MapBase implements Map { return latest(this[DRAFT_STATE]).keys(); } + // TODO: make these normal functions // TODO: values and entries iterators // @ts-ignore TODO: - // values: iterateMapValues, + values = iterateMapValues // @ts-ignore TODO: - // entries: iterateMapValues + entries = iterateMapValues } +if (hasSymbol) { + Object.defineProperty( + DraftMap.prototype, + Symbol.iterator, + // @ts-ignore TODO fix + proxyMethod(iterateMapValues) + ) +} + export function proxyMap(target, parent) { if (target instanceof DraftMap) { return target; // TODO: or copy? @@ -157,152 +167,9 @@ export function proxyMap(target, parent) { return new DraftMap(target, parent) // Object.defineProperties(target, mapTraps) - // if (hasSymbol) { - // Object.defineProperty( - // target, - // Symbol.iterator, - // // @ts-ignore TODO fix - // proxyMethod(iterateMapValues) - // ) - // } -} - -// TODO: eliminate these, and put in a Map superclass -// const mapTraps = finalizeTraps({ -// size: state => latest(state).size, -// has: state => key => latest(state).has(key), -// set: state => (key, value) => { -// if (latest(state).get(key) !== value) { -// prepareCopy(state) -// markChanged(state) -// state.assigned.set(key, true) -// state.copy.set(key, value) -// } -// return state.draft -// }, -// delete: state => key => { -// prepareCopy(state) -// markChanged(state) -// state.assigned.set(key, false) -// state.copy.delete(key) -// return false -// }, -// clear: state => () => { -// if (!state.copy) { -// prepareCopy(state) -// } -// markChanged(state) -// state.assigned = new Map() -// for (const key of latest(state).keys()) { -// state.assigned.set(key, false) -// } -// return state.copy.clear() -// }, -// // @ts-ignore TODO: -// forEach: (state, key, reciever) => cb => { -// latest(state).forEach((value, key, map) => { -// cb(reciever.get(key), key, map) -// }) -// }, -// get: state => key => { -// const value = latest(state).get(key) - -// if (state.finalizing || state.finalized || !isDraftable(value)) { -// return value -// } - -// if (value !== state.base.get(key)) { -// return value -// } -// const draft = state.scope.immer.createProxy(value, state) -// prepareCopy(state) -// state.copy.set(key, draft) -// return draft -// }, -// keys: state => () => latest(state).keys(), -// // @ts-ignore TODO: -// values: iterateMapValues, -// // @ts-ignore TODO: -// entries: iterateMapValues -// }) - -export function proxySet(target) { - Object.defineProperties(target, setTraps) - - if (hasSymbol) { - Object.defineProperty( - target, - Symbol.iterator, - // @ts-ignore TODO - proxyMethod(iterateSetValues) - ) - } -} - -const iterateSetValues = makeIterateSetValues() - -const setTraps = finalizeTraps({ - size: state => { - return latest(state).size - }, - add: state => value => { - if (!latest(state).has(value)) { - state.scope.immer.markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - state.copy.add(value) - } - return state.draft - }, - delete: state => value => { - state.scope.immer.markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.delete(value) - }, - has: state => key => { - return latest(state).has(key) - }, - clear: state => () => { - state.scope.immer.markChanged(state) - if (!state.copy) { - prepareCopy(state) - } - return state.copy.clear() - }, - keys: iterateSetValues, - entries: iterateSetValues, - values: iterateSetValues, - forEach: state => (cb, thisArg) => { - const iterator = iterateSetValues(state)() - let result = iterator.next() - while (!result.done) { - cb.call(thisArg, result.value, result.value, state.draft) - result = iterator.next() - } - } -}) - -function finalizeTraps(traps: {[prop: string]: (state: any) => Function }) { - return Object.keys(traps).reduce(function(acc, key) { - const builder = key === "size" ? proxyAttr : proxyMethod - acc[key] = builder(traps[key], key) - return acc - }, {}) -} - -function proxyAttr(fn) { - return { - get() { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return fn(state) - } - } } +// TODO: kill? function proxyMethod(trap, key) { return { get() { @@ -315,6 +182,7 @@ function proxyMethod(trap, key) { } } +// TODO: used? export function hasMapChanges(state) { const {base, draft} = state @@ -330,19 +198,3 @@ export function hasMapChanges(state) { }) return hasChanges } - -export function hasSetChanges(state) { - const {base, draft} = state - - if (base.size !== draft.size) return true - - // IE11 supports only forEach iteration - let hasChanges = false - // TODO: optimize: break on first diff - draft.forEach(function(value, key) { - if (!hasChanges) { - hasChanges = isDraftable(value) ? value.modified : !base.has(key) - } - }) - return hasChanges -} diff --git a/src/proxy.ts b/src/proxy.ts index b13682b3d..7e122cc97 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -20,7 +20,7 @@ import { latest } from "./common" import {ImmerScope} from "./scope" -import { proxyMap } from "./mapset" +import { proxyMap } from "./map" // Do nothing before being finalized. export function willFinalize() {} diff --git a/src/set.ts b/src/set.ts new file mode 100644 index 000000000..9ef913633 --- /dev/null +++ b/src/set.ts @@ -0,0 +1,174 @@ +import { + each, + has, + is, + isDraft, + isDraftable, + isEnumerable, + isMap, + isSet, + hasSymbol, + shallowCopy, + DRAFT_STATE, + iterateMapValues, + makeIterable, + makeIterateSetValues, + latest +} from "./common" + +// TODO: kill: +import { + assertUnrevoked, + ES5Draft, +} from "./es5" +import { ImmerScope } from "./scope"; +import { Immer } from "./immer"; + +// TODO: create own states +// TODO: clean up the maps and such from ES5 / Proxy states + +export interface SetState { + parent: any; // TODO: type + scope: ImmerScope; + modified: boolean; + finalizing: boolean; + finalized: boolean; + copy: Set | undefined; + // assigned: Map | undefined; + base: Set; + revoke(): void; + draft: ES5Draft; +} + +function prepareCopy(state: SetState) { + if (!state.copy) { + state.copy = new Set(state.base); + } +} + +// Make sure DraftSet declarion doesn't die if Map is not avialable... +const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : function FakeSet() {} as any + +// TODO: fix types for drafts +export class DraftSet extends SetBase implements Set { + [DRAFT_STATE]: SetState + constructor(target, parent) { + super() + this[DRAFT_STATE] = { + parent, + scope: parent ? parent.scope : ImmerScope.current, + modified: false, + finalized: false, + finalizing: false, + copy: undefined, + base: target, + draft: this as any, // TODO: fix typing + revoke() { + // TODO: make sure this marks the Map as revoked, and assert everywhere + } + }; + } + + get size(): number { + return latest(this[DRAFT_STATE]).size + } + + add(value: V): this { + const state = this[DRAFT_STATE]; + if (latest(state).has(value) !== value) { + prepareCopy(state) + state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly + state.copy!.add(value) + } + return this + } + + delete(value: V): boolean { + if (!this.has(value)) { + return false; + } + + const state = this[DRAFT_STATE]; + prepareCopy(state) + state.scope.immer.markChanged(state) + state.copy!.delete(value) + return true + } + + clear() { + const state = this[DRAFT_STATE]; + prepareCopy(state) + state.scope.immer.markChanged(state) + return state.copy!.clear() + } + + keys() { + return latest(this[DRAFT_STATE]).keys(); + } + + // TODO: values and entries iterators + // @ts-ignore TODO: + entries = iterateSetValues + // values: iterateMapValues, + // @ts-ignore TODO: + values= iterateSetValues + // entries: iterateMapValues + + forEach(cb, thisArg) { + const state = this[DRAFT_STATE] + const iterator = iterateSetValues(state)() + let result = iterator.next() + while (!result.done) { + cb.call(thisArg, result.value, result.value, state.draft) + result = iterator.next() + } + } +} + + +export function proxySet(target, parent) { + if (target instanceof DraftSet) { + return target; // TODO: or copy? + } + return new DraftSet(target, parent) +} + +if (hasSymbol) { + Object.defineProperty( + DraftSet.prototype, + Symbol.iterator, + // @ts-ignore TODO fix + proxyMethod(iterateMapValues) + ) +} + +const iterateSetValues = makeIterateSetValues() + +// TODO: kill? +function proxyMethod(trap, key) { + return { + get() { + return function(this: ES5Draft, ...args) { + const state = this[DRAFT_STATE] + assertUnrevoked(state) + return trap(state, key, state.draft)(...args) + } + } + } +} + +export function hasSetChanges(state) { + const {base, draft} = state + + if (base.size !== draft.size) return true + + // IE11 supports only forEach iteration + let hasChanges = false + // TODO: optimize: break on first diff + draft.forEach(function(value, key) { + if (!hasChanges) { + hasChanges = isDraftable(value) ? value.modified : !base.has(key) + } + }) + return hasChanges +} From 385c0a44c8c28fbdda2c20bbcb87d46ed45acdd9 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sun, 5 Jan 2020 19:55:19 +0000 Subject: [PATCH 06/28] fixed most tests --- __tests__/base.js | 15 ++++----- __tests__/frozen.js | 2 +- __tests__/hooks.js | 10 +++--- __tests__/map-set.js | 4 +-- __tests__/patch.js | 10 +++--- __tests__/readme.js | 2 +- src/common.ts | 73 +++++++++++++------------------------------ src/es5.ts | 5 ++- src/immer.ts | 3 +- src/map.ts | 66 ++++++++++++++++++++++++++++++--------- src/patches.ts | 1 + src/proxy.ts | 68 ++++++++-------------------------------- src/set.ts | 74 +++++++++++++++++++++----------------------- 13 files changed, 150 insertions(+), 183 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index 6a0a386c2..fd8336c17 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -319,7 +319,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(nextState).toBe(base) }) - it.skip("supports iteration", () => { + it("supports iteration", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -341,7 +341,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(result.get("second").a).toEqual(2) }) - it.skip("supports 'entries'", () => { + it("supports 'entries'", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -363,7 +363,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(result.get("second").a).toEqual(2) }) - it.skip("supports 'values'", () => { + it("supports 'values'", () => { const base = new Map([ ["first", {id: 1, a: 1}], ["second", {id: 2, a: 1}] @@ -545,7 +545,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) }) - describe.skip("set drafts", () => { + describe("set drafts", () => { it("supports iteration", () => { const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) const findById = (set, id) => { @@ -677,13 +677,14 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { it("can use 'delete' to remove items", () => { const nextState = produce(baseState, s => { expect(s.aSet.has("Luke")).toBe(true) - s.aSet.delete("Luke") + expect(s.aSet.delete("Luke")).toBe(true) + expect(s.aSet.delete("Luke")).toBe(false) expect(s.aSet.has("Luke")).toBe(false) }) expect(nextState.aSet).not.toBe(baseState.aSet) - expect(nextState.aSet.size).toBe(baseState.aSet.size - 1) expect(baseState.aSet.has("Luke")).toBe(true) expect(nextState.aSet.has("Luke")).toBe(false) + expect(nextState.aSet.size).toBe(baseState.aSet.size - 1) }) it("can use 'clear' to remove items", () => { @@ -1600,7 +1601,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) }) - describe.skip(`complex nesting map / set / object`, () => { + describe(`complex nesting map / set / object`, () => { const a = {a: 1} const b = {b: 2} const c = {c: 3} diff --git a/__tests__/frozen.js b/__tests__/frozen.js index bf764d2f3..e7b4ef4a8 100644 --- a/__tests__/frozen.js +++ b/__tests__/frozen.js @@ -156,7 +156,7 @@ function runTests(name, useProxies) { expect(produce(res, d => void d.set("a", 2))).not.toBe(res) }) - it.skip("will freeze sets", () => { + it("will freeze sets", () => { const base = new Set() const res = produce(base, draft => { base.add(1) diff --git a/__tests__/hooks.js b/__tests__/hooks.js index dce2c52fc..584ec8b7c 100644 --- a/__tests__/hooks.js +++ b/__tests__/hooks.js @@ -120,7 +120,7 @@ function createHookTests(useProxies) { }) }) - describe.skip("when draft is a Map", () => { + describe("when draft is a Map", () => { test("assign", () => { const key1 = {prop: "val1"} const key2 = {prop: "val2"} @@ -241,7 +241,7 @@ function createHookTests(useProxies) { }) }) - describe.skip("when draft is a Map -", () => { + describe("when draft is a Map -", () => { test("delete", () => { const key1 = {prop: "val1"} const key2 = {prop: "val2"} @@ -268,7 +268,7 @@ function createHookTests(useProxies) { }) }) - describe.skip("when draft is a Set -", () => { + describe("when draft is a Set -", () => { test("delete", () => { produce({a: new Set([1])}, s => { s.a.delete(1) @@ -296,7 +296,7 @@ function createHookTests(useProxies) { expect(calls).toShallowEqual([base.a.b, base.a, base]) }) - it.skip("is called in the right order for Maps", () => { + it("is called in the right order for Maps", () => { const base = new Map([["a", new Map([["b", 0]])]]) produce(base, s => { s.get("a").delete("b") @@ -304,7 +304,7 @@ function createHookTests(useProxies) { expect(calls).toShallowEqual([base.get("a"), base]) }) - it.skip("is called in the right order for Sets", () => { + it("is called in the right order for Sets", () => { const item1 = {a: 0} const base = new Set([item1]) produce(base, s => { diff --git a/__tests__/map-set.js b/__tests__/map-set.js index 287d06135..6cff982ac 100644 --- a/__tests__/map-set.js +++ b/__tests__/map-set.js @@ -57,7 +57,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) }) - test.skip("#466 - setNoPatches", () => { + test("#466 - setNoPatches", () => { const obj = { set: new Set() } @@ -72,7 +72,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ]) }) - test.skip("#466 - mapChangeBug ", () => { + test("#466 - mapChangeBug ", () => { const obj = { map: new Map([ ["a", new Map([["b", true], ["c", true], ["d", true]])], diff --git a/__tests__/patch.js b/__tests__/patch.js index 70ab1d0d6..da8b44ffa 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -247,7 +247,7 @@ describe("delete 5", () => { ) }) -describe.skip("delete 6", () => { +describe("delete 6", () => { runPatchTest( new Set(["x", 1]), d => { @@ -258,7 +258,7 @@ describe.skip("delete 6", () => { ) }) -describe.skip("delete 7", () => { +describe("delete 7", () => { runPatchTest( {x: new Set(["y", 1])}, d => { @@ -581,7 +581,7 @@ describe("arrays - splice (shrink)", () => { ) }) -describe.skip("sets - add - 1", () => { +describe("sets - add - 1", () => { runPatchTest( new Set([1]), d => { @@ -592,7 +592,7 @@ describe.skip("sets - add - 1", () => { ) }) -describe.skip("sets - add, delete, add - 1", () => { +describe("sets - add, delete, add - 1", () => { runPatchTest( new Set([1]), d => { @@ -605,7 +605,7 @@ describe.skip("sets - add, delete, add - 1", () => { ) }) -describe.skip("sets - add, delete, add - 2", () => { +describe("sets - add, delete, add - 2", () => { runPatchTest( new Set([2, 1]), d => { diff --git a/__tests__/readme.js b/__tests__/readme.js index e08e73ae1..8a1a9eb48 100644 --- a/__tests__/readme.js +++ b/__tests__/readme.js @@ -86,7 +86,7 @@ describe("readme example", () => { }) }) - it.skip("can update set", () => { + it("can update set", () => { const state = { title: "hello", tokenSet: new Set() diff --git a/src/common.ts b/src/common.ts index e08cee1ae..72fe99836 100644 --- a/src/common.ts +++ b/src/common.ts @@ -194,6 +194,14 @@ export function isSet(target): target is Set { return hasSet && target instanceof Set } + +export function makeIterable2(baseIterator: Iterator): IterableIterator { + // TODO: correct? + // TODO: don't use symbol + baseIterator[Symbol.iterator] = () => baseIterator + return baseIterator as any +} + export function makeIterable(next: () => {done: boolean; value: any}) { let self return (self = { @@ -202,62 +210,23 @@ export function makeIterable(next: () => {done: boolean; value: any}) { }) } -/** Map.prototype.values _-or-_ Map.prototype.entries */ -export function iterateMapValues(state, prop, receiver) { - const isEntries = prop !== "values" - return () => { - const iterator = latest(state)[Symbol.iterator]() - return makeIterable(() => { - const result = iterator.next() - if (!result.done) { - const [key] = result.value - const value = receiver.get(key) - result.value = isEntries ? [key, value] : value - } - return result - }) - } -} - -export function makeIterateSetValues() { - function iterateSetValues(state, prop?) { - const isEntries = prop === "entries" - return () => { - const iterator = latest(state)[Symbol.iterator]() - return makeIterable(() => { - const result = iterator.next() - if (!result.done) { - const value = wrapSetValue(state, result.value) - result.value = isEntries ? [value, value] : value - } - return result - }) - } - } - - function wrapSetValue(state, value) { - const key = original(value) || value - let draft = state.drafts.get(key) - if (!draft) { - if (state.finalized || !isDraftable(value) || state.finalizing) { - return value - } - draft = state.scope.immer.createProxy(value, state) - state.drafts.set(key, draft) - if (state.modified) { - state.copy.add(draft) - } - } - return draft - } - - return iterateSetValues -} - export function latest(state: any): any { return state.copy || state.base } +// export function iterateCollection(collection: Map | Set, transformer: (key, value) => any) { +// // TODO: create own iterator symbols for fallback +// const iterator = collection.entries() +// return makeIterable(() => { +// const result = iterator.next() +// if (!result.done) { +// const value = wrapSetValue(state, result.value) +// result.value = isEntries ? [value, value] : value +// } +// return result +// } as any) +// } + // TODO: duplicate of shallow clone export function clone(obj: T): T export function clone(obj) { diff --git a/src/es5.ts b/src/es5.ts index 13ab308b5..d952ecb37 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -13,7 +13,6 @@ import { DRAFT_STATE, iterateMapValues, makeIterable, - makeIterateSetValues, latest } from "./common" import {proxyMap, hasMapChanges} from "./map" @@ -64,6 +63,10 @@ export function willFinalize( } export function createProxy(base: T, parent: ES5State): ES5Draft { + if (!base || typeof base !== "object" || !isDraftable(base)) { + // TODO: || isDraft ? + return base as any // TODO: fix + } const scope = parent ? parent.scope : ImmerScope.current! if (isMap(base)) { const draft = proxyMap(base, parent) as any // TODO: typefix diff --git a/src/immer.ts b/src/immer.ts index 8a0862926..359503528 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -347,6 +347,7 @@ export class Immer implements ProducersFns { // Create the final copy, with added keys and without deleted keys. state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? } + root = state.copy } @@ -379,7 +380,7 @@ export class Immer implements ProducersFns { } // Unchanged drafts are never passed to the `onAssign` hook. - if (isDraftProp && value === get(state.base, prop)) return + if (isDraftProp && !isSetMember && value === get(state.base, prop)) return } // Unchanged draft properties are ignored. else if (isDraftProp && is(value, get(state.base, prop))) { diff --git a/src/map.ts b/src/map.ts index 1fdf97398..e46d914f5 100644 --- a/src/map.ts +++ b/src/map.ts @@ -10,9 +10,7 @@ import { hasSymbol, shallowCopy, DRAFT_STATE, - iterateMapValues, makeIterable, - makeIterateSetValues, latest } from "./common" @@ -132,6 +130,7 @@ export class DraftMap extends MapBase implements Map { if (value !== state.base.get(key)) { return value // either already drafted or reassigned } + // despite what it looks, this creates a draft only once, see above condition const draft = state.scope.immer.createProxy(value, state) prepareCopy(state) state.copy!.set(key, draft) @@ -142,24 +141,61 @@ export class DraftMap extends MapBase implements Map { return latest(this[DRAFT_STATE]).keys(); } - // TODO: make these normal functions - // TODO: values and entries iterators - // @ts-ignore TODO: - values = iterateMapValues - // @ts-ignore TODO: - entries = iterateMapValues + values() { + const iterator = this.keys() + return { + [Symbol.iterator]: () => this.values(), // TODO: don't use symbol directly + next: () => { + const r = iterator.next() + if (r.done) return r; + const value = this.get(r.value); + return { + done: false, value + } + } + } as any + } + + entries() { + const iterator = this.keys() + return { + [Symbol.iterator]: () => this.entries(), // TODO: don't use symbol directly + next: () => { + const r = iterator.next() + if (r.done) return r; + const value = this.get(r.value); + return { + done: false, value: [r.value, value] + } + } + } as any + } + + [Symbol.iterator]() { // TODO: don't use symbol directly + return this.entries() + } } -if (hasSymbol) { - Object.defineProperty( - DraftMap.prototype, - Symbol.iterator, - // @ts-ignore TODO fix - proxyMethod(iterateMapValues) - ) +/** Map.prototype.values _-or-_ Map.prototype.entries */ +export function iterateMapValues(state, prop, receiver) { + const isEntries = prop !== "values" + return () => { + const iterator = latest(state)[Symbol.iterator]() + return makeIterable(() => { + const result = iterator.next() + if (!result.done) { + const [key] = result.value + const value = receiver.get(key) + result.value = isEntries ? [key, value] : value + } + return result + }) + } } + + export function proxyMap(target, parent) { if (target instanceof DraftMap) { return target; // TODO: or copy? diff --git a/src/patches.ts b/src/patches.ts index e19c76e03..36f293e5a 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -109,6 +109,7 @@ function generateSetPatches( patches: Patch[], inversePatches: Patch[] ) { + // TODO: if this doesn't use assigned, drop assigned from SetState stuff let {base, copy} = state let i = 0 diff --git a/src/proxy.ts b/src/proxy.ts index 7e122cc97..7020e8cdb 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -16,11 +16,11 @@ import { assignSet, original, iterateMapValues, - makeIterateSetValues, latest } from "./common" import {ImmerScope} from "./scope" import { proxyMap } from "./map" +import { proxySet } from "./set" // Do nothing before being finalized. export function willFinalize() {} @@ -57,13 +57,25 @@ export function createProxy( base: T, parent: ES6State ): ES6Draft { + // TODO: dedupe + if (!base || typeof base !== "object" || !isDraftable(base)) { + // TODO: || isDraft ? + return base as any // TODO: fix + } + const scope = parent ? parent.scope : ImmerScope.current! + // TODO: dedupe this, it is the same for ES5 if (isMap(base)) { const draft = proxyMap(base, parent) as any // TODO: typefix scope.drafts.push(draft); return draft; } + if (isSet(base)) { + const draft = proxySet(base, parent) as any // TODO: typefix + scope.drafts.push(draft); + return draft; + } const state: ES6State = { // Track which produce call this is associated with. @@ -101,13 +113,6 @@ export function createProxy( traps = arrayTraps } - // Set drafts use a Map object to track which of its values are drafted. - // And we don't need the "assigned" property, because Set objects have no keys. - else if (isSet(base)) { - traps = setTraps - state.drafts = new Map() - } - const {revoke, proxy} = Proxy.revocable(target, traps) state.draft = proxy @@ -316,53 +321,6 @@ const mapTraps = makeTrapsForGetters>({ [hasSymbol ? Symbol.iterator : "@@iterator"]: iterateMapValues }) -const iterateSetValues = makeIterateSetValues() -/** - * Set drafts - */ - -const setTraps = makeTrapsForGetters>({ - //@ts-ignore - [DRAFT_STATE]: state => state, - size: state => latest(state).size, - has: state => key => latest(state).has(key), - add: state => value => { - if (!latest(state).has(value)) { - markChanged(state) - //@ts-ignore - state.copy.add(value) - } - return state.draft - }, - delete: state => value => { - markChanged(state) - //@ts-ignore - return state.copy.delete(value) - }, - clear: state => () => { - markChanged(state) - //@ts-ignore - return state.copy.clear() - }, - forEach: state => (cb, thisArg) => { - const iterator = iterateSetValues(state)() - let result = iterator.next() - while (!result.done) { - cb.call(thisArg, result.value, result.value, state.draft) - result = iterator.next() - } - }, - keys: iterateSetValues, - values: iterateSetValues, - entries: iterateSetValues, - [hasSymbol ? Symbol.iterator : "@@iterator"]: iterateSetValues -}) - -/** - * Helpers - */ - - // Access a property without creating an Immer draft. function peek(draft, prop) { const state = draft[DRAFT_STATE] diff --git a/src/set.ts b/src/set.ts index 9ef913633..eabb8ec5b 100644 --- a/src/set.ts +++ b/src/set.ts @@ -10,10 +10,10 @@ import { hasSymbol, shallowCopy, DRAFT_STATE, - iterateMapValues, makeIterable, - makeIterateSetValues, - latest + latest, + original, + makeIterable2 } from "./common" // TODO: kill: @@ -42,7 +42,11 @@ export interface SetState { function prepareCopy(state: SetState) { if (!state.copy) { - state.copy = new Set(state.base); + // create drafts for all entries to preserve insertion order + state.copy = new Set() + state.base.forEach(value => { + state.copy!.add(state.scope.immer.createProxy(value, state)) + }) } } @@ -73,9 +77,15 @@ export class DraftSet extends SetBase implements Set { return latest(this[DRAFT_STATE]).size } + has(value: V): boolean { + return latest(this[DRAFT_STATE]).has(value) + } + add(value: V): this { const state = this[DRAFT_STATE]; - if (latest(state).has(value) !== value) { + if (state.copy) { + state.copy.add(value) + } else if (!state.base.has(value)) { prepareCopy(state) state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly state.copy!.add(value) @@ -91,8 +101,7 @@ export class DraftSet extends SetBase implements Set { const state = this[DRAFT_STATE]; prepareCopy(state) state.scope.immer.markChanged(state) - state.copy!.delete(value) - return true + return state.copy!.delete(value) } clear() { @@ -102,21 +111,30 @@ export class DraftSet extends SetBase implements Set { return state.copy!.clear() } - keys() { - return latest(this[DRAFT_STATE]).keys(); + values(): IterableIterator { + const state = this[DRAFT_STATE] + prepareCopy(state) + return state.copy!.values() + } + + entries(): IterableIterator<[V, V]> { + const state = this[DRAFT_STATE] + prepareCopy(state) + return state.copy!.entries() + } + + keys(): IterableIterator { + return this.values() } - // TODO: values and entries iterators - // @ts-ignore TODO: - entries = iterateSetValues - // values: iterateMapValues, - // @ts-ignore TODO: - values= iterateSetValues - // entries: iterateMapValues + // TODO: factor out symbol + [Symbol.iterator]() { + return this.values() + } forEach(cb, thisArg) { const state = this[DRAFT_STATE] - const iterator = iterateSetValues(state)() + const iterator = this.values() let result = iterator.next() while (!result.done) { cb.call(thisArg, result.value, result.value, state.draft) @@ -133,29 +151,9 @@ export function proxySet(target, parent) { return new DraftSet(target, parent) } -if (hasSymbol) { - Object.defineProperty( - DraftSet.prototype, - Symbol.iterator, - // @ts-ignore TODO fix - proxyMethod(iterateMapValues) - ) -} +// const iterateSetValues = makeIterateSetValues() -const iterateSetValues = makeIterateSetValues() -// TODO: kill? -function proxyMethod(trap, key) { - return { - get() { - return function(this: ES5Draft, ...args) { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return trap(state, key, state.draft)(...args) - } - } - } -} export function hasSetChanges(state) { const {base, draft} = state From 7f5e301df1b1929be59a68f6db6e15b30087baeb Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sun, 5 Jan 2020 20:32:55 +0000 Subject: [PATCH 07/28] fixed remaining tests, fixes #466 --- __tests__/base.js | 13 +++++++++++++ __tests__/hooks.js | 2 +- __tests__/map-set.js | 43 ++++++++++++++++++++++++++++++++----------- __tests__/patch.js | 4 ++-- src/immer.ts | 2 +- src/set.ts | 19 +++++++++++++++---- 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index fd8336c17..3789ad49f 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -719,6 +719,19 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(base).toEqual(new Set([new Set(["Serenity"])])) expect(result).toEqual(new Set([new Set(["Serenity", "Firefly"])])) }) + + it("supports has / delete on elements from the original", () => { + const obj = {} + const set = new Set([obj]) + const next = produce(set, d => { + expect(d.has(obj)).toBe(true) + d.add(3) + expect(d.has(obj)).toBe(true) + d.delete(obj) + expect(d.has(obj)).toBe(false) + }) + expect(next).toEqual(new Set([3])) + }) }) it("supports `immerable` symbol on constructor", () => { diff --git a/__tests__/hooks.js b/__tests__/hooks.js index 584ec8b7c..1fe448f1a 100644 --- a/__tests__/hooks.js +++ b/__tests__/hooks.js @@ -159,7 +159,7 @@ function createHookTests(useProxies) { }) }) - describe.skip("when draft is a Set", () => { + describe("when draft is a Set", () => { test("assign", () => { produce({a: new Set([1, 2, 3])}, s => { s.a.add(4) diff --git a/__tests__/map-set.js b/__tests__/map-set.js index 6cff982ac..3f7b68766 100644 --- a/__tests__/map-set.js +++ b/__tests__/map-set.js @@ -5,11 +5,11 @@ import {each, shallowCopy, isEnumerable, DRAFT_STATE} from "../src/common" jest.setTimeout(1000) runBaseTest("proxy (no freeze)", true, false) -runBaseTest("proxy (autofreeze)", true, true) -runBaseTest("proxy (autofreeze)(patch listener)", true, true, true) -runBaseTest("es5 (no freeze)", false, false) -runBaseTest("es5 (autofreeze)", false, true) -runBaseTest("es5 (autofreeze)(patch listener)", false, true, true) +// runBaseTest("proxy (autofreeze)", true, true) +// runBaseTest("proxy (autofreeze)(patch listener)", true, true, true) +// runBaseTest("es5 (no freeze)", false, false) +// runBaseTest("es5 (autofreeze)", false, true) +// runBaseTest("es5 (autofreeze)(patch listener)", false, true, true) function runBaseTest(name, useProxies, autoFreeze, useListener) { const listener = useListener ? function() {} : undefined @@ -131,7 +131,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ]) }) - test.skip("#466 - mapChangeBug2 ", () => { + test("#466 - mapChangeBug2 ", () => { const obj = { map: new Map([ ["a", new Map([["b", true], ["c", true], ["d", true]])], @@ -141,14 +141,14 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ]) } const obj1 = produce(obj, draft => {}) - const result = produceWithPatches(obj1, draft => { + const [result, p, ip] = produceWithPatches(obj1, draft => { const aMap = draft.map.get("a") aMap.forEach((_, other) => { const otherMap = draft.map.get(other) otherMap.delete("a") }) }) - expect(result).toEqual([ + expect(result).toEqual( { map: new Map([ ["a", new Map([["b", true], ["c", true], ["d", true]])], @@ -156,21 +156,42 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { ["c", new Map([])], ["d", new Map([])] ]) - }, + } + ) + expect(p).toEqual( [ { op: "remove", path: ["map", "b", "a"] + }, + { + op: "remove", + path: ["map", "c", "a"] + }, + { + op: "remove", + path: ["map", "d", "a"] } - ], + ]) + expect(ip).toEqual( [ { op: "add", path: ["map", "b", "a"], value: true + }, + { + op: "add", + path: ["map", "c", "a"], + value: true + }, + { + op: "add", + path: ["map", "d", "a"], + value: true } ] - ]) + ) }) }) } diff --git a/__tests__/patch.js b/__tests__/patch.js index da8b44ffa..bd8fe42a4 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -618,7 +618,7 @@ describe("sets - add, delete, add - 2", () => { ) }) -describe.skip("sets - mutate - 1", () => { +describe.only("sets - mutate - 1", () => { const findById = (set, id) => { for (const item of set) { if (item.id === id) return item @@ -729,7 +729,7 @@ describe("same value replacement - 5", () => { ) }) -describe.skip("same value replacement - 6", () => { +describe("same value replacement - 6", () => { runPatchTest( new Set(["x", 3]), d => { diff --git a/src/immer.ts b/src/immer.ts index 359503528..a106f226c 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -380,7 +380,7 @@ export class Immer implements ProducersFns { } // Unchanged drafts are never passed to the `onAssign` hook. - if (isDraftProp && !isSetMember && value === get(state.base, prop)) return + if (isDraftProp && !isSet && value === get(state.base, prop)) return } // Unchanged draft properties are ignored. else if (isDraftProp && is(value, get(state.base, prop))) { diff --git a/src/set.ts b/src/set.ts index eabb8ec5b..0c67914f5 100644 --- a/src/set.ts +++ b/src/set.ts @@ -36,6 +36,7 @@ export interface SetState { copy: Set | undefined; // assigned: Map | undefined; base: Set; + drafts: Map; // maps the original value to the draft value in the new set revoke(): void; draft: ES5Draft; } @@ -45,7 +46,9 @@ function prepareCopy(state: SetState) { // create drafts for all entries to preserve insertion order state.copy = new Set() state.base.forEach(value => { - state.copy!.add(state.scope.immer.createProxy(value, state)) + const draft = state.scope.immer.createProxy(value, state); + state.copy!.add(draft) + state.drafts.set(value, draft) }) } } @@ -67,6 +70,7 @@ export class DraftSet extends SetBase implements Set { copy: undefined, base: target, draft: this as any, // TODO: fix typing + drafts: new Map(), revoke() { // TODO: make sure this marks the Map as revoked, and assert everywhere } @@ -78,7 +82,14 @@ export class DraftSet extends SetBase implements Set { } has(value: V): boolean { - return latest(this[DRAFT_STATE]).has(value) + const state = this[DRAFT_STATE] + // bit of trickery here, to be able to recognize both the value, and the draft of its value + if (!state.copy) { + return state.base.has(value) + } + if (state.copy.has(value)) return true; + if (state.drafts.has(value) && state.copy.has(state.drafts.get(value))) return true; + return false; } add(value: V): this { @@ -94,14 +105,14 @@ export class DraftSet extends SetBase implements Set { } delete(value: V): boolean { + const state = this[DRAFT_STATE]; if (!this.has(value)) { return false; } - const state = this[DRAFT_STATE]; prepareCopy(state) state.scope.immer.markChanged(state) - return state.copy!.delete(value) + return state.copy!.delete(value) || (state.drafts.has(value) ? state.copy!.delete(state.drafts.get(value)) : false) } clear() { From 65a377fddd6ba196b2c3155316363ae8228b8117 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 08:54:54 +0000 Subject: [PATCH 08/28] some build fixes --- __tests__/patch.js | 2 +- src/es5.ts | 1 - src/proxy.ts | 106 +++------------------------------------------ tsconfig.json | 3 +- 4 files changed, 10 insertions(+), 102 deletions(-) diff --git a/__tests__/patch.js b/__tests__/patch.js index bd8fe42a4..cdebb4b17 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -618,7 +618,7 @@ describe("sets - add, delete, add - 2", () => { ) }) -describe.only("sets - mutate - 1", () => { +describe("sets - mutate - 1", () => { const findById = (set, id) => { for (const item of set) { if (item.id === id) return item diff --git a/src/es5.ts b/src/es5.ts index d952ecb37..a37520d11 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -11,7 +11,6 @@ import { hasSymbol, shallowCopy, DRAFT_STATE, - iterateMapValues, makeIterable, latest } from "./common" diff --git a/src/proxy.ts b/src/proxy.ts index 7020e8cdb..74973c12f 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -15,12 +15,11 @@ import { assignMap, assignSet, original, - iterateMapValues, latest } from "./common" import {ImmerScope} from "./scope" -import { proxyMap } from "./map" -import { proxySet } from "./set" +import {proxyMap} from "./map" +import {proxySet} from "./set" // Do nothing before being finalized. export function willFinalize() {} @@ -68,13 +67,13 @@ export function createProxy( // TODO: dedupe this, it is the same for ES5 if (isMap(base)) { const draft = proxyMap(base, parent) as any // TODO: typefix - scope.drafts.push(draft); - return draft; + scope.drafts.push(draft) + return draft } if (isSet(base)) { const draft = proxySet(base, parent) as any // TODO: typefix - scope.drafts.push(draft); - return draft; + scope.drafts.push(draft) + return draft } const state: ES6State = { @@ -249,78 +248,6 @@ const reflectTraps = makeReflectTraps([ * Map drafts */ -const mapTraps = makeTrapsForGetters>({ - [DRAFT_STATE]: state => state, - size: state => latest(state).size, - has: state => key => latest(state).has(key), - set: state => (key, value) => { - const values = latest(state) - if (!values.has(key) || values.get(key) !== value) { - markChanged(state) - // @ts-ignore - state.assigned.set(key, true) - state.copy!.set(key, value) - } - return state.draft - }, - delete: state => key => { - if (latest(state).has(key)) { - markChanged(state) - // @ts-ignore - state.assigned.set(key, false) - return state.copy!.delete(key) - } - return false - }, - clear: state => () => { - markChanged(state) - state.assigned = new Map() - each(latest(state).keys(), (_, key) => { - // @ts-ignore - state.assigned.set(key, false) - }) - return state.copy!.clear() - }, - // @ts-ignore - forEach: (state, _, receiver) => (cb, thisArg) => - latest(state).forEach((_, key, map) => { - const value = receiver.get(key) - cb.call(thisArg, value, key, map) - }), - get: state => key => { - const drafts = state.modified ? state.copy : state.drafts - - // @ts-ignore TODO: ...or fix by using different ES6Draft types (but better just unify to maps) - if (drafts!.has(key)) { - // @ts-ignore - const value = drafts.get(key) - - if (isDraft(value) || !isDraftable(value)) return value - - const draft = createProxy(value, state) - // @ts-ignore - drafts.set(key, draft) - return draft - } - - const value = latest(state).get(key) - if (state.finalized || !isDraftable(value)) { - return value - } - - const draft = createProxy(value, state) - //@ts-ignore - drafts.set(key, draft) - return draft - }, - keys: state => () => latest(state).keys(), - //@ts-ignore - values: iterateMapValues, - //@ts-ignore - entries: iterateMapValues, - [hasSymbol ? Symbol.iterator : "@@iterator"]: iterateMapValues -}) - // Access a property without creating an Immer draft. function peek(draft, prop) { const state = draft[DRAFT_STATE] @@ -338,7 +265,7 @@ export function markChanged(state) { const {base, drafts, parent} = state if (!isMap(base) && !isSet(base)) { // TODO: drop creating copies here? - const copy = state.copy = shallowCopy(base) + const copy = (state.copy = shallowCopy(base)) assign(copy, drafts) state.drafts = null } @@ -369,22 +296,3 @@ function makeReflectTraps( {} as any ) } - -function makeTrapsForGetters( - getters: { - [K in keyof T]: ( - state: ES6State - ) => /* Skip first arg of: ProxyHandler[K] */ any - } -): ProxyHandler { - return assign({}, reflectTraps, { - get(state, prop, receiver) { - return getters.hasOwnProperty(prop) - ? getters[prop](state, prop, receiver) - : Reflect.get(state, prop, receiver) - }, - setPrototypeOf(state) { - throw new Error("Object.setPrototypeOf() cannot be used on an Immer draft") // prettier-ignore - } - }) -} diff --git a/tsconfig.json b/tsconfig.json index c0cd58280..5c65c158d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,6 +7,7 @@ "strict": true, "declaration": true, "importHelpers": false, - "noImplicitAny": false // TODO: true, + "noImplicitAny": false, // TODO: true, + "esModuleInterop": true } } From c53a75f793fe4fe761d98f0e4ba5876be690e4ce Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 09:07:48 +0000 Subject: [PATCH 09/28] Some initial cleanup --- src/common.ts | 108 +++++++++++++----------------------------- src/es5.ts | 1 - src/map.ts | 125 +++++++++++++++---------------------------------- src/patches.ts | 49 ++++++++++++------- src/proxy.ts | 1 - src/set.ts | 94 +++++++++++++++---------------------- 6 files changed, 141 insertions(+), 237 deletions(-) diff --git a/src/common.ts b/src/common.ts index 72fe99836..388aae147 100644 --- a/src/common.ts +++ b/src/common.ts @@ -1,4 +1,4 @@ -import {Objectish, ObjectishNoSet, ImmerState} from "./types" +import {Objectish, ObjectishNoSet} from "./types" /** Use a class type for `nothing` so its type is unique */ export class Nothing { @@ -105,41 +105,6 @@ export const ownKeys: (target) => PropertyKey[] = ) as any) : Object.getOwnPropertyNames -// TODO: duplicate of clone? TODO: should be used by prepareCopy of map / set -export function shallowCopy( - base: T, - invokeGetters?: boolean -): T -export function shallowCopy(base, invokeGetters = false) { - if (Array.isArray(base)) return base.slice() - if (isMap(base)) return new Map(base) - if (isSet(base)) return new Set(base) - const clone = Object.create(Object.getPrototypeOf(base)) - ownKeys(base).forEach(key => { - if (key === DRAFT_STATE) { - return // Never copy over draft state. - } - const desc = Object.getOwnPropertyDescriptor(base, key)! - let {value} = desc - if (desc.get) { - if (!invokeGetters) { - throw new Error("Immer drafts cannot have computed properties") - } - value = desc.get.call(base) - } - if (desc.enumerable) { - clone[key] = value - } else { - Object.defineProperty(clone, key, { - value, - writable: true, - configurable: true - }) - } - }) - return clone -} - export function each( obj: T, iter: (key: PropertyKey, value: any, source: T) => void @@ -194,49 +159,42 @@ export function isSet(target): target is Set { return hasSet && target instanceof Set } - -export function makeIterable2(baseIterator: Iterator): IterableIterator { - // TODO: correct? - // TODO: don't use symbol - baseIterator[Symbol.iterator] = () => baseIterator - return baseIterator as any -} - -export function makeIterable(next: () => {done: boolean; value: any}) { - let self - return (self = { - [Symbol.iterator]: () => self, - next - }) -} - export function latest(state: any): any { return state.copy || state.base } -// export function iterateCollection(collection: Map | Set, transformer: (key, value) => any) { -// // TODO: create own iterator symbols for fallback -// const iterator = collection.entries() -// return makeIterable(() => { -// const result = iterator.next() -// if (!result.done) { -// const value = wrapSetValue(state, result.value) -// result.value = isEntries ? [value, value] : value -// } -// return result -// } as any) -// } - -// TODO: duplicate of shallow clone -export function clone(obj: T): T -export function clone(obj) { - if (!isDraftable(obj)) return obj - if (Array.isArray(obj)) return obj.map(clone) - if (isMap(obj)) return new Map(obj) - if (isSet(obj)) return new Set(obj) - const cloned = Object.create(Object.getPrototypeOf(obj)) - for (const key in obj) cloned[key] = clone(obj[key]) - return cloned +export function shallowCopy( + base: T, + invokeGetters?: boolean +): T +export function shallowCopy(base, invokeGetters = false) { + if (Array.isArray(base)) return base.slice() + if (isMap(base)) return new Map(base) + if (isSet(base)) return new Set(base) + const clone = Object.create(Object.getPrototypeOf(base)) + ownKeys(base).forEach(key => { + if (key === DRAFT_STATE) { + return // Never copy over draft state. + } + const desc = Object.getOwnPropertyDescriptor(base, key)! + let {value} = desc + if (desc.get) { + if (!invokeGetters) { + throw new Error("Immer drafts cannot have computed properties") + } + value = desc.get.call(base) + } + if (desc.enumerable) { + clone[key] = value + } else { + Object.defineProperty(clone, key, { + value, + writable: true, + configurable: true + }) + } + }) + return clone } export function freeze( diff --git a/src/es5.ts b/src/es5.ts index a37520d11..5c2a56dc8 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -11,7 +11,6 @@ import { hasSymbol, shallowCopy, DRAFT_STATE, - makeIterable, latest } from "./common" import {proxyMap, hasMapChanges} from "./map" diff --git a/src/map.ts b/src/map.ts index e46d914f5..aa4930640 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,52 +1,36 @@ -import { - each, - has, - is, - isDraft, - isDraftable, - isEnumerable, - isMap, - isSet, - hasSymbol, - shallowCopy, - DRAFT_STATE, - makeIterable, - latest -} from "./common" +import {isDraftable, DRAFT_STATE, latest} from "./common" // TODO: kill: -import { - assertUnrevoked, - ES5Draft, -} from "./es5" -import { ImmerScope } from "./scope"; -import { Immer } from "./immer"; +import {assertUnrevoked, ES5Draft} from "./es5" +import {ImmerScope} from "./scope" +import {Immer} from "./immer" // TODO: create own states // TODO: clean up the maps and such from ES5 / Proxy states export interface MapState { - parent: any; // TODO: type - scope: ImmerScope; - modified: boolean; - finalizing: boolean; - finalized: boolean; - copy: Map | undefined; - assigned: Map | undefined; - base: Map; - revoke(): void; - draft: ES5Draft; + parent: any // TODO: type + scope: ImmerScope + modified: boolean + finalizing: boolean + finalized: boolean + copy: Map | undefined + assigned: Map | undefined + base: Map + revoke(): void + draft: ES5Draft } function prepareCopy(state: MapState) { if (!state.copy) { state.assigned = new Map() - state.copy = new Map(state.base); + state.copy = new Map(state.base) } } // Make sure DraftMap declarion doesn't die if Map is not avialable... -const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : function FakeMap() {} as any +const MapBase: MapConstructor = + typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) // TODO: fix types for drafts export class DraftMap extends MapBase implements Map { @@ -66,7 +50,7 @@ export class DraftMap extends MapBase implements Map { revoke() { // TODO: make sure this marks the Map as revoked, and assert everywhere } - }; + } } get size(): number { @@ -78,23 +62,23 @@ export class DraftMap extends MapBase implements Map { } set(key: K, value: V): this { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] if (latest(state).get(key) !== value) { prepareCopy(state) state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly state.assigned!.set(key, true) state.copy!.set(key, value) - state.assigned!.set(key, true); + state.assigned!.set(key, true) } return this } delete(key: K): boolean { if (!this.has(key)) { - return false; + return false } - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] prepareCopy(state) state.scope.immer.markChanged(state) state.assigned!.set(key, false) @@ -103,7 +87,7 @@ export class DraftMap extends MapBase implements Map { } clear() { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] prepareCopy(state) state.scope.immer.markChanged(state) state.assigned = new Map() @@ -115,14 +99,14 @@ export class DraftMap extends MapBase implements Map { // @ts-ignore TODO: forEach(cb) { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] latest(state).forEach((_value, key, _map) => { cb(this.get(key), key, this) }) } - get(key: K): V /* TODO: Draft */ { - const state = this[DRAFT_STATE]; + get(key: K): V /* TODO: Draft */ { + const state = this[DRAFT_STATE] const value = latest(state).get(key) if (state.finalizing || state.finalized || !isDraftable(value)) { return value @@ -138,7 +122,7 @@ export class DraftMap extends MapBase implements Map { } keys() { - return latest(this[DRAFT_STATE]).keys(); + return latest(this[DRAFT_STATE]).keys() } values() { @@ -147,10 +131,11 @@ export class DraftMap extends MapBase implements Map { [Symbol.iterator]: () => this.values(), // TODO: don't use symbol directly next: () => { const r = iterator.next() - if (r.done) return r; - const value = this.get(r.value); + if (r.done) return r + const value = this.get(r.value) return { - done: false, value + done: false, + value } } } as any @@ -162,63 +147,29 @@ export class DraftMap extends MapBase implements Map { [Symbol.iterator]: () => this.entries(), // TODO: don't use symbol directly next: () => { const r = iterator.next() - if (r.done) return r; - const value = this.get(r.value); + if (r.done) return r + const value = this.get(r.value) return { - done: false, value: [r.value, value] + done: false, + value: [r.value, value] } } } as any } - [Symbol.iterator]() { // TODO: don't use symbol directly + [Symbol.iterator]() { + // TODO: don't use symbol directly return this.entries() } } - -/** Map.prototype.values _-or-_ Map.prototype.entries */ -export function iterateMapValues(state, prop, receiver) { - const isEntries = prop !== "values" - return () => { - const iterator = latest(state)[Symbol.iterator]() - return makeIterable(() => { - const result = iterator.next() - if (!result.done) { - const [key] = result.value - const value = receiver.get(key) - result.value = isEntries ? [key, value] : value - } - return result - }) - } -} - - - export function proxyMap(target, parent) { if (target instanceof DraftMap) { - return target; // TODO: or copy? + return target // TODO: or copy? } return new DraftMap(target, parent) - // Object.defineProperties(target, mapTraps) - -} - -// TODO: kill? -function proxyMethod(trap, key) { - return { - get() { - return function(this: ES5Draft, ...args) { - const state = this[DRAFT_STATE] - assertUnrevoked(state) - return trap(state, key, state.draft)(...args) - } - } - } } -// TODO: used? export function hasMapChanges(state) { const {base, draft} = state diff --git a/src/patches.ts b/src/patches.ts index 36f293e5a..c32ddbfb4 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,4 +1,4 @@ -import {get, each, isMap, isSet, has, clone} from "./common" +import {get, each, isMap, isSet, has} from "./common" import {Patch, ImmerState} from "./types" export function generatePatches( @@ -86,21 +86,22 @@ function generatePatchesFromAssigned( inversePatches: Patch[] ) { const {base, copy} = state - if (state.assigned) each(state.assigned, (key, assignedValue) => { - const origValue = get(base, key) - const value = get(copy, key) - const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" - if (origValue === value && op === "replace") return - const path = basePath.concat(key as any) - patches.push(op === "remove" ? {op, path} : {op, path, value}) - inversePatches.push( - op === "add" - ? {op: "remove", path} - : op === "remove" - ? {op: "add", path, value: origValue} - : {op: "replace", path, value: origValue} - ) - }) + if (state.assigned) + each(state.assigned, (key, assignedValue) => { + const origValue = get(base, key) + const value = get(copy, key) + const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" + if (origValue === value && op === "replace") return + const path = basePath.concat(key as any) + patches.push(op === "remove" ? {op, path} : {op, path, value}) + inversePatches.push( + op === "add" + ? {op: "remove", path} + : op === "remove" + ? {op: "add", path, value: origValue} + : {op: "replace", path, value: origValue} + ) + }) } function generateSetPatches( @@ -161,7 +162,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore } - const value = clone(patch.value) // used to clone patch to ensure original patch is not modified, see #411 + const value = deepClone(patch.value) // used to clone patch to ensure original patch is not modified, see #411 const key = path[path.length - 1] switch (op) { @@ -206,3 +207,17 @@ export function applyPatches(draft: T, patches: Patch[]): T { return draft } + +// TODO: optimize: this is quite a performance hit, can we detect intelligently when it is needed? +// E.g. auto-draft when new objects from outside are assigned and modified? +// (See failing test when deepClone just returns obj) +function deepClone(obj) { + if (!obj || typeof obj !== "object") return obj + if (Array.isArray(obj)) return obj.map(deepClone) + if (isMap(obj)) + return new Map(Array.from(obj.entries()).map(([k, v]) => [k, deepClone(v)])) + if (isSet(obj)) return new Set(Array.from(obj.values()).map(deepClone)) + const cloned = Object.create(Object.getPrototypeOf(obj)) + for (const key in obj) cloned[key] = deepClone(obj[key]) + return cloned +} diff --git a/src/proxy.ts b/src/proxy.ts index 74973c12f..2b6aff2fe 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -10,7 +10,6 @@ import { isSet, hasSymbol, shallowCopy, - makeIterable, DRAFT_STATE, assignMap, assignSet, diff --git a/src/set.ts b/src/set.ts index 0c67914f5..7d8b2f9c1 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,60 +1,41 @@ -import { - each, - has, - is, - isDraft, - isDraftable, - isEnumerable, - isMap, - isSet, - hasSymbol, - shallowCopy, - DRAFT_STATE, - makeIterable, - latest, - original, - makeIterable2 -} from "./common" +import {isDraftable, DRAFT_STATE, latest} from "./common" // TODO: kill: -import { - assertUnrevoked, - ES5Draft, -} from "./es5" -import { ImmerScope } from "./scope"; -import { Immer } from "./immer"; +import {assertUnrevoked, ES5Draft} from "./es5" +import {ImmerScope} from "./scope" // TODO: create own states // TODO: clean up the maps and such from ES5 / Proxy states export interface SetState { - parent: any; // TODO: type - scope: ImmerScope; - modified: boolean; - finalizing: boolean; - finalized: boolean; - copy: Set | undefined; + parent: any // TODO: type + scope: ImmerScope + modified: boolean + finalizing: boolean + finalized: boolean + copy: Set | undefined // assigned: Map | undefined; - base: Set; - drafts: Map; // maps the original value to the draft value in the new set - revoke(): void; - draft: ES5Draft; + base: Set + drafts: Map // maps the original value to the draft value in the new set + revoke(): void + draft: ES5Draft } function prepareCopy(state: SetState) { if (!state.copy) { - // create drafts for all entries to preserve insertion order - state.copy = new Set() - state.base.forEach(value => { - const draft = state.scope.immer.createProxy(value, state); - state.copy!.add(draft) - state.drafts.set(value, draft) - }) + // create drafts for all entries to preserve insertion order + state.copy = new Set() + state.base.forEach(value => { + const draft = state.scope.immer.createProxy(value, state) + state.copy!.add(draft) + state.drafts.set(value, draft) + }) } } // Make sure DraftSet declarion doesn't die if Map is not avialable... -const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : function FakeSet() {} as any +const SetBase: SetConstructor = + typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) // TODO: fix types for drafts export class DraftSet extends SetBase implements Set { @@ -74,7 +55,7 @@ export class DraftSet extends SetBase implements Set { revoke() { // TODO: make sure this marks the Map as revoked, and assert everywhere } - }; + } } get size(): number { @@ -87,13 +68,14 @@ export class DraftSet extends SetBase implements Set { if (!state.copy) { return state.base.has(value) } - if (state.copy.has(value)) return true; - if (state.drafts.has(value) && state.copy.has(state.drafts.get(value))) return true; - return false; + if (state.copy.has(value)) return true + if (state.drafts.has(value) && state.copy.has(state.drafts.get(value))) + return true + return false } add(value: V): this { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] if (state.copy) { state.copy.add(value) } else if (!state.base.has(value)) { @@ -105,18 +87,23 @@ export class DraftSet extends SetBase implements Set { } delete(value: V): boolean { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] if (!this.has(value)) { - return false; + return false } prepareCopy(state) state.scope.immer.markChanged(state) - return state.copy!.delete(value) || (state.drafts.has(value) ? state.copy!.delete(state.drafts.get(value)) : false) + return ( + state.copy!.delete(value) || + (state.drafts.has(value) + ? state.copy!.delete(state.drafts.get(value)) + : false) + ) } clear() { - const state = this[DRAFT_STATE]; + const state = this[DRAFT_STATE] prepareCopy(state) state.scope.immer.markChanged(state) return state.copy!.clear() @@ -154,18 +141,13 @@ export class DraftSet extends SetBase implements Set { } } - export function proxySet(target, parent) { if (target instanceof DraftSet) { - return target; // TODO: or copy? + return target // TODO: or copy? } return new DraftSet(target, parent) } -// const iterateSetValues = makeIterateSetValues() - - - export function hasSetChanges(state) { const {base, draft} = state From 3842f8568a12cddfb3b66aef83d27d6b0a8f073e Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 09:42:43 +0000 Subject: [PATCH 10/28] Introduced proper state types --- src/es5.ts | 45 ++++++++++++++++++++------------ src/map.ts | 11 +++++--- src/patches.ts | 15 ++++++----- src/proxy.ts | 70 ++++++++++++++++++++++++++++---------------------- src/set.ts | 12 +++++---- src/types.ts | 43 ++++++++++++++++++++++++++----- 6 files changed, 127 insertions(+), 69 deletions(-) diff --git a/src/es5.ts b/src/es5.ts index 5c2a56dc8..b21b646cf 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -17,28 +17,37 @@ import {proxyMap, hasMapChanges} from "./map" import {proxySet, hasSetChanges} from "./set" import {ImmerScope} from "./scope" -import {ImmerState} from "./types" - -export interface ES5Draft { - [DRAFT_STATE]: ES5State -} +import {ImmerState, Drafted, AnyObject, AnyMap} from "./types" // TODO: merge with ImmerState? -export interface ES5State { +interface ES5BaseState { scope: ImmerScope modified: boolean finalizing: boolean finalized: boolean - assigned: Map | {[key: string]: any} - parent: ES5State - base: T - draft: T & ES5Draft - drafts: Map | null - copy: T | null + assigned: {[key: string]: any} + parent: ImmerState + drafts: AnyMap | null // TODO: don't use map? revoke() revoked: boolean } +export interface ES5ObjectState extends ES5BaseState { + type: "es5_object" + draft: Drafted + base: AnyObject + copy: AnyObject | null +} + +export interface ES5ArrayState extends ES5BaseState { + type: "es5_array" + draft: Drafted + base: AnyObject + copy: AnyObject | null +} + +type ES5State = ES5ArrayState | ES5ObjectState + export function willFinalize( scope: ImmerScope, result: any, @@ -60,7 +69,10 @@ export function willFinalize( } } -export function createProxy(base: T, parent: ES5State): ES5Draft { +export function createProxy( + base: T, + parent: ImmerState +): Drafted { if (!base || typeof base !== "object" || !isDraftable(base)) { // TODO: || isDraft ? return base as any // TODO: fix @@ -84,7 +96,8 @@ export function createProxy(base: T, parent: ES5State): ES5Draft { proxyProperty(draft, prop, isArray || isEnumerable(base, prop)) }) - const state: ES5State = { + const state: ES5ObjectState | ES5ArrayState = { + type: isArray ? "es5_array" : ("es5_object" as any), scope, modified: false, finalizing: false, // es5 only @@ -104,7 +117,7 @@ export function createProxy(base: T, parent: ES5State): ES5Draft { return draft } -function revoke(this: ES5State) { +function revoke(this: ES5BaseState) { this.revoked = true } @@ -141,7 +154,7 @@ function set(state: ES5State, prop, value) { markChanged(state) prepareCopy(state) } - state.copy[prop] = value + state.copy![prop] = value } export function markChanged(state) { diff --git a/src/map.ts b/src/map.ts index aa4930640..57c60abf3 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,24 +1,26 @@ import {isDraftable, DRAFT_STATE, latest} from "./common" // TODO: kill: -import {assertUnrevoked, ES5Draft} from "./es5" +import {assertUnrevoked} from "./es5" import {ImmerScope} from "./scope" import {Immer} from "./immer" +import {AnyMap, Drafted} from "./types" // TODO: create own states // TODO: clean up the maps and such from ES5 / Proxy states export interface MapState { + type: "map" parent: any // TODO: type scope: ImmerScope modified: boolean finalizing: boolean finalized: boolean - copy: Map | undefined + copy: AnyMap | undefined assigned: Map | undefined - base: Map + base: AnyMap revoke(): void - draft: ES5Draft + draft: Drafted } function prepareCopy(state: MapState) { @@ -38,6 +40,7 @@ export class DraftMap extends MapBase implements Map { constructor(target, parent) { super() this[DRAFT_STATE] = { + type: "map", parent, scope: parent ? parent.scope : ImmerScope.current, modified: false, diff --git a/src/patches.ts b/src/patches.ts index c32ddbfb4..c8a630412 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,5 +1,6 @@ import {get, each, isMap, isSet, has} from "./common" import {Patch, ImmerState} from "./types" +import {SetState} from "./set" export function generatePatches( state: ImmerState, @@ -7,17 +8,18 @@ export function generatePatches( patches: Patch[], inversePatches: Patch[] ) { + // TODO: use a proper switch here const generatePatchesFn = Array.isArray(state.base) ? generateArrayPatches : isSet(state.base) ? generateSetPatches : generatePatchesFromAssigned - generatePatchesFn(state, basePath, patches, inversePatches) + generatePatchesFn(state as any, basePath, patches, inversePatches) } function generateArrayPatches( - state: ImmerState, + state: any, // TODO: type properly with ImmerState basePath: (string | number)[], patches: Patch[], inversePatches: Patch[] @@ -80,7 +82,7 @@ function generateArrayPatches( // This is used for both Map objects and normal objects. function generatePatchesFromAssigned( - state: ImmerState, + state: any, // TODO: type properly with ImmerState basePath: (number | string)[], patches: Patch[], inversePatches: Patch[] @@ -105,17 +107,16 @@ function generatePatchesFromAssigned( } function generateSetPatches( - state: ImmerState, + state: SetState, basePath: (number | string)[], patches: Patch[], inversePatches: Patch[] ) { - // TODO: if this doesn't use assigned, drop assigned from SetState stuff let {base, copy} = state let i = 0 base.forEach(value => { - if (!copy.has(value)) { + if (!copy!.has(value)) { const path = basePath.concat([i]) patches.push({ op: "remove", @@ -131,7 +132,7 @@ function generateSetPatches( i++ }) i = 0 - copy.forEach(value => { + copy!.forEach(value => { if (!base.has(value)) { const path = basePath.concat([i]) patches.push({ diff --git a/src/proxy.ts b/src/proxy.ts index 2b6aff2fe..2ea74185c 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -19,33 +19,39 @@ import { import {ImmerScope} from "./scope" import {proxyMap} from "./map" import {proxySet} from "./set" +import {AnyObject, Drafted, ImmerState, AnyArray} from "./types" // Do nothing before being finalized. export function willFinalize() {} -interface ES6Draft {} - -interface ES6State { +interface ProxyBaseState { scope: ImmerScope modified: boolean finalized: boolean - assigned: - | { - [property: string]: boolean - } - | Map // TODO: always use a Map? - parent: ES6State - base: T - draft: ES6Draft | null - drafts: - | { - [property: string]: ES6Draft - } - | Map // TODO: always use a Map? - copy: T | null + assigned: { + [property: string]: boolean + } + parent: ImmerState + drafts: { + [property: string]: Drafted + } revoke: null | (() => void) } +export interface ProxyObjectState extends ProxyBaseState { + type: "proxy_object" + base: AnyObject + copy: AnyObject | null + draft: Drafted +} + +export interface ProxyArrayState extends ProxyBaseState { + type: "proxy_array" + base: AnyArray + copy: AnyArray | null + draft: Drafted +} + /** * Returns a new draft of the `base` object. * @@ -53,8 +59,9 @@ interface ES6State { */ export function createProxy( base: T, - parent: ES6State -): ES6Draft { + parent: ProxyObjectState | ProxyArrayState +): Drafted { + // TODO: extract create proxy and refine // TODO: dedupe if (!base || typeof base !== "object" || !isDraftable(base)) { // TODO: || isDraft ? @@ -75,7 +82,9 @@ export function createProxy( return draft } - const state: ES6State = { + const isArray = Array.isArray(base) + const state: ProxyObjectState | ProxyArrayState = { + type: isArray ? "proxy_array" : ("proxy_object" as any), // Track which produce call this is associated with. scope, // True for both shallow and deep changes. @@ -89,7 +98,7 @@ export function createProxy( // The base state. base, // The base proxy. - draft: null, + draft: null as any, // set below // Any property proxies. drafts: {}, // The base copy with any updated values. @@ -113,24 +122,24 @@ export function createProxy( const {revoke, proxy} = Proxy.revocable(target, traps) - state.draft = proxy + state.draft = proxy as any state.revoke = revoke scope.drafts!.push(proxy) - return proxy + return proxy as any } /** * Object drafts */ -const objectTraps: ProxyHandler = { +const objectTraps: ProxyHandler = { get(state, prop) { if (prop === DRAFT_STATE) return state let {drafts} = state // Check for existing draft in unmodified state. if (!state.modified && has(drafts, prop)) { - return drafts[prop] + return drafts[prop as any] } const value = latest(state)[prop] @@ -143,10 +152,11 @@ const objectTraps: ProxyHandler = { // Assigned values are never drafted. This catches any drafts we created, too. if (value !== peek(state.base, prop)) return value // Store drafts on the copy (when one exists). + // @ts-ignore TODO: this line seems of? drafts = state.copy } - return (drafts[prop] = createProxy(value, state)) + return (drafts[prop as any] = createProxy(value, state)) }, has(state, prop) { return prop in latest(state) @@ -154,7 +164,7 @@ const objectTraps: ProxyHandler = { ownKeys(state) { return Reflect.ownKeys(latest(state)) }, - set(state, prop, value) { + set(state, prop: string /* strictly not, but helps TS */, value) { if (!state.modified) { const baseValue = peek(state.base, prop) // Optimize based on value's truthiness. Truthy values are guaranteed to @@ -168,10 +178,10 @@ const objectTraps: ProxyHandler = { markChanged(state) } state.assigned[prop] = true - state.copy[prop] = value + state.copy![prop] = value return true }, - deleteProperty(state, prop) { + deleteProperty(state, prop: string) { // The `undefined` check is a fast path for pre-existing keys. if (peek(state.base, prop) !== undefined || prop in state.base) { state.assigned[prop] = false @@ -210,7 +220,7 @@ const objectTraps: ProxyHandler = { * Array drafts */ -const arrayTraps: ProxyHandler<[ES6State]> = {} +const arrayTraps: ProxyHandler<[ProxyArrayState]> = {} each(objectTraps, (key, fn) => { arrayTraps[key] = function() { arguments[0] = arguments[0][0] diff --git a/src/set.ts b/src/set.ts index 7d8b2f9c1..9d83abc89 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,24 +1,25 @@ import {isDraftable, DRAFT_STATE, latest} from "./common" // TODO: kill: -import {assertUnrevoked, ES5Draft} from "./es5" +import {assertUnrevoked} from "./es5" import {ImmerScope} from "./scope" +import {AnySet, Drafted} from "./types" // TODO: create own states // TODO: clean up the maps and such from ES5 / Proxy states export interface SetState { + type: "set" parent: any // TODO: type scope: ImmerScope modified: boolean finalizing: boolean finalized: boolean - copy: Set | undefined - // assigned: Map | undefined; - base: Set + copy: AnySet | undefined + base: AnySet drafts: Map // maps the original value to the draft value in the new set revoke(): void - draft: ES5Draft + draft: Drafted } function prepareCopy(state: SetState) { @@ -43,6 +44,7 @@ export class DraftSet extends SetBase implements Set { constructor(target, parent) { super() this[DRAFT_STATE] = { + type: "set", parent, scope: parent ? parent.scope : ImmerScope.current, modified: false, diff --git a/src/types.ts b/src/types.ts index 5ec69ee57..43c1c1fad 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,15 +1,44 @@ -import {Nothing} from "./common" +import {Nothing, DRAFT_STATE} from "./common" +import {SetState} from "./set" +import {MapState} from "./map" +import {ProxyObjectState, ProxyArrayState} from "./proxy" +import {ES5ObjectState, ES5ArrayState} from "./es5" export type Objectish = any[] | Map | Set | {} export type ObjectishNoSet = any[] | Map | {} -export interface ImmerState { - parent?: ImmerState - base: T - copy: T - assigned: {[prop: string]: boolean; [index: number]: boolean} -} +// export interface ImmerState { +// parent?: ImmerState +// base: T +// copy: T +// assigned: {[prop: string]: boolean; [index: number]: boolean} +// } + +export type AnyObject = {[key: string]: any} +export type AnyArray = Array +export type AnySet = Set +export type AnyMap = Map + +export type DraftType = + | "proxy_object" + | "proxy_array" + | "es5_object" + | "es5_array" + | "map" + | "set" + +export type ImmerState = + | ProxyObjectState + | ProxyArrayState + | ES5ObjectState + | ES5ArrayState + | MapState + | SetState + +export type Drafted = { + [DRAFT_STATE]: T +} & Base type Tail = ((...t: T) => any) extends (( _: any, From 7b0ad536c97a3395c47a5365aff87ee200d25425 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 16:51:55 +0000 Subject: [PATCH 11/28] Stop abusing modules as exports --- src/es5.ts | 32 ++++++++++++++++---------------- src/immer.ts | 49 ++++++++++++++++++++++++++++++++++--------------- src/proxy.ts | 16 +++++----------- 3 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/es5.ts b/src/es5.ts index b21b646cf..6c33dac63 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -48,7 +48,7 @@ export interface ES5ArrayState extends ES5BaseState { type ES5State = ES5ArrayState | ES5ObjectState -export function willFinalize( +export function willFinalizeES5( scope: ImmerScope, result: any, isReplaced: boolean @@ -69,7 +69,7 @@ export function willFinalize( } } -export function createProxy( +export function createES5Proxy( base: T, parent: ImmerState ): Drafted { @@ -141,7 +141,7 @@ function get(state, prop) { // Create a draft if the value is unmodified. if (value === peek(state.base, prop) && isDraftable(value)) { prepareCopy(state) - return (state.copy[prop] = createProxy(value, state)) + return (state.copy[prop] = createES5Proxy(value, state)) } return value } @@ -151,16 +151,16 @@ function set(state: ES5State, prop, value) { state.assigned[prop] = true if (!state.modified) { if (is(value, peek(latest(state), prop))) return - markChanged(state) + markChangedES5(state) prepareCopy(state) } state.copy![prop] = value } -export function markChanged(state) { +export function markChangedES5(state) { if (!state.modified) { state.modified = true - if (state.parent) markChanged(state.parent) + if (state.parent) markChangedES5(state.parent) } } @@ -221,13 +221,13 @@ function markChangesSweep(drafts) { const state = drafts[i][DRAFT_STATE] if (!state.modified) { if (Array.isArray(state.base)) { - if (hasArrayChanges(state)) markChanged(state) + if (hasArrayChanges(state)) markChangedES5(state) } else if (isMap(state.base)) { - if (hasMapChanges(state)) markChanged(state) + if (hasMapChanges(state)) markChangedES5(state) } else if (isSet(state.base)) { - if (hasSetChanges(state)) markChanged(state) + if (hasSetChanges(state)) markChangedES5(state) } else if (hasObjectChanges(state)) { - markChanged(state) + markChangedES5(state) } } } @@ -242,7 +242,7 @@ function markChangesRecursively(object) { const {base, draft, assigned} = state if (isSet(object)) { if (hasSetChanges(state)) { - markChanged(state) + markChangedES5(state) object.forEach(v => { markChangesRecursively(v) }) @@ -253,7 +253,7 @@ function markChangesRecursively(object) { if (assigned && base.get(key) === undefined && !has(base, key)) { // TODO: this code seems invalid for Maps! assigned.set(key, true) - markChanged(state) + markChangedES5(state) } else if (!assigned || !assigned.get(key)) { // TODO: === false? // Only untouched properties trigger recursion. @@ -268,7 +268,7 @@ function markChangesRecursively(object) { // The `undefined` check is a fast path for pre-existing keys. if (draft.get(key) === undefined && !has(draft, key)) { assigned.set(key, false) - markChanged(state) + markChangedES5(state) } }) // } @@ -280,7 +280,7 @@ function markChangesRecursively(object) { if (base[key] === undefined && !has(base, key)) { // TODO: this code seems invalid for Maps! assigned[key] = true - markChanged(state) + markChangedES5(state) } else if (!assigned[key]) { // TODO: === false ? // Only untouched properties trigger recursion. @@ -294,11 +294,11 @@ function markChangesRecursively(object) { // The `undefined` check is a fast path for pre-existing keys. if (draft[key] === undefined && !has(draft, key)) { assigned[key] = false - markChanged(state) + markChangedES5(state) } }) } else if (hasArrayChanges(state)) { - markChanged(state) + markChangedES5(state) assigned.length = true if (draft.length < base.length) { for (let i = draft.length; i < base.length; i++) assigned[i] = false diff --git a/src/immer.ts b/src/immer.ts index a106f226c..a893e6859 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -1,6 +1,5 @@ -// TODO: destructure these, as the * as import makes it unclear which methods get installed on the Immer class -import * as legacyProxy from "./es5" -import * as modernProxy from "./proxy" +import {createES5Proxy, willFinalizeES5, markChangedES5} from "./es5" +import {createProxy, markChanged} from "./proxy" import {applyPatches, generatePatches} from "./patches" import { @@ -21,7 +20,15 @@ import { latest } from "./common" import {ImmerScope} from "./scope" -import {ImmerState, IProduce, IProduceWithPatches, Objectish, PatchListener, Draft, Patch} from "./types" +import { + ImmerState, + IProduce, + IProduceWithPatches, + Objectish, + PatchListener, + Draft, + Patch +} from "./types" function verifyMinified() {} @@ -47,16 +54,9 @@ interface ProducersFns { export class Immer implements ProducersFns { useProxies: boolean = false autoFreeze: boolean = false - onAssign?: ( - state: ImmerState, - prop: string | number, - value: unknown - ) => void + onAssign?: (state: ImmerState, prop: string | number, value: unknown) => void onDelete?: (state: ImmerState, prop: string | number) => void onCopy?: (state: ImmerState) => void - createProxy!:(value: T, parent: any) => T; - willFinalize!: (scope: ImmerScope, thing: any, isReplaced: boolean) => void; - markChanged!:(state: any) => void; // TODO: immerState? constructor(config?: { useProxies?: boolean @@ -182,7 +182,10 @@ export class Immer implements ProducersFns { return proxy as any } - finishDraft>(draft: D, patchListener: PatchListener): D extends Draft ? T : never { + finishDraft>( + draft: D, + patchListener: PatchListener + ): D extends Draft ? T : never { const state = draft && draft[DRAFT_STATE] if (!state || !state.isManual) { throw new Error("First argument to `finishDraft` must be a draft returned by `createDraft`") // prettier-ignore @@ -212,7 +215,6 @@ export class Immer implements ProducersFns { */ setUseProxies(value: boolean) { this.useProxies = value - assign(this, value ? modernProxy : legacyProxy) } applyPatches(base: Objectish, patches: Patch[]) { @@ -275,6 +277,23 @@ export class Immer implements ProducersFns { return result !== NOTHING ? result : undefined } + createProxy(value: any, parent: any) { + if (this.useProxies) return createProxy(value, parent) + else return createES5Proxy(value, parent) + } + + willFinalize(scope: ImmerScope, thing: any, isReplaced: boolean) { + if (!this.useProxies) willFinalizeES5(scope, thing, isReplaced) + } + + markChanged(state: any) { + if (this.useProxies) { + markChanged(state) + } else { + markChangedES5(state) + } + } + /** * @internal * Finalize a draft, returning either the unmodified base state or a modified @@ -343,7 +362,7 @@ export class Immer implements ProducersFns { // state.copy = shallowCopy(state.base, false) // } // else - if (!this.useProxies && !isMap(root) && !isSet(root)) { + if (!this.useProxies && !isMap(root) && !isSet(root)) { // Create the final copy, with added keys and without deleted keys. state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? } diff --git a/src/proxy.ts b/src/proxy.ts index 2ea74185c..1b0d533af 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -21,9 +21,6 @@ import {proxyMap} from "./map" import {proxySet} from "./set" import {AnyObject, Drafted, ImmerState, AnyArray} from "./types" -// Do nothing before being finalized. -export function willFinalize() {} - interface ProxyBaseState { scope: ImmerScope modified: boolean @@ -296,12 +293,9 @@ function prepareCopy(state) { function makeReflectTraps( names: (keyof typeof Reflect)[] ): ProxyHandler { - return names.reduce( - (traps, name) => { - // @ts-ignore - traps[name] = (state, ...args) => Reflect[name](latest(state), ...args) - return traps - }, - {} as any - ) + return names.reduce((traps, name) => { + // @ts-ignore + traps[name] = (state, ...args) => Reflect[name](latest(state), ...args) + return traps + }, {} as any) } From 073bc099a2aa35778f243e41e07b4e09a2cbec74 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 16:52:08 +0000 Subject: [PATCH 12/28] Upgraded prettier to support optional chaining --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index d0a9bf5e4..15690d4ca 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ "jest": "^24.7.1", "lodash": "^4.17.4", "lodash.clonedeep": "^4.5.0", - "prettier": "1.17.0", + "prettier": "1.19.1", "pretty-quick": "^1.8.0", "regenerator-runtime": "^0.11.1", "rimraf": "^2.6.2", diff --git a/yarn.lock b/yarn.lock index b4a8c2ead..cfa7df9b3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5876,10 +5876,10 @@ preserve@^0.2.0: resolved "https://registry.yarnpkg.com/preserve/-/preserve-0.2.0.tgz#815ed1f6ebc65926f865b310c0713bcb3315ce4b" integrity sha1-gV7R9uvGWSb4ZbMQwHE7yzMVzks= -prettier@1.17.0: - version "1.17.0" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.17.0.tgz#53b303676eed22cc14a9f0cec09b477b3026c008" - integrity sha512-sXe5lSt2WQlCbydGETgfm1YBShgOX4HxQkFPvbxkcwgDvGDeqVau8h+12+lmSVlP3rHPz0oavfddSZg/q+Szjw== +prettier@1.19.1: + version "1.19.1" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb" + integrity sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew== pretty-format@^24.7.0: version "24.7.0" From c182c1ea27bb87830ad026a77321155599d60796 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 20:39:03 +0000 Subject: [PATCH 13/28] clean up create proxy --- src/es5.ts | 29 ++++++----------------------- src/immer.ts | 33 ++++++++++++++++++++++++++++----- src/proxy.ts | 32 ++++---------------------------- src/types.ts | 6 +++--- 4 files changed, 41 insertions(+), 59 deletions(-) diff --git a/src/es5.ts b/src/es5.ts index 6c33dac63..030cf1fbd 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -26,7 +26,7 @@ interface ES5BaseState { finalizing: boolean finalized: boolean assigned: {[key: string]: any} - parent: ImmerState + parent?: ImmerState drafts: AnyMap | null // TODO: don't use map? revoke() revoked: boolean @@ -71,24 +71,8 @@ export function willFinalizeES5( export function createES5Proxy( base: T, - parent: ImmerState + parent?: ImmerState ): Drafted { - if (!base || typeof base !== "object" || !isDraftable(base)) { - // TODO: || isDraft ? - return base as any // TODO: fix - } - const scope = parent ? parent.scope : ImmerScope.current! - if (isMap(base)) { - const draft = proxyMap(base, parent) as any // TODO: typefix - scope.drafts.push(draft) - return draft - } - if (isSet(base)) { - const draft = proxySet(base, parent) as any // TODO: typefix - scope.drafts.push(draft) - return draft - } - const isArray = Array.isArray(base) const draft = clonePotentialDraft(base) @@ -98,22 +82,21 @@ export function createES5Proxy( const state: ES5ObjectState | ES5ArrayState = { type: isArray ? "es5_array" : ("es5_object" as any), - scope, + scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalizing: false, // es5 only finalized: false, - assigned: isMap(base) ? new Map() : {}, + assigned: {}, parent, base, draft, - drafts: isSet(base) ? new Map() : null, + drafts: null, copy: null, revoke, revoked: false // es5 only } createHiddenProperty(draft, DRAFT_STATE, state) - scope.drafts!.push(draft) return draft } @@ -141,7 +124,7 @@ function get(state, prop) { // Create a draft if the value is unmodified. if (value === peek(state.base, prop) && isDraftable(value)) { prepareCopy(state) - return (state.copy[prop] = createES5Proxy(value, state)) + return (state.copy[prop] = state.scope.immer.createProxy(value, state)) } return value } diff --git a/src/immer.ts b/src/immer.ts index a893e6859..dc327f73b 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -17,7 +17,9 @@ import { DRAFT_STATE, NOTHING, freeze, - latest + latest, + isPlainObject, + DRAFTABLE } from "./common" import {ImmerScope} from "./scope" import { @@ -27,8 +29,11 @@ import { Objectish, PatchListener, Draft, - Patch + Patch, + Drafted } from "./types" +import {proxyMap} from "./map" +import {proxySet} from "./set" function verifyMinified() {} @@ -277,9 +282,27 @@ export class Immer implements ProducersFns { return result !== NOTHING ? result : undefined } - createProxy(value: any, parent: any) { - if (this.useProxies) return createProxy(value, parent) - else return createES5Proxy(value, parent) + createProxy(value: any, parent?: ImmerState) { + if (!value || typeof value !== "object") return value + + let draft: Drafted + + if ( + isPlainObject(value) || + Array.isArray(value) || + value[DRAFTABLE] || + value?.constructor?.[DRAFTABLE] + ) { + draft = this.useProxies + ? createProxy(value, parent) + : createES5Proxy(value, parent) + } else if (isMap(value)) draft = proxyMap(value, parent) + else if (isSet(value)) draft = proxySet(value, parent) + else return value + + const scope = parent ? parent.scope : ImmerScope.current! + scope.drafts.push(draft) + return draft } willFinalize(scope: ImmerScope, thing: any, isReplaced: boolean) { diff --git a/src/proxy.ts b/src/proxy.ts index 1b0d533af..40f840577 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -28,7 +28,7 @@ interface ProxyBaseState { assigned: { [property: string]: boolean } - parent: ImmerState + parent?: ImmerState drafts: { [property: string]: Drafted } @@ -56,34 +56,13 @@ export interface ProxyArrayState extends ProxyBaseState { */ export function createProxy( base: T, - parent: ProxyObjectState | ProxyArrayState + parent?: ImmerState ): Drafted { - // TODO: extract create proxy and refine - // TODO: dedupe - if (!base || typeof base !== "object" || !isDraftable(base)) { - // TODO: || isDraft ? - return base as any // TODO: fix - } - - const scope = parent ? parent.scope : ImmerScope.current! - - // TODO: dedupe this, it is the same for ES5 - if (isMap(base)) { - const draft = proxyMap(base, parent) as any // TODO: typefix - scope.drafts.push(draft) - return draft - } - if (isSet(base)) { - const draft = proxySet(base, parent) as any // TODO: typefix - scope.drafts.push(draft) - return draft - } - const isArray = Array.isArray(base) const state: ProxyObjectState | ProxyArrayState = { type: isArray ? "proxy_array" : ("proxy_object" as any), // Track which produce call this is associated with. - scope, + scope: parent ? parent.scope : ImmerScope.current!, // True for both shallow and deep changes. modified: false, // Used during finalization. @@ -118,11 +97,8 @@ export function createProxy( } const {revoke, proxy} = Proxy.revocable(target, traps) - state.draft = proxy as any state.revoke = revoke - - scope.drafts!.push(proxy) return proxy as any } @@ -153,7 +129,7 @@ const objectTraps: ProxyHandler = { drafts = state.copy } - return (drafts[prop as any] = createProxy(value, state)) + return (drafts[prop as any] = state.scope.immer.createProxy(value, state)) }, has(state, prop) { return prop in latest(state) diff --git a/src/types.ts b/src/types.ts index 43c1c1fad..18dc0e49c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -36,14 +36,14 @@ export type ImmerState = | MapState | SetState -export type Drafted = { +export type Drafted = { [DRAFT_STATE]: T } & Base -type Tail = ((...t: T) => any) extends (( +type Tail = ((...t: T) => any) extends ( _: any, ...tail: infer TT -) => any) +) => any ? TT : [] From 19a6b66820f8aea979237a5cb15e0ccb606e2e48 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 21:19:43 +0000 Subject: [PATCH 14/28] =?UTF-8?q?=F0=9F=8E=B5...killing=20me=20softly,=20w?= =?UTF-8?q?ith=20your=20fingers...=20=F0=9F=8E=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/common.ts | 30 ++++-------------------------- src/es5.ts | 43 +++++++------------------------------------ src/immer.ts | 2 +- src/map.ts | 19 ------------------- src/patches.ts | 45 +++++++++++++++++++++++---------------------- src/proxy.ts | 24 ------------------------ src/set.ts | 19 ------------------- 7 files changed, 35 insertions(+), 147 deletions(-) diff --git a/src/common.ts b/src/common.ts index 388aae147..469e0838d 100644 --- a/src/common.ts +++ b/src/common.ts @@ -65,26 +65,6 @@ export function original(value: T): T | undefined { // otherwise return undefined } -// We use Maps as `drafts` for Sets, not Objects -// See proxy.js -export function assignSet(target: Map, override) { - override.forEach(value => { - // When we add new drafts we have to remove their originals if present - const prev = original(value) - if (prev) target.delete(prev) - // @ts-ignore TODO investigate - target.add(value) - }) - return target -} - -// We use Maps as `drafts` for Maps, not Objects -// See proxy.js -export function assignMap(target: Map, override: Map) { - override.forEach((value, key) => target.set(key, value)) - return target -} - export const assign = Object.assign || ((target, ...overrides) => { @@ -100,9 +80,9 @@ export const ownKeys: (target) => PropertyKey[] = ? Reflect.ownKeys : typeof Object.getOwnPropertySymbols !== "undefined" ? obj => - Object.getOwnPropertyNames(obj).concat(Object.getOwnPropertySymbols( - obj - ) as any) + Object.getOwnPropertyNames(obj).concat( + Object.getOwnPropertySymbols(obj) as any + ) : Object.getOwnPropertyNames export function each( @@ -115,7 +95,7 @@ export function each(obj, iter) { } else if (obj && typeof obj === "object") { ownKeys(obj).forEach(key => iter(key, obj[key], obj)) } else { - throw new Error("Cannot iterate primitive " + obj) + throw new Error("Nope") } } @@ -169,8 +149,6 @@ export function shallowCopy( ): T export function shallowCopy(base, invokeGetters = false) { if (Array.isArray(base)) return base.slice() - if (isMap(base)) return new Map(base) - if (isSet(base)) return new Set(base) const clone = Object.create(Object.getPrototypeOf(base)) ownKeys(base).forEach(key => { if (key === DRAFT_STATE) { diff --git a/src/es5.ts b/src/es5.ts index 030cf1fbd..32b8b86b2 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -13,11 +13,10 @@ import { DRAFT_STATE, latest } from "./common" -import {proxyMap, hasMapChanges} from "./map" -import {proxySet, hasSetChanges} from "./set" import {ImmerScope} from "./scope" import {ImmerState, Drafted, AnyObject, AnyMap} from "./types" +import {markChanged} from "./proxy" // TODO: merge with ImmerState? interface ES5BaseState { @@ -206,9 +205,10 @@ function markChangesSweep(drafts) { if (Array.isArray(state.base)) { if (hasArrayChanges(state)) markChangedES5(state) } else if (isMap(state.base)) { - if (hasMapChanges(state)) markChangedES5(state) + // TODO: use switch + continue } else if (isSet(state.base)) { - if (hasSetChanges(state)) markChangedES5(state) + continue } else if (hasObjectChanges(state)) { markChangedES5(state) } @@ -223,38 +223,9 @@ function markChangesRecursively(object) { const state = object[DRAFT_STATE] if (!state) return const {base, draft, assigned} = state - if (isSet(object)) { - if (hasSetChanges(state)) { - markChangedES5(state) - object.forEach(v => { - markChangesRecursively(v) - }) - } - } else if (isMap(object)) { - // if (hasMapChanges(object)) { - object.forEach((value, key) => { - if (assigned && base.get(key) === undefined && !has(base, key)) { - // TODO: this code seems invalid for Maps! - assigned.set(key, true) - markChangedES5(state) - } else if (!assigned || !assigned.get(key)) { - // TODO: === false? - // Only untouched properties trigger recursion. - markChangesRecursively(draft.get(key)) - } - }) - // Look for removed keys. - // TODO: is this code needed? - // TODO: use each? - if (assigned) - base.forEach((value, key) => { - // The `undefined` check is a fast path for pre-existing keys. - if (draft.get(key) === undefined && !has(draft, key)) { - assigned.set(key, false) - markChangedES5(state) - } - }) - // } + if (isSet(object) || isMap(object)) { + // TODO: create switch here + return } else if (!Array.isArray(object)) { // Look for added keys. // TODO: use each? diff --git a/src/immer.ts b/src/immer.ts index dc327f73b..ae14df950 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -422,7 +422,7 @@ export class Immer implements ProducersFns { } // Unchanged drafts are never passed to the `onAssign` hook. - if (isDraftProp && !isSet && value === get(state.base, prop)) return + // if (isDraftProp && !isSet && value === get(state.base, prop)) return } // Unchanged draft properties are ignored. else if (isDraftProp && is(value, get(state.base, prop))) { diff --git a/src/map.ts b/src/map.ts index 57c60abf3..c54ee983b 100644 --- a/src/map.ts +++ b/src/map.ts @@ -167,24 +167,5 @@ export class DraftMap extends MapBase implements Map { } export function proxyMap(target, parent) { - if (target instanceof DraftMap) { - return target // TODO: or copy? - } return new DraftMap(target, parent) } - -export function hasMapChanges(state) { - const {base, draft} = state - - if (base.size !== draft.size) return true - - // IE11 supports only forEach iteration - let hasChanges = false - // TODO: optimize: break on first difference - draft.forEach(function(value, key) { - if (!hasChanges) { - hasChanges = isDraftable(value) ? value.modified : value !== base.get(key) - } - }) - return hasChanges -} diff --git a/src/patches.ts b/src/patches.ts index c8a630412..548074fc2 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -88,22 +88,21 @@ function generatePatchesFromAssigned( inversePatches: Patch[] ) { const {base, copy} = state - if (state.assigned) - each(state.assigned, (key, assignedValue) => { - const origValue = get(base, key) - const value = get(copy, key) - const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" - if (origValue === value && op === "replace") return - const path = basePath.concat(key as any) - patches.push(op === "remove" ? {op, path} : {op, path, value}) - inversePatches.push( - op === "add" - ? {op: "remove", path} - : op === "remove" - ? {op: "add", path, value: origValue} - : {op: "replace", path, value: origValue} - ) - }) + each(state.assigned, (key, assignedValue) => { + const origValue = get(base, key) + const value = get(copy, key) + const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" + if (origValue === value && op === "replace") return + const path = basePath.concat(key as any) + patches.push(op === "remove" ? {op, path} : {op, path, value}) + inversePatches.push( + op === "add" + ? {op: "remove", path} + : op === "remove" + ? {op: "add", path, value: origValue} + : {op: "replace", path, value: origValue} + ) + }) } function generateSetPatches( @@ -163,7 +162,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore } - const value = deepClone(patch.value) // used to clone patch to ensure original patch is not modified, see #411 + const value = deepClonePatchValue(patch.value) // used to clone patch to ensure original patch is not modified, see #411 const key = path[path.length - 1] switch (op) { @@ -212,13 +211,15 @@ export function applyPatches(draft: T, patches: Patch[]): T { // TODO: optimize: this is quite a performance hit, can we detect intelligently when it is needed? // E.g. auto-draft when new objects from outside are assigned and modified? // (See failing test when deepClone just returns obj) -function deepClone(obj) { +function deepClonePatchValue(obj) { if (!obj || typeof obj !== "object") return obj - if (Array.isArray(obj)) return obj.map(deepClone) + if (Array.isArray(obj)) return obj.map(deepClonePatchValue) if (isMap(obj)) - return new Map(Array.from(obj.entries()).map(([k, v]) => [k, deepClone(v)])) - if (isSet(obj)) return new Set(Array.from(obj.values()).map(deepClone)) + return new Map( + Array.from(obj.entries()).map(([k, v]) => [k, deepClonePatchValue(v)]) + ) + // Not needed: if (isSet(obj)) return new Set(Array.from(obj.values()).map(deepClone)) const cloned = Object.create(Object.getPrototypeOf(obj)) - for (const key in obj) cloned[key] = deepClone(obj[key]) + for (const key in obj) cloned[key] = deepClonePatchValue(obj[key]) return cloned } diff --git a/src/proxy.ts b/src/proxy.ts index 40f840577..0fb42abc3 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -213,19 +213,6 @@ arrayTraps.set = function(state, prop, value) { return objectTraps.set!.call(this, state[0], prop, value, state[0]) } -// Used by Map and Set drafts -const reflectTraps = makeReflectTraps([ - "ownKeys", - "has", - "set", - "deleteProperty", - "defineProperty", - "getOwnPropertyDescriptor", - "preventExtensions", - "isExtensible", - "getPrototypeOf" -]) - /** * Map drafts */ @@ -264,14 +251,3 @@ function prepareCopy(state) { state.copy = shallowCopy(state.base) } } - -/** Create traps that all use the `Reflect` API on the `latest(state)` */ -function makeReflectTraps( - names: (keyof typeof Reflect)[] -): ProxyHandler { - return names.reduce((traps, name) => { - // @ts-ignore - traps[name] = (state, ...args) => Reflect[name](latest(state), ...args) - return traps - }, {} as any) -} diff --git a/src/set.ts b/src/set.ts index 9d83abc89..0271211c1 100644 --- a/src/set.ts +++ b/src/set.ts @@ -144,24 +144,5 @@ export class DraftSet extends SetBase implements Set { } export function proxySet(target, parent) { - if (target instanceof DraftSet) { - return target // TODO: or copy? - } return new DraftSet(target, parent) } - -export function hasSetChanges(state) { - const {base, draft} = state - - if (base.size !== draft.size) return true - - // IE11 supports only forEach iteration - let hasChanges = false - // TODO: optimize: break on first diff - draft.forEach(function(value, key) { - if (!hasChanges) { - hasChanges = isDraftable(value) ? value.modified : !base.has(key) - } - }) - return hasChanges -} From 74c05361cf45f5a04b888410063c542a2b831b7b Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 6 Jan 2020 21:23:03 +0000 Subject: [PATCH 15/28] Removed unused stuff --- src/es5.ts | 2 -- src/immer.ts | 1 - src/map.ts | 4 +--- src/proxy.ts | 7 ------- src/scope.ts | 4 ++-- src/set.ts | 4 ++-- src/types.ts | 7 ------- 7 files changed, 5 insertions(+), 24 deletions(-) diff --git a/src/es5.ts b/src/es5.ts index 32b8b86b2..e47d27f92 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -8,7 +8,6 @@ import { isEnumerable, isMap, isSet, - hasSymbol, shallowCopy, DRAFT_STATE, latest @@ -16,7 +15,6 @@ import { import {ImmerScope} from "./scope" import {ImmerState, Drafted, AnyObject, AnyMap} from "./types" -import {markChanged} from "./proxy" // TODO: merge with ImmerState? interface ES5BaseState { diff --git a/src/immer.ts b/src/immer.ts index ae14df950..42ad0de90 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -17,7 +17,6 @@ import { DRAFT_STATE, NOTHING, freeze, - latest, isPlainObject, DRAFTABLE } from "./common" diff --git a/src/map.ts b/src/map.ts index c54ee983b..cf7fb850d 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,9 +1,6 @@ import {isDraftable, DRAFT_STATE, latest} from "./common" -// TODO: kill: -import {assertUnrevoked} from "./es5" import {ImmerScope} from "./scope" -import {Immer} from "./immer" import {AnyMap, Drafted} from "./types" // TODO: create own states @@ -35,6 +32,7 @@ const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) // TODO: fix types for drafts +// TODO: assert unrevoked export class DraftMap extends MapBase implements Map { [DRAFT_STATE]: MapState constructor(target, parent) { diff --git a/src/proxy.ts b/src/proxy.ts index 0fb42abc3..ae984d1c5 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -5,20 +5,13 @@ import { has, is, isDraftable, - isDraft, isMap, isSet, - hasSymbol, shallowCopy, DRAFT_STATE, - assignMap, - assignSet, - original, latest } from "./common" import {ImmerScope} from "./scope" -import {proxyMap} from "./map" -import {proxySet} from "./set" import {AnyObject, Drafted, ImmerState, AnyArray} from "./types" interface ProxyBaseState { diff --git a/src/scope.ts b/src/scope.ts index c734dde2a..3a73f466a 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -1,6 +1,6 @@ import {DRAFT_STATE} from "./common" -import {ImmerState, Patch, PatchListener} from "./types" -import { Immer } from "./immer" +import {Patch, PatchListener} from "./types" +import {Immer} from "./immer" /** Each scope represents a `produce` call. */ export class ImmerScope { diff --git a/src/set.ts b/src/set.ts index 0271211c1..36e9a9f46 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,7 +1,6 @@ -import {isDraftable, DRAFT_STATE, latest} from "./common" +import {DRAFT_STATE, latest} from "./common" // TODO: kill: -import {assertUnrevoked} from "./es5" import {ImmerScope} from "./scope" import {AnySet, Drafted} from "./types" @@ -39,6 +38,7 @@ const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) // TODO: fix types for drafts +// TODO: assert unrevoked export class DraftSet extends SetBase implements Set { [DRAFT_STATE]: SetState constructor(target, parent) { diff --git a/src/types.ts b/src/types.ts index 18dc0e49c..9f9a79129 100644 --- a/src/types.ts +++ b/src/types.ts @@ -8,13 +8,6 @@ export type Objectish = any[] | Map | Set | {} export type ObjectishNoSet = any[] | Map | {} -// export interface ImmerState { -// parent?: ImmerState -// base: T -// copy: T -// assigned: {[prop: string]: boolean; [index: number]: boolean} -// } - export type AnyObject = {[key: string]: any} export type AnyArray = Array export type AnySet = Set From 79a1d2c6e97f2f8ecb40312c53c08d334c773eee Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 7 Jan 2020 16:22:59 +0000 Subject: [PATCH 16/28] Dropped Object.assign --- __tests__/polyfills.js | 27 --------------------------- src/common.ts | 20 +++++--------------- src/immer.ts | 9 +++++++-- src/proxy.ts | 5 +++-- src/types.ts | 4 ++-- 5 files changed, 17 insertions(+), 48 deletions(-) diff --git a/__tests__/polyfills.js b/__tests__/polyfills.js index 0a4a39ac6..53a1415cd 100644 --- a/__tests__/polyfills.js +++ b/__tests__/polyfills.js @@ -30,33 +30,6 @@ describe("Symbol", () => { }) }) -describe("Object.assign", () => { - const {assign} = common - - it("only copies enumerable keys", () => { - const src = {a: 1} - Object.defineProperty(src, "b", {value: 1}) - const dest = {} - assign(dest, src) - expect(dest.a).toBe(1) - expect(dest.b).toBeUndefined() - }) - - it("only copies own properties", () => { - const src = Object.create({a: 1}) - src.b = 1 - const dest = {} - assign(dest, src) - expect(dest.a).toBeUndefined() - expect(dest.b).toBe(1) - }) - - it("can handle null", () => { - const res = assign({a: 1, b: 2}, null, {a: 2, c: 2}) - expect(res).toEqual({a: 2, b: 2, c: 2}) - }) -}) - describe("Reflect.ownKeys", () => { const {ownKeys} = common diff --git a/src/common.ts b/src/common.ts index 469e0838d..6c4efde74 100644 --- a/src/common.ts +++ b/src/common.ts @@ -1,4 +1,4 @@ -import {Objectish, ObjectishNoSet} from "./types" +import {Objectish, ObjectishNoSet, Drafted, AnyObject, AnyArray} from "./types" /** Use a class type for `nothing` so its type is unique */ export class Nothing { @@ -50,7 +50,7 @@ export function isDraftable(value: any): boolean { ) } -export function isPlainObject(value): boolean { +export function isPlainObject(value): value is AnyObject | AnyArray { if (!value || typeof value !== "object") return false if (Array.isArray(value)) return true const proto = Object.getPrototypeOf(value) @@ -58,23 +58,13 @@ export function isPlainObject(value): boolean { } /** Get the underlying object that is represented by the given draft */ -export function original(value: T): T | undefined { +export function original(value: Drafted): T | undefined { if (value && value[DRAFT_STATE]) { - return value[DRAFT_STATE].base + return value[DRAFT_STATE].base as any } // otherwise return undefined } -export const assign = - Object.assign || - ((target, ...overrides) => { - overrides.forEach(override => { - if (typeof override === "object" && override !== null) - Object.keys(override).forEach(key => (target[key] = override[key])) - }) - return target - }) - export const ownKeys: (target) => PropertyKey[] = typeof Reflect !== "undefined" && Reflect.ownKeys ? Reflect.ownKeys @@ -99,7 +89,7 @@ export function each(obj, iter) { } } -export function isEnumerable(base: {}, prop: PropertyKey): boolean { +export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { const desc = Object.getOwnPropertyDescriptor(base, prop) return desc && desc.enumerable ? true : false } diff --git a/src/immer.ts b/src/immer.ts index 42ad0de90..f8051db53 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -3,7 +3,6 @@ import {createProxy, markChanged} from "./proxy" import {applyPatches, generatePatches} from "./patches" import { - assign, each, has, is, @@ -73,7 +72,13 @@ export class Immer implements ProducersFns { onDelete?: (state: ImmerState, prop: string | number) => void onCopy?: (state: ImmerState) => void }) { - assign(this, configDefaults, config) + each(configDefaults, (key, value) => { + this[key] = value + }) + config && + each(config, (key, value) => { + this[key] = value + }) this.setUseProxies(this.useProxies) this.produce = this.produce.bind(this) this.produceWithPatches = this.produceWithPatches.bind(this) diff --git a/src/proxy.ts b/src/proxy.ts index ae984d1c5..879e39548 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -1,6 +1,5 @@ "use strict" import { - assign, each, has, is, @@ -228,7 +227,9 @@ export function markChanged(state) { if (!isMap(base) && !isSet(base)) { // TODO: drop creating copies here? const copy = (state.copy = shallowCopy(base)) - assign(copy, drafts) + each(drafts, (key, value) => { + copy[key] = value + }) state.drafts = null } diff --git a/src/types.ts b/src/types.ts index 9f9a79129..a9c855aed 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,9 +4,9 @@ import {MapState} from "./map" import {ProxyObjectState, ProxyArrayState} from "./proxy" import {ES5ObjectState, ES5ArrayState} from "./es5" -export type Objectish = any[] | Map | Set | {} +export type Objectish = AnyObject | AnyArray | AnyMap | AnySet -export type ObjectishNoSet = any[] | Map | {} +export type ObjectishNoSet = AnyObject | AnyArray | AnyMap export type AnyObject = {[key: string]: any} export type AnyArray = Array From aeba1ffd58f7905b9a626b2a99d943118a36c8aa Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 12:40:37 +0000 Subject: [PATCH 17/28] Further cleanup in progress --- src/common.ts | 25 +++-- src/es5.ts | 1 - src/finalize.ts | 243 ++++++++++++++++++++++++++++++++++++++++++++++++ src/immer.ts | 201 ++------------------------------------- src/map.ts | 2 +- src/proxy.ts | 1 - src/set.ts | 2 +- 7 files changed, 269 insertions(+), 206 deletions(-) create mode 100644 src/finalize.ts diff --git a/src/common.ts b/src/common.ts index 6c4efde74..fd11c1282 100644 --- a/src/common.ts +++ b/src/common.ts @@ -1,4 +1,13 @@ -import {Objectish, ObjectishNoSet, Drafted, AnyObject, AnyArray} from "./types" +import { + Objectish, + ObjectishNoSet, + Drafted, + AnyObject, + AnyArray, + AnyMap, + AnySet, + ImmerState +} from "./types" /** Use a class type for `nothing` so its type is unique */ export class Nothing { @@ -77,7 +86,7 @@ export const ownKeys: (target) => PropertyKey[] = export function each( obj: T, - iter: (key: PropertyKey, value: any, source: T) => void + iter: (key: string | number, value: any, source: T) => void ) export function each(obj, iter) { if (Array.isArray(obj) || isMap(obj) || isSet(obj)) { @@ -102,11 +111,11 @@ export function has(thing: ObjectishNoSet, prop: PropertyKey): boolean { : Object.prototype.hasOwnProperty.call(thing, prop) } -export function get(thing: ObjectishNoSet, prop: PropertyKey) { +export function get(thing: ObjectishNoSet, prop: PropertyKey): any { return isMap(thing) ? thing.get(prop) : thing[prop] } -export function is(x, y): boolean { +export function is(x: any, y: any): boolean { // From: https://github.com/facebook/fbjs/blob/c69904a511b900266935168223063dd8772dfc40/packages/fbjs/src/core/shallowEqual.js if (x === y) { return x !== 0 || 1 / x === 1 / y @@ -119,21 +128,21 @@ export const hasSymbol = typeof Symbol !== "undefined" export const hasMap = typeof Map !== "undefined" -export function isMap(target): target is Map { +export function isMap(target: any): target is AnyMap { return hasMap && target instanceof Map } export const hasSet = typeof Set !== "undefined" -export function isSet(target): target is Set { +export function isSet(target: any): target is AnySet { return hasSet && target instanceof Set } -export function latest(state: any): any { +export function latest(state: ImmerState): any { return state.copy || state.base } -export function shallowCopy( +export function shallowCopy( base: T, invokeGetters?: boolean ): T diff --git a/src/es5.ts b/src/es5.ts index e47d27f92..0c5c0c937 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -144,7 +144,6 @@ export function markChangedES5(state) { } } -// TODO: kill export function prepareCopy(state) { if (!state.copy) state.copy = clonePotentialDraft(state.base) } diff --git a/src/finalize.ts b/src/finalize.ts new file mode 100644 index 000000000..37fbe80b5 --- /dev/null +++ b/src/finalize.ts @@ -0,0 +1,243 @@ +import {Immer} from "./immer" +import {ImmerState, Drafted} from "./types" +import {ImmerScope} from "./scope" +import { + isSet, + has, + is, + get, + each, + isMap, + isEnumerable, + DRAFT_STATE, + NOTHING, + freeze, + shallowCopy +} from "./common" +import {isDraft, isDraftable} from "./index" +import {SetState} from "./set" +import {generatePatches} from "./patches" + +type PatchPath = Array | null + +export function processResult(immer: Immer, result: any, scope: ImmerScope) { + const baseDraft = scope.drafts![0] + const isReplaced = result !== undefined && result !== baseDraft + immer.willFinalize(scope, result, isReplaced) + if (isReplaced) { + if (baseDraft[DRAFT_STATE].modified) { + scope.revoke() + throw new Error("An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.") // prettier-ignore + } + if (isDraftable(result)) { + // Finalize the result in case it contains (or is) a subset of the draft. + result = finalize(immer, result, null, scope) + maybeFreeze(immer, result) + } + if (scope.patches) { + scope.patches.push({ + op: "replace", + path: [], + value: result + }) + scope.inversePatches!.push({ + op: "replace", + path: [], + value: baseDraft[DRAFT_STATE].base + }) + } + } else { + // Finalize the base draft. + result = finalize(immer, baseDraft, [], scope) + } + scope.revoke() + if (scope.patches) { + scope.patchListener!(scope.patches, scope.inversePatches!) + } + return result !== NOTHING ? result : undefined +} + +function finalize( + immer: Immer, + draft: Drafted, + path: PatchPath, + scope: ImmerScope +) { + const state = draft[DRAFT_STATE] + if (!state) { + if (Object.isFrozen(draft)) return draft + return finalizeTree(immer, draft, null, scope) + } + // Never finalize drafts owned by another scope. + if (state.scope !== scope) { + return draft + } + if (!state.modified) { + maybeFreeze(immer, state.base, true) + return state.base + } + if (!state.finalized) { + state.finalized = true + finalizeTree(immer, state.draft, path, scope) + + // We cannot really delete anything inside of a Set. We can only replace the whole Set. + if (immer.onDelete && !isSet(state.base)) { + // The `assigned` object is unreliable with ES5 drafts. + if (immer.useProxies) { + const {assigned} = state + each(assigned, (prop, exists) => { + if (!exists) immer.onDelete?.(state, prop as any) + }) + } else { + // TODO: Figure it out for Maps and Sets if we need to support ES5 + const {base, copy} = state + each(base, prop => { + if (!has(copy, prop)) immer.onDelete?.(state, prop as any) + }) + } + } + if (immer.onCopy) { + immer.onCopy(state) + } + + // At this point, all descendants of `state.copy` have been finalized, + // so we can be sure that `scope.canAutoFreeze` is accurate. + if (immer.autoFreeze && scope.canAutoFreeze) { + freeze(state.copy, false) + } + + if (path && scope.patches) { + generatePatches(state, path, scope.patches, scope.inversePatches!) + } + } + return state.copy +} + +function finalizeTree( + immer: Immer, + root: Drafted, + rootPath: PatchPath, + scope: ImmerScope +) { + const state = root[DRAFT_STATE] + if (state) { + // TODO: kill isMap / isSet here + if (!immer.useProxies && !isMap(root) && !isSet(root)) { + // Create the final copy, with added keys and without deleted keys. + state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? + } + + root = state.copy + } + + const needPatches = !!rootPath && !!scope.patches + + each(root, (key, value) => + finalizeProperty( + immer, + root, + state, + key, + value, + root, + needPatches, + scope, + rootPath + ) + ) + return root +} + +function finalizeProperty( + // TODO: can do with less args? + immer: Immer, + root: ImmerState, + state: ImmerState, + prop: string | number, + value: any, + parent: ImmerState, + needPatches: boolean, + scope: ImmerScope, + rootPath: PatchPath +) { + if (value === parent) { + throw Error("Immer forbids circular references") + } + + // In the `finalizeTree` method, only the `root` object may be a draft. + const isDraftProp = !!state && parent === root + const isSetMember = isSet(parent) + + if (isDraft(value)) { + const path = + isDraftProp && + needPatches && + !isSetMember && // Set objects are atomic since they have no keys. + !has((state as Exclude).assigned!, prop) // Skip deep patches for assigned keys. + ? rootPath!.concat(prop) + : null + + // Drafts owned by `scope` are finalized here. + value = finalize(immer, value, path, scope) + replace(parent, prop, value) + + // Drafts from another scope must prevent auto-freezing. + if (isDraft(value)) { + scope.canAutoFreeze = false + } + + // Unchanged drafts are never passed to the `onAssign` hook. + // if (isDraftProp && !isSet && value === get(state.base, prop)) return + } + // Unchanged draft properties are ignored. + else if (isDraftProp && is(value, get(state.base, prop))) { + return + } + // Search new objects for unfinalized drafts. Frozen objects should never contain drafts. + else if (isDraftable(value) && !Object.isFrozen(value)) { + each(value, (key, v) => + finalizeProperty( + immer, + root, + state, + key, + v, + value, + needPatches, + scope, + rootPath + ) + ) + maybeFreeze(immer, value) + } + + if (isDraftProp && immer.onAssign && !isSetMember) { + immer.onAssign(state, prop, value) + } +} + +function replace(parent: Drafted, prop: PropertyKey, value: any) { + // TODO: kill isMap clal here + if (isMap(parent)) { + parent.set(prop, value) + } else if (isSet(parent)) { + // In this case, the `prop` is actually a draft. + parent.delete(prop) + parent.add(value) + } else if (Array.isArray(parent) || isEnumerable(parent, prop)) { + // Preserve non-enumerable properties. + parent[prop] = value + } else { + Object.defineProperty(parent, prop, { + value, + writable: true, + configurable: true + }) + } +} + +export function maybeFreeze(immer: Immer, value: any, deep = false) { + if (immer.autoFreeze && !isDraft(value)) { + freeze(value, deep) + } +} diff --git a/src/immer.ts b/src/immer.ts index f8051db53..9a9cc3c0a 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -31,7 +31,8 @@ import { Drafted } from "./types" import {proxyMap} from "./map" -import {proxySet} from "./set" +import {proxySet, SetState} from "./set" +import {processResult, maybeFreeze} from "./finalize" function verifyMinified() {} @@ -144,7 +145,7 @@ export class Immer implements ProducersFns { return result.then( result => { scope.usePatches(patchListener) - return this.processResult(result, scope) + return processResult(this, result, scope) }, error => { scope.revoke() @@ -153,12 +154,12 @@ export class Immer implements ProducersFns { ) } scope.usePatches(patchListener) - return this.processResult(result, scope) + return processResult(this, result, scope) } else { result = recipe(base) if (result === NOTHING) return undefined if (result === undefined) result = base - this.maybeFreeze(result, true) + maybeFreeze(this, result, true) return result } } @@ -188,7 +189,7 @@ export class Immer implements ProducersFns { const proxy = this.createProxy(base, undefined) proxy[DRAFT_STATE].isManual = true scope.leave() - return proxy as any + return proxy } finishDraft>( @@ -204,7 +205,7 @@ export class Immer implements ProducersFns { } const {scope} = state scope.usePatches(patchListener) - return this.processResult(undefined, scope) + return processResult(this, undefined, scope) } /** @@ -248,44 +249,6 @@ export class Immer implements ProducersFns { ) } - /** @internal */ - processResult(result: any, scope: ImmerScope) { - const baseDraft = scope.drafts![0] - const isReplaced = result !== undefined && result !== baseDraft - this.willFinalize(scope, result, isReplaced) - if (isReplaced) { - if (baseDraft[DRAFT_STATE].modified) { - scope.revoke() - throw new Error("An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.") // prettier-ignore - } - if (isDraftable(result)) { - // Finalize the result in case it contains (or is) a subset of the draft. - result = this.finalize(result, null, scope) - this.maybeFreeze(result) - } - if (scope.patches) { - scope.patches.push({ - op: "replace", - path: [], - value: result - }) - scope.inversePatches!.push({ - op: "replace", - path: [], - value: baseDraft[DRAFT_STATE].base - }) - } - } else { - // Finalize the base draft. - result = this.finalize(baseDraft, [], scope) - } - scope.revoke() - if (scope.patches) { - scope.patchListener!(scope.patches, scope.inversePatches!) - } - return result !== NOTHING ? result : undefined - } - createProxy(value: any, parent?: ImmerState) { if (!value || typeof value !== "object") return value @@ -320,154 +283,4 @@ export class Immer implements ProducersFns { markChangedES5(state) } } - - /** - * @internal - * Finalize a draft, returning either the unmodified base state or a modified - * copy of the base state. - */ - finalize(draft: any, path: string[] | null, scope: ImmerScope) { - const state = draft[DRAFT_STATE] - if (!state) { - if (Object.isFrozen(draft)) return draft - return this.finalizeTree(draft, null, scope) - } - // Never finalize drafts owned by another scope. - if (state.scope !== scope) { - return draft - } - if (!state.modified) { - this.maybeFreeze(state.base, true) - return state.base - } - if (!state.finalized) { - state.finalized = true - this.finalizeTree(state.draft, path, scope) - - // We cannot really delete anything inside of a Set. We can only replace the whole Set. - if (this.onDelete && !isSet(state.base)) { - // The `assigned` object is unreliable with ES5 drafts. - if (this.useProxies) { - const {assigned} = state - each(assigned, (prop, exists) => { - if (!exists) this.onDelete?.(state, prop as any) - }) - } else { - // TODO: Figure it out for Maps and Sets if we need to support ES5 - const {base, copy} = state - each(base, prop => { - if (!has(copy, prop)) this.onDelete?.(state, prop as any) - }) - } - } - if (this.onCopy) { - this.onCopy(state) - } - - // At this point, all descendants of `state.copy` have been finalized, - // so we can be sure that `scope.canAutoFreeze` is accurate. - if (this.autoFreeze && scope.canAutoFreeze) { - freeze(state.copy, false) - } - - if (path && scope.patches) { - generatePatches(state, path, scope.patches, scope.inversePatches!) - } - } - return state.copy - } - - /** - * @internal - * Finalize all drafts in the given state tree. - */ - finalizeTree(root: any, rootPath: string[] | null, scope: ImmerScope) { - const state = root[DRAFT_STATE] - if (state) { - // TODO: remove crap - // if (state.modified && !state.copy) { - // state.copy = shallowCopy(state.base, false) - // } - // else - if (!this.useProxies && !isMap(root) && !isSet(root)) { - // Create the final copy, with added keys and without deleted keys. - state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? - } - - root = state.copy - } - - const needPatches = !!rootPath && !!scope.patches - const finalizeProperty = (prop, value, parent) => { - if (value === parent) { - throw Error("Immer forbids circular references") - } - - // In the `finalizeTree` method, only the `root` object may be a draft. - const isDraftProp = !!state && parent === root - const isSetMember = isSet(parent) - - if (isDraft(value)) { - const path = - isDraftProp && - needPatches && - !isSetMember && // Set objects are atomic since they have no keys. - !has(state.assigned, prop) // Skip deep patches for assigned keys. - ? rootPath!.concat(prop) - : null - - // Drafts owned by `scope` are finalized here. - value = this.finalize(value, path, scope) - replace(parent, prop, value) - - // Drafts from another scope must prevent auto-freezing. - if (isDraft(value)) { - scope.canAutoFreeze = false - } - - // Unchanged drafts are never passed to the `onAssign` hook. - // if (isDraftProp && !isSet && value === get(state.base, prop)) return - } - // Unchanged draft properties are ignored. - else if (isDraftProp && is(value, get(state.base, prop))) { - return - } - // Search new objects for unfinalized drafts. Frozen objects should never contain drafts. - else if (isDraftable(value) && !Object.isFrozen(value)) { - each(value, finalizeProperty) - this.maybeFreeze(value) - } - - if (isDraftProp && this.onAssign && !isSetMember) { - this.onAssign(state, prop, value) - } - } - - each(root, finalizeProperty) - return root - } - maybeFreeze(value, deep = false) { - if (this.autoFreeze && !isDraft(value)) { - freeze(value, deep) - } - } -} - -function replace(parent, prop, value) { - if (isMap(parent)) { - parent.set(prop, value) - } else if (isSet(parent)) { - // In this case, the `prop` is actually a draft. - parent.delete(prop) - parent.add(value) - } else if (Array.isArray(parent) || isEnumerable(parent, prop)) { - // Preserve non-enumerable properties. - parent[prop] = value - } else { - Object.defineProperty(parent, prop, { - value, - writable: true, - configurable: true - }) - } } diff --git a/src/map.ts b/src/map.ts index cf7fb850d..a173258b4 100644 --- a/src/map.ts +++ b/src/map.ts @@ -32,7 +32,7 @@ const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) // TODO: fix types for drafts -// TODO: assert unrevoked +// TODO: assert unrevoked, use freeze for that export class DraftMap extends MapBase implements Map { [DRAFT_STATE]: MapState constructor(target, parent) { diff --git a/src/proxy.ts b/src/proxy.ts index 879e39548..30bdb9b03 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -239,7 +239,6 @@ export function markChanged(state) { } } -// TODO: unify with ES5 version function prepareCopy(state) { if (!state.copy) { state.copy = shallowCopy(state.base) diff --git a/src/set.ts b/src/set.ts index 36e9a9f46..d8310ca1c 100644 --- a/src/set.ts +++ b/src/set.ts @@ -38,7 +38,7 @@ const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) // TODO: fix types for drafts -// TODO: assert unrevoked +// TODO: assert unrevoked, use freeze for that export class DraftSet extends SetBase implements Set { [DRAFT_STATE]: SetState constructor(target, parent) { From dd1808ee61fb2645c08fa996ca83120f80f17120 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 19:47:00 +0000 Subject: [PATCH 18/28] TS type improvements --- src/common.ts | 66 ++++++++++++++++++++++++++++----- src/es5.ts | 97 ++++++++++++++++++++----------------------------- src/finalize.ts | 62 +++++++++---------------------- src/immer.ts | 76 ++++++++++++++++++-------------------- src/map.ts | 34 ++++++++--------- src/patches.ts | 22 ++++++----- src/proxy.ts | 26 ++++++++----- src/scope.ts | 4 +- src/set.ts | 46 ++++++++++++----------- src/types.ts | 2 +- tsconfig.json | 2 +- 11 files changed, 224 insertions(+), 213 deletions(-) diff --git a/src/common.ts b/src/common.ts index fd11c1282..3fb512ce4 100644 --- a/src/common.ts +++ b/src/common.ts @@ -59,7 +59,7 @@ export function isDraftable(value: any): boolean { ) } -export function isPlainObject(value): value is AnyObject | AnyArray { +export function isPlainObject(value: any): boolean { if (!value || typeof value !== "object") return false if (Array.isArray(value)) return true const proto = Object.getPrototypeOf(value) @@ -74,7 +74,7 @@ export function original(value: Drafted): T | undefined { // otherwise return undefined } -export const ownKeys: (target) => PropertyKey[] = +export const ownKeys: (target: AnyObject) => PropertyKey[] = typeof Reflect !== "undefined" && Reflect.ownKeys ? Reflect.ownKeys : typeof Object.getOwnPropertySymbols !== "undefined" @@ -87,14 +87,14 @@ export const ownKeys: (target) => PropertyKey[] = export function each( obj: T, iter: (key: string | number, value: any, source: T) => void -) -export function each(obj, iter) { +): void +export function each(obj: any, iter: any) { if (Array.isArray(obj) || isMap(obj) || isSet(obj)) { - obj.forEach((entry, index) => iter(index, entry, obj)) + obj.forEach((entry: any, index: any) => iter(index, entry, obj)) } else if (obj && typeof obj === "object") { ownKeys(obj).forEach(key => iter(key, obj[key], obj)) } else { - throw new Error("Nope") + die() } } @@ -103,6 +103,8 @@ export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { return desc && desc.enumerable ? true : false } +// TODO: use draft meta data if present? +// make refelction methods to determine types? export function has(thing: ObjectishNoSet, prop: PropertyKey): boolean { return !thing ? false @@ -111,8 +113,38 @@ export function has(thing: ObjectishNoSet, prop: PropertyKey): boolean { : Object.prototype.hasOwnProperty.call(thing, prop) } -export function get(thing: ObjectishNoSet, prop: PropertyKey): any { - return isMap(thing) ? thing.get(prop) : thing[prop] +// TODO: use draft meta data if present? +export function get(thing: AnyMap | AnyObject, prop: PropertyKey): any { + return isMap(thing) ? thing.get(prop) : thing[prop as any] +} + +// TODO: measure fast vs slow path, use type switch instead +export function set(parent: Drafted, prop: PropertyKey, value: any) { + // fast path switch on meta data + if (parent[DRAFT_STATE]) + switch (parent[DRAFT_STATE].type) { + case "map": + parent.set(prop, value) + break + case "set": + parent.delete(prop) + parent.add(value) + break + default: + parent[prop] = value + } + + // slow path + parent[prop] = value + if (isMap(parent)) { + parent.set(prop, value) + } else if (isSet(parent)) { + // In this case, the `prop` is actually a draft. + parent.delete(prop) + parent.add(value) + } else { + parent[prop] = value + } } export function is(x: any, y: any): boolean { @@ -146,7 +178,7 @@ export function shallowCopy( base: T, invokeGetters?: boolean ): T -export function shallowCopy(base, invokeGetters = false) { +export function shallowCopy(base: any, invokeGetters = false) { if (Array.isArray(base)) return base.slice() const clone = Object.create(Object.getPrototypeOf(base)) ownKeys(base).forEach(key => { @@ -191,3 +223,19 @@ export function freeze( function dontMutateFrozenCollections() { throw new Error("This object has been frozen and should not be mutated") } + +export function createHiddenProperty( + target: AnyObject, + prop: PropertyKey, + value: any +) { + Object.defineProperty(target, prop, { + value: value, + enumerable: false, + writable: true + }) +} + +export function die(): never { + throw new Error("Illegal state, please file a bug") +} diff --git a/src/es5.ts b/src/es5.ts index 0c5c0c937..41ec5fe43 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -10,13 +10,13 @@ import { isSet, shallowCopy, DRAFT_STATE, - latest + latest, + createHiddenProperty } from "./common" import {ImmerScope} from "./scope" -import {ImmerState, Drafted, AnyObject, AnyMap} from "./types" +import {ImmerState, Drafted, AnyObject, AnyMap, Objectish} from "./types" -// TODO: merge with ImmerState? interface ES5BaseState { scope: ImmerScope modified: boolean @@ -24,8 +24,7 @@ interface ES5BaseState { finalized: boolean assigned: {[key: string]: any} parent?: ImmerState - drafts: AnyMap | null // TODO: don't use map? - revoke() + revoke(): void revoked: boolean } @@ -69,7 +68,7 @@ export function willFinalizeES5( export function createES5Proxy( base: T, parent?: ImmerState -): Drafted { +): Drafted { const isArray = Array.isArray(base) const draft = clonePotentialDraft(base) @@ -87,7 +86,6 @@ export function createES5Proxy( parent, base, draft, - drafts: null, copy: null, revoke, revoked: false // es5 only @@ -101,9 +99,8 @@ function revoke(this: ES5BaseState) { this.revoked = true } -// TODO: type these methods // Access a property without creating an Immer draft. -function peek(draft, prop) { +function peek(draft: Drafted, prop: PropertyKey) { const state = draft[DRAFT_STATE] if (state && !state.finalizing) { state.finalizing = true @@ -114,19 +111,19 @@ function peek(draft, prop) { return draft[prop] } -function get(state, prop) { +function get(state: ES5State, prop: string | number) { assertUnrevoked(state) const value = peek(latest(state), prop) if (state.finalizing) return value // Create a draft if the value is unmodified. if (value === peek(state.base, prop) && isDraftable(value)) { prepareCopy(state) - return (state.copy[prop] = state.scope.immer.createProxy(value, state)) + return (state.copy![prop] = state.scope.immer.createProxy(value, state)) } return value } -function set(state: ES5State, prop, value) { +function set(state: ES5State, prop: string | number, value: any) { assertUnrevoked(state) state.assigned[prop] = true if (!state.modified) { @@ -137,19 +134,19 @@ function set(state: ES5State, prop, value) { state.copy![prop] = value } -export function markChangedES5(state) { +export function markChangedES5(state: ImmerState) { if (!state.modified) { state.modified = true if (state.parent) markChangedES5(state.parent) } } -function prepareCopy(state) { +function prepareCopy(state: ES5State) { if (!state.copy) state.copy = clonePotentialDraft(state.base) } -function clonePotentialDraft(base) { - const state = base && base[DRAFT_STATE] +function clonePotentialDraft(base: Objectish) { + const state = base && (base as any)[DRAFT_STATE] if (state) { state.finalizing = true const draft = shallowCopy(state.draft, true) @@ -161,9 +158,13 @@ function clonePotentialDraft(base) { // property descriptors are recycled to make sure we don't create a get and set closure per property, // but share them all instead -const descriptors = {} +const descriptors: {[prop: string]: PropertyDescriptor} = {} -function proxyProperty(draft, prop, enumerable) { +function proxyProperty( + draft: Drafted, + prop: string | number, + enumerable: boolean +) { let desc = descriptors[prop] if (desc) { desc.enumerable = enumerable @@ -171,10 +172,10 @@ function proxyProperty(draft, prop, enumerable) { descriptors[prop] = desc = { configurable: true, enumerable, - get() { + get(this: any) { return get(this[DRAFT_STATE], prop) }, - set(value) { + set(this: any, value) { set(this[DRAFT_STATE], prop, value) } } @@ -182,7 +183,7 @@ function proxyProperty(draft, prop, enumerable) { Object.defineProperty(draft, prop, desc) } -export function assertUnrevoked(state) { +export function assertUnrevoked(state: ES5State) { if (state.revoked === true) throw new Error( "Cannot use a proxy that has been revoked. Did you pass an object from inside an immer function to an async process? " + @@ -191,7 +192,7 @@ export function assertUnrevoked(state) { } // This looks expensive, but only proxies are visited, and only objects without known changes are scanned. -function markChangesSweep(drafts) { +function markChangesSweep(drafts: Drafted[]) { // The natural order of drafts in the `scope` array is based on when they // were accessed. By processing drafts in reverse natural order, we have a // better chance of processing leaf nodes first. When a leaf node is known to @@ -199,56 +200,46 @@ function markChangesSweep(drafts) { for (let i = drafts.length - 1; i >= 0; i--) { const state = drafts[i][DRAFT_STATE] if (!state.modified) { - if (Array.isArray(state.base)) { - if (hasArrayChanges(state)) markChangedES5(state) - } else if (isMap(state.base)) { - // TODO: use switch - continue - } else if (isSet(state.base)) { - continue - } else if (hasObjectChanges(state)) { - markChangedES5(state) + switch (state.type) { + case "es5_array": + if (hasArrayChanges(state)) markChangedES5(state) + break + case "es5_object": + if (hasObjectChanges(state)) markChangedES5(state) + break } } } } -// TODO: refactor this to work per object-type -// TODO: Set / Map shouldn't be ES specific -function markChangesRecursively(object) { +function markChangesRecursively(object: any) { if (!object || typeof object !== "object") return const state = object[DRAFT_STATE] if (!state) return - const {base, draft, assigned} = state - if (isSet(object) || isMap(object)) { - // TODO: create switch here - return - } else if (!Array.isArray(object)) { + const {base, draft, assigned, type} = state + if (type === "es5_object") { // Look for added keys. - // TODO: use each? - Object.keys(draft).forEach(key => { + // TODO: looks quite duplicate to hasObjectChanges + each(draft, key => { + if ((key as any) === DRAFT_STATE) return // The `undefined` check is a fast path for pre-existing keys. if (base[key] === undefined && !has(base, key)) { - // TODO: this code seems invalid for Maps! assigned[key] = true markChangedES5(state) } else if (!assigned[key]) { - // TODO: === false ? // Only untouched properties trigger recursion. markChangesRecursively(draft[key]) } }) // Look for removed keys. - // TODO: is this code needed? - // TODO: use each? - Object.keys(base).forEach(key => { + each(base, key => { // The `undefined` check is a fast path for pre-existing keys. if (draft[key] === undefined && !has(draft, key)) { assigned[key] = false markChangedES5(state) } }) - } else if (hasArrayChanges(state)) { + } else if (type === "es5_array" && hasArrayChanges(state)) { markChangedES5(state) assigned.length = true if (draft.length < base.length) { @@ -263,7 +254,7 @@ function markChangesRecursively(object) { } } -function hasObjectChanges(state) { +function hasObjectChanges(state: ES5ObjectState) { const {base, draft} = state // Search for added keys and changed keys. Start at the back, because @@ -292,7 +283,7 @@ function hasObjectChanges(state) { return keys.length !== Object.keys(base).length } -function hasArrayChanges(state) { +function hasArrayChanges(state: ES5ArrayState) { const {draft} = state if (draft.length !== state.base.length) return true // See #116 @@ -308,11 +299,3 @@ function hasArrayChanges(state) { // For all other cases, we don't have to compare, as they would have been picked up by the index setters return false } - -function createHiddenProperty(target, prop, value) { - Object.defineProperty(target, prop, { - value: value, - enumerable: false, - writable: true - }) -} diff --git a/src/finalize.ts b/src/finalize.ts index 37fbe80b5..dbf819e1f 100644 --- a/src/finalize.ts +++ b/src/finalize.ts @@ -12,13 +12,12 @@ import { DRAFT_STATE, NOTHING, freeze, - shallowCopy + shallowCopy, + set } from "./common" import {isDraft, isDraftable} from "./index" import {SetState} from "./set" -import {generatePatches} from "./patches" - -type PatchPath = Array | null +import {generatePatches, PatchPath} from "./patches" export function processResult(immer: Immer, result: any, scope: ImmerScope) { const baseDraft = scope.drafts![0] @@ -31,7 +30,7 @@ export function processResult(immer: Immer, result: any, scope: ImmerScope) { } if (isDraftable(result)) { // Finalize the result in case it contains (or is) a subset of the draft. - result = finalize(immer, result, null, scope) + result = finalize(immer, result, scope) maybeFreeze(immer, result) } if (scope.patches) { @@ -48,7 +47,7 @@ export function processResult(immer: Immer, result: any, scope: ImmerScope) { } } else { // Finalize the base draft. - result = finalize(immer, baseDraft, [], scope) + result = finalize(immer, baseDraft, scope, []) } scope.revoke() if (scope.patches) { @@ -60,13 +59,13 @@ export function processResult(immer: Immer, result: any, scope: ImmerScope) { function finalize( immer: Immer, draft: Drafted, - path: PatchPath, - scope: ImmerScope + scope: ImmerScope, + path?: PatchPath ) { const state = draft[DRAFT_STATE] if (!state) { if (Object.isFrozen(draft)) return draft - return finalizeTree(immer, draft, null, scope) + return finalizeTree(immer, draft, scope) } // Never finalize drafts owned by another scope. if (state.scope !== scope) { @@ -78,7 +77,7 @@ function finalize( } if (!state.finalized) { state.finalized = true - finalizeTree(immer, state.draft, path, scope) + finalizeTree(immer, state.draft, scope, path) // We cannot really delete anything inside of a Set. We can only replace the whole Set. if (immer.onDelete && !isSet(state.base)) { @@ -89,7 +88,6 @@ function finalize( if (!exists) immer.onDelete?.(state, prop as any) }) } else { - // TODO: Figure it out for Maps and Sets if we need to support ES5 const {base, copy} = state each(base, prop => { if (!has(copy, prop)) immer.onDelete?.(state, prop as any) @@ -116,22 +114,18 @@ function finalize( function finalizeTree( immer: Immer, root: Drafted, - rootPath: PatchPath, - scope: ImmerScope + scope: ImmerScope, + rootPath?: PatchPath ) { const state = root[DRAFT_STATE] if (state) { - // TODO: kill isMap / isSet here - if (!immer.useProxies && !isMap(root) && !isSet(root)) { + if (state.type === "es5_object" || state.type === "es5_array") { // Create the final copy, with added keys and without deleted keys. - state.copy = shallowCopy(state.draft, true) // TODO: optimization, can we get rid of this and just use state.copy? + state.copy = shallowCopy(state.draft, true) } - root = state.copy } - const needPatches = !!rootPath && !!scope.patches - each(root, (key, value) => finalizeProperty( immer, @@ -155,10 +149,10 @@ function finalizeProperty( state: ImmerState, prop: string | number, value: any, - parent: ImmerState, + parent: Drafted, needPatches: boolean, scope: ImmerScope, - rootPath: PatchPath + rootPath?: PatchPath ) { if (value === parent) { throw Error("Immer forbids circular references") @@ -175,11 +169,11 @@ function finalizeProperty( !isSetMember && // Set objects are atomic since they have no keys. !has((state as Exclude).assigned!, prop) // Skip deep patches for assigned keys. ? rootPath!.concat(prop) - : null + : undefined // Drafts owned by `scope` are finalized here. - value = finalize(immer, value, path, scope) - replace(parent, prop, value) + value = finalize(immer, value, scope, path) + set(parent, prop, value) // Drafts from another scope must prevent auto-freezing. if (isDraft(value)) { @@ -216,26 +210,6 @@ function finalizeProperty( } } -function replace(parent: Drafted, prop: PropertyKey, value: any) { - // TODO: kill isMap clal here - if (isMap(parent)) { - parent.set(prop, value) - } else if (isSet(parent)) { - // In this case, the `prop` is actually a draft. - parent.delete(prop) - parent.add(value) - } else if (Array.isArray(parent) || isEnumerable(parent, prop)) { - // Preserve non-enumerable properties. - parent[prop] = value - } else { - Object.defineProperty(parent, prop, { - value, - writable: true, - configurable: true - }) - } -} - export function maybeFreeze(immer: Immer, value: any, deep = false) { if (immer.autoFreeze && !isDraft(value)) { freeze(value, deep) diff --git a/src/immer.ts b/src/immer.ts index 9a9cc3c0a..2097b0b5e 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -1,23 +1,18 @@ import {createES5Proxy, willFinalizeES5, markChangedES5} from "./es5" import {createProxy, markChanged} from "./proxy" -import {applyPatches, generatePatches} from "./patches" +import {applyPatches} from "./patches" import { each, - has, - is, isDraft, isSet, - get, isMap, isDraftable, - isEnumerable, - shallowCopy, DRAFT_STATE, NOTHING, - freeze, isPlainObject, - DRAFTABLE + DRAFTABLE, + die } from "./common" import {ImmerScope} from "./scope" import { @@ -31,7 +26,7 @@ import { Drafted } from "./types" import {proxyMap} from "./map" -import {proxySet, SetState} from "./set" +import {proxySet} from "./set" import {processResult, maybeFreeze} from "./finalize" function verifyMinified() {} @@ -48,7 +43,7 @@ const configDefaults = { onAssign: null, onDelete: null, onCopy: null -} +} as const interface ProducersFns { produce: IProduce @@ -74,12 +69,9 @@ export class Immer implements ProducersFns { onCopy?: (state: ImmerState) => void }) { each(configDefaults, (key, value) => { - this[key] = value + // @ts-ignore + this[key] = config?.[key] ?? value }) - config && - each(config, (key, value) => { - this[key] = value - }) this.setUseProxies(this.useProxies) this.produce = this.produce.bind(this) this.produceWithPatches = this.produceWithPatches.bind(this) @@ -104,15 +96,19 @@ export class Immer implements ProducersFns { * @param {Function} patchListener - optional function that will be called with all the patches produced here * @returns {any} a new state, or the initial state if nothing was modified */ - produce(base, recipe?, patchListener?) { + produce(base: any, recipe?: any, patchListener?: any) { // curried invocation if (typeof base === "function" && typeof recipe !== "function") { const defaultBase = recipe recipe = base const self = this - return function curriedProduce(this: any, base = defaultBase, ...args) { - return self.produce(base, draft => recipe.call(this, draft, ...args)) // prettier-ignore + return function curriedProduce( + this: any, + base = defaultBase, + ...args: any[] + ) { + return self.produce(base, (draft: Drafted) => recipe.call(this, draft, ...args)) // prettier-ignore } } @@ -164,29 +160,30 @@ export class Immer implements ProducersFns { } } - produceWithPatches(arg1, arg2?, arg3?): any { + produceWithPatches(arg1: any, arg2?: any, arg3?: any): any { if (typeof arg1 === "function") { const self = this - return (state, ...args) => - this.produceWithPatches(state, draft => arg1(draft, ...args)) + return (state: any, ...args: any[]) => + this.produceWithPatches(state, (draft: any) => arg1(draft, ...args)) } // non-curried form if (arg3) throw new Error("A patch listener cannot be passed to produceWithPatches") - let patches, inversePatches - const nextState = this.produce(arg1, arg2, (p, ip) => { + let patches: Patch[], inversePatches: Patch[] + const nextState = this.produce(arg1, arg2, (p: Patch[], ip: Patch[]) => { patches = p inversePatches = ip }) - return [nextState, patches, inversePatches] + return [nextState, patches!, inversePatches!] } - createDraft(base: T): Draft { + createDraft(base: T): Drafted { if (!isDraftable(base)) { throw new Error("First argument to `createDraft` must be a plain object, an array, or an immerable object") // prettier-ignore } const scope = ImmerScope.enter(this) const proxy = this.createProxy(base, undefined) + // @ts-ignore TODO: add to structures proxy[DRAFT_STATE].isManual = true scope.leave() return proxy @@ -230,7 +227,7 @@ export class Immer implements ProducersFns { applyPatches(base: Objectish, patches: Patch[]) { // If a patch replaces the entire state, take that replacement as base // before applying patches - let i + let i: number for (i = patches.length - 1; i >= 0; i--) { const patch = patches[i] if (patch.path.length === 0 && patch.op === "replace") { @@ -244,28 +241,25 @@ export class Immer implements ProducersFns { return applyPatches(base, patches) } // Otherwise, produce a copy of the base state. - return this.produce(base, draft => + return this.produce(base, (draft: Drafted) => applyPatches(draft, patches.slice(i + 1)) ) } - createProxy(value: any, parent?: ImmerState) { - if (!value || typeof value !== "object") return value - + createProxy( + value: T, + parent?: ImmerState + ): Drafted { let draft: Drafted - - if ( - isPlainObject(value) || - Array.isArray(value) || - value[DRAFTABLE] || - value?.constructor?.[DRAFTABLE] - ) { + // TODO: use switch + type discovery> + if (isMap(value)) draft = proxyMap(value, parent) + else if (isSet(value)) draft = proxySet(value, parent) + else { + // createProxy should be guarded by isDraftable, so we know we can safely draft draft = this.useProxies ? createProxy(value, parent) : createES5Proxy(value, parent) - } else if (isMap(value)) draft = proxyMap(value, parent) - else if (isSet(value)) draft = proxySet(value, parent) - else return value + } const scope = parent ? parent.scope : ImmerScope.current! scope.drafts.push(draft) @@ -276,7 +270,7 @@ export class Immer implements ProducersFns { if (!this.useProxies) willFinalizeES5(scope, thing, isReplaced) } - markChanged(state: any) { + markChanged(state: ImmerState) { if (this.useProxies) { markChanged(state) } else { diff --git a/src/map.ts b/src/map.ts index a173258b4..965adb5ad 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,14 +1,11 @@ import {isDraftable, DRAFT_STATE, latest} from "./common" import {ImmerScope} from "./scope" -import {AnyMap, Drafted} from "./types" - -// TODO: create own states -// TODO: clean up the maps and such from ES5 / Proxy states +import {AnyMap, Drafted, ImmerState} from "./types" export interface MapState { type: "map" - parent: any // TODO: type + parent?: ImmerState scope: ImmerScope modified: boolean finalizing: boolean @@ -20,13 +17,6 @@ export interface MapState { draft: Drafted } -function prepareCopy(state: MapState) { - if (!state.copy) { - state.assigned = new Map() - state.copy = new Map(state.base) - } -} - // Make sure DraftMap declarion doesn't die if Map is not avialable... const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) @@ -35,12 +25,12 @@ const MapBase: MapConstructor = // TODO: assert unrevoked, use freeze for that export class DraftMap extends MapBase implements Map { [DRAFT_STATE]: MapState - constructor(target, parent) { + constructor(target: AnyMap, parent?: ImmerState) { super() this[DRAFT_STATE] = { type: "map", parent, - scope: parent ? parent.scope : ImmerScope.current, + scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalized: false, finalizing: false, @@ -98,11 +88,10 @@ export class DraftMap extends MapBase implements Map { return state.copy!.clear() } - // @ts-ignore TODO: - forEach(cb) { + forEach(cb: (value: V, key: K, self: this) => void, thisArg?: any) { const state = this[DRAFT_STATE] - latest(state).forEach((_value, key, _map) => { - cb(this.get(key), key, this) + latest(state).forEach((_value: V, key: K, _map: this) => { + cb.call(thisArg, this.get(key), key, this) }) } @@ -164,6 +153,13 @@ export class DraftMap extends MapBase implements Map { } } -export function proxyMap(target, parent) { +export function proxyMap(target: AnyMap, parent?: ImmerState) { return new DraftMap(target, parent) } + +function prepareCopy(state: MapState) { + if (!state.copy) { + state.assigned = new Map() + state.copy = new Map(state.base) + } +} diff --git a/src/patches.ts b/src/patches.ts index 548074fc2..7dc5b33db 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,10 +1,12 @@ -import {get, each, isMap, isSet, has} from "./common" +import {get, each, isMap, isSet, has, die} from "./common" import {Patch, ImmerState} from "./types" import {SetState} from "./set" +export type PatchPath = (string | number)[] + export function generatePatches( state: ImmerState, - basePath: (string | number)[], + basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { @@ -20,7 +22,7 @@ export function generatePatches( function generateArrayPatches( state: any, // TODO: type properly with ImmerState - basePath: (string | number)[], + basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { @@ -83,7 +85,7 @@ function generateArrayPatches( // This is used for both Map objects and normal objects. function generatePatchesFromAssigned( state: any, // TODO: type properly with ImmerState - basePath: (number | string)[], + basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { @@ -107,7 +109,7 @@ function generatePatchesFromAssigned( function generateSetPatches( state: SetState, - basePath: (number | string)[], + basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { @@ -153,7 +155,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { patches.forEach(patch => { const {path, op} = patch - if (!path.length) throw new Error("Illegal state") + if (!path.length) die() let base = draft for (let i = 0; i < path.length - 1; i++) { @@ -175,6 +177,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { // if value is an object, then it's assigned by reference // in the following add or remove ops, the value field inside the patch will also be modifyed // so we use value from the cloned patch + // @ts-ignore base[key] = value } break @@ -189,7 +192,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { ? base.set(key, value) : isSet(base) ? base.add(value) - : (base[key] = value) + : ((base as any)[key] = value) break case "remove": Array.isArray(base) @@ -198,7 +201,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { ? base.delete(key) : isSet(base) ? base.delete(patch.value) - : delete base[key] + : delete (base as any)[key] break default: throw new Error("Unsupported patch operation: " + op) @@ -211,7 +214,8 @@ export function applyPatches(draft: T, patches: Patch[]): T { // TODO: optimize: this is quite a performance hit, can we detect intelligently when it is needed? // E.g. auto-draft when new objects from outside are assigned and modified? // (See failing test when deepClone just returns obj) -function deepClonePatchValue(obj) { +function deepClonePatchValue(obj: T): T +function deepClonePatchValue(obj: any) { if (!obj || typeof obj !== "object") return obj if (Array.isArray(obj)) return obj.map(deepClonePatchValue) if (isMap(obj)) diff --git a/src/proxy.ts b/src/proxy.ts index 30bdb9b03..4db18fed3 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -11,7 +11,7 @@ import { latest } from "./common" import {ImmerScope} from "./scope" -import {AnyObject, Drafted, ImmerState, AnyArray} from "./types" +import {AnyObject, Drafted, ImmerState, AnyArray, Objectish} from "./types" interface ProxyBaseState { scope: ImmerScope @@ -41,17 +41,19 @@ export interface ProxyArrayState extends ProxyBaseState { draft: Drafted } +type ProxyState = ProxyObjectState | ProxyArrayState + /** * Returns a new draft of the `base` object. * * The second argument is the parent draft-state (used internally). */ -export function createProxy( +export function createProxy( base: T, parent?: ImmerState -): Drafted { +): Drafted { const isArray = Array.isArray(base) - const state: ProxyObjectState | ProxyArrayState = { + const state: ProxyState = { type: isArray ? "proxy_array" : ("proxy_object" as any), // Track which produce call this is associated with. scope: parent ? parent.scope : ImmerScope.current!, @@ -97,7 +99,7 @@ export function createProxy( /** * Object drafts */ -const objectTraps: ProxyHandler = { +const objectTraps: ProxyHandler = { get(state, prop) { if (prop === DRAFT_STATE) return state let {drafts} = state @@ -143,6 +145,7 @@ const objectTraps: ProxyHandler = { markChanged(state) } state.assigned[prop] = true + // @ts-ignore state.copy![prop] = value return true }, @@ -156,6 +159,7 @@ const objectTraps: ProxyHandler = { // if an originally not assigned property was deleted delete state.assigned[prop] } + // @ts-ignore if (state.copy) delete state.copy[prop] return true }, @@ -187,6 +191,7 @@ const objectTraps: ProxyHandler = { const arrayTraps: ProxyHandler<[ProxyArrayState]> = {} each(objectTraps, (key, fn) => { + // @ts-ignore arrayTraps[key] = function() { arguments[0] = arguments[0][0] return fn.apply(this, arguments) @@ -210,7 +215,7 @@ arrayTraps.set = function(state, prop, value) { */ // Access a property without creating an Immer draft. -function peek(draft, prop) { +function peek(draft: Drafted, prop: PropertyKey): any { const state = draft[DRAFT_STATE] const desc = Reflect.getOwnPropertyDescriptor( state ? latest(state) : draft, @@ -219,17 +224,20 @@ function peek(draft, prop) { return desc && desc.value } -// TODO: unify with ES5 version, by getting rid of the drafts vs copy distinction? -export function markChanged(state) { +export function markChanged(state: ImmerState) { if (!state.modified) { state.modified = true + // @ts-ignore TODO const {base, drafts, parent} = state + // TODO: get rid of isMap and isSet if (!isMap(base) && !isSet(base)) { // TODO: drop creating copies here? const copy = (state.copy = shallowCopy(base)) each(drafts, (key, value) => { + // @ts-ignore copy[key] = value }) + // @ts-ignore TODO state.drafts = null } @@ -239,7 +247,7 @@ export function markChanged(state) { } } -function prepareCopy(state) { +function prepareCopy(state: ProxyState) { if (!state.copy) { state.copy = shallowCopy(state.base) } diff --git a/src/scope.ts b/src/scope.ts index 3a73f466a..caf52768e 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -1,5 +1,5 @@ import {DRAFT_STATE} from "./common" -import {Patch, PatchListener} from "./types" +import {Patch, PatchListener, Drafted} from "./types" import {Immer} from "./immer" /** Each scope represents a `produce` call. */ @@ -55,6 +55,6 @@ export class ImmerScope { } } -function revoke(draft) { +function revoke(draft: Drafted) { draft[DRAFT_STATE].revoke() } diff --git a/src/set.ts b/src/set.ts index d8310ca1c..33900bb31 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,38 +1,26 @@ -import {DRAFT_STATE, latest} from "./common" +import {DRAFT_STATE, latest, isDraftable} from "./common" // TODO: kill: import {ImmerScope} from "./scope" -import {AnySet, Drafted} from "./types" +import {AnySet, Drafted, ImmerState} from "./types" // TODO: create own states // TODO: clean up the maps and such from ES5 / Proxy states export interface SetState { type: "set" - parent: any // TODO: type + parent?: ImmerState scope: ImmerScope modified: boolean finalizing: boolean finalized: boolean copy: AnySet | undefined base: AnySet - drafts: Map // maps the original value to the draft value in the new set + drafts: Map // maps the original value to the draft value in the new set revoke(): void draft: Drafted } -function prepareCopy(state: SetState) { - if (!state.copy) { - // create drafts for all entries to preserve insertion order - state.copy = new Set() - state.base.forEach(value => { - const draft = state.scope.immer.createProxy(value, state) - state.copy!.add(draft) - state.drafts.set(value, draft) - }) - } -} - // Make sure DraftSet declarion doesn't die if Map is not avialable... const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) @@ -41,12 +29,12 @@ const SetBase: SetConstructor = // TODO: assert unrevoked, use freeze for that export class DraftSet extends SetBase implements Set { [DRAFT_STATE]: SetState - constructor(target, parent) { + constructor(target: AnySet, parent?: ImmerState) { super() this[DRAFT_STATE] = { type: "set", parent, - scope: parent ? parent.scope : ImmerScope.current, + scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalized: false, finalizing: false, @@ -132,17 +120,33 @@ export class DraftSet extends SetBase implements Set { return this.values() } - forEach(cb, thisArg) { + forEach(cb: (value: V, key: V, self: this) => void, thisArg?: any) { const state = this[DRAFT_STATE] const iterator = this.values() let result = iterator.next() while (!result.done) { - cb.call(thisArg, result.value, result.value, state.draft) + cb.call(thisArg, result.value, result.value, this) result = iterator.next() } } } -export function proxySet(target, parent) { +export function proxySet(target: AnySet, parent?: ImmerState) { return new DraftSet(target, parent) } + +function prepareCopy(state: SetState) { + if (!state.copy) { + // create drafts for all entries to preserve insertion order + state.copy = new Set() + state.base.forEach(value => { + if (isDraftable(value)) { + const draft = state.scope.immer.createProxy(value, state) + state.drafts.set(value, draft) + state.copy!.add(draft) + } else { + state.copy!.add(value) + } + }) + } +} diff --git a/src/types.ts b/src/types.ts index a9c855aed..380450a70 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,7 +5,6 @@ import {ProxyObjectState, ProxyArrayState} from "./proxy" import {ES5ObjectState, ES5ArrayState} from "./es5" export type Objectish = AnyObject | AnyArray | AnyMap | AnySet - export type ObjectishNoSet = AnyObject | AnyArray | AnyMap export type AnyObject = {[key: string]: any} @@ -29,6 +28,7 @@ export type ImmerState = | MapState | SetState +// The _internal_ type used for drafts (not to be confused with Draft, which is public facing) export type Drafted = { [DRAFT_STATE]: T } & Base diff --git a/tsconfig.json b/tsconfig.json index 5c65c158d..a4a149bad 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,7 +7,7 @@ "strict": true, "declaration": true, "importHelpers": false, - "noImplicitAny": false, // TODO: true, + "noImplicitAny": true, "esModuleInterop": true } } From 00425e184e3482aa92bc766ea5877b36eaf1ab6d Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 20:20:23 +0000 Subject: [PATCH 19/28] Processed bunch of todos --- src/common.ts | 37 ++++++++++++++++++------------------- src/es5.ts | 26 +++++++++++++++++--------- src/immer.ts | 1 - src/map.ts | 32 +++++++++++++------------------- src/patches.ts | 43 ++++++++++++++++++++++++++++--------------- src/proxy.ts | 48 ++++++++++++++++++++++++------------------------ src/scope.ts | 6 ++---- src/set.ts | 26 ++++++++------------------ src/types.ts | 9 +++++++++ 9 files changed, 119 insertions(+), 109 deletions(-) diff --git a/src/common.ts b/src/common.ts index 3fb512ce4..919666d63 100644 --- a/src/common.ts +++ b/src/common.ts @@ -16,13 +16,16 @@ export class Nothing { private _: any } +const hasSymbol = typeof Symbol !== "undefined" +export const hasMap = typeof Map !== "undefined" +export const hasSet = typeof Set !== "undefined" + /** * The sentinel value returned by producers to replace the draft with undefined. */ -export const NOTHING: Nothing = - typeof Symbol !== "undefined" - ? Symbol("immer-nothing") - : ({["immer-nothing"]: true} as any) +export const NOTHING: Nothing = hasSymbol + ? Symbol("immer-nothing") + : ({["immer-nothing"]: true} as any) /** * To let Immer treat your class instances as plain immutable objects @@ -32,15 +35,17 @@ export const NOTHING: Nothing = * Otherwise, your class instance will never be drafted, which means it won't be * safe to mutate in a produce callback. */ -export const DRAFTABLE: unique symbol = - typeof Symbol !== "undefined" && Symbol.for - ? Symbol.for("immer-draftable") - : ("__$immer_draftable" as any) +export const DRAFTABLE: unique symbol = hasSymbol + ? Symbol("immer-draftable") + : ("__$immer_draftable" as any) -export const DRAFT_STATE: unique symbol = - typeof Symbol !== "undefined" && Symbol.for - ? Symbol.for("immer-state") - : ("__$immer_state" as any) +export const DRAFT_STATE: unique symbol = hasSymbol + ? Symbol("immer-state") + : ("__$immer_state" as any) + +export const iteratorSymbol: typeof Symbol.iterator = hasSymbol + ? Symbol.iterator + : ("@@iterator" as any) /** Returns true if the given value is an Immer draft */ export function isDraft(value: any): boolean { @@ -105,7 +110,7 @@ export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { // TODO: use draft meta data if present? // make refelction methods to determine types? -export function has(thing: ObjectishNoSet, prop: PropertyKey): boolean { +export function has(thing: any, prop: PropertyKey): boolean { return !thing ? false : isMap(thing) @@ -156,16 +161,10 @@ export function is(x: any, y: any): boolean { } } -export const hasSymbol = typeof Symbol !== "undefined" - -export const hasMap = typeof Map !== "undefined" - export function isMap(target: any): target is AnyMap { return hasMap && target instanceof Map } -export const hasSet = typeof Set !== "undefined" - export function isSet(target: any): target is AnySet { return hasSet && target instanceof Set } diff --git a/src/es5.ts b/src/es5.ts index 41ec5fe43..ad66182b8 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -15,13 +15,18 @@ import { } from "./common" import {ImmerScope} from "./scope" -import {ImmerState, Drafted, AnyObject, AnyMap, Objectish} from "./types" +import { + ImmerState, + Drafted, + AnyObject, + AnyMap, + Objectish, + ImmerBaseState, + AnyArray +} from "./types" -interface ES5BaseState { - scope: ImmerScope - modified: boolean +interface ES5BaseState extends ImmerBaseState { finalizing: boolean - finalized: boolean assigned: {[key: string]: any} parent?: ImmerState revoke(): void @@ -38,8 +43,8 @@ export interface ES5ObjectState extends ES5BaseState { export interface ES5ArrayState extends ES5BaseState { type: "es5_array" draft: Drafted - base: AnyObject - copy: AnyObject | null + base: AnyArray + copy: AnyArray | null } type ES5State = ES5ArrayState | ES5ObjectState @@ -80,7 +85,7 @@ export function createES5Proxy( type: isArray ? "es5_array" : ("es5_object" as any), scope: parent ? parent.scope : ImmerScope.current!, modified: false, - finalizing: false, // es5 only + finalizing: false, finalized: false, assigned: {}, parent, @@ -88,7 +93,8 @@ export function createES5Proxy( draft, copy: null, revoke, - revoked: false // es5 only + revoked: false, + isManual: false } createHiddenProperty(draft, DRAFT_STATE, state) @@ -118,6 +124,7 @@ function get(state: ES5State, prop: string | number) { // Create a draft if the value is unmodified. if (value === peek(state.base, prop) && isDraftable(value)) { prepareCopy(state) + // @ts-ignore return (state.copy![prop] = state.scope.immer.createProxy(value, state)) } return value @@ -131,6 +138,7 @@ function set(state: ES5State, prop: string | number, value: any) { markChangedES5(state) prepareCopy(state) } + // @ts-ignore state.copy![prop] = value } diff --git a/src/immer.ts b/src/immer.ts index 2097b0b5e..ab9e89a7c 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -183,7 +183,6 @@ export class Immer implements ProducersFns { } const scope = ImmerScope.enter(this) const proxy = this.createProxy(base, undefined) - // @ts-ignore TODO: add to structures proxy[DRAFT_STATE].isManual = true scope.leave() return proxy diff --git a/src/map.ts b/src/map.ts index 965adb5ad..66fe48ff0 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,15 +1,11 @@ -import {isDraftable, DRAFT_STATE, latest} from "./common" +import {isDraftable, DRAFT_STATE, latest, iteratorSymbol} from "./common" import {ImmerScope} from "./scope" -import {AnyMap, Drafted, ImmerState} from "./types" +import {AnyMap, Drafted, ImmerState, ImmerBaseState} from "./types" -export interface MapState { +export interface MapState extends ImmerBaseState { type: "map" - parent?: ImmerState - scope: ImmerScope - modified: boolean finalizing: boolean - finalized: boolean copy: AnyMap | undefined assigned: Map | undefined base: AnyMap @@ -21,8 +17,6 @@ export interface MapState { const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) -// TODO: fix types for drafts -// TODO: assert unrevoked, use freeze for that export class DraftMap extends MapBase implements Map { [DRAFT_STATE]: MapState constructor(target: AnyMap, parent?: ImmerState) { @@ -37,10 +31,11 @@ export class DraftMap extends MapBase implements Map { copy: undefined, assigned: undefined, base: target, - draft: this as any, // TODO: fix typing + draft: this, revoke() { // TODO: make sure this marks the Map as revoked, and assert everywhere - } + }, + isManual: false } } @@ -95,7 +90,7 @@ export class DraftMap extends MapBase implements Map { }) } - get(key: K): V /* TODO: Draft */ { + get(key: K): V { const state = this[DRAFT_STATE] const value = latest(state).get(key) if (state.finalizing || state.finalized || !isDraftable(value)) { @@ -111,14 +106,14 @@ export class DraftMap extends MapBase implements Map { return draft } - keys() { + keys(): IterableIterator { return latest(this[DRAFT_STATE]).keys() } - values() { + values(): IterableIterator { const iterator = this.keys() return { - [Symbol.iterator]: () => this.values(), // TODO: don't use symbol directly + [iteratorSymbol]: () => this.values(), next: () => { const r = iterator.next() if (r.done) return r @@ -131,10 +126,10 @@ export class DraftMap extends MapBase implements Map { } as any } - entries() { + entries(): IterableIterator<[K, V]> { const iterator = this.keys() return { - [Symbol.iterator]: () => this.entries(), // TODO: don't use symbol directly + [iteratorSymbol]: () => this.entries(), next: () => { const r = iterator.next() if (r.done) return r @@ -147,8 +142,7 @@ export class DraftMap extends MapBase implements Map { } as any } - [Symbol.iterator]() { - // TODO: don't use symbol directly + [iteratorSymbol]() { return this.entries() } } diff --git a/src/patches.ts b/src/patches.ts index 7dc5b33db..5bbb13f14 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,6 +1,9 @@ import {get, each, isMap, isSet, has, die} from "./common" import {Patch, ImmerState} from "./types" import {SetState} from "./set" +import {ES5ArrayState, ES5ObjectState} from "./es5" +import {ProxyArrayState, ProxyObjectState} from "./proxy" +import {MapState} from "./map" export type PatchPath = (string | number)[] @@ -9,27 +12,37 @@ export function generatePatches( basePath: PatchPath, patches: Patch[], inversePatches: Patch[] -) { - // TODO: use a proper switch here - const generatePatchesFn = Array.isArray(state.base) - ? generateArrayPatches - : isSet(state.base) - ? generateSetPatches - : generatePatchesFromAssigned - - generatePatchesFn(state as any, basePath, patches, inversePatches) +): void { + switch (state.type) { + case "proxy_object": + case "es5_object": + case "map": + return generatePatchesFromAssigned( + state, + basePath, + patches, + inversePatches + ) + case "es5_array": + case "proxy_array": + return generateArrayPatches(state, basePath, patches, inversePatches) + case "set": + return generateSetPatches(state, basePath, patches, inversePatches) + } } function generateArrayPatches( - state: any, // TODO: type properly with ImmerState + state: ES5ArrayState | ProxyArrayState, basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { - let {base, copy, assigned} = state + let {base, assigned, copy} = state + if (!copy) die() // Reduce complexity by ensuring `base` is never longer. - if (copy.length < base.length) { + if (copy!.length < base.length) { + // @ts-ignore ;[base, copy] = [copy, base] ;[patches, inversePatches] = [inversePatches, patches] } @@ -84,15 +97,15 @@ function generateArrayPatches( // This is used for both Map objects and normal objects. function generatePatchesFromAssigned( - state: any, // TODO: type properly with ImmerState + state: MapState | ES5ObjectState | ProxyObjectState, basePath: PatchPath, patches: Patch[], inversePatches: Patch[] ) { const {base, copy} = state - each(state.assigned, (key, assignedValue) => { + each(state.assigned!, (key, assignedValue) => { const origValue = get(base, key) - const value = get(copy, key) + const value = get(copy!, key) const op = !assignedValue ? "remove" : has(base, key) ? "replace" : "add" if (origValue === value && op === "replace") return const path = basePath.concat(key as any) diff --git a/src/proxy.ts b/src/proxy.ts index 4db18fed3..d92312bc9 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -11,20 +11,24 @@ import { latest } from "./common" import {ImmerScope} from "./scope" -import {AnyObject, Drafted, ImmerState, AnyArray, Objectish} from "./types" - -interface ProxyBaseState { - scope: ImmerScope - modified: boolean - finalized: boolean +import { + AnyObject, + Drafted, + ImmerState, + AnyArray, + Objectish, + ImmerBaseState +} from "./types" + +interface ProxyBaseState extends ImmerBaseState { assigned: { [property: string]: boolean } parent?: ImmerState - drafts: { + drafts?: { [property: string]: Drafted } - revoke: null | (() => void) + revoke(): void } export interface ProxyObjectState extends ProxyBaseState { @@ -74,7 +78,8 @@ export function createProxy( // The base copy with any updated values. copy: null, // Called by the `produce` function. - revoke: null + revoke: null as any, + isManual: false } // the traps must target something, a bit like the 'real' base. @@ -106,7 +111,7 @@ const objectTraps: ProxyHandler = { // Check for existing draft in unmodified state. if (!state.modified && has(drafts, prop)) { - return drafts[prop as any] + return drafts![prop as any] } const value = latest(state)[prop] @@ -119,11 +124,11 @@ const objectTraps: ProxyHandler = { // Assigned values are never drafted. This catches any drafts we created, too. if (value !== peek(state.base, prop)) return value // Store drafts on the copy (when one exists). - // @ts-ignore TODO: this line seems of? + // @ts-ignore drafts = state.copy } - return (drafts[prop as any] = state.scope.immer.createProxy(value, state)) + return (drafts![prop as any] = state.scope.immer.createProxy(value, state)) }, has(state, prop) { return prop in latest(state) @@ -138,7 +143,7 @@ const objectTraps: ProxyHandler = { // never be undefined, so we can avoid the `in` operator. Lastly, truthy // values may be drafts, but falsy values are never drafts. const isUnchanged = value - ? is(baseValue, value) || value === state.drafts[prop] + ? is(baseValue, value) || value === state.drafts![prop] : is(baseValue, value) && prop in state.base if (isUnchanged) return true prepareCopy(state) @@ -227,22 +232,17 @@ function peek(draft: Drafted, prop: PropertyKey): any { export function markChanged(state: ImmerState) { if (!state.modified) { state.modified = true - // @ts-ignore TODO - const {base, drafts, parent} = state - // TODO: get rid of isMap and isSet - if (!isMap(base) && !isSet(base)) { - // TODO: drop creating copies here? - const copy = (state.copy = shallowCopy(base)) - each(drafts, (key, value) => { + if (state.type === "proxy_object" || state.type === "proxy_array") { + const copy = (state.copy = shallowCopy(state.base)) + each(state.drafts!, (key, value) => { // @ts-ignore copy[key] = value }) - // @ts-ignore TODO - state.drafts = null + state.drafts = undefined } - if (parent) { - markChanged(parent) + if (state.parent) { + markChanged(state.parent) } } } diff --git a/src/scope.ts b/src/scope.ts index caf52768e..8068a17e7 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -22,9 +22,6 @@ export class ImmerScope { // Whenever the modified draft contains a draft from another scope, we // need to prevent auto-freezing so the unowned draft can be finalized. this.canAutoFreeze = true - - // To avoid prototype lookups: - this.patches = null as any // TODO: } usePatches(patchListener: PatchListener) { @@ -39,7 +36,7 @@ export class ImmerScope { this.leave() this.drafts.forEach(revoke) // @ts-ignore - this.drafts = null // TODO: // Make draft-related methods throw. + this.drafts = null } leave() { @@ -56,5 +53,6 @@ export class ImmerScope { } function revoke(draft: Drafted) { + // TODO: switch per type and remove from data structures draft[DRAFT_STATE].revoke() } diff --git a/src/set.ts b/src/set.ts index 33900bb31..a3e9e0951 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,19 +1,11 @@ -import {DRAFT_STATE, latest, isDraftable} from "./common" +import {DRAFT_STATE, latest, isDraftable, iteratorSymbol} from "./common" -// TODO: kill: import {ImmerScope} from "./scope" -import {AnySet, Drafted, ImmerState} from "./types" +import {AnySet, Drafted, ImmerState, ImmerBaseState} from "./types" -// TODO: create own states -// TODO: clean up the maps and such from ES5 / Proxy states - -export interface SetState { +export interface SetState extends ImmerBaseState { type: "set" - parent?: ImmerState - scope: ImmerScope - modified: boolean finalizing: boolean - finalized: boolean copy: AnySet | undefined base: AnySet drafts: Map // maps the original value to the draft value in the new set @@ -25,8 +17,6 @@ export interface SetState { const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) -// TODO: fix types for drafts -// TODO: assert unrevoked, use freeze for that export class DraftSet extends SetBase implements Set { [DRAFT_STATE]: SetState constructor(target: AnySet, parent?: ImmerState) { @@ -40,11 +30,12 @@ export class DraftSet extends SetBase implements Set { finalizing: false, copy: undefined, base: target, - draft: this as any, // TODO: fix typing + draft: this, drafts: new Map(), revoke() { // TODO: make sure this marks the Map as revoked, and assert everywhere - } + }, + isManual: false } } @@ -70,7 +61,7 @@ export class DraftSet extends SetBase implements Set { state.copy.add(value) } else if (!state.base.has(value)) { prepareCopy(state) - state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly + state.scope.immer.markChanged(state) state.copy!.add(value) } return this @@ -115,8 +106,7 @@ export class DraftSet extends SetBase implements Set { return this.values() } - // TODO: factor out symbol - [Symbol.iterator]() { + [iteratorSymbol]() { return this.values() } diff --git a/src/types.ts b/src/types.ts index 380450a70..b106511e8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,6 +3,7 @@ import {SetState} from "./set" import {MapState} from "./map" import {ProxyObjectState, ProxyArrayState} from "./proxy" import {ES5ObjectState, ES5ArrayState} from "./es5" +import {ImmerScope} from "./scope" export type Objectish = AnyObject | AnyArray | AnyMap | AnySet export type ObjectishNoSet = AnyObject | AnyArray | AnyMap @@ -20,6 +21,14 @@ export type DraftType = | "map" | "set" +export interface ImmerBaseState { + parent?: ImmerState + scope: ImmerScope + modified: boolean + finalized: boolean + isManual: boolean +} + export type ImmerState = | ProxyObjectState | ProxyArrayState From c7f8f5ba4773240012beb794ca44eaff461b73f8 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 21:59:41 +0000 Subject: [PATCH 20/28] Several refactorings --- __tests__/tsconfig.json | 3 +- package.json | 9 +++- src/common.ts | 102 ++++++++++++++++++++++------------------ src/es5.ts | 3 -- src/finalize.ts | 2 - src/immer.ts | 24 ++++------ src/patches.ts | 65 ++++++++++++------------- src/proxy.ts | 6 +-- src/set.ts | 1 - tsconfig.json | 3 +- 10 files changed, 111 insertions(+), 107 deletions(-) diff --git a/__tests__/tsconfig.json b/__tests__/tsconfig.json index 8e92c580a..19c0864b7 100644 --- a/__tests__/tsconfig.json +++ b/__tests__/tsconfig.json @@ -2,6 +2,7 @@ "include": ["*", "../dist"], "compilerOptions": { "lib": ["es2015"], - "strict": true + "strict": true, + "noUnusedLocals": false } } diff --git a/package.json b/package.json index 15690d4ca..309500021 100644 --- a/package.json +++ b/package.json @@ -91,6 +91,13 @@ "\\.js$": "babel-jest", "\\.ts$": "ts-jest" }, - "preset": "ts-jest/presets/js-with-babel" + "preset": "ts-jest/presets/js-with-babel", + "globals": { + "ts-jest": { + "tsConfig": { + "noUnusedLocals": false + } + } + } } } diff --git a/src/common.ts b/src/common.ts index 919666d63..50f77a8bb 100644 --- a/src/common.ts +++ b/src/common.ts @@ -1,6 +1,5 @@ import { Objectish, - ObjectishNoSet, Drafted, AnyObject, AnyArray, @@ -13,6 +12,7 @@ import { export class Nothing { // This lets us do `Exclude` // TODO: do this better, use unique symbol instead + // @ts-ignore private _: any } @@ -57,6 +57,7 @@ export function isDraftable(value: any): boolean { if (!value) return false return ( isPlainObject(value) || + Array.isArray(value) || !!value[DRAFTABLE] || !!value.constructor[DRAFTABLE] || isMap(value) || @@ -66,7 +67,6 @@ export function isDraftable(value: any): boolean { export function isPlainObject(value: any): boolean { if (!value || typeof value !== "object") return false - if (Array.isArray(value)) return true const proto = Object.getPrototypeOf(value) return !proto || proto === Object.prototype } @@ -94,12 +94,10 @@ export function each( iter: (key: string | number, value: any, source: T) => void ): void export function each(obj: any, iter: any) { - if (Array.isArray(obj) || isMap(obj) || isSet(obj)) { - obj.forEach((entry: any, index: any) => iter(index, entry, obj)) - } else if (obj && typeof obj === "object") { + if (getArchtype(obj) === Archtype.Object) { ownKeys(obj).forEach(key => iter(key, obj[key], obj)) } else { - die() + obj.forEach((entry: any, index: any) => iter(index, entry, obj)) } } @@ -108,47 +106,59 @@ export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { return desc && desc.enumerable ? true : false } -// TODO: use draft meta data if present? -// make refelction methods to determine types? -export function has(thing: any, prop: PropertyKey): boolean { - return !thing - ? false +export enum Archtype { + Object, + Array, + Map, + Set +} + +const toArchType = { + proxy_object: Archtype.Object, + es5_object: Archtype.Object, + proxy_array: Archtype.Array, + es5_array: Archtype.Array, + map: Archtype.Map, + set: Archtype.Set +} + +export function getArchtype(thing: any): Archtype { + if (!thing) die() + if (thing[DRAFT_STATE]) { + // @ts-ignore + return toArchType[(thing as Drafted)[DRAFT_STATE].type] + } + return Array.isArray(thing) + ? Archtype.Array : isMap(thing) + ? Archtype.Map + : isSet(thing) + ? Archtype.Set + : Archtype.Object +} + +export function has(thing: any, prop: PropertyKey): boolean { + return getArchtype(thing) === Archtype.Map ? thing.has(prop) : Object.prototype.hasOwnProperty.call(thing, prop) } -// TODO: use draft meta data if present? export function get(thing: AnyMap | AnyObject, prop: PropertyKey): any { - return isMap(thing) ? thing.get(prop) : thing[prop as any] -} - -// TODO: measure fast vs slow path, use type switch instead -export function set(parent: Drafted, prop: PropertyKey, value: any) { - // fast path switch on meta data - if (parent[DRAFT_STATE]) - switch (parent[DRAFT_STATE].type) { - case "map": - parent.set(prop, value) - break - case "set": - parent.delete(prop) - parent.add(value) - break - default: - parent[prop] = value - } - - // slow path - parent[prop] = value - if (isMap(parent)) { - parent.set(prop, value) - } else if (isSet(parent)) { - // In this case, the `prop` is actually a draft. - parent.delete(prop) - parent.add(value) - } else { - parent[prop] = value + // @ts-ignore + return getArchtype(thing) === Archtype.Map ? thing.get(prop) : thing[prop] +} + +export function set(thing: any, propOrOldValue: PropertyKey, value: any) { + switch (getArchtype(thing)) { + case Archtype.Map: + thing.set(propOrOldValue, value) + break + case Archtype.Set: + thing.delete(propOrOldValue) + thing.add(value) + break + default: + thing[propOrOldValue] = value } } @@ -205,14 +215,12 @@ export function shallowCopy(base: any, invokeGetters = false) { return clone } -export function freeze( - obj: T, - deep: boolean = false -): void { +export function freeze(obj: any, deep: boolean = false): void { if (!isDraftable(obj) || isDraft(obj) || Object.isFrozen(obj)) return - if (isSet(obj)) { + const type = getArchtype(obj) + if (type === Archtype.Set) { obj.add = obj.clear = obj.delete = dontMutateFrozenCollections as any - } else if (isMap(obj)) { + } else if (type === Archtype.Map) { obj.set = obj.clear = obj.delete = dontMutateFrozenCollections as any } Object.freeze(obj) diff --git a/src/es5.ts b/src/es5.ts index ad66182b8..3c871faf3 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -6,8 +6,6 @@ import { isDraft, isDraftable, isEnumerable, - isMap, - isSet, shallowCopy, DRAFT_STATE, latest, @@ -19,7 +17,6 @@ import { ImmerState, Drafted, AnyObject, - AnyMap, Objectish, ImmerBaseState, AnyArray diff --git a/src/finalize.ts b/src/finalize.ts index dbf819e1f..b252481d0 100644 --- a/src/finalize.ts +++ b/src/finalize.ts @@ -7,8 +7,6 @@ import { is, get, each, - isMap, - isEnumerable, DRAFT_STATE, NOTHING, freeze, diff --git a/src/immer.ts b/src/immer.ts index ab9e89a7c..b041ae487 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -9,10 +9,7 @@ import { isMap, isDraftable, DRAFT_STATE, - NOTHING, - isPlainObject, - DRAFTABLE, - die + NOTHING } from "./common" import {ImmerScope} from "./scope" import { @@ -162,7 +159,6 @@ export class Immer implements ProducersFns { produceWithPatches(arg1: any, arg2?: any, arg3?: any): any { if (typeof arg1 === "function") { - const self = this return (state: any, ...args: any[]) => this.produceWithPatches(state, (draft: any) => arg1(draft, ...args)) } @@ -249,16 +245,14 @@ export class Immer implements ProducersFns { value: T, parent?: ImmerState ): Drafted { - let draft: Drafted - // TODO: use switch + type discovery> - if (isMap(value)) draft = proxyMap(value, parent) - else if (isSet(value)) draft = proxySet(value, parent) - else { - // createProxy should be guarded by isDraftable, so we know we can safely draft - draft = this.useProxies - ? createProxy(value, parent) - : createES5Proxy(value, parent) - } + // precondition: createProxy should be guarded by isDraftable, so we know we can safely draft + const draft: Drafted = isMap(value) + ? proxyMap(value, parent) + : isSet(value) + ? proxySet(value, parent) + : this.useProxies + ? createProxy(value, parent) + : createES5Proxy(value, parent) const scope = parent ? parent.scope : ImmerScope.current! scope.drafts.push(draft) diff --git a/src/patches.ts b/src/patches.ts index 5bbb13f14..f76d08698 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,4 +1,4 @@ -import {get, each, isMap, isSet, has, die} from "./common" +import {get, each, isMap, has, die, getArchtype, Archtype} from "./common" import {Patch, ImmerState} from "./types" import {SetState} from "./set" import {ES5ArrayState, ES5ObjectState} from "./es5" @@ -170,52 +170,53 @@ export function applyPatches(draft: T, patches: Patch[]): T { if (!path.length) die() - let base = draft + let base: any = draft for (let i = 0; i < path.length - 1; i++) { base = get(base, path[i]) if (!base || typeof base !== "object") throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore } + const type = getArchtype(base) const value = deepClonePatchValue(patch.value) // used to clone patch to ensure original patch is not modified, see #411 - const key = path[path.length - 1] switch (op) { case "replace": - if (isMap(base)) { - base.set(key, value) - } else if (isSet(base)) { - throw new Error('Sets cannot have "replace" patches.') - } else { - // if value is an object, then it's assigned by reference - // in the following add or remove ops, the value field inside the patch will also be modifyed - // so we use value from the cloned patch - // @ts-ignore - base[key] = value + switch (type) { + case Archtype.Map: + return base.set(key, value) + case Archtype.Set: + throw new Error('Sets cannot have "replace" patches.') + default: + // if value is an object, then it's assigned by reference + // in the following add or remove ops, the value field inside the patch will also be modifyed + // so we use value from the cloned patch + // @ts-ignore + return (base[key] = value) } break case "add": - if (isSet(base)) { - base.delete(patch.value) + switch (type) { + case Archtype.Array: + return base.splice(key as any, 0, value) + case Archtype.Map: + return base.set(key, value) + case Archtype.Set: + return base.add(value) + default: + return (base[key] = value) } - - Array.isArray(base) - ? base.splice(key as any, 0, value) - : isMap(base) - ? base.set(key, value) - : isSet(base) - ? base.add(value) - : ((base as any)[key] = value) - break case "remove": - Array.isArray(base) - ? base.splice(key as any, 1) - : isMap(base) - ? base.delete(key) - : isSet(base) - ? base.delete(patch.value) - : delete (base as any)[key] - break + switch (type) { + case Archtype.Array: + return base.splice(key as any, 1) + case Archtype.Map: + return base.delete(key) + case Archtype.Set: + return base.delete(patch.value) + default: + return delete base[key] + } default: throw new Error("Unsupported patch operation: " + op) } diff --git a/src/proxy.ts b/src/proxy.ts index d92312bc9..284fd3130 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -4,8 +4,6 @@ import { has, is, isDraftable, - isMap, - isSet, shallowCopy, DRAFT_STATE, latest @@ -90,7 +88,7 @@ export function createProxy( // Note that in the case of an array, we put the state in an array to have better Reflect defaults ootb let target: T = state as any let traps: ProxyHandler> = objectTraps - if (Array.isArray(base)) { + if (isArray) { target = [state] as any traps = arrayTraps } @@ -175,7 +173,7 @@ const objectTraps: ProxyHandler = { const desc = Reflect.getOwnPropertyDescriptor(owner, prop) if (desc) { desc.writable = true - desc.configurable = !Array.isArray(owner) || prop !== "length" + desc.configurable = state.type !== "proxy_array" || prop !== "length" } return desc }, diff --git a/src/set.ts b/src/set.ts index a3e9e0951..1a5c11e7c 100644 --- a/src/set.ts +++ b/src/set.ts @@ -111,7 +111,6 @@ export class DraftSet extends SetBase implements Set { } forEach(cb: (value: V, key: V, self: this) => void, thisArg?: any) { - const state = this[DRAFT_STATE] const iterator = this.values() let result = iterator.next() while (!result.done) { diff --git a/tsconfig.json b/tsconfig.json index a4a149bad..5283ccd98 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,6 +8,7 @@ "declaration": true, "importHelpers": false, "noImplicitAny": true, - "esModuleInterop": true + "esModuleInterop": true, + "noUnusedLocals": true } } From fd76f1c18267bf12c464891e6e388f1d6f646516 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 22:16:04 +0000 Subject: [PATCH 21/28] Enums FTW --- src/common.ts | 34 +++++++++++++++------------------- src/es5.ts | 17 +++++++++-------- src/finalize.ts | 7 +++++-- src/map.ts | 6 +++--- src/patches.ts | 23 ++++++++++++++--------- src/proxy.ts | 17 +++++++++++------ src/set.ts | 6 +++--- src/types.ts | 21 ++++++++++++++------- 8 files changed, 74 insertions(+), 57 deletions(-) diff --git a/src/common.ts b/src/common.ts index 50f77a8bb..d920dbc4f 100644 --- a/src/common.ts +++ b/src/common.ts @@ -5,7 +5,9 @@ import { AnyArray, AnyMap, AnySet, - ImmerState + ImmerState, + ProxyType, + Archtype } from "./types" /** Use a class type for `nothing` so its type is unique */ @@ -106,27 +108,21 @@ export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { return desc && desc.enumerable ? true : false } -export enum Archtype { - Object, - Array, - Map, - Set -} - -const toArchType = { - proxy_object: Archtype.Object, - es5_object: Archtype.Object, - proxy_array: Archtype.Array, - es5_array: Archtype.Array, - map: Archtype.Map, - set: Archtype.Set -} - export function getArchtype(thing: any): Archtype { if (!thing) die() if (thing[DRAFT_STATE]) { - // @ts-ignore - return toArchType[(thing as Drafted)[DRAFT_STATE].type] + switch ((thing as Drafted)[DRAFT_STATE].type) { + case ProxyType.ES5Object: + case ProxyType.ProxyObject: + return Archtype.Object + case ProxyType.ES5Array: + case ProxyType.ProxyArray: + return Archtype.Array + case ProxyType.Map: + return Archtype.Map + case ProxyType.Set: + return Archtype.Set + } } return Array.isArray(thing) ? Archtype.Array diff --git a/src/es5.ts b/src/es5.ts index 3c871faf3..e21607739 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -19,7 +19,8 @@ import { AnyObject, Objectish, ImmerBaseState, - AnyArray + AnyArray, + ProxyType } from "./types" interface ES5BaseState extends ImmerBaseState { @@ -31,14 +32,14 @@ interface ES5BaseState extends ImmerBaseState { } export interface ES5ObjectState extends ES5BaseState { - type: "es5_object" + type: ProxyType.ES5Object draft: Drafted base: AnyObject copy: AnyObject | null } export interface ES5ArrayState extends ES5BaseState { - type: "es5_array" + type: ProxyType.ES5Array draft: Drafted base: AnyArray copy: AnyArray | null @@ -79,7 +80,7 @@ export function createES5Proxy( }) const state: ES5ObjectState | ES5ArrayState = { - type: isArray ? "es5_array" : ("es5_object" as any), + type: isArray ? ProxyType.ES5Array : (ProxyType.ES5Object as any), scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalizing: false, @@ -206,10 +207,10 @@ function markChangesSweep(drafts: Drafted[]) { const state = drafts[i][DRAFT_STATE] if (!state.modified) { switch (state.type) { - case "es5_array": + case ProxyType.ES5Array: if (hasArrayChanges(state)) markChangedES5(state) break - case "es5_object": + case ProxyType.ES5Object: if (hasObjectChanges(state)) markChangedES5(state) break } @@ -222,7 +223,7 @@ function markChangesRecursively(object: any) { const state = object[DRAFT_STATE] if (!state) return const {base, draft, assigned, type} = state - if (type === "es5_object") { + if (type === ProxyType.ES5Object) { // Look for added keys. // TODO: looks quite duplicate to hasObjectChanges each(draft, key => { @@ -244,7 +245,7 @@ function markChangesRecursively(object: any) { markChangedES5(state) } }) - } else if (type === "es5_array" && hasArrayChanges(state)) { + } else if (type === ProxyType.ES5Array && hasArrayChanges(state)) { markChangedES5(state) assigned.length = true if (draft.length < base.length) { diff --git a/src/finalize.ts b/src/finalize.ts index b252481d0..1b682f799 100644 --- a/src/finalize.ts +++ b/src/finalize.ts @@ -1,5 +1,5 @@ import {Immer} from "./immer" -import {ImmerState, Drafted} from "./types" +import {ImmerState, Drafted, ProxyType} from "./types" import {ImmerScope} from "./scope" import { isSet, @@ -117,7 +117,10 @@ function finalizeTree( ) { const state = root[DRAFT_STATE] if (state) { - if (state.type === "es5_object" || state.type === "es5_array") { + if ( + state.type === ProxyType.ES5Object || + state.type === ProxyType.ES5Array + ) { // Create the final copy, with added keys and without deleted keys. state.copy = shallowCopy(state.draft, true) } diff --git a/src/map.ts b/src/map.ts index 66fe48ff0..252be262a 100644 --- a/src/map.ts +++ b/src/map.ts @@ -1,10 +1,10 @@ import {isDraftable, DRAFT_STATE, latest, iteratorSymbol} from "./common" import {ImmerScope} from "./scope" -import {AnyMap, Drafted, ImmerState, ImmerBaseState} from "./types" +import {AnyMap, Drafted, ImmerState, ImmerBaseState, ProxyType} from "./types" export interface MapState extends ImmerBaseState { - type: "map" + type: ProxyType.Map finalizing: boolean copy: AnyMap | undefined assigned: Map | undefined @@ -22,7 +22,7 @@ export class DraftMap extends MapBase implements Map { constructor(target: AnyMap, parent?: ImmerState) { super() this[DRAFT_STATE] = { - type: "map", + type: ProxyType.Map, parent, scope: parent ? parent.scope : ImmerScope.current!, modified: false, diff --git a/src/patches.ts b/src/patches.ts index f76d08698..127a608d9 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,5 +1,5 @@ -import {get, each, isMap, has, die, getArchtype, Archtype} from "./common" -import {Patch, ImmerState} from "./types" +import {get, each, isMap, has, die, getArchtype} from "./common" +import {Patch, ImmerState, ProxyType, Archtype} from "./types" import {SetState} from "./set" import {ES5ArrayState, ES5ObjectState} from "./es5" import {ProxyArrayState, ProxyObjectState} from "./proxy" @@ -14,20 +14,25 @@ export function generatePatches( inversePatches: Patch[] ): void { switch (state.type) { - case "proxy_object": - case "es5_object": - case "map": + case ProxyType.ProxyObject: + case ProxyType.ES5Object: + case ProxyType.Map: return generatePatchesFromAssigned( state, basePath, patches, inversePatches ) - case "es5_array": - case "proxy_array": + case ProxyType.ES5Array: + case ProxyType.ProxyArray: return generateArrayPatches(state, basePath, patches, inversePatches) - case "set": - return generateSetPatches(state, basePath, patches, inversePatches) + case ProxyType.Set: + return generateSetPatches( + (state as any) as SetState, + basePath, + patches, + inversePatches + ) } } diff --git a/src/proxy.ts b/src/proxy.ts index 284fd3130..cdc77e090 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -15,7 +15,8 @@ import { ImmerState, AnyArray, Objectish, - ImmerBaseState + ImmerBaseState, + ProxyType } from "./types" interface ProxyBaseState extends ImmerBaseState { @@ -30,14 +31,14 @@ interface ProxyBaseState extends ImmerBaseState { } export interface ProxyObjectState extends ProxyBaseState { - type: "proxy_object" + type: ProxyType.ProxyObject base: AnyObject copy: AnyObject | null draft: Drafted } export interface ProxyArrayState extends ProxyBaseState { - type: "proxy_array" + type: ProxyType.ProxyArray base: AnyArray copy: AnyArray | null draft: Drafted @@ -56,7 +57,7 @@ export function createProxy( ): Drafted { const isArray = Array.isArray(base) const state: ProxyState = { - type: isArray ? "proxy_array" : ("proxy_object" as any), + type: isArray ? ProxyType.ProxyArray : (ProxyType.ProxyObject as any), // Track which produce call this is associated with. scope: parent ? parent.scope : ImmerScope.current!, // True for both shallow and deep changes. @@ -173,7 +174,8 @@ const objectTraps: ProxyHandler = { const desc = Reflect.getOwnPropertyDescriptor(owner, prop) if (desc) { desc.writable = true - desc.configurable = state.type !== "proxy_array" || prop !== "length" + desc.configurable = + state.type !== ProxyType.ProxyArray || prop !== "length" } return desc }, @@ -230,7 +232,10 @@ function peek(draft: Drafted, prop: PropertyKey): any { export function markChanged(state: ImmerState) { if (!state.modified) { state.modified = true - if (state.type === "proxy_object" || state.type === "proxy_array") { + if ( + state.type === ProxyType.ProxyObject || + state.type === ProxyType.ProxyArray + ) { const copy = (state.copy = shallowCopy(state.base)) each(state.drafts!, (key, value) => { // @ts-ignore diff --git a/src/set.ts b/src/set.ts index 1a5c11e7c..8d51c347a 100644 --- a/src/set.ts +++ b/src/set.ts @@ -1,10 +1,10 @@ import {DRAFT_STATE, latest, isDraftable, iteratorSymbol} from "./common" import {ImmerScope} from "./scope" -import {AnySet, Drafted, ImmerState, ImmerBaseState} from "./types" +import {AnySet, Drafted, ImmerState, ImmerBaseState, ProxyType} from "./types" export interface SetState extends ImmerBaseState { - type: "set" + type: ProxyType.Set finalizing: boolean copy: AnySet | undefined base: AnySet @@ -22,7 +22,7 @@ export class DraftSet extends SetBase implements Set { constructor(target: AnySet, parent?: ImmerState) { super() this[DRAFT_STATE] = { - type: "set", + type: ProxyType.Set, parent, scope: parent ? parent.scope : ImmerScope.current!, modified: false, diff --git a/src/types.ts b/src/types.ts index b106511e8..570bab1b1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -12,14 +12,21 @@ export type AnyObject = {[key: string]: any} export type AnyArray = Array export type AnySet = Set export type AnyMap = Map +export enum Archtype { + Object, + Array, + Map, + Set +} -export type DraftType = - | "proxy_object" - | "proxy_array" - | "es5_object" - | "es5_array" - | "map" - | "set" +export enum ProxyType { + ProxyObject, + ProxyArray, + ES5Object, + ES5Array, + Map, + Set +} export interface ImmerBaseState { parent?: ImmerState From 28ce953e616ea4e0aa5e0d1abbeacc8bc47e4c86 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 22:37:14 +0000 Subject: [PATCH 22/28] Cleanup --- src/es5.ts | 5 +++- src/finalize.ts | 67 +++++++++++++++++++------------------------------ 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/es5.ts b/src/es5.ts index e21607739..52c387df5 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -225,7 +225,10 @@ function markChangesRecursively(object: any) { const {base, draft, assigned, type} = state if (type === ProxyType.ES5Object) { // Look for added keys. - // TODO: looks quite duplicate to hasObjectChanges + // TODO: looks quite duplicate to hasObjectChanges, + // probably there is a faster way to detect changes, as sweep + recurse seems to do some + // unnecessary work. + // also: probably we can store the information we detect here, to speed up tree finalization! each(draft, key => { if ((key as any) === DRAFT_STATE) return // The `undefined` check is a fast path for pre-existing keys. diff --git a/src/finalize.ts b/src/finalize.ts index 1b682f799..b431ecbaf 100644 --- a/src/finalize.ts +++ b/src/finalize.ts @@ -126,88 +126,73 @@ function finalizeTree( } root = state.copy } - const needPatches = !!rootPath && !!scope.patches each(root, (key, value) => - finalizeProperty( - immer, - root, - state, - key, - value, - root, - needPatches, - scope, - rootPath - ) + finalizeProperty(immer, scope, root, state, root, key, value, rootPath) ) return root } function finalizeProperty( - // TODO: can do with less args? immer: Immer, - root: ImmerState, - state: ImmerState, - prop: string | number, - value: any, - parent: Drafted, - needPatches: boolean, scope: ImmerScope, + root: Drafted, + rootState: ImmerState, + parentValue: Drafted, + prop: string | number, + childValue: any, rootPath?: PatchPath ) { - if (value === parent) { + if (childValue === parentValue) { throw Error("Immer forbids circular references") } // In the `finalizeTree` method, only the `root` object may be a draft. - const isDraftProp = !!state && parent === root - const isSetMember = isSet(parent) + const isDraftProp = !!rootState && parentValue === root + const isSetMember = isSet(parentValue) - if (isDraft(value)) { + if (isDraft(childValue)) { const path = + rootPath && isDraftProp && - needPatches && !isSetMember && // Set objects are atomic since they have no keys. - !has((state as Exclude).assigned!, prop) // Skip deep patches for assigned keys. + !has((rootState as Exclude).assigned!, prop) // Skip deep patches for assigned keys. ? rootPath!.concat(prop) : undefined // Drafts owned by `scope` are finalized here. - value = finalize(immer, value, scope, path) - set(parent, prop, value) + childValue = finalize(immer, childValue, scope, path) + set(parentValue, prop, childValue) // Drafts from another scope must prevent auto-freezing. - if (isDraft(value)) { + if (isDraft(childValue)) { scope.canAutoFreeze = false } - - // Unchanged drafts are never passed to the `onAssign` hook. - // if (isDraftProp && !isSet && value === get(state.base, prop)) return } // Unchanged draft properties are ignored. - else if (isDraftProp && is(value, get(state.base, prop))) { + else if (isDraftProp && is(childValue, get(rootState.base, prop))) { return } // Search new objects for unfinalized drafts. Frozen objects should never contain drafts. - else if (isDraftable(value) && !Object.isFrozen(value)) { - each(value, (key, v) => + // TODO: the recursion over here looks weird, shouldn't non-draft stuff have it's own recursion? + // especially the passing on of root and rootState doesn't make sense... + else if (isDraftable(childValue) && !Object.isFrozen(childValue)) { + each(childValue, (key, grandChild) => finalizeProperty( immer, + scope, root, - state, + rootState, + childValue, key, - v, - value, - needPatches, - scope, + grandChild, rootPath ) ) - maybeFreeze(immer, value) + maybeFreeze(immer, childValue) } if (isDraftProp && immer.onAssign && !isSetMember) { - immer.onAssign(state, prop, value) + immer.onAssign(rootState, prop, childValue) } } From 12c38c693653cff666e60fa4ec83700354e55e88 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 22:52:15 +0000 Subject: [PATCH 23/28] Made sure Map and Set proxies are revoked, and less closures are created --- __tests__/base.js | 136 ++++++++++++++++++++++++++++++++++++++++------ src/es5.ts | 10 +--- src/map.ts | 13 +++-- src/scope.ts | 11 +++- src/set.ts | 17 ++++-- 5 files changed, 148 insertions(+), 39 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index 3789ad49f..437403239 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -188,7 +188,10 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) it("supports iteration", () => { - const base = [{id: 1, a: 1}, {id: 2, a: 1}] + const base = [ + {id: 1, a: 1}, + {id: 2, a: 1} + ] const findById = (collection, id) => { for (const item of collection) { if (item.id === id) return item @@ -386,7 +389,10 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) it("supports 'keys", () => { - const base = new Map([["first", Symbol()], ["second", Symbol()]]) + const base = new Map([ + ["first", Symbol()], + ["second", Symbol()] + ]) const result = produce(base, draft => { expect([...draft.keys()]).toEqual(["first", "second"]) draft.set("third", Symbol()) @@ -543,11 +549,27 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) expect(next).toBe(base) }) + + it("revokes map proxies", () => { + let m + produce(baseState, s => { + m = s.aMap + }) + expect(() => m.get("x")).toThrow( + "Cannot use a proxy that has been revoked" + ) + expect(() => m.set("x", 3)).toThrow( + "Cannot use a proxy that has been revoked" + ) + }) }) describe("set drafts", () => { it("supports iteration", () => { - const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) + const base = new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) const findById = (set, id) => { for (const item of set) { if (item.id === id) return item @@ -561,12 +583,25 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { obj2.a = 2 }) expect(result).not.toBe(base) - expect(base).toEqual(new Set([{id: 1, a: 1}, {id: 2, a: 1}])) - expect(result).toEqual(new Set([{id: 1, a: 2}, {id: 2, a: 2}])) + expect(base).toEqual( + new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) + ) + expect(result).toEqual( + new Set([ + {id: 1, a: 2}, + {id: 2, a: 2} + ]) + ) }) it("supports 'entries'", () => { - const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) + const base = new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) const findById = (set, id) => { for (const [item1, item2] of set.entries()) { expect(item1).toBe(item2) @@ -581,12 +616,25 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { obj2.a = 2 }) expect(result).not.toBe(base) - expect(base).toEqual(new Set([{id: 1, a: 1}, {id: 2, a: 1}])) - expect(result).toEqual(new Set([{id: 1, a: 2}, {id: 2, a: 2}])) + expect(base).toEqual( + new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) + ) + expect(result).toEqual( + new Set([ + {id: 1, a: 2}, + {id: 2, a: 2} + ]) + ) }) it("supports 'values'", () => { - const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) + const base = new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) const findById = (set, id) => { for (const item of set.values()) { if (item.id === id) return item @@ -600,12 +648,25 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { obj2.a = 2 }) expect(result).not.toBe(base) - expect(base).toEqual(new Set([{id: 1, a: 1}, {id: 2, a: 1}])) - expect(result).toEqual(new Set([{id: 1, a: 2}, {id: 2, a: 2}])) + expect(base).toEqual( + new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) + ) + expect(result).toEqual( + new Set([ + {id: 1, a: 2}, + {id: 2, a: 2} + ]) + ) }) it("supports 'keys'", () => { - const base = new Set([{id: 1, a: 1}, {id: 2, a: 1}]) + const base = new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) const findById = (set, id) => { for (const item of set.keys()) { if (item.id === id) return item @@ -619,12 +680,25 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { obj2.a = 2 }) expect(result).not.toBe(base) - expect(base).toEqual(new Set([{id: 1, a: 1}, {id: 2, a: 1}])) - expect(result).toEqual(new Set([{id: 1, a: 2}, {id: 2, a: 2}])) + expect(base).toEqual( + new Set([ + {id: 1, a: 1}, + {id: 2, a: 1} + ]) + ) + expect(result).toEqual( + new Set([ + {id: 1, a: 2}, + {id: 2, a: 2} + ]) + ) }) it("supports forEach with mutation after reads", () => { - const base = new Set([{id: 1, a: 1}, {id: 2, a: 2}]) + const base = new Set([ + {id: 1, a: 1}, + {id: 2, a: 2} + ]) const result = produce(base, draft => { let sum1 = 0 draft.forEach(({a}) => { @@ -639,8 +713,18 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(sum2).toBe(23) }) expect(result).not.toBe(base) - expect(base).toEqual(new Set([{id: 1, a: 1}, {id: 2, a: 2}])) - expect(result).toEqual(new Set([{id: 1, a: 11}, {id: 2, a: 12}])) + expect(base).toEqual( + new Set([ + {id: 1, a: 1}, + {id: 2, a: 2} + ]) + ) + expect(result).toEqual( + new Set([ + {id: 1, a: 11}, + {id: 2, a: 12} + ]) + ) }) it("state stays the same if the same item is added", () => { @@ -732,6 +816,19 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { }) expect(next).toEqual(new Set([3])) }) + + it("revokes sets", () => { + let m + produce(baseState, s => { + m = s.aSet + }) + expect(() => m.has("x")).toThrow( + "Cannot use a proxy that has been revoked" + ) + expect(() => m.add("x")).toThrow( + "Cannot use a proxy that has been revoked" + ) + }) }) it("supports `immerable` symbol on constructor", () => { @@ -1620,7 +1717,10 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { const c = {c: 3} const set1 = new Set([a, b]) const set2 = new Set([c]) - const map = new Map([["set1", set1], ["set2", set2]]) + const map = new Map([ + ["set1", set1], + ["set2", set2] + ]) const base = {map} function first(set) { diff --git a/src/es5.ts b/src/es5.ts index 52c387df5..f7ac428b9 100644 --- a/src/es5.ts +++ b/src/es5.ts @@ -22,12 +22,13 @@ import { AnyArray, ProxyType } from "./types" +import {MapState} from "./map" +import {SetState} from "./set" interface ES5BaseState extends ImmerBaseState { finalizing: boolean assigned: {[key: string]: any} parent?: ImmerState - revoke(): void revoked: boolean } @@ -90,7 +91,6 @@ export function createES5Proxy( base, draft, copy: null, - revoke, revoked: false, isManual: false } @@ -99,10 +99,6 @@ export function createES5Proxy( return draft } -function revoke(this: ES5BaseState) { - this.revoked = true -} - // Access a property without creating an Immer draft. function peek(draft: Drafted, prop: PropertyKey) { const state = draft[DRAFT_STATE] @@ -189,7 +185,7 @@ function proxyProperty( Object.defineProperty(draft, prop, desc) } -export function assertUnrevoked(state: ES5State) { +export function assertUnrevoked(state: ES5State | MapState | SetState) { if (state.revoked === true) throw new Error( "Cannot use a proxy that has been revoked. Did you pass an object from inside an immer function to an async process? " + diff --git a/src/map.ts b/src/map.ts index 252be262a..595437129 100644 --- a/src/map.ts +++ b/src/map.ts @@ -2,6 +2,7 @@ import {isDraftable, DRAFT_STATE, latest, iteratorSymbol} from "./common" import {ImmerScope} from "./scope" import {AnyMap, Drafted, ImmerState, ImmerBaseState, ProxyType} from "./types" +import {assertUnrevoked} from "./es5" export interface MapState extends ImmerBaseState { type: ProxyType.Map @@ -9,7 +10,7 @@ export interface MapState extends ImmerBaseState { copy: AnyMap | undefined assigned: Map | undefined base: AnyMap - revoke(): void + revoked: boolean draft: Drafted } @@ -32,10 +33,8 @@ export class DraftMap extends MapBase implements Map { assigned: undefined, base: target, draft: this, - revoke() { - // TODO: make sure this marks the Map as revoked, and assert everywhere - }, - isManual: false + isManual: false, + revoked: false } } @@ -49,6 +48,7 @@ export class DraftMap extends MapBase implements Map { set(key: K, value: V): this { const state = this[DRAFT_STATE] + assertUnrevoked(state) if (latest(state).get(key) !== value) { prepareCopy(state) state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly @@ -65,6 +65,7 @@ export class DraftMap extends MapBase implements Map { } const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) state.scope.immer.markChanged(state) state.assigned!.set(key, false) @@ -74,6 +75,7 @@ export class DraftMap extends MapBase implements Map { clear() { const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) state.scope.immer.markChanged(state) state.assigned = new Map() @@ -92,6 +94,7 @@ export class DraftMap extends MapBase implements Map { get(key: K): V { const state = this[DRAFT_STATE] + assertUnrevoked(state) const value = latest(state).get(key) if (state.finalizing || state.finalized || !isDraftable(value)) { return value diff --git a/src/scope.ts b/src/scope.ts index 8068a17e7..43c3fa74d 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -1,5 +1,5 @@ import {DRAFT_STATE} from "./common" -import {Patch, PatchListener, Drafted} from "./types" +import {Patch, PatchListener, Drafted, ProxyType} from "./types" import {Immer} from "./immer" /** Each scope represents a `produce` call. */ @@ -53,6 +53,11 @@ export class ImmerScope { } function revoke(draft: Drafted) { - // TODO: switch per type and remove from data structures - draft[DRAFT_STATE].revoke() + const state = draft[DRAFT_STATE] + if ( + state.type === ProxyType.ProxyObject || + state.type === ProxyType.ProxyArray + ) + state.revoke() + else state.revoked = true } diff --git a/src/set.ts b/src/set.ts index 8d51c347a..6f1a3f35c 100644 --- a/src/set.ts +++ b/src/set.ts @@ -2,14 +2,15 @@ import {DRAFT_STATE, latest, isDraftable, iteratorSymbol} from "./common" import {ImmerScope} from "./scope" import {AnySet, Drafted, ImmerState, ImmerBaseState, ProxyType} from "./types" +import {assertUnrevoked} from "./es5" export interface SetState extends ImmerBaseState { type: ProxyType.Set - finalizing: boolean + finalizing: boolean // TODO: kill? copy: AnySet | undefined base: AnySet drafts: Map // maps the original value to the draft value in the new set - revoke(): void + revoked: boolean draft: Drafted } @@ -32,9 +33,7 @@ export class DraftSet extends SetBase implements Set { base: target, draft: this, drafts: new Map(), - revoke() { - // TODO: make sure this marks the Map as revoked, and assert everywhere - }, + revoked: false, isManual: false } } @@ -45,6 +44,7 @@ export class DraftSet extends SetBase implements Set { has(value: V): boolean { const state = this[DRAFT_STATE] + assertUnrevoked(state) // bit of trickery here, to be able to recognize both the value, and the draft of its value if (!state.copy) { return state.base.has(value) @@ -57,6 +57,7 @@ export class DraftSet extends SetBase implements Set { add(value: V): this { const state = this[DRAFT_STATE] + assertUnrevoked(state) if (state.copy) { state.copy.add(value) } else if (!state.base.has(value)) { @@ -68,11 +69,12 @@ export class DraftSet extends SetBase implements Set { } delete(value: V): boolean { - const state = this[DRAFT_STATE] if (!this.has(value)) { return false } + const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) state.scope.immer.markChanged(state) return ( @@ -85,6 +87,7 @@ export class DraftSet extends SetBase implements Set { clear() { const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) state.scope.immer.markChanged(state) return state.copy!.clear() @@ -92,12 +95,14 @@ export class DraftSet extends SetBase implements Set { values(): IterableIterator { const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) return state.copy!.values() } entries(): IterableIterator<[V, V]> { const state = this[DRAFT_STATE] + assertUnrevoked(state) prepareCopy(state) return state.copy!.entries() } From 82298d4f21ae692b0f76165e087291a56a142c77 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Jan 2020 22:56:16 +0000 Subject: [PATCH 24/28] Some final cleanup --- src/common.ts | 3 +-- src/map.ts | 6 ++---- src/proxy.ts | 2 ++ src/set.ts | 2 -- src/types.ts | 9 --------- 5 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/common.ts b/src/common.ts index d920dbc4f..04e8fb473 100644 --- a/src/common.ts +++ b/src/common.ts @@ -13,9 +13,8 @@ import { /** Use a class type for `nothing` so its type is unique */ export class Nothing { // This lets us do `Exclude` - // TODO: do this better, use unique symbol instead // @ts-ignore - private _: any + private _!: unique symbol } const hasSymbol = typeof Symbol !== "undefined" diff --git a/src/map.ts b/src/map.ts index 595437129..11687852e 100644 --- a/src/map.ts +++ b/src/map.ts @@ -6,7 +6,6 @@ import {assertUnrevoked} from "./es5" export interface MapState extends ImmerBaseState { type: ProxyType.Map - finalizing: boolean copy: AnyMap | undefined assigned: Map | undefined base: AnyMap @@ -28,7 +27,6 @@ export class DraftMap extends MapBase implements Map { scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalized: false, - finalizing: false, copy: undefined, assigned: undefined, base: target, @@ -51,7 +49,7 @@ export class DraftMap extends MapBase implements Map { assertUnrevoked(state) if (latest(state).get(key) !== value) { prepareCopy(state) - state.scope.immer.markChanged(state) // TODO: this needs to bubble up recursively correctly + state.scope.immer.markChanged(state) state.assigned!.set(key, true) state.copy!.set(key, value) state.assigned!.set(key, true) @@ -96,7 +94,7 @@ export class DraftMap extends MapBase implements Map { const state = this[DRAFT_STATE] assertUnrevoked(state) const value = latest(state).get(key) - if (state.finalizing || state.finalized || !isDraftable(value)) { + if (state.finalized || !isDraftable(value)) { return value } if (value !== state.base.get(key)) { diff --git a/src/proxy.ts b/src/proxy.ts index cdc77e090..009dde046 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -94,6 +94,8 @@ export function createProxy( traps = arrayTraps } + // TODO: optimization: might be faster, cheaper if we created a non-revocable proxy + // and administrate revoking ourselves const {revoke, proxy} = Proxy.revocable(target, traps) state.draft = proxy as any state.revoke = revoke diff --git a/src/set.ts b/src/set.ts index 6f1a3f35c..6825d68ba 100644 --- a/src/set.ts +++ b/src/set.ts @@ -6,7 +6,6 @@ import {assertUnrevoked} from "./es5" export interface SetState extends ImmerBaseState { type: ProxyType.Set - finalizing: boolean // TODO: kill? copy: AnySet | undefined base: AnySet drafts: Map // maps the original value to the draft value in the new set @@ -28,7 +27,6 @@ export class DraftSet extends SetBase implements Set { scope: parent ? parent.scope : ImmerScope.current!, modified: false, finalized: false, - finalizing: false, copy: undefined, base: target, draft: this, diff --git a/src/types.ts b/src/types.ts index 570bab1b1..e0562c002 100644 --- a/src/types.ts +++ b/src/types.ts @@ -214,12 +214,3 @@ export interface IProduceWithPatches { recipe: (draft: D) => Return ): [Produced, Patch[], Patch[]] } - -// Backward compatibility with --target es5 -// TODO: still needed? -// declare global { -// interface Set {} -// interface Map {} -// interface WeakSet {} -// interface WeakMap {} -// } From 6e76122fcffea0395ff947a290fa834e4f81432b Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 9 Jan 2020 18:44:54 +0000 Subject: [PATCH 25/28] Document and test things that are _not_ drafted --- __tests__/base.js | 34 ++++++++++++++++++++++++++++++++++ docs/complex-objects.md | 2 ++ docs/pitfalls.md | 17 +++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/__tests__/base.js b/__tests__/base.js index 437403239..cb61bfbdc 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -562,6 +562,26 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { "Cannot use a proxy that has been revoked" ) }) + + it("does not draft map keys", () => { + // anything else would be terribly confusing + const key = {a: 1} + const map = new Map([[key, 2]]) + const next = produce(map, d => { + const dKey = Array.from(d.keys())[0] + expect(isDraft(dKey)).toBe(false) + expect(dKey).toBe(key) + dKey.a += 1 + d.set(dKey, d.get(dKey) + 1) + d.set(key, d.get(key) + 1) + expect(d.get(key)).toBe(4) + expect(key.a).toBe(2) + }) + const entries = Array.from(next.entries()) + expect(entries).toEqual([[key, 4]]) + expect(entries[0][0]).toBe(key) + expect(entries[0][0].a).toBe(2) + }) }) describe("set drafts", () => { @@ -1355,6 +1375,20 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) { expect(result[0].a).toEqual(2) }) + it("does not draft external data", () => { + const externalData = {x: 3} + const base = {} + const next = produce(base, draft => { + // potentially, we *could* draft external data automatically, but only if those statements are not switched... + draft.y = externalData + draft.y.x += 1 + externalData.x += 1 + }) + expect(next).toEqual({y: {x: 5}}) + expect(externalData.x).toBe(5) + expect(next.y).toBe(externalData) + }) + autoFreeze && test("issue #469, state not frozen", () => { const project = produce( diff --git a/docs/complex-objects.md b/docs/complex-objects.md index e5dc5a309..79fb26d80 100644 --- a/docs/complex-objects.md +++ b/docs/complex-objects.md @@ -72,3 +72,5 @@ For arrays, only numeric properties and the `length` property can be mutated. Cu When working with `Date` objects, you should always create a new `Date` instance instead of mutating an existing `Date` object. Maps and Sets that are produced by Immer will be made artificially immutable. This means that they will throw an exception when trying mutative methods like `set`, `clear` etc. outside a producer. + +_Note: The **keys** of a map are never drafted! This is done to avoid confusing semantics and keep keys always referentially equal_ diff --git a/docs/pitfalls.md b/docs/pitfalls.md index d5a2610df..04e749765 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -10,3 +10,20 @@ title: Pitfalls 1. Since Immer uses proxies, reading huge amounts of data from state comes with an overhead (especially in the ES5 implementation). If this ever becomes an issue (measure before you optimize!), do the current state analysis before entering the producer function or read from the `currentState` rather than the `draftState`. Also, realize that immer is opt-in everywhere, so it is perfectly fine to manually write super performance critical reducers, and use immer for all the normal ones. Also note that `original` can be used to get the original state of an object, which is cheaper to read. 1. Always try to pull `produce` 'up', for example `for (let x of y) produce(base, d => d.push(x))` is exponentially slower than `produce(base, d => { for (let x of y) d.push(x)})` 1. It is possible to return values from producers, except, it is not possible to return `undefined` that way, as it is indistinguishable from not updating the draft at all! If you want to replace the draft with `undefined`, just return `nothing` from the producer. +1. Note that data that comes from the closure, and not from the base state, will never be drafted, even when the data has become part of the new draft: + +```javascript +function onReceiveTodo(todo) { + const nextTodos = produce(todos, draft => { + draft.todos[todo.id] = todo + // Note, because 'todo' is coming from external, and not from the 'draft', + // it isn't draft so the following modification affects the original todo! + draft.todos[todo.id].done = true + + // The reason for this, is that it means that the behavior of the 2 lines above + // is equivalent to code, making this whole process more consistent + todo.done = true + draft.todos[todo.id] = todo + }) +} +``` From 4ce36b65f0b513950dea8ebbd3fd7aa9c4d53e60 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 9 Jan 2020 19:16:24 +0000 Subject: [PATCH 26/28] Fixed #492, finishDraft doesn't require two arguments --- src/immer.ts | 4 ++-- src/scope.ts | 2 +- tsconfig.json | 10 +++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/immer.ts b/src/immer.ts index b041ae487..a6adedc16 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -186,9 +186,9 @@ export class Immer implements ProducersFns { finishDraft>( draft: D, - patchListener: PatchListener + patchListener?: PatchListener ): D extends Draft ? T : never { - const state = draft && draft[DRAFT_STATE] + const state: ImmerState = draft && draft[DRAFT_STATE] if (!state || !state.isManual) { throw new Error("First argument to `finishDraft` must be a draft returned by `createDraft`") // prettier-ignore } diff --git a/src/scope.ts b/src/scope.ts index 43c3fa74d..9c66a91d3 100644 --- a/src/scope.ts +++ b/src/scope.ts @@ -24,7 +24,7 @@ export class ImmerScope { this.canAutoFreeze = true } - usePatches(patchListener: PatchListener) { + usePatches(patchListener?: PatchListener) { if (patchListener) { this.patches = [] this.inversePatches = [] diff --git a/tsconfig.json b/tsconfig.json index 3db7e91cb..576514ec2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,6 @@ { - "include": ["src"], "compilerOptions": { + "rootDir": "src/", "lib": ["es2015"], // TODO: change to ES5 and make sure maps still work "target": "ES6", @@ -11,6 +11,10 @@ "esModuleInterop": true, "noUnusedLocals": true, "sourceMap": true, - "declarationMap": true - } + "declarationMap": true, + "noEmit": true + }, + "files": ["src/index.ts"], + "include": ["src/**/*.ts"], + "exclude": ["node_modules/**/*.ts"] } From 617772f67d12262047949646551af68f83a924fe Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 9 Jan 2020 19:30:17 +0000 Subject: [PATCH 27/28] Fixed tsconfig setup --- tsconfig.json | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tsconfig.json b/tsconfig.json index 576514ec2..4c36b7612 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,8 +1,6 @@ { "compilerOptions": { - "rootDir": "src/", "lib": ["es2015"], - // TODO: change to ES5 and make sure maps still work "target": "ES6", "strict": true, "declaration": true, @@ -12,9 +10,8 @@ "noUnusedLocals": true, "sourceMap": true, "declarationMap": true, - "noEmit": true + "noEmit": true, + "moduleResolution": "node" }, - "files": ["src/index.ts"], - "include": ["src/**/*.ts"], - "exclude": ["node_modules/**/*.ts"] + "files": ["./src/index.ts"] } From f4a4701a956c2e1fc63f39830622cec319757922 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 9 Jan 2020 19:42:04 +0000 Subject: [PATCH 28/28] Added istanbul rules to get 100% test coverage --- src/common.ts | 6 ++++-- src/finalize.ts | 6 +++--- src/immer.ts | 11 +++++++---- src/map.ts | 3 +++ src/patches.ts | 6 ++++-- src/set.ts | 3 ++- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/common.ts b/src/common.ts index 04e8fb473..370384c14 100644 --- a/src/common.ts +++ b/src/common.ts @@ -88,7 +88,7 @@ export const ownKeys: (target: AnyObject) => PropertyKey[] = Object.getOwnPropertyNames(obj).concat( Object.getOwnPropertySymbols(obj) as any ) - : Object.getOwnPropertyNames + : /* istanbul ignore next */ Object.getOwnPropertyNames export function each( obj: T, @@ -108,6 +108,7 @@ export function isEnumerable(base: AnyObject, prop: PropertyKey): boolean { } export function getArchtype(thing: any): Archtype { + /* istanbul ignore next */ if (!thing) die() if (thing[DRAFT_STATE]) { switch ((thing as Drafted)[DRAFT_STATE].type) { @@ -210,7 +211,7 @@ export function shallowCopy(base: any, invokeGetters = false) { return clone } -export function freeze(obj: any, deep: boolean = false): void { +export function freeze(obj: any, deep: boolean): void { if (!isDraftable(obj) || isDraft(obj) || Object.isFrozen(obj)) return const type = getArchtype(obj) if (type === Archtype.Set) { @@ -238,6 +239,7 @@ export function createHiddenProperty( }) } +/* istanbul ignore next */ export function die(): never { throw new Error("Illegal state, please file a bug") } diff --git a/src/finalize.ts b/src/finalize.ts index b431ecbaf..4ecaf10e1 100644 --- a/src/finalize.ts +++ b/src/finalize.ts @@ -78,17 +78,17 @@ function finalize( finalizeTree(immer, state.draft, scope, path) // We cannot really delete anything inside of a Set. We can only replace the whole Set. - if (immer.onDelete && !isSet(state.base)) { + if (immer.onDelete && state.type !== ProxyType.Set) { // The `assigned` object is unreliable with ES5 drafts. if (immer.useProxies) { const {assigned} = state each(assigned, (prop, exists) => { - if (!exists) immer.onDelete?.(state, prop as any) + if (!exists) immer.onDelete!(state, prop as any) }) } else { const {base, copy} = state each(base, prop => { - if (!has(copy, prop)) immer.onDelete?.(state, prop as any) + if (!has(copy, prop)) immer.onDelete!(state, prop as any) }) } } diff --git a/src/immer.ts b/src/immer.ts index a6adedc16..d777d67ae 100644 --- a/src/immer.ts +++ b/src/immer.ts @@ -9,7 +9,8 @@ import { isMap, isDraftable, DRAFT_STATE, - NOTHING + NOTHING, + die } from "./common" import {ImmerScope} from "./scope" import { @@ -26,6 +27,7 @@ import {proxyMap} from "./map" import {proxySet} from "./set" import {processResult, maybeFreeze} from "./finalize" +/* istanbul ignore next */ function verifyMinified() {} const configDefaults = { @@ -36,7 +38,8 @@ const configDefaults = { autoFreeze: typeof process !== "undefined" ? process.env.NODE_ENV !== "production" - : verifyMinified.name === "verifyMinified", + : /* istanbul ignore next */ + verifyMinified.name === "verifyMinified", onAssign: null, onDelete: null, onCopy: null @@ -163,8 +166,8 @@ export class Immer implements ProducersFns { this.produceWithPatches(state, (draft: any) => arg1(draft, ...args)) } // non-curried form - if (arg3) - throw new Error("A patch listener cannot be passed to produceWithPatches") + /* istanbul ignore next */ + if (arg3) die() let patches: Patch[], inversePatches: Patch[] const nextState = this.produce(arg1, arg2, (p: Patch[], ip: Patch[]) => { patches = p diff --git a/src/map.ts b/src/map.ts index 11687852e..e46a8f7b7 100644 --- a/src/map.ts +++ b/src/map.ts @@ -14,6 +14,7 @@ export interface MapState extends ImmerBaseState { } // Make sure DraftMap declarion doesn't die if Map is not avialable... +/* istanbul ignore next */ const MapBase: MapConstructor = typeof Map !== "undefined" ? Map : (function FakeMap() {} as any) @@ -117,6 +118,7 @@ export class DraftMap extends MapBase implements Map { [iteratorSymbol]: () => this.values(), next: () => { const r = iterator.next() + /* istanbul ignore next */ if (r.done) return r const value = this.get(r.value) return { @@ -133,6 +135,7 @@ export class DraftMap extends MapBase implements Map { [iteratorSymbol]: () => this.entries(), next: () => { const r = iterator.next() + /* istanbul ignore next */ if (r.done) return r const value = this.get(r.value) return { diff --git a/src/patches.ts b/src/patches.ts index 127a608d9..28892c64c 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -43,10 +43,11 @@ function generateArrayPatches( inversePatches: Patch[] ) { let {base, assigned, copy} = state + /* istanbul ignore next */ if (!copy) die() // Reduce complexity by ensuring `base` is never longer. - if (copy!.length < base.length) { + if (copy.length < base.length) { // @ts-ignore ;[base, copy] = [copy, base] ;[patches, inversePatches] = [inversePatches, patches] @@ -173,6 +174,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { patches.forEach(patch => { const {path, op} = patch + /* istanbul ignore next */ if (!path.length) die() let base: any = draft @@ -190,6 +192,7 @@ export function applyPatches(draft: T, patches: Patch[]): T { switch (type) { case Archtype.Map: return base.set(key, value) + /* istanbul ignore next */ case Archtype.Set: throw new Error('Sets cannot have "replace" patches.') default: @@ -199,7 +202,6 @@ export function applyPatches(draft: T, patches: Patch[]): T { // @ts-ignore return (base[key] = value) } - break case "add": switch (type) { case Archtype.Array: diff --git a/src/set.ts b/src/set.ts index 6825d68ba..a952b4af4 100644 --- a/src/set.ts +++ b/src/set.ts @@ -14,6 +14,7 @@ export interface SetState extends ImmerBaseState { } // Make sure DraftSet declarion doesn't die if Map is not avialable... +/* istanbul ignore next */ const SetBase: SetConstructor = typeof Set !== "undefined" ? Set : (function FakeSet() {} as any) @@ -79,7 +80,7 @@ export class DraftSet extends SetBase implements Set { state.copy!.delete(value) || (state.drafts.has(value) ? state.copy!.delete(state.drafts.get(value)) - : false) + : /* istanbul ignore next */ false) ) }