search

jitsi / jitsi-meet

Jitsi Meet - Secure, Simple and Scalable Video Conferences that you use as a standalone app or embed in your web application.
https://jitsi.org/meet
Apache License 2.0
23.26k stars 6.75k forks source link

[RNSDK] Upgrade peer dependencies for @jitsi/react-native-sdk #14850

Closed alexander-at-t closed 2 months ago

alexander-at-t commented 5 months ago

What problem are you trying to solve?

Using Jitsi React Native SDK in modern application is blocked, because it has old peer dependencies.

Current peer dependencies of latest (2.2.1) version:

"peerDependencies": {
        "@amplitude/react-native": "2.7.0",
        "@braintree/sanitize-url": "7.0.0",
        "@giphy/react-native-sdk": "2.3.0",
        "@react-native/metro-config": "0.72.9",
        "@react-native-async-storage/async-storage": "1.19.4",
        "@react-native-community/clipboard": "1.5.1",
        "@react-native-community/netinfo": "11.1.0",
        "@react-native-community/slider": "4.4.3",
        "@react-native-google-signin/google-signin": "10.1.0",
        "react-native": "*",
        "react": "*",
        "react-native-background-timer": "2.4.1",
        "react-native-calendar-events": "2.2.0",
        "react-native-default-preference": "1.4.4",
        "react-native-device-info": "10.9.0",
        "react-native-get-random-values": "1.9.0",
        "react-native-gesture-handler": "2.9.0",
        "react-native-immersive-mode": "2.0.1",
        "react-native-keep-awake": "4.0.0",
        "react-native-pager-view": "6.2.0",
        "react-native-paper": "5.10.3",
        "react-native-performance": "5.0.0",
        "react-native-orientation-locker": "1.6.0",
        "react-native-safe-area-context": "4.7.1",
        "react-native-screens": "3.24.0",
        "react-native-sound": "0.11.2",
        "react-native-splash-screen": "3.3.0",
        "react-native-svg": "13.13.0",
        "react-native-video": "6.0.0-alpha.11",
        "react-native-watch-connectivity": "1.1.0",
        "react-native-webrtc": "118.0.6",
        "react-native-webview": "13.5.1",
        "text-encoding": "0.7.0"
    },

What solution would you like to see?

Upgrade dependencies regularly (at least each 4 months).

Some examples of outdated peer dependencies:

Is there an alternative?

No response

saghul commented 5 months ago

Others that having a newer version, is there anything in those deps that you need? Last I checked they work and there are no security problems with them, so we are not in a rush to update.

alexander-at-t commented 4 months ago

@saghul

Others that having a newer version, is there anything in those deps that you need? Last I checked they work and there are no security problems with them, so we are not in a rush to update.

Yes, support of new React Native versions 0.73 and 0.74.

Example why it is not supported currently:

peer react-native@"^0.0.0-0 || 0.60 - 0.72 || 1000.0.0" from @react-native-async-storage/async-storage@1.19.4
saghul commented 4 months ago

Is that the only one?

alexander-at-t commented 4 months ago

@saghul

I've tested with react-native 0.74.2 (currently latest stable release).

Full list of updates:

Maybe you could consider specifying peer dependencies with ^ semver, instead of exact match.

After these updates, app is built successfully, but after enter to the call (probably requires at least one another participant with video enabled), the following exception is thrown:

Exception thrown while executing UI block: -[RTCVideoView setOnClick:]: unrecognized selector sent to instance 0x109d11430

I found a suggested solution here: https://github.com/jitsi/jitsi-meet/issues/14441#issuecomment-2057655553 On basis of that, I've created a patch file: react-native-webrtc+118.0.6.patch

It works.

saghul commented 4 months ago

Full list of updates:

Are those required to run on RN 0.74, or just packages with updates?

Maybe you could consider specifying peer dependencies with ^ semver, instead of exact match.

We've been bitten by that before, never again.

After these updates, app is built successfully, but after enter to the call (probably requires at least one another participant with video enabled), the following exception is thrown:

Exception thrown while executing UI block: -[RTCVideoView setOnClick:]: unrecognized selector sent to instance 0x109d11430

We are the maintainers of react-native-webrtc too, thanks for the heads up, we'll take care of that!

alexander-at-t commented 4 months ago

Are those required to run on RN 0.74, or just packages with updates?

These versions are recommended to work stable with Expo 51 that uses RN 0.74.

npx expo-doctor@latest

https://expo.dev/changelog/2024/05-07-sdk-51#%EF%B8%8F-upgrading-your-app

In first order, I've started manual research, package by package, but it takes too much time. So I've decided to better use some recommended list.

Examples of findings during manual research:

saghul commented 4 months ago

Sorry, but I'm not interested in their recommendations. Does something actually not work? There is the storage library and the declared supported RN versions, but is there something elase?

alexander-at-t commented 4 months ago

Does something actually not work?

I don't understand the question. In my previous messages I described 2 kinds of problems with RN 0.73-0.74:

Also please take into account React Native Releases Support Policy: https://github.com/reactwg/react-native-releases/blob/main/docs/support.md

0.72 will be unsupported soon, so it makes sense to update @jitsi/react-native-sdk to support latest RN versions, to make sure it is useful for modern apps.

There is the storage library and the declared supported RN versions, but is there something elase?

npm blocked it during installation phase, I didn't try to test it. So I cannot answer on your question.

I've already spent several working days to find working set of dependencies with overrides and patches of @jitsi/react-native-sdk.

Sorry, but I'm not interested in their recommendations.

@jitsi/react-native-sdk has a problem of using it in modern applications, because it depends on old react-native version.

Expo is an officially recommended React Native framework (https://reactnative.dev/blog/2024/06/25/use-a-framework-to-build-react-native-apps), so I think they have good experience and authority for React Native world. Maybe you can reconsider usage of their recommendations.

saghul commented 4 months ago

Hey. Sorry for my response earlier, I was rushing and as I read it now, I sound like an asshole. I should've waited until I had the time to give a proper response. Here is goes.

Some back-story, which may help undertand why I hold certain opinions.

Expo (then Exponent) was first published in August 2016: https://github.com/expo/expo/commit/2a38a11e3964a11c884d2cd2581f1c89c6391ef0 The first bits of Jitsi Meet RN code were published on October of the same yeah https://github.com/jitsi/jitsi-meet/commit/7f3ff13c18429e98990888f525b308cc6480a4d9 even though work had been happening previously, on a different repo.

That is, we've seen a lot over the years.

I used to always want to stay current because otherwise RN updates were painful, but updating too hastily was also painful, we encountered many bugs updating. Early adopting Hermes was also a mistake we had to undo, for example.

A bit about the architecture of the code:

So, this is not a "traditional" SDK, in a way it's an "app as an SDK" of sorts, since what the main component exposes really is,is the full app.

This means we have dozens of dependencies, that are key to the functionality of the app.

When not using the RN SDK, the RN library versions don't really matter. If it works, and there are no security bugs, the incentive to update fades somewhat, because the risk of introducing new bugs trumps the new features we may not need.

Now, when you consume our RN SDK in an actual RN app, shenanigans begin :-/ So now let's go back to our previous conversation.

I don't understand the question. In my previous messages I described 2 kinds of problems with RN 0.73-0.74:

* peer dependencies incompatibilities

* exceptions

What I meant here is, that, while some depenencies might be slightly off, I'd be interested in knowing which are causing the trouble, not a list of everything that is not the latest, because as I mentioned earlier, statibility is very important to us.

Looks like updating async-storage and fixing up rn-webrtc might be enough?

Also please take into account React Native Releases Support Policy: https://github.com/reactwg/react-native-releases/blob/main/docs/support.md

0.72 will be unsupported soon, so it makes sense to update @jitsi/react-native-sdk to support latest RN versions, to make sure it is useful for modern apps.

I am aware. The problem here is the same, we might not need the uncertainty of the new features, and I will admit that my frustration with unfixed longstanding bugs certainly plays a part here. Example: https://github.com/facebook/react-native/issues/33686

So these days I became more conservative with updates, because more and more people rely on our apps / SDKs, and they expect stability.

That said, I think we can find some middle ground by updating the blocking dependencies first, thus allowing those who want to use a certain version to do so, while we stay in the one we prefer a release cycle or 2 more.

npm blocked it during installation phase, I didn't try to test it. So I cannot answer on your question.

Gotcha. I'll look into updating those 2 first, and see what'd next.

I've already spent several working days to find working set of dependencies with overrides and patches of @jitsi/react-native-sdk.

Even though we have been using RN for many years, the RN SDK itself is the youngest, apologies for the pain you are enduring.

Expo is an officially recommended React Native framework (https://reactnative.dev/blog/2024/06/25/use-a-framework-to-build-react-native-apps), so I think they have good experience and authority for React Native world. Maybe you can reconsider usage of their recommendations.

I understand Expo is useful for new projects, it's just not a good match for this one, because it has diverged from a "traditional" RN project so much already. I am also biased because over the years, when Expo was not as simple to use with native dependencies the interactions I had over at rn-webrtc gave me negative vibes. Things seem to be better now, which is great! I'm glad RN is simpler to use than it was when we started!

Once again, sorry for the reply earlier. We'll try to fix things to make it work with more modern RN versions.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.