Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On Maintenance and potential forking #160

Open
fungilation opened this issue Apr 20, 2019 · 23 comments
Open

On Maintenance and potential forking #160

fungilation opened this issue Apr 20, 2019 · 23 comments

Comments

@fungilation
Copy link
Collaborator

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:

  • Is @kfiroo completely unresponsive and out of reach, for transitioning maintenance of this repo?
  • If so, how best to transition community to new official fork?
  • If forking, new naming on npm: what should it be?

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
Copy link

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
Copy link
Collaborator Author

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
Copy link

bwillem commented Apr 22, 2019

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
Copy link

jr-k commented May 1, 2019

Thanks @fungilation running great on RN 0.59.5

  • Is @kfiroo completely unresponsive and out of reach, for transitioning maintenance of this repo?

2 years without nothing and 15 pending pull requests, let's assume he is unreachable 99% sure.

  • If so, how best to transition community to new official fork?

Try to create a npm dispute by explaining them the situation (https://docs.npmjs.com/misc/disputes.html) or just create a new repository with a new name and post an issue with a clear name saying "Repository has moved" or something..

  • If forking, new naming on npm: what should it be?

react-native-cached-image2 or react-native-cached-image-enhanced or whatever, the goal is to provide a working caching image module, anything will do the job I guess.

@fungilation
Copy link
Collaborator Author

Ok emailed Kfir and cc'ed [email protected]. 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
Copy link
Owner

kfiroo commented May 2, 2019

@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
Copy link
Collaborator Author

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
Copy link
Owner

kfiroo commented May 5, 2019

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

@bendadaniel
Copy link

Is there npm package of that fork?

@paschaldev
Copy link

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

@olinations
Copy link

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

@augustbjornberg
Copy link

augustbjornberg commented May 22, 2019

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
Copy link
Collaborator Author

fungilation commented May 22, 2019

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
Copy link

augustbjornberg commented May 25, 2019

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
Copy link

👀 👀

@augustbjornberg
Copy link

augustbjornberg commented Jun 4, 2019

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
Copy link

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

@bwillem
Copy link

bwillem commented Jun 17, 2019

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

  • react-native-fast-image uses memory instead of file system for cacheing, which is why it's faster. if your app already has memory issues this will add to them. RNFI seems to manage its memory usage well so hasn't been an issue for me.
  • RNFI is not a drop-in replacement for ReactNative.Image, it has its own API.
  • RNFI is very sensitive to malformed URIs and will crash at runtime if it gets a string it doesn't recognize, so make sure you sanitize uris you're passing to it.

@fungilation
Copy link
Collaborator Author

Interesting comparison. Thanks!

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

@c-ancia
Copy link

c-ancia commented Aug 16, 2019

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
Copy link
Collaborator Author

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

@bwillem
Copy link

bwillem commented Aug 16, 2019

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
Copy link

c-ancia commented Aug 16, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests