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

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

Open
alexander-at-t opened this issue Jun 19, 2024 · 9 comments
Open

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

alexander-at-t opened this issue Jun 19, 2024 · 9 comments
Labels
feature-request Issue which suggest an idea, enhancement or feature to implement

Comments

@alexander-at-t
Copy link

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:

  • @react-native-community/clipboard published 4 years ago, use expo-clipboard (it works in bare react native without expo too)
  • react-native-svg 13.13.0 is published almost 1 year ago, upgrade to latest version

Is there an alternative?

No response

@alexander-at-t alexander-at-t added the feature-request Issue which suggest an idea, enhancement or feature to implement label Jun 19, 2024
@saghul
Copy link
Member

saghul commented Jun 19, 2024

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
Copy link
Author

@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/[email protected]

@saghul
Copy link
Member

saghul commented Jun 25, 2024

Is that the only one?

@alexander-at-t
Copy link
Author

@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: #14441 (comment)
On basis of that, I've created a patch file: react-native-webrtc+118.0.6.patch

It works.

@saghul
Copy link
Member

saghul commented Jun 27, 2024

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
Copy link
Author

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
Copy link
Member

saghul commented Jun 27, 2024

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
Copy link
Author

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:

  • peer dependencies incompatibilities
  • exceptions

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
Copy link
Member

saghul commented Jun 27, 2024

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: expo/expo@2a38a11 The first bits of Jitsi Meet RN code were published on October of the same yeah 7f3ff13 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:

  • This repo contains the shared codebase for the web app and the mobile app.
  • The mobile apps are NOT RN apps, they are fully native apps
  • We use RN for a native SDK, which the apps consume
  • The native SDKs are also wrapped with some Dart for a Flutter SDK
  • The RN SDK is basically exposing the root component our native SDK exposes

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: facebook/react-native#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Issue which suggest an idea, enhancement or feature to implement
Projects
None yet
Development

No branches or pull requests

2 participants