mobxjs / mobx-state-tree

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

Objects in array are died after array.replace is called which use types.union with dispatcher and return types.model with types.identifier #2013

Closed HughHzWu closed 1 year ago

HughHzWu commented 1 year ago

Bug report

Sandbox link or minimal reproduction code sandbox demo

Describe the expected behavior Objects should be alived

Describe the observed behavior Objects are died

coolsoftwaretyler commented 1 year ago

I think the problem is that you're declaring id as a types.identifier, which needs to be unique. So the reference to the old, dead node stays around.

If you use an actually unique value for the id, and use some field like title for the string value to display, the nodes remain alive. Here's an example sandbox forked from yours:

https://codesandbox.io/s/mobx-state-tree-todolist-forked-n9xksf

You'll notice I'm using Math.random(), which isn't necessarily guaranteed to be unique. For that, you might want to use a library like uuid. The ids change, but the nodes remain alive. The title also stays the same.

Does that fix your problem, or did you expect the replaced objects to remain alive even matched against those same ids? If so, can you say more about the use case for that?

coolsoftwaretyler commented 1 year ago

Hey @HughHzWu - just checking in here. Did that help you out? I am happy to help you continue troubleshooting further, but I'll probably close this issue out if I don't hear back from you in the next two weeks.

HughHzWu commented 1 year ago

Hi @coolsoftwaretyler . Here's an other example using types.union without dispatcher. It runs good.

https://codesandbox.io/s/mobx-state-tree-todolist-forked-lkc2sx

coolsoftwaretyler commented 1 year ago

@HughHzWu - thanks for the additional context! That was really helpful.

I'm not 100% certain, but I think the reason your first Sandbox is failing is because the model type returned by your dispatcher is an anonymous model, and the IDs between the first type of anonymous model are not being tracked with each new invocation of replace, because each invocation of replace creates a new, different, anonymous model type, so the IDs are getting lost or something.

Here's a Sandbox where I defined baseModel, and returned it from the dispatcher, instead of a new types.model(...) each time. They seem to be staying alive: https://codesandbox.io/s/mobx-state-tree-todolist-forked-hcw5ht

Are you seeing this same problem in a real-world example where you aren't using anonymous models?

HughHzWu commented 1 year ago

@coolsoftwaretyler Yes, I got it. Thank you for your answer :)

coolsoftwaretyler commented 1 year ago

Excellent! I am going to close out this issue. Have a good one!