Please consider the following code:
Parent:
const Messages = (props) => {
const [targetUserId, setTargetUserId] = useState(null);
const [currentChat, setCurrentChat] = useState(null);
useEffect(() => {
const { userId } = props;
const initiateChat = async (targetUser) => {
const chatroom = `${
userId < targetUser
? `${userId}_${targetUser}`
: `${targetUser}_${userId}`
}`;
const chatsRef = doc(database, 'chats', chatroom);
const docSnap = await getDoc(chatsRef);
if (docSnap.exists()) {
setCurrentChat(chatroom);
} else {
await setDoc(chatsRef, { empty: true });
}
};
if (props.location.targetUser) {
initiateChat(props.location.targetUser.userId);
setTargetUserId(props.location.targetUser.userId);
}
}, [props]);
return (
...
<Chat currentChat={currentChat} />
...
);
};
Child:
const Chat = (props) => {
const {currentChat} = props;
useEffect(() => {
const unsubscribeFromChat = () => {
try {
onSnapshot(
collection(database, 'chats', currentChat, 'messages'),
(snapshot) => {
// ... //
}
);
} catch (error) {
console.log(error);
}
};
return () => {
unsubscribeFromChat();
};
}, []);
...
The issue I'm dealing with is that Child's UseEffect clean up function, which depends on the chatroom prop passed from its parent, throws a TypeError error because apparently chatroom is null. Namely, it becomes null when the parent component unmounts, the component works just fine while it's mounted and props are recognized properly.
I've tried different approaches to fix this. The only way I could make this work if when I moved child component's useEffect into the parent component and defined currentChat using useRef() which honestly isn't ideal.
Why is this happening? Shouldn't useEffect clean-up function depend on previous state? Is there a proper way to fix this?
currentChat is a dependency of that effect. If it's null, the the unsubscribe should just early return.
const {currentChat} = props;
useEffect(() => {
const unsubscribeFromChat = () => {
if(!currentChat) return;
try {
onSnapshot(
collection(database, 'chats', currentChat, 'messages'),
(snapshot) => {
// ... //
}
);
} catch (error) {
console.log(error);
}
};
return () => {
unsubscribeFromChat();
};
}, [currentChat]);
But that doesn't smell like the best solution. I think you should handle all the subscribing/unsubscribing in the same component. You shouldn't subscribe in the parent and then unsubscribe in the child.
EDIT:
Ah, there's a bunch of stuff going on here that's not good. You've got your userId coming in from props - props.location.targetUser.userId and then you're setting it as state. It's NOT state, it's only a prop. State is something a component owns, some data that a component has created, some data that emanates from that component, that component is it's source of truth (you get the idea). If your component didn't create it (like userId which is coming in on props via the location.targetUser object) then it's not state. Trying to keep the prop in sync with state and worry about all the edge cases is a fruitless exercise. It's just not state.
Also, it's a codesmell to have [props] as a dependency of an effect. You should split out the pieces of props that that effect actually needs to detect changes in and put them in the dependency array individually.