Add reference equality checking to Pitfalls?

[ianstormtaylor]

🙋‍♂ Question

I ran into an issue because reference equality checking doesn't work with Immer proxies. After researching all the traps available to proxies the limitation makes total sense, but I think it's a pitfall if you aren't familiar with proxies. (I just assumed there was a trap for equality checks to make objects fully transparent.)

Here's an example of what's not possible:

import produce from 'immer'

const a = { value: 7 }
const b = { value: 4 }
const c = { value: 7 }
const values = [a, b, c]

const remove = produce((list, element) => {
  const index = list.indexOf(element);
  if (index > -1) list.splice(index, 1);
})

remove(values, a)

Since the proxies aren't referentially equal, you can't compare an existing element to the proxied elements to get its index. This makes using Immer in cases with elements without distinguishing properties impossible. (You need some sort of unique property like .id to identify the element in question for this to work.)

Is this correct?

I think it would be useful to add to the Pitfalls docs if so, and I'm happy to create a PR if you agree.

Link to repro

https://codesandbox.io/s/lucid-hooks-mst6l

[mweststrate]

PR welcome! Note that you can solve this with using the 'original' function, e.g original(list).indexOf, or, if the list might be changed already making the returned index possibly incorrect, 'list.findIndex(x => original(x) === element)' should do the trick. If you can, I'd perform the search before entering the produce, because that will be the fastest.

…

On Thu, 7 Jan 2021, 19:58 Ian Storm Taylor, ***@***.***> wrote: 🙋‍♂ Question I ran into an issue because reference equality checking doesn't work with Immer proxies. After researching all the traps available to proxies the limitation makes total sense, but I think it's a pitfall if you aren't familiar with proxies. (I just assumed there was a trap for equality checks to make objects fully transparent.) Here's an example of what's not possible: import produce from 'immer' const a = { value: 7 } const b = { value: 4 } const c = { value: 7 } const values = [a, b, c] const remove = produce((list, element) => { const index = list.indexOf(element); if (index > -1) list.splice(index, 1); }) remove(values, a) Since the proxies aren't referentially equal, you can't compare an existing element to the proxied elements to get its index. This makes using Immer in cases with elements without distinguishing properties impossible. (You need some sort of unique property like .id to identify the element in question for this to work.) Is this correct? I think it would be useful to add to the Pitfalls docs if so, and I'm happy to create a PR if you agree. Link to repro https://codesandbox.io/s/lucid-hooks-mst6l — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#730>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBEYMQXXJJNFPK7S77DSYYG75ANCNFSM4VZOKN5Q> .

[ianstormtaylor]

@mweststrate ah cool, I didn't think of that! Thanks that's a great solution, I'll write up a PR that mentions that.

[mweststrate]

🎉 This issue has been resolved in version 8.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