localTracks can be captured in stale state when calling connect

[shaibt]

Describe the bug The useRoom (src/components/VideoProvider/useRoom/useRoom.tsx) hook component uses a React ref (useRef) for storing the state of the connectoptions object. According to the inline comments:

useEffect(() => {
// This allows the connect function to always access the most recent version of the options object. This allows us to
// reliably use the connect function at any time.
optionsRef.current = options;
  }, [options]);

localTracks prop does not get the same treatment so potentially, although they are used as a dependency for the connect function, the localTracks state can be captured by a consuming function that does not re-render every time connect is updated.

const connect = useCallback(
token => {
  setIsConnecting(true);
  return Video.connect(token, { ...optionsRef.current, tracks: localTracks }).then(
    ...
  );
},
[localTracks, onError]
  );

IMHO, It would be safer to also store the localTracks state in the options ref so connect is indeed reliable to use at any time. Something like:

useEffect(() => {
// This allows the connect function to always access the most recent version of the options object. This allows us to
// reliably use the connect function at any time.
optionsRef.current =  { ...optionsRef.current, tracks: localTracks };
}, [options, localTracks]);

and then:

return Video.connect(token, optionsRef.current).then(

[timmydoza]

Thanks for the interesting post @shaibt!

I think you raise a very good point. I'm not sure why the localTracks array doesn't get the same treatment as the options object here. It makes sense that it could be a ref too. I'm in the middle of building some new features for these apps, so I might not get around to this one for a bit, but I'll definitely give it some thought.

Thanks for sharing!