Skip to content

Commit a2caade

Browse files
committed
Add Optional Support For Multiple References to an Object
# What This Does Some state trees may need to reference an object more than once (such as the tree for my [fomod](https://www.npmjs.com/package/fomod) library). In essence, we store existing drafts when an off-by-default Immer class configuration option is enabled. This should be a painless solution. Specifics are described below. ## Implementation Details * Two `WeakMap` are used to keep track of draft states and related data at different parts of the immerification process: * `existingStateMap_` maps a given base object to the first draft state created for it. This state includes a reference to the revokable draft. * If a state is referenced multiple times, it will be given a new `revoke_()` function that, once called the first time, calls the old `revoke_()` function. The result is that the final `revoke_()` must be called once for every requested draft before the Proxy is finally revoked. Since a proxy which has has its `revoke_()` method called should be considered revoked by all code paths, duplicate calls should not be an issue. * During finalization, `encounteredObjects` keeps track of objects we've finalized and doesn't traverse an object if it's already seen it. It prevents infinite recursion when circular references are present. * Introduced the `extraParents_` property to the `ImmerBaseState` interface. This keeps track of additional values that would normally be attached to `parent_` so that functionality such as marking the parent state as modified is retained for objects with multiple parent objects * For Maps and Sets, a proxy is established between the state and DraftMap/DraftSet classes to handle multiple references to these native classes while preserving the idea of having one DraftSet per reference. * For Sets, each child draft has a single symbol value set so that a copy is prepared. (discussion needed; see TODOs below) * During finalization, objects may have drafted children and, thus, even unmodified children are finalized in multi-ref mode * To enable the feature, it is the same as other Immer class configuration options (such as `useStrictShallowCopy`). That is, either specify it in the config object passed to the class's constructor OR call the relevant method, `setAllowMultiRefs()` > [!NOTE] > Because of the extra computation involved with checking every proxied object against a map and traversing every object in a tree, enabling multi-ref will have a significant performance impact—even on trees which contain no repeated references. # Tests The file `__tests__/multiref.ts` contains a number of tests related to this multi-reference support implementation. Such tests seek to verify that: * Direct circular references (which Immer tests for normally) do not throw an error when multi-ref is enabled * When the properties of multiple references are modified, all references are modified * Unmodified references to the same object are kept * The same copy is provided for every reference (new references are strictly equivalent [`===`] just as the references before `produce()` would have been) Tests are performed on all relevant object archetypes where applicable. # Outstanding Discussion TODOs * [ ] What to do regarding documentation * [ ] Possible alternate solution for preparing copies for multi-reference DraftSet children * [ ] Add an error for when WeakMap isn't supported in the current environment? (supported in every noteworthy browser and server environment since late 2015)
1 parent 8e465ab commit a2caade

File tree

10 files changed

+722
-97
lines changed

10 files changed

+722
-97
lines changed

__tests__/__prod_snapshots__/base.js.snap

Lines changed: 128 additions & 0 deletions
Large diffs are not rendered by default.

__tests__/__prod_snapshots__/manual.js.snap

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
exports[`manual - proxy cannot finishDraft twice 1`] = `"Cannot perform 'get' on a proxy that has been revoked"`;
44

5+
exports[`manual - proxy cannot modify after finish 1`] = `"Cannot perform 'set' on a proxy that has been revoked"`;
6+
57
exports[`manual - proxy should check arguments 1`] = `"[Immer] minified error nr: 8. Full error at: https://bit.ly/3cXEKWf"`;
68

79
exports[`manual - proxy should check arguments 2`] = `"[Immer] minified error nr: 8. Full error at: https://bit.ly/3cXEKWf"`;

