Let's start of by why things are going wrong. The issue is that when you call onNewItem(newItem) you are using an outdated reference to the onNewItem. For this reason items within the function will still be set to the initial value.
You partially solved this by providing an dependency array to React.useCallback. This will update onNewItem when a new value of items is available. However since React.useEffect does not list onNewItem as a dependency it keeps using the old version of onNewItem.
With this being said you might consider adding onNewItem, to the dependency array of React.useEffect. Although this is the correct action, just adding this to dependency array is not enough.
What is the problem you get when you add onNewItem to the depency array of React.useEffect? There is no cleanup function, so you will subscribe to the channel multiple times with different onmessage handlers (different versions of onNewItem).
So taking all the above into account a solution might look something like this:
function TodoListCard() {
const [items, setItems] = React.useState(null);
const [events, setEvents] = React.useState(null);
React.useEffect(() => {
const pEvents = fetch('/items')
.then(r => r.json())
.then(setItems)
.then(() => new EventSource('/events/subscribe'));
pEvents.then(setEvents);
return () => pEvents.then(events => events.close());
}, []);
React.useEffect(() => {
if (!events) return;
events.onmessage = (event) => {
const parsedData = JSON.parse(event.data);
switch (parsedData.type) {
case "add":
var newItem = {
id: parsedData.id,
name: parsedData.name,
completed: parsedData.completed
};
onNewItem(newItem);
break;
default:
break;
}
};
}, [events, onNewItem]);
const onNewItem = React.useCallback(newItem => {
const isPresent = items.some(item => item.id === newItem.id);
if (isPresent) return;
setItems([...items, newItem]);
}, [items]);
return (
// ...
);
}
I've moved the EventSource creation inside the first React.useEffect since that only needs to happen once the component is mounted (and needs to close the connection on unmount). An empty dependency array will only call the function on mount, and calls the cleanup function on unmount.
The second React.useEffect now has the dependency array [events, onNewItem], because when events is set the onmessage handler needs to be attached. And if the onNewItem callback updates to a new version you should attach it as the new onmessage handler (replacing the old handler). This doesn't need a cleanup function anymore since, opening and closing events is already handled.
Although the above should do the job. If managing a specific state is becoming more complicated it might be better to opt for useReducer instead of useState.
function reducer(items, action) {
switch (action.type) {
case "add":
const isPresent = items.some(item => item.id == action.item.id);
if (isPresent) return items;
return [...items, action.item];
case "replace all":
return action.items;
case "complete": // <- unused example case
return items.map(item => {
if (item.id != action.id) return item;
return {...item, completed: true};
});
// ...
default: // silently ignore unsupported operations
return items;
}
}
function TodoListCard() {
const [items, dispatch] = React.useReducer(reducer, null);
React.useEffect(() => {
const pEvents = fetch('/items')
.then(r => r.json())
.then(items => dispatch({type: "replace all", items}))
.then(() => new EventSource('/events/subscribe'));
pEvents.then(events => {
events.onmessage = (event) => {
const {type, ...item} = JSON.parse(event.data);
dispatch({type, item});
};
});
return () => pEvents.then(events => events.close());
}, []);
// if you still need onNewItem for your render:
const onNewItem = React.useCallback(item => {
dispatch({type: "add", item});
}, []);
return (
// ...
);
}
The above extracts all the items management logic into a "reducer" function. The dispatch function returned by useReducer is guaranteed to be stable by React, so you can omit it from dependency arrays (but you don't have to).
