Skip to content

Commit d6c1202

Browse files
authored
perf: Rewrite finalization system to use a callback approach instead of tree traversal (#1183)
BREAKING CHANGE: enable loose iteration by default * Simplify some iteration checks * Allow passing type to get/set utils to skip archetype lookup * Convert assigned_ to Map * Enable loose iteration * Replace recursive tree finalization with targeted callbacks Ported Mutative's "finalization callback" approach as a more targeted and performant implementation for finalization compared to the existing recursive tree traversal approach: - Added cleanup callbacks for each draft that's created - Added callbacks to handle root drafts, assigned values, and recursing inside of plain values - Updated state creation to return [draft, state] to avoid a lookup - Rewrote patch generation system to work with callbacks instead of during tree traversal * Update self-reference test with new behavior * Apply code review suggestions * Byte-shave scopes and patch plugin usage * Inline finalizeAssigned * Move fixPotentialSetContents to plugin * Byte-shave typeof utils * Byte-shave Object references * Byte-shave field names and arrow functions
1 parent 521d5ed commit d6c1202

14 files changed

Lines changed: 715 additions & 351 deletions

File tree

__tests__/__snapshots__/base.js.snap

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=
1010

1111
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=false > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
1212

13-
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=false > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
14-
1513
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=false > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
1614

1715
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=false > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -48,8 +46,6 @@ exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=
4846

4947
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=true > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
5048

51-
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=true > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
52-
5349
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=true > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
5450

5551
exports[`base functionality - auto-freeze=false:shallow-copy=false:use-listener=true > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -86,8 +82,6 @@ exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=f
8682

8783
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=false > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
8884

89-
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=false > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
90-
9185
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=false > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
9286

9387
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=false > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -124,8 +118,6 @@ exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=t
124118

125119
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=true > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
126120

127-
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=true > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
128-
129121
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=true > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
130122

131123
exports[`base functionality - auto-freeze=false:shallow-copy=true:use-listener=true > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -162,8 +154,6 @@ exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=f
162154

163155
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=false > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
164156

165-
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=false > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
166-
167157
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=false > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
168158

169159
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=false > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -200,8 +190,6 @@ exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=t
200190

201191
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=true > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
202192

203-
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=true > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
204-
205193
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=true > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
206194

207195
exports[`base functionality - auto-freeze=true:shallow-copy=false:use-listener=true > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -238,8 +226,6 @@ exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=fa
238226

239227
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=false > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
240228

241-
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=false > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
242-
243229
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=false > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
244230

245231
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=false > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;
@@ -276,8 +262,6 @@ exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=tr
276262

277263
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=true > recipe functions > cannot return a modified child draft 1`] = `[Error: [Immer] An immer producer returned a new value *and* modified its draft. Either return a new value *or* modify the draft.]`;
278264

279-
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=true > recipe functions > cannot return an object that references itself 1`] = `[Error: [Immer] Immer forbids circular references]`;
280-
281265
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=true > revokes the draft once produce returns 1`] = `[TypeError: Cannot perform 'get' on a proxy that has been revoked]`;
282266

283267
exports[`base functionality - auto-freeze=true:shallow-copy=true:use-listener=true > revokes the draft once produce returns 2`] = `[TypeError: Cannot perform 'set' on a proxy that has been revoked]`;

__tests__/base.js

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2367,9 +2367,9 @@ function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) {
23672367
draft.y = 1
23682368
draft.z = NaN
23692369
if (!isProd) {
2370-
expect(draft[DRAFT_STATE].assigned_.x).toBe(true)
2371-
expect(draft[DRAFT_STATE].assigned_.y).toBe(undefined)
2372-
expect(draft[DRAFT_STATE].assigned_.z).toBe(undefined)
2370+
expect(draft[DRAFT_STATE].assigned_.get("x")).toBe(true)
2371+
expect(draft[DRAFT_STATE].assigned_.get("y")).toBe(undefined)
2372+
expect(draft[DRAFT_STATE].assigned_.get("z")).toBe(undefined)
23732373
}
23742374
})
23752375
expect(nextState.x).toBe("s2")
@@ -2756,13 +2756,31 @@ function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) {
27562756
expect(next[0]).toBe(next[1])
27572757
})
27582758

2759-
// This actually seems to pass now!
2760-
it("cannot return an object that references itself", () => {
2759+
// As of the finalization callback rewrite, the
2760+
// the original `() => res.self` check passes without throwing,
2761+
// but we still will not have self-references
2762+
// when returning updated values
2763+
it("can return self-references, but not for modified values", () => {
27612764
const res = {}
27622765
res.self = res
2763-
expect(() => {
2764-
produce(res, () => res.self)
2765-
}).toThrowErrorMatchingSnapshot()
2766+
2767+
// the call will pass
2768+
const next = produce(res, draft => {
2769+
draft.a = 42
2770+
draft.self.b = 99
2771+
})
2772+
2773+
// Root object and first child were both copied
2774+
expect(next).not.toBe(next.self)
2775+
// Second child is the first circular reference
2776+
expect(next.self.self).not.toBe(next.self)
2777+
// And it's turtles all the way down
2778+
expect(next.self.self.self).toBe(next.self.self.self.self)
2779+
expect(next.a).toBe(42)
2780+
expect(next.self.b).toBe(99)
2781+
// The child copy did not receive the update
2782+
// to the root object
2783+
expect(next.self.a).toBe(undefined)
27662784
})
27672785
})
27682786

__tests__/map-set.js

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,21 @@ function runBaseTest(name, autoFreeze, useListener) {
124124
[
125125
{
126126
op: "remove",
127-
path: ["map", "b", "a"]
127+
path: ["map", "d", "a"]
128128
},
129129
{
130130
op: "remove",
131131
path: ["map", "c", "a"]
132132
},
133133
{
134134
op: "remove",
135-
path: ["map", "d", "a"]
135+
path: ["map", "b", "a"]
136136
}
137137
],
138138
[
139139
{
140140
op: "add",
141-
path: ["map", "b", "a"],
141+
path: ["map", "d", "a"],
142142
value: true
143143
},
144144
{
@@ -148,7 +148,7 @@ function runBaseTest(name, autoFreeze, useListener) {
148148
},
149149
{
150150
op: "add",
151-
path: ["map", "d", "a"],
151+
path: ["map", "b", "a"],
152152
value: true
153153
}
154154
]
@@ -197,31 +197,32 @@ function runBaseTest(name, autoFreeze, useListener) {
197197
expect(p).toEqual([
198198
{
199199
op: "remove",
200-
path: ["map", "b", "a"]
200+
path: ["map", "d", "a"]
201201
},
202202
{
203203
op: "remove",
204204
path: ["map", "c", "a"]
205205
},
206206
{
207207
op: "remove",
208-
path: ["map", "d", "a"]
208+
path: ["map", "b", "a"]
209209
}
210210
])
211211
expect(ip).toEqual([
212212
{
213213
op: "add",
214-
path: ["map", "b", "a"],
214+
path: ["map", "d", "a"],
215215
value: true
216216
},
217+
217218
{
218219
op: "add",
219220
path: ["map", "c", "a"],
220221
value: true
221222
},
222223
{
223224
op: "add",
224-
path: ["map", "d", "a"],
225+
path: ["map", "b", "a"],
225226
value: true
226227
}
227228
])

0 commit comments

Comments
 (0)