search

immerjs / immer

Create the next immutable state by mutating the current one
https://immerjs.github.io/immer/
MIT License
27.65k stars 848 forks source link

feat: deepCurrent #1092

Closed lveillard closed 7 months ago

lveillard commented 8 months ago

Related to this issue: https://github.com/immerjs/immer/issues/1091

This is just a draft, if it goes validated I can test it, add some tests and apply any changes you might ask for!

mweststrate commented 7 months ago

Thanks for the effort of creating this PR! I think this might actually be a bug in the current current (lol) implementation, as it is supposed to be able to handle recursion already by the looks of it: https://github.com/immerjs/immer/blob/main/src/core/current.ts#L33

From a quick scan your implementation seems also to be exactly the same as current? Would you mind first setting up a unit test showing the problem in the existing current implementation?

lveillard commented 7 months ago

Thanks for the effort of creating this PR! I think this might actually be a bug in the current current (lol) implementation, as it is supposed to be able to handle recursion already by the looks of it: https://github.com/immerjs/immer/blob/main/src/core/current.ts#L33

From a quick scan your implementation seems also to be exactly the same as current? Would you mind first setting up a unit test showing the problem in the existing current implementation?

True! Somehow deepCurrent works for my use case and current does not 🤷‍♂️. Will try to find some time to build that test triggering the issue, I guess is related to using symbols with objects as values.

mweststrate commented 7 months ago

Closing in favour of #1105, that fixes the underlying issue (I think). Thanks for reporting and taking the effort for this PR though!