Hard dependency on react-native

[benkaiser]

Hey @AbelTesfaye,

I see in the last update to this repo, a hard dependency was taken on the 'react-native' package in url/index.js.

This has broken the build where I use react-native-ytdl in several web applications with no hard-dependency on react-native. Is there a possible fix we could consider that does not take a hard dependency on the react-native package itself?

Thanks!

[benkaiser]

In case anyone else runs into this issue and needs a browser-only version, here is my fork: https://github.com/benkaiser/react-native-ytdl

[benkaiser]

@AbelTesfaye I found an issue in how the cache was storing promises for the identity tokens. Looks like we need to make the getOrSet function in cache.js an async function, move that await on the value to the main part of the function, and then only set the value after the awaiting is done (so it isn't a promise anymore).

See my changes: https://github.com/benkaiser/react-native-ytdl/commit/f860f6d90d063d67bcedf2cd0490304c2a375e1b

I also made json page first, since it seemed to more reliably return results instead of the HTML option. Should we instead switch it to a Promise.race, and wait for the first one that succeeds to win? Rather than waiting a bunch of retires on one method before failing to the next?

[AbelTesfaye]

Hey @benkaiser,

I don't think we can avoid the react-native dependency since the URL polyfill will not work on IOS 10 without it plus there are some other dependencies that need it i.e. BlobModule

[AbelTesfaye]

This library doesn't use getOrSet synchronously anywhere, but if it will sometime in the future, then it's going to fail. Below is what I mean by synchronous usage:

const c = new Cache()
const cReturn = c.getOrSet("MY_KEY", () => "test")
console.log('cReturn:', cReturn) // it should return "test" but will instead return a promise object

The bug will be subtle and will be difficult to find later on, might be best not to modify the method in the first place.

[AbelTesfaye]

I think a few versions back, the JSON page was failing for certain videos, hence why the HTML page has more precedence. This might have changed now but i'm not sure. But for react-native-ytdl, I think using the same strategy as node-ytdl-core will be better in the long run since it will require minimal changes to be made when new versions need to be ported.