Description
🐛 Bug Report
When adding to a Set with produce, the insertion order is not maintained when the original Set contains a draftable object.
Link to repro
See #820 for a failing test (copied below)
it("maintains order when adding", () => {
const objs = [
{
id: "a"
},
{
id: "b"
}
]
const set = new Set([objs[0]])
const newSet = produce(set, draft => {
draft.add(objs[1])
})
expect(Array.from(newSet)).toEqual([objs[0], objs[1]])
})To Reproduce
Run the "maintains order when adding" test in the "set drafts" section of __tests__/base.js.
Observed behavior
After adding a second object to the Set with produce, it is ordered before the original first object in the Set.
Expected behavior
{ id: 'b' } should appear after { id: 'a' } when iterating through newSet, matching the behavior of a normal mutation-based update.
Debugging Observations
As mentioned above, I've only noticed this in the situation where the original Set (pre-produce) contains a draftable object. For example, this test passes:
it("maintains order when adding", () => {
const objs = [
"a",
{
id: "b"
}
]
const set = new Set([objs[0]])
const newSet = produce(set, draft => {
draft.add(objs[1])
})
// passes
expect(Array.from(newSet)).toEqual([objs[0], objs[1]])
})but the opposite does not:
it("maintains order when adding", () => {
const objs = [
{
id: "a"
},
"b"
]
const set = new Set([objs[0]])
const newSet = produce(set, draft => {
draft.add(objs[1])
})
// does not pass
expect(Array.from(newSet)).toEqual([objs[0], objs[1]])
})Possible Root Cause
(take all of this with a grain of salt - my first time digging into the repo 😅 )
After a bit of digging, this function seems to be causing the order change during finalize:
Lines 131 to 138 in dc3f66c
finalizeProperty is called on both objects (Draft of { id: 'a' } and vanilla object { id: 'b' }). On the first call, the Draft is converted back to a vanilla object and the Set value is replaced:
Lines 130 to 131 in dc3f66c
Since Set values can't be directly updated, set deletes the previous version (Draft of { id: 'a' }) and adds the new version (vanilla object), ultimately breaking the original order.
Environment
We only accept bug reports against the latest Immer version.
- Immer version: v9.0.3
- I filed this report against the latest version of Immer
- Occurs with
setUseProxies(true) - Occurs with
setUseProxies(false)(ES5 only)
This appears to have been introduced in v5.2.0 which contains a rewrite of the Map and Set implementations, though I've only been able to test on v5.2.1 due to #502