cugs•9mo ago

Multiple optimistic updates and old data from refetches

Hi all, I'm new to tRPC and React Query and I'm just trying to get my head around what exactly I'm doing wrong when trying to implement optimistic updates. To help illustrate the issue I'm having, I have a list of items that are deletable, and I want to be able to optimistically update the UI when they're deleted, which I have working, except for this one bug... When I delete one item, then delete a second item about 1-2 seconds later (i.e. before the first deletion has a chance to refetch the server data) I notice that the UI updates from the server with the state of only the first deletion, hence the UI optimistically deletes both items, then refetches, which shows one of the deleted items still there, then shortly after it correctly updates again to the correct state after the 2nd refetch. To solve this issue, I thought that I should be using .cancel() to prevent out-of-date data being returned by refetches that came before the latter mutation, but for whatever reason, that isn't working. (Likely cause I'm stoopid 🙃) To better illustrate, I've included a log of the order of execution: (You can see exactly where these console.logs are placed in my code which I'll also include below)

@acme/expo:dev:  LOG  onMutate <--- first deletion via UI
@acme/expo:dev:  LOG  onMutate <--- second deletion via UI
@acme/expo:dev:  LOG  onSettled <--- first deletion is settled
@acme/expo:dev:  LOG  refetch <--- this is the reason for the incorrect state being shown 😭
@acme/expo:dev:  LOG  onSettled <--- second deletion is settled
@acme/expo:dev:  LOG  refetch <--- this updates the UI to the correct state

@acme/expo:dev:  LOG  onMutate <--- first deletion via UI
@acme/expo:dev:  LOG  onMutate <--- second deletion via UI
@acme/expo:dev:  LOG  onSettled <--- first deletion is settled
@acme/expo:dev:  LOG  refetch <--- this is the reason for the incorrect state being shown 😭
@acme/expo:dev:  LOG  onSettled <--- second deletion is settled
@acme/expo:dev:  LOG  refetch <--- this updates the UI to the correct state

I've actually been stuck on this for an embarassingly long time, so any help would be GREATLY appreciated 🙏

4 Replies

cugs•9mo ago

These are the relevant bits of code:

const { isFetching: fetchingExercises, data: exercises = [] } =
  trpc.exercise.all.useQuery(
    { workoutId: workoutId },
    {
      onSettled: () => {
        console.log("refetch");
      },
      enabled: !!workoutId.length,
    },
  );

const deleteExercise = trpc.exercise.delete.useMutation({
  onMutate: async (variables) => {
    console.log("onMutate");
    await trpcContext.exercise.all.cancel();
    const previousExercises =
      trpcContext.exercise.all.getData({ workoutId: workoutId }) || [];
    trpcContext.exercise.all.setData(
      { workoutId: workoutId },
      previousExercises.filter(
        (exercise) => exercise.id !== variables.exerciseId,
      ),
    );
    return { previousExercises };
  },
  onError(_, __, context) {
    if (context?.previousExercises) {
      trpcContext.exercise.all.setData(
        { workoutId: workoutId },
        context.previousExercises,
      );
    }
  },
  async onSettled() {
    console.log("onSettled");
    await trpcContext.exercise.all.cancel(); // This call is not actually necessary afaik, just ruling things out...
    trpcContext.exercise.all.invalidate({ workoutId: workoutId });
  },
});

const { isFetching: fetchingExercises, data: exercises = [] } =
  trpc.exercise.all.useQuery(
    { workoutId: workoutId },
    {
      onSettled: () => {
        console.log("refetch");
      },
      enabled: !!workoutId.length,
    },
  );

const deleteExercise = trpc.exercise.delete.useMutation({
  onMutate: async (variables) => {
    console.log("onMutate");
    await trpcContext.exercise.all.cancel();
    const previousExercises =
      trpcContext.exercise.all.getData({ workoutId: workoutId }) || [];
    trpcContext.exercise.all.setData(
      { workoutId: workoutId },
      previousExercises.filter(
        (exercise) => exercise.id !== variables.exerciseId,
      ),
    );
    return { previousExercises };
  },
  onError(_, __, context) {
    if (context?.previousExercises) {
      trpcContext.exercise.all.setData(
        { workoutId: workoutId },
        context.previousExercises,
      );
    }
  },
  async onSettled() {
    console.log("onSettled");
    await trpcContext.exercise.all.cancel(); // This call is not actually necessary afaik, just ruling things out...
    trpcContext.exercise.all.invalidate({ workoutId: workoutId });
  },
});

ghard1314•9mo ago

When you call invalidate that triggers a refetch from the server which is going to override any optimistic updates you made to your cache like you are seeing. There may be some ways to make this play nicely with your existing optimistic update, but my question is: do you really need to refetch data? Unless your expecting this data to change from another source beside this users interactions, there shouldnt really be a need to get fresh data from the server right? Another idea would be to put the refetch on a debounce that gets renewed anytime another delete action is taken. This way you still get fresh data eventually that should match your local cache, but it wont override many fast changes

cugs•9mo ago

Hey @ghard1314 thanks so much for your help!!! 🙂

When you call invalidate that triggers a refetch from the server which is going to override any optimistic updates you made to your cache like you are seeing.

Makes sense. But my thinking was that calling trpcContext.exercise.all.cancel(); would cancel any previous exercise.all.invalidate() invoked refetches, is this not the point of cancel?

do you really need to refetch data?

Good pickup. I don't need to, you're correct. For this example I only need to handle rolling back on error, and I will likely change this particular code to do nothing in the onSettled callback. But the reason I'm still wanting to get to understand this, is because I'm using this pattern in a few places, and some of these use-cases do need to make use of the onSettled callback in this way. You've helped me understand the crux of the issue: cancel does not cancel refetches invoked via invalidate. This seems strange to me, but it's likely just due to my lack of familiarity/understanding of React Query, if I was to guess. I'm starting to believe this has more to do with me employing anti-patterns in my code rather than not understanding the library 😅 I might have to rethink my approach.

ghard1314•9mo ago

I think you were originally right about cancel stopping refetches from invalidate but thats not really helping you here when you have back to back mutations. Lets look at a quick timeline of whats happening here when you make 2 back to back delete mutations before the first one settles.

onMutate(exercise 1):
  client -> server: delete exercise
  client -> server: cancel all previous "all" queries (none happening right now)
  client -> client: remove deleted exercise from cache // cache is now correct without exercise 1
onMutate(exercise 2):
  client -> server: delete exercise
  client -> server: cancel all previous "all" queries // still none happening here because exercise 1 hasn't settled yet
  client -> client: remove deleted exercise from cache // cache is now correct without exercise 2
onSettled(exercise 1):
  client -> server: cancel all previous "all" queries // still none happening here because exercise 2 hasn't settled yet
  client -> server: get all exercises (invalidate) // this will return exercise list with exercise 2 still in it since second mutation hasnt settled yet <-----
onSettled(exercise 2):
   client -> server: cancel all previous "all" queries // still none happening here because exercise 1 already finished settling
   client -> server: get all exercises (invalidate) // this will return exercise list without exercise 1 or 2

onMutate(exercise 1):
  client -> server: delete exercise
  client -> server: cancel all previous "all" queries (none happening right now)
  client -> client: remove deleted exercise from cache // cache is now correct without exercise 1
onMutate(exercise 2):
  client -> server: delete exercise
  client -> server: cancel all previous "all" queries // still none happening here because exercise 1 hasn't settled yet
  client -> client: remove deleted exercise from cache // cache is now correct without exercise 2
onSettled(exercise 1):
  client -> server: cancel all previous "all" queries // still none happening here because exercise 2 hasn't settled yet
  client -> server: get all exercises (invalidate) // this will return exercise list with exercise 2 still in it since second mutation hasnt settled yet <-----
onSettled(exercise 2):
   client -> server: cancel all previous "all" queries // still none happening here because exercise 1 already finished settling
   client -> server: get all exercises (invalidate) // this will return exercise list without exercise 1 or 2