Optimistic Update Undo Proposal (or Suggestions)

[Aryk]

Sometimes, when I do a mutation, I'm adding records to collections that are not stored in a mobx map. Sometimes, I simply append the items optimistics to an array in the component state.

If there is an error though, I need to undo this....Optimistic update will undo instances added to the MST before I have a chance to remove it from my arrays in the state...and there is no way for me to hook into this to do something before mobx undoes itself.

Currently:

  function mutate(mutation, variables, optimisticUpdate) {
    if (optimisticUpdate) {
      var recorder = recordPatches(self);
      optimisticUpdate();
      recorder.stop();
      var q = query(mutation, variables, {
        fetchPolicy: "network-only"
      });
      q.currentPromise().catch(function () {
        recorder.undo();
      });
      return q;
    } else {
      return query(mutation, variables, {
        fetchPolicy: "network-only"
      });
    }
  } // N.b: the T is ignored, but it does simplify code generation

My proposal:

  function mutate(mutation, variables, optimisticUpdate) {
    if (optimisticUpdate) {
      var recorder = recordPatches(self);
      var undo = optimisticUpdate();
      recorder.stop();
      var q = query(mutation, variables, {
        fetchPolicy: "network-only"
      });
      q.currentPromise().catch(function () {
        if (typeof(undo) === "function") undo();
        recorder.undo();
      });
      return q;
    } else {
      return query(mutation, variables, {
        fetchPolicy: "network-only"
      });
    }
  } // N.b: the T is ignored, but it does simplify code generation

[Aryk]

hey @chrisdrackett, curious what you think about this? Without this, it's very hard to use React's local state to keep any items from mobx.

[chrisdrackett]

Personally when using mobx I do all I can to keep all my application state in mobx and use hooks only for UI stuff that is too light or local to warrant putting into mobx. I'm curious as to what the use case is to not use mobx here as with mst-gql its all fairly automatic and "free".

Anyways, I'm not against this proposal specifically I think ideally this is optional and the current syntax keeps working. Not sure if others have thoughts!

[Aryk]

Well, thats the thing. Say you are on a page and you are doing infinite scroll...you make a call, get 10 items and put them into local state (which is what I'm doing). Seems like super overkill to be putting that into mobx-state-tree for being used simply on one screen. However, say I need to delete an item from that list but the call fails. If I'm soft deleting, then I could switch a flag, but most of the time I'm wiping the data clean from view and backend so I need to pop that item out of the array.

Ideally in your optimistic update function, you can return a function that will undo things not in mobx-state-tree.

Or would you simply put infinite scroll on one page in mobx state tree? Curious how others do it. Seems like a very common thing to be needed for people that append items returned from the api into local state on the component for the purposes of infinite scroll. I'm pretty sure most mobile applications use an infinite scroll approach as well and need deletable items.

So I guess, my main point is that a) This is common, especially for mobile apps that do not query all items as once and use an intermediary place to keep collections (like local component state) and b) the change required seems like high ROI...2 lines for the ability to undo local state and any other cleanup necessary.

[chrisdrackett]

We put everything in local state, but I'm not going to assume that is nessasarly standard. ^_^

I agree that this seems like a nice thing to have, so I would be happy to review a PR. I do think for something like this we need to make sure to add it to one of the examples and possibliy our tests as well.

[Aryk]

Makes sense, I will see if I can cook something up but I'm in the middle of an app launch across iOS and Android so I have a long list of bugs to fix.