Reputation: 306
I'm creating a wrapper hooks library around pusher-js
to publish into the wild. For each hooks (i.e. useChannel
, usePresenceChannel
, useTrigger
), I need to keep a reference to the Pusher instance, i.e. new Pusher() stored in a Context Provider. I am allowing third party auth to be passed in so I need to create the Pusher instance on the fly. I'm unsure whether I should be storing this in a useState or useRef.
The eslint-plugin-react-hooks
rules complain with various combinations of useState and useRef to store this. I'm also seeing undesirable side effects when trying to clean up correctly with each. I'm unsure what's considered best practice.
Here's a simplified implementation with the important details. See comments 1. 2. and 3. below for my questions.
// PusherContext.jsx
export const PusherContext = React.createContext();
export function PusherProvider({key, config, ...props}){
// 1. Should I store third party libraries like this?
const clientRef = useRef();
// vs const [client, setClient] = useState();
// when config changes, i.e. config.auth, re-create instance
useEffect(() => {
clientRef.current && clientRef.current.disconnect();
clientRef.current = new Pusher(key, {...config});
}, [clientRef, config]);
return <PusherContext.Provider value={{ client }} {...props} />
}
// useChannel.js
export function useChannel(
channelName,
eventName,
callback,
callbackDeps
){
const { client } = useContext(PusherContext);
const callbackRef = useCallback(callback, callbackDeps);
// 2. Or should I be using state instead?
const [channel, setChannel] = useState();
// vs. const channelRef = useRef();
useEffect(() => {
if(client.current){
const pusherChannel = client.current.subscribe(channelName);
pusherChannel.bind(eventName, callbackRef.current);
setChannel(pusherChannel);
}
// can't cleanup here because callbackRef changes often.
// 3. eslint: Mutable values like 'client.current' aren't valid dependencies because mutating them doesn't re-render the component
}, [client.current, callbackRef])
// cleanup for unbinding the event
// re-bind makes sense with an updated callback ref
useEffect(() => {
channel.unbind(eventName)
}, [client, channel, callbackRef, eventName]);
// cleanup for unsubscribing from the channel
useEffect(() => {
clientRef.unsubscribe(channelName);
}, [client, channelName])
}
Any advice, past examples or patterns are greatly appreciated as I want to nail this one!
Upvotes: 4
Views: 3150
Reputation: 36915
I'd use the ref to hold a new instance of Pusher
as recommended by Dan.
You don't need to clean up initially by doing null check and disconnect (clientRef.current && clientRef.current.disconnect()
) in the inside effect because whenever the useEffect
is run, React will disconnect when you handle it in the return statement.
export function PusherProvider({ key, config, ...props }) {
// 1. Should I store third party libraries like this?
const clientRef = useRef();
// vs const [client, setClient] = useState();
// when config changes, i.e. config.auth, re-create instance
// useEffect(() => {
// clientRef.current && clientRef.current.disconnect();
// clientRef.current = new Pusher(key, { ...config });
// }, [clientRef, config]);
// Create an instance, and disconnect on the next render
// whenever clientRef or config changes.
useEffect(() => {
clientRef.current = new Pusher(key, { ...config });
// React will take care of disconnect on next effect run.
return () => clientRef.current.disconnect();
}, [clientRef, config]);
return <PusherContext.Provider value={{ client }} {...props} />;
}
For the second case, I tried as much to write suggestions in-line below.
The gist is that, un/subscription
are related events, thus should be handled in the same effect (as it was the case for PusherProvider
).
// useChannel.js
export function useChannel(channelName, eventName, callback, callbackDeps) {
const { client } = useContext(PusherContext);
const callbackRef = useCallback(callback, callbackDeps);
// 2. Or should I be using state instead?
// I believe a state makes sense as un/subscription depends on the channel name.
// And it's easier to trigger the effect using deps array below.
const [channel, setChannel] = useState();
useEffect(() => {
// do not run the effect if we don't have the Pusher available.
if (!client.current) return;
const pusherChannel = client.current.subscribe(channelName);
pusherChannel.bind(eventName, callbackRef.current);
setChannel(pusherChannel);
// Co-locate the concern by letting React
// to take care of un/subscription on each channel name changes
return () => client.current.unsubscribe(channelName);
// Added `channelName` in the list as the un/subscription occurs on channel name changes.
}, [client.current, callbackRef, channelName]);
// This.. I am not sure... 🤔
// cleanup for unbinding the event
// re-bind makes sense with an updated callback ref
useEffect(() => {
channel.unbind(eventName);
}, [client, channel, callbackRef, eventName]);
// Moved it up to the first `useEffect` to co-locate the logic
// // cleanup for unsubscribing from the channel
// useEffect(() => {
// clientRef.unsubscribe(channelName);
// }, [client, channelName]);
}
Upvotes: 2