opentok-react-native icon indicating copy to clipboard operation
opentok-react-native copied to clipboard

OTSession component doesn't update eventHandlers prop natively

Open impe93 opened this issue 4 years ago • 5 comments

Bug Report

Current behavior

In the component OTSession the prop eventHandlers is updated only during the component init as proved here and not when prop update. This can lead to some problems.

In my case, I do some specific logic based on the subscriber streamId that I save in the component state after the streamCreated event has triggered. The problem is that when I update the streamId the callback passed to the event object field streamPropertyChanged does not update natively

Steps to reproduce

  • Try to change functions references that are assigned to the OTSession eventHandlers prop fileds
  • The reference used by OTSession of those functions never changes

Example Project

This should be a working example, the streamPropertyChanged in sessionEventHandlers passed to the OTSession component never change his function reference because under the hood in OTSession the prop is never updated

import React, { useState, useMemo, useCallback } from 'react';
import { View } from 'react-native';
import {
  OTSession,
  OTPublisher,
  OTSubscriber,
  StreamCreatedEvent,
  OTSessionEventHandlers,
  Stream
} from 'opentok-react-native';

type StreamEvent = {
  stream: Stream
}

export const App = () => {
  const someApiKey = useMemo(() => 'someApiKey', [])
  const someSessionId = useMemo(() => 'someSessionId', [])
  const someToken = useMemo(() => 'someToken', [])

  const [streamIdToObserve, setStreamIdToObserve] = useState<string | undefined>()
  const [hasSpecialistVideo, setHasSpecialistVideo] = useState(true)
  const [hasSpecialistAudio, setHasSpecialistAudio] = useState(true)
  const [aspectRatio, setAspectRation] = useState<number>(1)

  const updateStreamsProps = useCallback(
    ({ stream }: StreamEvent) => {
      console.log('Stream Updated', stream.streamId)
      console.log('Stream to Observe', streamIdToObserve)
      const { width, height, hasAudio, hasVideo, streamId } = stream
      if (streamId === streamIdToObserve) {
        const streamAspectRatio = width / height
        setAspectRation(streamAspectRatio)
        setHasSpecialistAudio(hasAudio)
        setHasSpecialistVideo(hasVideo)
      }
    },
    [streamIdToObserve]
  )

  const onStreamCreated = useCallback((createEvent: StreamCreatedEvent) => {
    console.log('Stream Created', createEvent.streamId)
    setStreamIdToObserve(createEvent.streamId)
    const { width, height, hasAudio, hasVideo } = createEvent
    const streamAspectRatio = width / height

    setAspectRation(streamAspectRatio)
    setHasSpecialistAudio(hasAudio)
    setHasSpecialistVideo(hasVideo)
  }, [])

  // This object is never updated natively because OTSession never update eventHandlers when updated
  const sessionEventHandlers: OTSessionEventHandlers = useMemo(() => {
    console.log('Update session events handlers')
    return {
      streamCreated: onStreamCreated,
      streamPropertyChanged: updateStreamsProps,
    }
  }, [
    onStreamCreated,
    updateStreamsProps
  ])
   
  return (
    <View style={{ flex: 1, flexDirection: 'column', paddingHorizontal: 100, paddingVertical: 50 }}>
      <OTSession eventHandlers={sessionEventHandlers} apiKey={someApiKey} sessionId={someSessionId} token={someToken}>
        <OTPublisher style={{ width: 200, height: 200 }} />
        <OTSubscriber style={{ width: 200, height: 200 }} />
      </OTSession>
    </View>
  );
}

What is the current bug behavior?

OTSession component is not update the eventHandlers prop natively but only on component init

What is the expected correct behavior?

OTSession component update the eventHandlers prop natively every time the prop reference change

impe93 avatar Aug 12 '21 13:08 impe93

Hi @impe93 , I believe I didn't fully understand this issue. Where are you updating the eventHandlers prop in the example?

enricop89 avatar Feb 03 '22 14:02 enricop89

Hi @enricop89 ! sessionEventHandlers is updated everytime onStreamCreated or updateStreamsProps have new references. So eventHandlers has a new reference every time sessionEventHandlers update (when useMemo is triggered). As I write in the issue, I've noticed that internally doesn't update the reference to the object passed to eventHandlers, but only updates the first time.

EDIT: I don't work anymore with opentok so I can't confirm that is still a problem in some newer versions!

impe93 avatar Feb 04 '22 23:02 impe93

Thanks, I still don't understand the use case and why you want to update the reference to functions (onStreamCreated or updateStreamsProps). Can't you just use the following (without passing a function basically):

 const sessionEventHandlers: OTSessionEventHandlers = {
      streamCreated: onStreamCreated,
      streamPropertyChanged: updateStreamsProps,
    }

enricop89 avatar Feb 09 '22 11:02 enricop89

Well, useMemo doesn't create a function but returns a value and always returns the same object reference if dependencies (second parameters list) don't update. So is the same thing that you write in your comment but is a bit optimized, you probably want to read more about useMemo.

The problem here is that if I want (and before I wanted to do that) to update the streamCreated and streamPropertyChanged functions I can't do it because setNativeEvents (internally in OTSession) is called only in the constructor (component initialization) and not during the component props update

impe93 avatar Feb 09 '22 15:02 impe93

@impe93 thanks for the explanation, I will tag this issue as "feature request" 😀 . Thanks again!

enricop89 avatar Feb 10 '22 11:02 enricop89