search

itinance / react-native-fs

Native filesystem access for react-native
MIT License
4.96k stars 983 forks source link

New version requires a minimum of API 24 #606

Closed anshul-kai closed 5 years ago

anshul-kai commented 5 years ago

getContentLengthLong requires API 24. API 16 (Android 6.0) is still the most dominant OS by market share. I think we need to revert the recent changes to Downloader.java and use an Integer content length.

Happy to provide a PR if needed. Just a 2 line change.

zizzle0 commented 5 years ago

Thanks for pointing out, but latest statistics shows a different picture: https://de.statista.com/statistik/daten/studie/180113/umfrage/anteil-der-verschiedenen-android-versionen-auf-geraeten-mit-android-os/

Android 6: 21 %, Android 7: 28 % Android 9: 8%

For backward com compatibility we should keep with older versions of RN, weil the most recent still is more open to the "modern world". What do you and others think about? Discussion is open :)

itinance commented 5 years ago

If there were no urgent updates happened in latest releases i would go with @zizzle0 as we already did with other critical version upgrades of RN itself.

v2.12 for those that want to be compatible with older Android 6.0, and all newer for the others. What do you think, @a-koka?

anshul-kai commented 5 years ago

Thanks for bringing this to my attention @zizzle0. Glad to know that Android 6.0 is no longer the market dominator but a 20% market share is still not something we can ignore. React native also sets minSdkVersion to 16 which is what most libraries support from what I've noticed.

An Integer contentLength could still download a 2 GB file which seems pretty large to me but perhaps I'm overlooking some use cases. We can also modify my PR to use the newer getContentLengthLong on devices it's available on.

Is there a reason why we're debating merging this PR? Or are we debating releasing the change? I'm just a little confused regarding the proposal.

itinance commented 5 years ago

We are debating about how we should come to a consensus in your current Issue but without to complete ignore https://github.com/itinance/react-native-fs/pull/583, which solved an IntegerOverflow-Issue that happens on large files > 2 GB.

I confirm that we can't ignore Android 6-users. And the use case with large files might be happen in only a few apps.

I think the best approach would be to use the getContentLengthLong if it is available and otherwise fall back to the former version.

anshul-kai commented 5 years ago

Excellent! I'll update the PR with that suggestion.

itinance commented 5 years ago

New release is out, supporting both Android 6 and also integer-overflow-fixed variant on Android 7, thx to @a-koka