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

TypeError: _this.isCollectingLocalStats is not a function (it is undefined) #2489

Closed
Romick2005 opened this issue Mar 26, 2024 · 18 comments
Closed

Comments

@Romick2005
Copy link
Contributor

After jitsiConnectionEstablished I tried to call JitsiMeetJS.createLocalTracks({ devices: ['audio'] });
But got an error: "error": [TypeError: _this.isCollectingLocalStats is not a function (it is undefined)]

I am using latest [email protected]

Romick2005 added a commit to Romick2005/lib-jitsi-meet that referenced this issue Mar 26, 2024
…eetJS.isCollectingLocalStats as deprecated with warning to use Statistics.isCollectingLocalStats instead.
@damencho
Copy link
Member

Can you reproduce the same error on alpha.jitsi.net?

@saghul
Copy link
Member

saghul commented Mar 26, 2024

I am using latest [email protected]

Are you using our SDKs or using this library directly?

@Romick2005
Copy link
Contributor Author

Using raw git code pointing main to the index.js. Directly.

@saghul
Copy link
Member

saghul commented Mar 26, 2024

Did you initialize the library by calling JitsiMeetJS.init ?

@Romick2005
Copy link
Contributor Author

Did you initialize the library by calling JitsiMeetJS.init ?

Yes, sir. Only one time on project initial setup.
lib-jitsi-meet version is git reset --hard 006457f.

JitsiMeetJS.init({
  disableAudioLevels: false,
  useIPv6: false,
  disableThirdPartyRequests: true,
});

@Romick2005
Copy link
Contributor Author

Romick2005 commented Mar 26, 2024

Can you reproduce the same error on alpha.jitsi.net?

Sorry, but I am not sure how to check it there as I using react-native mobile app together with latest lib-jitsi-meet code.

@damencho
Copy link
Member

Can you reproduce the same error on alpha.jitsi.net?

Sorry, but I am not sure how to check it there as I using react-native mobile app together with latest lib-jitsi-meet code.

Yeah, I was wondering how is possible jitsi-meet web and mobile already using this and works without a problem, that's why I was asking about some test and I did not see the react-native line.

@Romick2005
Copy link
Contributor Author

More info.
It is createLocalTracks from JitsiMeetJS.ts with extra console.log:
image

This is my code and how it is used:
image

And this is output from console:
image

@Romick2005
Copy link
Contributor Author

PS Could it be related with generator creating some different this for an object call? Because if I use it like regular promise it is working )
Working code sample:

image

@saghul
Copy link
Member

saghul commented Mar 26, 2024

Any chance you are calling that before calling JitsiMeetJS.init?

@Romick2005
Copy link
Contributor Author

Romick2005 commented Mar 26, 2024

Chances is 0%. It is calling after JitsiMeetJS.init - 100%. And as you can see JitsiMeetJS.isCollectingLocalStats is defined before calling JitsiMeetJS.createLocalTracks inside saga generators:
yield call(JitsiMeetJS.createLocalTracks, { devices: ['audio'] });
where call is from
import { call } from 'redux-saga/effects';
and then error comes:
TypeError: _this.isCollectingLocalStats is not a function (it is undefined)

@saghul
Copy link
Member

saghul commented Mar 26, 2024

I wonder if call is changint the this bidning?

@Romick2005
Copy link
Contributor Author

So it is better to have arrow function or better go with my fix.
Please check it and merge if possible.

Thank you!

@Romick2005
Copy link
Contributor Author

I wonder if call is changint the this bidning?

I assume yes. It is changing this context. So to be safe, please merge my no context dependent fix )

Romick2005 added a commit to Romick2005/lib-jitsi-meet that referenced this issue Mar 26, 2024
@saghul
Copy link
Member

saghul commented Mar 27, 2024

Sorry, I won't do that. As I said, your fix doesn't work because Staristics is not part of the public API.

That library is monkeying with this because you are not calling the function directly, as you should.

This I see nothing to fix on our end.

Use an arrow function please.

It's likely you'll run into a myriad of problems, even if you sidestepped this one.

@saghul saghul closed this as not planned Won't fix, can't repro, duplicate, stale Mar 27, 2024
@Romick2005
Copy link
Contributor Author

Romick2005 commented Mar 27, 2024

I think you are wrong here as my fix makes things more clear.

  1. You do not have to import whole LocalStatsCollector into JitsiMeetJS just for calling isLocalStatsSupported only one. It is better to have isCollectingLocalStats inside Statistics as it is related to this and it is more proper place for this method and Statistics file already have imported LocalStatsCollector as LocalStatsCollector. So it is double win win.
  2. This fix will make method isCollectingLocalStats not dependent on this context as in reality it should not be as have no context base dependency. So having it as static method in Statistics is also win-win.

It is sad that you do not see any improvement in my fix.

PS Can you please ask some alternative opinion on this? Looks like you do not like changes even for good.

@saghul
Copy link
Member

saghul commented Mar 27, 2024

I appreciate you trying to fix things here. That's why I'm spending some time trying to show you why this fix is not correct.

This library is used by tons of customers, you can't just decide "this is better" so I'm deprecating the function.

It's a marginal improvement at best, and definitely not worth the churn.

Last, you keep ignoring the fact that Statistics is not exposed to the library consumers in UMD form, which means the non-deprecated API is not even reachable.

@Romick2005
Copy link
Contributor Author

I think you are wrong again and did not pay attention to my changes. I've removed deprecated warning as you suggested, because it make sense.

So please be more attentive next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants