mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.9k stars 639 forks source link

fix splitCache, extra nodes were being removed #2054

Closed scytacki closed 11 months ago

scytacki commented 11 months ago

https://github.com/mobxjs/mobx-state-tree/issues/2053

When detach is used to remove nodes from the tree, splitCache is called to pull this node and its children out of the identifierCache of the original tree. The implementation of this splitting had a bug which would remove additional nodes that are not children of the detached node.

The issue was that it was using a string match of the path of the node being removed with all the other nodes in the indentifierCache. So if the node being removed had a path of /items/1, then this would match /items/10, or items/199.

Additionally this PR removes empty sets in the indentifierCache when that is the result of splitCache. This matches the behavior of notifiedDied.

coolsoftwaretyler commented 11 months ago

Thanks, @scytacki! I will take a look at this in a few days. Looks very good at first glance, and I'm guessing y'all have merged it into your fork/are using it, so I'm sure we'll merge it pretty quickly.

Really appreciate you merging it back into MST! Thanks for that!

scytacki commented 11 months ago

Thanks @coolsoftwaretyler. Yes we have merged it into our fork. We haven't deployed it to our production app yet, but we will likely be doing that this week.