search

joltup / rn-fetch-blob

A project committed to making file access and data transfer easier, efficient for React Native developers.
MIT License
2.81k stars 770 forks source link

Remove polyfills #357

Open alpha0010 opened 5 years ago

alpha0010 commented 5 years ago

Fixes #183

erennyuksell commented 5 years ago

is this PR tested? should i merge it into https://github.com/Frekansapp/rn-fetch-blob here?

alpha0010 commented 5 years ago

I have only tested the parts used by our apps. So, I cannot confirm if json stream integration works as expected. Also, slicing blobs stored native side (as opposed to in js memory) might act differently; uncertain, as I do not use said feature.

erennyuksell commented 5 years ago

@alpha0010 What do you think about this solution? https://github.com/joltup/rn-fetch-blob/issues/183#issuecomment-450826541

erennyuksell commented 5 years ago

You didn't remove this interface. Can this lines cause problems? https://github.com/joltup/rn-fetch-blob/blob/979810b3d9471bdc001d05ec6ffd089a96b0a882/index.d.ts#L26-L35

alpha0010 commented 5 years ago

You are right; that would cause incorrect checks for typescript users.

alpha0010 commented 5 years ago

@alpha0010 What do you think about this solution? #183 (comment)

Would result in undefined usage. The name of the native module and (recommended name for the) js module are unfortunately the same. However, index.js exports a js object with methods that do not exist on the native module, but are required by the polyfill.

ThaJay commented 4 years ago

Looking good! Is this not ready to use yet?

Traviskn commented 4 years ago

I would definitely love to remove these polyfills if possible, it seems we need more testing to confirm if anything will be broken by removing them

sadafk831 commented 4 years ago

@alpha0010 @Frekansapp @ThaJay @Traviskn will this be the breaking changes, as most of the people using the Firebase still use the fix of global.XMLHttpRequest = RNFetchBlob.polyfill.XMLHttpRequest So once the polyfills are removed, these will break there app

ThaJay commented 4 years ago

@sadafk831 Thank you for communicating the reasoning behind this. There have been multiple solutions proposed in the relevant issue: https://github.com/joltup/rn-fetch-blob/issues/183 but nobody has stepped up to fix it thus far.

alpha0010 commented 4 years ago

That might be okay: https://facebook.github.io/react-native/docs/network.html#using-other-networking-libraries