Bug in React Intermediate course - Optimistic Updates(chapter 18, 19)

Exact bug in useMutation:onSuccess event below:

onSuccess: (savedTodo, newTodo) => {
queryClient.setQueryData<Todo>(CACHE_KEY_TODOS, (todos) =>
todos?.map((todo) =>
todo === newTodo ? savedTodo : todo

Here in your code todo === newTodo not seems matching so its not updating with savedTodo. So when we add more than one item it shows warning “Warning: Encountered two children with the same key, 0. Keys should be unique so that components maintain their identity across updates. Non-unique”. Please can you tell me why ?

1 Like

I agree. There is a problem with this line:
todos?.map((todo) =>
todo === newTodo ? savedTodo : todo)
since both todo and newTodo are objects, it totally can’t use strict equality (===) to compare them

I hope Mosh will check it and fix this issue soon


I just fixed the above issue like that:

todos?.map((todo) =>
todo.id === 0 ? savedTodo : todo)

the replacing based on the id: 0 of the newTodo we added on onMutate func

1 Like

It’s not really an issue because the jsonplaceholder API is a fake API and returns the same ID for every POSTed user. Since every single todo we get back from the server comes back with the same ID and we are using that ID as the key for each item in the list we get that duplicate warning. A real API wouldn’t have this behavior and we wouldn’t see this warning.

Comparing the objects with === works fine in this scenario. Both objects have the same fields with the same values and both are the same type. Using todo.id === 0 does not solve anything because === works fine in this case and the duplicate warning will still appear because the issue is with the API, not our code.

1 Like

have you tried to compare two objects with strict equality? object is not primitive type, it’s reference type. If you would like two objects are equal, you have to reference the second one to first one
let obj1 = {a: 5}
let obj2 = obj1
result is true with strict equality

Ps: I have checked the data in fakeAPI has no any object starts with id = 0 instead of start with id = 1, that’s why we don’t have to worry duplicate data later

yes, sure, the fakeAPI always returns an obj id 201, but the main idea here is we need to find the item we add to UI first and change it back to the fetched item. And I agree with you that no guarantee APIs don’t have item id 0 in the future. But now I just stick with it for temporary

to solve the duplicate key warning, I coded as this - the key combines index and id 201:
{todos?.map((todo, index) => (
<li key={${index}${todo.id}} className=“list-group-item”>



Looks like you are correct, === will check for reference equality. I double checked the comparison there for the todos and it returns false, which means the list of todos does not get updated with what we get back from the server.

While checking for an ID of 0 works in this case, there’s no guarantee other APIs won’t have another item with an ID of 0 already. Looks like the easiest solution in this case is to stringify() the objects and then compare them as strings. Writing out a deep equals function seems like the most fool-proof way of doing it but it requires a bit of work to setup.

1 Like

That still doesn’t solve the duplicate warning but I still believe that the warning is a non-issue. Every POST to the fake API returns an object with an id of 201 so every new item will have the same, duplicate, ID.

1 Like

Thank you very much for all of your response!
I know that json api will only return id 201 on add and i am not saying about that object and duplicate warning it raising.

@s2tomhum, yes todo.id===0 check can be used if === cannot be used!

For those who dont understand my issue, please see more details:
At first onMutate we are adding newTodo object in the todos list as below(as its optimistic update):
queryClient.setQueryData<Todo>(CACHE_KEY_TODOS, (todos) => [
…(todos || ),

Then onSuccess we are getting 2 objects as arguments ie.savedTodo, newTodo
onSuccess: (savedTodo, newTodo) => {
queryClient.setQueryData(CACHE_KEY_TODOS, (todos) =>
todos?.map((todo) =>
todo === newTodo ? savedTodo : todo)

newTodo object here is the same object that we are added with setQueryData(I have tested with some code and made sure). Then here, when we check for equality in the todos list it must have the same referentially equal object in the todos list. But its not matching any todo with this newTodo. So I think setQueryData may be again destructuring our object. Can anyone confirm?

Anyway thank you all for your help!

1 Like

I just helped you to solve that issue. I met it as you. You can’t compare todo === newTodo, since both are objects, so if you try to execute that expression, the map method always returns the new array of todo objects, not including savedTodo object.

And the solution I used in that map method is compare todo.id and 0 to get new array including savedTodo object.

@s2tomhum is correct. The equality check for the todo objects fails because it is not comparing the fields in the object. They might look the exact same, have the same ID, etc. but unless they happen to be the same object in memory, aka same reference value, then they will never be the same. That’s why @s2tomhum used id === 0, and I suggested using stringify() or creating a deep comparison function. Both methods will work to compare the objects in the way that is needed to update the list correctly.

That’s why it never matches any todo in the list and therefore never updates the todo with the savedTodo.

I wasted about an hour on this tonight trying to figure out what I’d done wrong. Thought I was going crazy until I checked the useMutation documentation and discovered it returns a new object. Was about to start a new thread here, good thing I checked first and found this one.

I’ve done enough of Mosh’s courses by now to know that errors like this are rare. This one should definitely be fixed.

1 Like

Bug is still present. @Mosh could you fix this? It is quite serious mistake.