__tests__/multiref.ts

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
import {Immer, enableMapSet} from "../src/immer"
2+
import {inspect} from "util"
3+
4+
// Implementation note: TypeScript says ES5 doesn't support iterating directly over a Set so I've used Array.from().
5+
// If the project is moved to a later JS feature set, we can drop the Array.from() and do `for (const value of ref)` instead.
6+
7+
test("modified circular object", () => {
8+
const immer = new Immer({allowMultiRefs: true})
9+
const base = {a: 1, b: null} as any
10+
base.b = base
11+
12+
const envs = ["production", "development", "testing"]
13+
for (const env of envs) {
14+
process.env.NODE_ENV = env
15+
expect(() => {
16+
const next = immer.produce(base, (draft: any) => {
17+
draft.a = 2
18+
})
19+
expect(next).toEqual({a: 2, b: next})
20+
}).not.toThrow()
21+
}
22+
})
23+
24+
test("unmodified circular object", () => {
25+
const immer = new Immer({allowMultiRefs: true})
26+
const base = {a: 1, b: null} as any
27+
base.b = base
28+
29+
const envs = ["production", "development", "testing"]
30+
for (const env of envs) {
31+
process.env.NODE_ENV = env
32+
expect(() => {
33+
const next = immer.produce({state: null}, (draft: any) => {
34+
draft.state = base
35+
})
36+
expect(next.state).toBe(base)
37+
}).not.toThrow()
38+
}
39+
})
40+
41+
describe("access value & change child's child value", () => {
42+
describe("with object", () => {
43+
const immer = new Immer({allowMultiRefs: true})
44+
const sameRef = {someNumber: 1, someString: "one"}
45+
const objectOfRefs = {a: sameRef, b: sameRef, c: sameRef, d: sameRef}
46+
47+
const base = {
48+
objectRef1: objectOfRefs,
49+
objectRef2: objectOfRefs,
50+
objectRef3: objectOfRefs,
51+
objectRef4: objectOfRefs
52+
}
53+
const next = immer.produce(base, draft => {
54+
draft.objectRef2.b.someNumber = 2
55+
draft.objectRef3.c.someString = "two"
56+
})
57+
58+
it("should have kept the Object refs the same", () => {
59+
expect(next.objectRef1).toBe(next.objectRef2),
60+
expect(next.objectRef2).toBe(next.objectRef3),
61+
expect(next.objectRef3).toBe(next.objectRef4)
62+
})
63+
64+
it("should have updated the values across everything", () => {
65+
function verifyObjectReference(
66+
ref: {[key: string]: {someNumber: number; someString: string}},
67+
objectNum: number
68+
) {
69+
verifySingleReference(ref.a, objectNum, "a")
70+
verifySingleReference(ref.b, objectNum, "b")
71+
verifySingleReference(ref.c, objectNum, "c")
72+
verifySingleReference(ref.d, objectNum, "d")
73+
}
74+
75+
function verifySingleReference(
76+
ref: {someNumber: number; someString: string},
77+
objectNum: number,
78+
refKey: string
79+
) {
80+
//it(`should have updated the values across everything (ref ${refKey.toUpperCase()} in object #${objectNum})`, () => {
81+
expect(ref.someNumber).toBe(2)
82+
expect(ref.someString).toBe("two")
83+
//})
84+
}
85+
86+
verifyObjectReference(next.objectRef1, 1)
87+
verifyObjectReference(next.objectRef2, 2)
88+
verifyObjectReference(next.objectRef3, 3)
89+
verifyObjectReference(next.objectRef4, 4)
90+
});
91+
})
92+
93+
describe("with map", () => {
94+
const immer = new Immer({allowMultiRefs: true})
95+
enableMapSet()
96+
const sameRef = {someNumber: 1, someString: "one"}
97+
const mapOfRefs = new Map([
98+
["a", sameRef],
99+
["b", sameRef],
100+
["c", sameRef],
101+
["d", sameRef]
102+
])
103+
104+
const base = {
105+
mapRef1: mapOfRefs,
106+
mapRef2: mapOfRefs,
107+
mapRef3: mapOfRefs,
108+
mapRef4: mapOfRefs
109+
}
110+
const next = immer.produce(base, draft => {
111+
draft.mapRef2.get("b")!.someNumber = 2
112+
draft.mapRef3.get("c")!.someString = "two"
113+
})
114+
115+
it("should have kept the Map refs the same", () => {
116+
expect(next.mapRef1).toBe(next.mapRef2),
117+
expect(next.mapRef2).toBe(next.mapRef3),
118+
expect(next.mapRef3).toBe(next.mapRef4)
119+
})
120+
121+
it("should have updated the values across everything", () => {
122+
function verifyMapReference(
123+
ref: Map<string, {someNumber: number; someString: string}>,
124+
mapNum: number
125+
) {
126+
verifySingleReference(ref.get("a")!, mapNum, "a")
127+
verifySingleReference(ref.get("b")!, mapNum, "b")
128+
verifySingleReference(ref.get("c")!, mapNum, "c")
129+
verifySingleReference(ref.get("d")!, mapNum, "d")
130+
131+
//it(`should have the same child refs (map #${mapNum})`, () => {
132+
expect(ref.get("a")).toBe(ref.get("b")),
133+
expect(ref.get("b")).toBe(ref.get("c")),
134+
expect(ref.get("c")).toBe(ref.get("d"))
135+
//})
136+
}
137+
138+
function verifySingleReference(
139+
ref: {someNumber: number; someString: string},
140+
mapNum: number,
141+
refKey: string
142+
) {
143+
//it(`should have updated the values across everything (ref ${refKey.toUpperCase()} in map #${mapNum})`, () => {
144+
expect(ref.someNumber).toBe(2)
145+
expect(ref.someString).toBe("two")
146+
//})
147+
}
148+
149+
verifyMapReference(next.mapRef1, 1)
150+
verifyMapReference(next.mapRef2, 2)
151+
verifyMapReference(next.mapRef3, 3)
152+
verifyMapReference(next.mapRef4, 4)
153+
154+
});
155+
})
156+
157+
describe("with array", () => {
158+
const immer = new Immer({allowMultiRefs: true})
159+
const sameRef = {someNumber: 1, someString: "one"}
160+
const arrayOfRefs = [sameRef, sameRef, sameRef, sameRef]
161+
162+
const base = {
163+
arrayRef1: arrayOfRefs,
164+
arrayRef2: arrayOfRefs,
165+
arrayRef3: arrayOfRefs,
166+
arrayRef4: arrayOfRefs
167+
}
168+
const next = immer.produce(base, draft => {
169+
draft.arrayRef2[1].someNumber = 2
170+
draft.arrayRef3[2].someString = "two"
171+
})
172+
173+
it("should have kept the Array refs the same", () => {
174+
expect(next.arrayRef1).toBe(next.arrayRef2),
175+
expect(next.arrayRef2).toBe(next.arrayRef3),
176+
expect(next.arrayRef3).toBe(next.arrayRef4)
177+
})
178+
179+
it("should have updated the values across everything", () => {
180+
function verifyArrayReference(
181+
ref: {someNumber: number; someString: string}[],
182+
arrayNum: number
183+
) {
184+
let i = 0
185+
for (const value of ref) {
186+
//it(`should have updated the values across everything (ref #${i++} in array #${arrayNum})`, () => {
187+
verifySingleReference(value)
188+
//})
189+
}
190+
}
191+
192+
function verifySingleReference(ref: {
193+
someNumber: number
194+
someString: string
195+
}) {
196+
expect(ref.someNumber).toBe(2)
197+
expect(ref.someString).toBe("two")
198+
}
199+
200+
verifyArrayReference(next.arrayRef1, 1)
201+
verifyArrayReference(next.arrayRef2, 2)
202+
verifyArrayReference(next.arrayRef3, 3)
203+
verifyArrayReference(next.arrayRef4, 4)
204+
});
205+
})
206+
207+
describe("with set", () => {
208+
const immer = new Immer({allowMultiRefs: true})
209+
enableMapSet()
210+
const sameRef = {someNumber: 1, someString: "one"}
211+
const setOfRefs = new Set([{sameRef}, {sameRef}, {sameRef}, {sameRef}])
212+
213+
const base = {
214+
setRef1: setOfRefs,
215+
setRef2: setOfRefs,
216+
setRef3: setOfRefs,
217+
setRef4: setOfRefs
218+
}
219+
//console.log("base", inspect(base, {depth: 6, colors: true, compact: true}))
220+
221+
const next = immer.produce(base, draft => {
222+
const set2Values = draft.setRef2.values()
223+
set2Values.next()
224+
set2Values.next().value.sameRef.someNumber = 2
225+
226+
const set3Values = draft.setRef3.values()
227+
set3Values.next()
228+
set3Values.next()
229+
set3Values.next().value.sameRef.someString = "two"
230+
})
231+
232+
//console.log(
233+
// "next",
234+
// inspect(next, {
235+
// depth: 20,
236+
// colors: true,
237+
// compact: true,
238+
// breakLength: Infinity
239+
// })
240+
//)
241+
242+
it("should have kept the Set refs the same", () => {
243+
expect(next.setRef1).toBe(next.setRef2),
244+
expect(next.setRef2).toBe(next.setRef3),
245+
expect(next.setRef3).toBe(next.setRef4)
246+
})
247+
248+
it("should have updated the values across everything", () => {
249+
function verifySetReference(
250+
ref: Set<{sameRef: {someNumber: number; someString: string}}>,
251+
setLetter: string
252+
) {
253+
//it(`should have the same child refs (set ${setLetter.toUpperCase()})`, () => {
254+
let first = ref.values().next().value.sameRef
255+
for (const value of Array.from(ref)) {
256+
expect(value.sameRef).toBe(first)
257+
}
258+
//})
259+
260+
let i = 0
261+
for (const value of Array.from(ref)) {
262+
//it(`should have updated the values across everything (ref #${i++} in set ${setLetter.toUpperCase()})`, () => {
263+
verifySingleReference(value.sameRef)
264+
//})
265+
}
266+
}
267+
268+
function verifySingleReference(ref: {
269+
someNumber: number
270+
someString: string
271+
}) {
272+
expect(ref.someNumber).toBe(2)
273+
expect(ref.someString).toBe("two")
274+
}
275+
276+
verifySetReference(next.setRef1, "a")
277+
verifySetReference(next.setRef2, "b")
278+
verifySetReference(next.setRef3, "c")
279+
verifySetReference(next.setRef4, "d")
280+
281+
});
282+
})
283+
})

0 commit comments

Comments
 (0)