Optimistic Updates Docs Are Flawed??

[rochajulian]

Hi guys! I have a question regarding optimistic updates, particularly the examples found in the docs. I'll paste them below to explain my question:

[Updating a list of todos when adding a new todo](https://tanstack.com/query/latest/docs/framework/react/guides/optimistic-updates#updating-a-list-of-todos-when-adding-a-new-todo)

const queryClient = useQueryClient()

useMutation({
  mutationFn: updateTodo,
  // When mutate is called:
  onMutate: async (newTodo) => {
    // Cancel any outgoing refetches
    // (so they don't overwrite our optimistic update)
    await queryClient.cancelQueries({ queryKey: ['todos'] })

    // Snapshot the previous value
    const previousTodos = queryClient.getQueryData(['todos'])

    // Optimistically update to the new value
    queryClient.setQueryData(['todos'], (old) => [...old, newTodo])

    // Return a context object with the snapshotted value
    return { previousTodos }
  },
  // If the mutation fails,
  // use the context returned from onMutate to roll back
  onError: (err, newTodo, context) => {
    queryClient.setQueryData(['todos'], context.previousTodos)
  },
  // Always refetch after error or success:
  onSettled: () => {
    queryClient.invalidateQueries({ queryKey: ['todos'] })
  },
})

[Updating a single todo](https://tanstack.com/query/latest/docs/framework/react/guides/optimistic-updates#updating-a-single-todo)

useMutation({
  mutationFn: updateTodo,
  // When mutate is called:
  onMutate: async (newTodo) => {
    // Cancel any outgoing refetches
    // (so they don't overwrite our optimistic update)
    await queryClient.cancelQueries({ queryKey: ['todos', newTodo.id] })

    // Snapshot the previous value
    const previousTodo = queryClient.getQueryData(['todos', newTodo.id])

    // Optimistically update to the new value
    queryClient.setQueryData(['todos', newTodo.id], newTodo)

    // Return a context with the previous and new todo
    return { previousTodo, newTodo }
  },
  // If the mutation fails, use the context we returned above
  onError: (err, newTodo, context) => {
    queryClient.setQueryData(
      ['todos', context.newTodo.id],
      context.previousTodo,
    )
  },
  // Always refetch after error or success:
  onSettled: (newTodo) => {
    queryClient.invalidateQueries({ queryKey: ['todos', newTodo.id] })
  },
})

My question is: in the update mutation, particularly in it's onSettled, shouldn't we also be invalidating the ['todos'] query?

Suppose that when we fetch all todos, we utilize useQuery and we cache them all under a ['todos'] query key. When we want to optimistically add a new todo, we use the first mutation in the example to add it to the cached ['todos'], all of this makes sense to me...but what about when we want to optimistically update a todo? Well of course we'd use the second mutation in the example to grab the previous value in cache and replace it with the new one, which is all done under a different key, being ['todos', todoID]. Now, when we reach the onSettled we invalidate that query. Cool, but why are we forgetting about the 'parent' query ['todos']?

It seems to me that there's a flaw here where the ['todos', todoID] gets updated fine but ['todos'] will now be out of sync. If I utilize getQueryData(['todos, todoID']) for a todo I just updated, I should see the new data cached. BUT if I utilize getQueryData(['todos']) wouldn't I see that specific todo's old data amongst the rest of the todos since I didn't touch that query key's cache?

My solution: if my observation is valid, my attempt at a solution would be to simply also invalidate the ['todos']. But if I'm going to do that, I am unsure if I should also be optimistically updating the ['lists'] as well? See below for a draft:

useMutation({
  mutationFn: updateTodo,
  // When mutate is called:
  onMutate: async (newTodo) => {
    // Cancel any outgoing refetches
    await queryClient.cancelQueries({ queryKey: ["todos", newTodo.id] });
    await queryClient.cancelQueries({ queryKey: ["todos"] });

    // Snapshot the previous values
    const previousTodo = queryClient.getQueryData(["todos", newTodo.id]);
    const previousTodos = queryClient.getQueryData(["todos"]);

    // Optimistically update the single todo
    queryClient.setQueryData(["todos", newTodo.id], newTodo);

    // Optimistically update the todos array
    queryClient.setQueryData(["todos"], (oldTodos) =>
      oldTodos.map((todo) => (todo.id === newTodo.id ? newTodo : todo))
    );

    // Return a context with the previous todo and previous todos
    return { previousTodo, previousTodos };
  },
  // If the mutation fails, use the context we returned above
  onError: (err, newTodo, context) => {
    // Roll back single todo
    queryClient.setQueryData(["todos", newTodo.id], context.previousTodo);

    // Roll back the todos array
    queryClient.setQueryData(["todos"], context.previousTodos);
  },
  // Always refetch after error or success:
  onSettled: (newTodo) => {
    queryClient.invalidateQueries({ queryKey: ["todos", newTodo.id] });
    queryClient.invalidateQueries({ queryKey: ["todos"] });
  }
});

As you can see, now both the ['todos', todoID] and ['todos'] caches get optimistically updated and then invalidated. From my understanding, this approach now keeps BOTH in sync with each other. If I utilize getQueryData(['todos, todoID']) for a todo I just updated, I should see the new data cached. AND if I utilize getQueryData(['todos']) I should ALSO see that specific todo's new data amongst the rest of the todos. But now this begins to seem more complex than it probably should be. With my approach I am left wondering why to even use the ["todos", todoID] if its going to add this extra step. Why not just use ['todos'] for all CRUD operations?

I hope this has been detailed enough. Though I feel as though I have a good understanding of the example found in the docs, I am left confused for this case. I wonder why it isn't documented and I am left wondering if perhaps its me that's overthinking this. I would appreciate any help in clarifying why or why not my approach is correct, it would mean a lot.

[TkDodo]

in the update mutation, particularly in it's onSettled, shouldn't we also be invalidating the ['todos'] query?

sure, you can do that.