Invalidate Parent (Not just Immediate object) on Types.SafeReference

[ChristopherGabba]

Bug report

• [x ] I've checked documentation and searched for existing issues and discussions
• [x ] I've made sure my project is based on the latest MST version
• Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code

I'm having trouble getting my app to not have reference issues.
Here is my code:

import { Instance, SnapshotIn, SnapshotOut, types } from "mobx-state-tree"
import { withSetPropAction } from "./helpers/withSetPropAction"
import { FriendshipOrGroup, Group, GroupModel } from "./Group"
import { FriendshipModel } from "./Friendship"
import { Correspondence } from "app/services/api/AWS/API"

export const RecentModel = types
  .model("Recent", {
    id: types.identifier,
    friendship: types.safeReference(FriendshipModel, {
      onInvalidated(event) {
        event.removeRef()
      },
    }),
    lastCorrespondence: types.frozen<Correspondence>(),
    updatedAt: types.Date,
    sentCount: types.maybe(types.maybeNull(types.number)),
    viewedCount: types.maybe(types.maybeNull(types.number)),
  })
  .actions(withSetPropAction)

Exact Sequence:

1. I fetch data from my backend (aka fresh start)
2. I delete a Friendship object that is referenced by this Recent object (which deletes it from the backend AND MST), but I DO NOT delete the Recent object from the backend / MST. This immediately should invalidate the Recent object, when the Friendship object is removed from the tree.
3. When I then pull-to-refresh (aka fetch the data again, this time it has one less Friendship, but the Recent object still comes in)
4. The recent object correctly hits the 'onInvalidated' function call but for some reason, my lists fail to load and its like the reference is still there. I get an Promise Rejection and the app sits in a stale state and my lists fail to update.

Describe the expected behavior

The app should effectively seamlessly ignore the Recent that is now referencing an invalid Friendship object.

Describe the observed behavior

The app throws a promise rejection and my Friendslist stays in the loading state. Now I have a workaround that works for some reason:

    friendship: types.safeReference(FriendshipModel, {
      onInvalidated(event) {
        // event.removeRef() This line clearly isn't working in this instance, do not know why
        const recentToDeleteFromStore = event.parent
        const rootNode = getRoot(event.parent)
       // A tree cannot be modified without an action so I have to unprotect it first
        unprotect(rootNode)
        destroy(recentToDeleteFromStore)
        protect(rootNode)
      },
    }),

Another work around is that I add a simplified filter / middleware type of function so that I check the references of all the backend data before applying it to MST. This has also worked well but is kind of slow because it annoyingly parses through a bunch of objects.

In my mind this is the same thing as effectively running event.parent.removeRef() if it existed (I kind of think it should, this is basically what I'm looking for).

When the code above runs, the app seamlessly ignores the Recent that is referencing the invalid Friendship and everything loads as normal. Would love your thoughts @coolsoftwaretyler as to what I'm doing wrong or issues with my strategy.

[coolsoftwaretyler]

Hey @ChristopherGabba - thanks for the issue. I'm traveling right now so I'll take a look soon, but it may be closer to the end of the week.

[ChristopherGabba]

Hey @ChristopherGabba - thanks for the issue. I'm traveling right now so I'll take a look soon, but it may be closer to the end of the week.

Not a problem at all, I appreciate it very much!

[coolsoftwaretyler]

Hey @ChristopherGabba - thanks for your patience on this. Just got back from travel and then had some illness that has kept me from checking things out.

The solution you called a "workaround" is, perhaps frustratingly, the "correct" way to do this in MobX-State-Tree. Our reference system is not as sophisticated as you might want it to be. This comes up from time to time. For the most part, references are just a key/value store that we expose via the API, so you're responsible for your own cleanup.

Your first version of the onInvalidated hook is pretty much how we implement it by default. See https://mobx-state-tree.js.org/concepts/references#customizable-references. The removeRef function describes itself:

 // a function to remove the reference from its parent (or set to undefined in the case of models)

So we're not going to handle removing parents through that call, unlike a cascading delete in a relational database that might do that for you.

That said, I'd like to consider making this easier for you. You mentioned maybe an event.parent.removeRef method. What if we exposed removeRef as a part of the API so you could call it on any arbitrary reference? I don't want to just expose the parent object, because it's possible a use case might require multiple ancestors to call removeRef. But with a standalone removeRef function, users could write their own invalidation logic to handle different scenarios.

Would that resolve your issue here, or is it insufficient in some way?

[ChristopherGabba]

@coolsoftwaretyler That would be great, I'm kind of indifferent on what the API looks like. What you are proposing is this?

import { Instance, SnapshotIn, SnapshotOut, types, removeRef } from "mobx-state-tree" //<-- Added import
import { withSetPropAction } from "./helpers/withSetPropAction"
import { FriendshipOrGroup, Group, GroupModel } from "./Group"
import { FriendshipModel } from "./Friendship"
import { Correspondence } from "app/services/api/AWS/API"


export const RecentModel = types
  .model("Recent", {
    id: types.identifier,
    friendship: types.safeReference(FriendshipModel, {
      onInvalidated(event) {
       removeRef(event.parent) //<---------------------------- SEE HERE
      },
    }),
    lastCorrespondence: types.frozen<Correspondence>(),
    updatedAt: types.Date,
    sentCount: types.maybe(types.maybeNull(types.number)),
    viewedCount: types.maybe(types.maybeNull(types.number)),
  })
  .actions(withSetPropAction)

If so that feels pretty clean! Honestly though, I have EXTREMELY enjoyed MST with the exception of references. I have loved the views and actions and the whole setup, but references... not so much (types.reference vs. types.safeReference). I may be simple minded, but why would you ever want your app to crash on a reference invalidation. If you don't intenionally wrap your reference or safeReference with types.maybe or types.maybeNull, which tells your model its okay to be undefined or null, in my opinion it should always invalidate the entire parent object instead of crashing and maybe just throw a console error? Maybe the best approach is if I write my own reference logic like so -- I noticed this other API in the docs where you can write your own reference. Would there be something wrong with this?

const FriendshipReference =  types.reference(FriendshipModel, {
        // given an identifier, find the user
        get(identifier /* string */, parent: any /*Store*/) {
            if(parent.friendships.find(u => u.id === identifier) {
                  return parent.friendships.find(u => u.id === identifier) 
           } else {
                  // Trigger custom invalidation logic for parent here from tree here
           }
        },
        // given a user, produce the identifier that should be stored
        set(value /* Friendship*/) {
            return value.id
        }
    })

Could you suggest what that parent invalidation logic would look like in this situation?

One last comment, I obviously enjoy that you can only edit a tree with an action but would it not make sense to remove that requirement within the reference invalidation function? or at least expose the tree actions someone within the onInvalidated logic.

[coolsoftwaretyler]

Yeah! No promises on implementation, so I can't say it would definitely look like that (i.e. removing all children, so maybe we need a few invocations).

I like it! We have definitely been trying to reduce API surface but I think having the ability to remove a reference is pretty useful.

I'll mark this as looking for help/PR. Should be pretty straightforward if you want to try it out, or we can wait for someone else to volunteer (or I can get to it myself eventually).