fix typings for issue 690 #691
Conversation
| * in response to calling the OTPublisher.getRtcStatsReport() method. | ||
| */ | ||
| rtcStatsReport?: CallbackWithParam<PublisherRtcStatsReport[]>, any>; | ||
| rtcStatsReport?: CallbackWithParam<PublisherRtcStatsReport[], any>; |
There was a problem hiding this comment.
This is a required fix
| @@ -420,7 +413,14 @@ declare module "opentok-react-native" { | |||
| /** | |||
| * https://github.com/opentok/opentok-react-native/blob/master/docs/OTPublisher.md | |||
| */ | |||
| export class OTPublisher extends React.Component<OTPublisherProps> {} | |||
| export class OTPublisher extends React.Component<OTPublisherProps, unknown> { | |||
There was a problem hiding this comment.
React.Component requires two type arguments: <Props, State>
I did not want to dive too deep in what should be declared for State; thus unknown is a good start.
| * in response to calling the OTSubscriber.getRtcStatsReport() method. | ||
| */ | ||
| rtcStatsReport?: CallbackWithParam<RtcStatsReport>, any>; | ||
| rtcStatsReport?: CallbackWithParam<RtcStatsReport, any>; |
There was a problem hiding this comment.
this is a required fix
| @@ -551,9 +544,16 @@ declare module "opentok-react-native" { | |||
| /** | |||
| * https://github.com/opentok/opentok-react-native/blob/main/docs/OTSubscriber.md#custom-rendering-of-streams | |||
| */ | |||
| export class OTSubscriberView extends React.Component<OTSubscriberViewProps> {} | |||
| export class OTSubscriberView extends React.Component<OTSubscriberViewProps, unknown> {} | |||
There was a problem hiding this comment.
same:
React.Component requires two type arguments: <Props, State>
I did not want to dive too deep in what should be declared for State; thus unknown is a good start.
There was a problem hiding this comment.
Thanks for the PR @MartinHarkins . I added a commit to rev the version and add a note to the change log. I am asking one other team member here to review this, and then we'll merge it in and ship it.
Contributing checklist
Solves issue(s)
Fixes: #690
The typings file was broken. Essentially the basic fixes needed are along those lines:
Another issue was with the recent typings added on
OTSessionProps,OTSubscriberPropsandOTPublisherPropsSome methods were added onto the JS component, like
OTSession#getCapabilities...However, they are not in fact
props.The issue with those typings is that they force consumers of those three components, to fill in those props; despite the fact that they cannot be used anywhere in the component implementations.
The only reason they could be used was if a developer subclassed for example
OTSession, and then called those props themselves.Those methods are a part of the components, but using them actually requires subclassing said components. There is no practical way to use them otherwise.
Additionally;
OTSession,OTPublisherandOTSubscribershould probably have those functions removed from the declaredpropTypesanddefaultProps.I can do that but I need confirmation before I perform that change.
In anycase, if possible this PR could be merged and published quickly in order to fix the typings which are completely broken at the moment.