Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTSession component doesn't update eventHandlers prop natively #536

Open
impe93 opened this issue Aug 12, 2021 · 5 comments
Open

OTSession component doesn't update eventHandlers prop natively #536

impe93 opened this issue Aug 12, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@impe93
Copy link

impe93 commented Aug 12, 2021

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

@enricop89
Copy link
Contributor

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

@impe93
Copy link
Author

impe93 commented Feb 4, 2022

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!

@enricop89
Copy link
Contributor

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,
    }

@impe93
Copy link
Author

impe93 commented Feb 9, 2022

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

@enricop89
Copy link
Contributor

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

@enricop89 enricop89 added the enhancement New feature or request label Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants