YouTube.js icon indicating copy to clipboard operation
YouTube.js copied to clipboard

Add support for `react-native` platform

Open nugmanoff opened this issue 1 year ago • 3 comments

Following PR adds support for react-native as a target platform. Thank you for making the library platform-agnostic and including the guide on how to add support for a new one – it helped a ton!

I've added some docs explaining what steps need to be taken on RN side in order for things to work.

There are some rough edges around typing that could easily be fixed. Decided to throw it up here first to get some feedback.

nugmanoff avatar Feb 04 '24 05:02 nugmanoff

Your work is awesome, however I think you should make mmkv optional. Forcing using mmkv make this module unable to be used along with react-native-debugger (since mmkv does not work without jsi). Request response from Innertube always complicate to debug, missing the help from network debugging and a good console viewer like react-native-debugger could be a big deal with us.

monokaijs avatar Feb 23 '24 17:02 monokaijs

hey @monokaijs! thanks for the feedback – appreciate it! I am pretty newbie-ish when it comes to RN, so good to know the limitations of mmkv that you've mentioned; what alternatives would you suggest to use as a backing storage for cache to be used with this Youtube.js?

nugmanoff avatar Feb 24 '24 18:02 nugmanoff

How do you think about customizable cache adapter? Or allow caching customizations? I used to try to use youtubei.js in my RN app, at that time, I used AsyncStorage as cache engine and it worked flawlessly. I think you should make Caching optional since we can implement our own cache mechanism, in this case, I see mmkv is just an option of developer, not the library itself.

monokaijs avatar Feb 25 '24 11:02 monokaijs

@monokaijs You can already pass your own implementation of the ICache interface to YouTube.js' InnerTube.create method, this pull request just provides a default implementation, through the UniversalCache class. YouTube.js won't use any cache unless you pass an instance of a cache, so it won't cause any problems. I think this is fine as it is.

absidue avatar Mar 31 '24 19:03 absidue

I see no need this implementation anymore. With React Native, after adding some polyfills and configure metro and builder to support YouTube.js, you can use this module without any hassle. I've successfully implemented this module into my project with just some polyfills (which are also available in this PR). I think no big difference between ReactNative and browser platform except fetch function (which we do not have to proxy in ReactNative since security headers are modificable).

monokaijs avatar Mar 31 '24 20:03 monokaijs

This PR has been automatically marked as stale because it has not had recent activity. Remove the stale label or comment or this will be closed in 2 days

github-actions[bot] avatar May 31 '24 02:05 github-actions[bot]

Sorry for letting this get stale, I need some opinions on whether this is still needed or not. The last comment in this thread makes me think the browser implementation is enough, but I have no experience with react native to verify that.

I already reviewed this a while ago so I'll merge it once we come to a conclusion.

LuanRT avatar Jun 01 '24 23:06 LuanRT

I think Browser Implementation is enough.

monokaijs avatar Jun 02 '24 05:06 monokaijs

Hey @LuanRT I think it might work with browser implementation directly by passing custom Cache implementation in Innertube.create call. But I believe it is more about the bigger picture of indicating that YouTube.js is supported and can be run on react-native and having an out-of-the box way to do that (i.e. providing default Cache implementation using react native specific libraries and necessary documentation).

I think the last documentation bit on glueing everything together on the React Native runtime side (using polyfills) is the most important thing to consider here. It took me quite some time to figure out how to properly pick and apply polyfill libraries on React Native side and it also includes some configs around bundler and babel. But now basically anyone who is looking for using YouTube.js can just install it as a dependency (and leverage the default Cache impl) and drop the polyfills setup file that is presented in the docs and have everything up and running in a few minutes. Without it one would need to figure out the Cache and polyfills for themselves (and might very well give up on the last one, thinking that it is not compatible with RN runtime at all).

tl;dr I think having this merged would clearly benefit YouTube.js by making the fact that it could be used and run on React Native more explicit and whole set up process more streamlined and drop-in, at the expense of having almost similar to web platform setup file (except for Cache implementation) – so almost no maintenance overhead.

@monokaijs thank you for your thoughts on this one as well! while I do agree with you to some extent, I think the fact that you have discovered my fork because it was opened as PR in this repo and that helped you have YouTube.js running in your RN project only highlights the fact that why it is important to have this explicit support merged

nugmanoff avatar Jun 05 '24 14:06 nugmanoff

Thanks! : ) I'll be merging this now. It should be available in v10 (which is going to be released later today, probably).

LuanRT avatar Jun 09 '24 21:06 LuanRT