search

kfiroo / react-native-cached-image

CachedImage component for react-native
MIT License
942 stars 467 forks source link

On Maintenance and potential forking #160

Open fungilation opened 5 years ago

fungilation commented 5 years ago

I've been asked to make my fork official but haven't done such before. And I honestly don't want to waste my time, I have full time job(s). I'd do it if it's truly of value to the community and enough people want this repo to stay alive and updated (alongside RN updates which keep breaking this).

Looking at https://www.npmjs.com/package/react-native-cached-image, 3k downloads/week doesn't seem huge but not tiny either. So if there's interest:

Let me know your thoughts. My fork is updated and tested to work in my own apps, up to RN 0.59.5: https://github.com/fungilation/react-native-cached-image/commits/master

onedevlad commented 5 years ago

Hey, @fungilation I'm running RN 0.59.3 and I get a warning about AsyncStorage being extracted from rn core while using your fork. It seems it's dropped by react-native-clcasher which isn't maintained anymore. Can you do anything about this?

fungilation commented 5 years ago

Yes, I patched it with https://github.com/ds300/patch-package. Here's the resulting patch:

diff --git a/node_modules/react-native-clcasher/MemoryCache.js b/node_modules/react-native-clcasher/MemoryCache.js
index 4bb65b8..00a373e 100644
--- a/node_modules/react-native-clcasher/MemoryCache.js
+++ b/node_modules/react-native-clcasher/MemoryCache.js
@@ -1,5 +1,6 @@
 // import React, {Component} from 'react';
-import {AsyncStorage, Platform} from 'react-native';
+import {Platform} from 'react-native';
+import AsyncStorage from '@react-native-community/async-storage';

 const PREFIX = 'react-native-cacher:values:';
 const DEFAULT_EXPIRES = 999999;
bwillem commented 5 years ago

I think people will organically start using the fork that's active. Thanks for your contributions, it's definitely of value to the community.

jr-k commented 5 years ago

Thanks @fungilation running great on RN 0.59.5

fungilation commented 5 years ago

Ok emailed Kfir and cc'ed support@npmjs.com. I'd like to keep the same npm package name to not fragment userbase. Will see if Kfir actually respond to email, if not by github.

image

kfiroo commented 5 years ago

@fungilation Hey, thank you for sending the email, my github account doesn't get as much attention as it should 😅 I've added you as a collaborator on github. Please send me your npm username and I'll add you as a maintainer there too

Sorry for the delayed response! 🙏

fungilation commented 5 years ago

Thanks @kfiroo. I believe I got all the permissions I need, including merge access here. I much prefer this over forking, no fragmenting the community either.

My npm username is garon

kfiroo commented 5 years ago

@fungilation You are now a maintainer in npm too :)

bendadaniel commented 5 years ago

Is there npm package of that fork?

paschaldev commented 5 years ago

@fungilation Any update on the npm fork thing? I currently installed from your github fork directly

olinations commented 5 years ago

Can someone explain how to install the forked version if the fork is not going to be merged.

augustbjornberg commented 5 years ago

package.json

"react-native-cached-image": "https://github.com/fungilation/react-native-cached-image.git#master"

Just a headsup. There seems to be some not so minor bugs in the fork when using the latest version of RN (0.59.8).

fungilation commented 5 years ago

Bugs such as? My app is using my fork, on RN 0.59.8.

I'm just too busy to merge my fork and do npm releases, will when I can.

augustbjornberg commented 5 years ago

You can read about the bug here.

I've probably spent upwards of 10 hours just trying to figure out what causes it. If you could have a look at it that would be amazing @fungilation.

stvmachine commented 5 years ago

👀 👀

augustbjornberg commented 5 years ago

If anyone else is looking for an easy replacement for this module i can highly recommend react-native-fast-image. I was able to completely replace the now broken react-native-cached-image in my app in less than 30 minutes, with no noticeable loss in functionality. An added bonus is react-native-fast-image also seems to perform noticeably better, and has some nice additional features i might make use of in the future.

paschaldev commented 5 years ago

@augustbjornberg Thanks for this. I'm adding it to my project right away

bwillem commented 5 years ago

I've used both a bit now and some difference I've noticed are

fungilation commented 5 years ago

Interesting comparison. Thanks!

react-native-cached-image been resilient and hasn't crashed on me yet in my app.

c-ancia commented 5 years ago

I know this article might be a bit old, but I stumbled onto it while looking for memory leaks in my app and it turned out my app went from unexpectedly crashing to completely stable after I changed from react-native-fast-image to react-native-cached-image.

I'm still leaving the link here in case it might be useful to someone : https://www.freecodecamp.org/news/finding-memory-leaks-react-native-app-ios-46e6eeb50c8c/

fungilation commented 5 years ago

That's interesting. I was thinking of trying/switching to react-native-fast-image myself

bwillem commented 5 years ago

I could not reproduce the leaks in the above article. I assumed this was patched in an update, cause from what I can tell fast-image is cleaning up after itself properly now.

c-ancia commented 5 years ago

Alright, then maybe I had an old version of the package installed.