wcandillon / react-native-expo-image-cache

React Native Image Cache and Progressive Loading based on Expo
MIT License
672 stars 125 forks source link

Image caching improvements #129

Open davidminor opened 5 years ago

davidminor commented 5 years ago

Some improvements that I suggested in #126 & #127 -

The first commit adds an optional cache key property to Image to use instead of the uri as the cache key.

The second commit avoids repeat downloads if multiple Image components reference the same uri.

jeffreybello commented 5 years ago

+1 to this. This feature is sooooo good!

davidminor commented 5 years ago

@wcandillon any chance of getting this merged soon?

mo-hummus commented 4 years ago

Hey man you are AMAZING!

wcandillon commented 4 years ago

@davidminor Thks for that, can you look into the merge conflicts?

mo-hummus commented 4 years ago

Before merging this it needs some work though, there is an edge case where if you delete a cached image then change the component's key the promise is never deleted it wont redownload/load the image, i resolved this by adding a check if the path has been resolved before

export class CacheEntry {
  uri;

  options;

  downloadPromise;

  pathResolved = false;

  constructor(uri, options) {
    this.uri = uri;
    this.options = options;
  }

  async getPath() {
    const { uri, options } = this;
    const { path, exists, tmpPath } = await getCacheEntry(uri);

    if (exists) {
      return path;
    }

    if (!this.downloadPromise) {
      this.pathResolved = false;
      this.downloadPromise = this.download(path, tmpPath);
    }

    if(this.downloadPromise && this.pathResolved) {
      this.pathResolved = false;
      this.downloadPromise = this.download(path, tmpPath);
    }

    return this.downloadPromise;
  }

  async download(path, tmpPath){
    const { uri, options } = this;
    const result = await FileSystem.createDownloadResumable(uri, tmpPath, options).downloadAsync();

    // If the image download failed, we don't cache anything
    if (result && result.status !== 200) {
      this.downloadPromise = undefined;
      return undefined;
    }
    await FileSystem.moveAsync({ from: tmpPath, to: path }).then(()=> {
      this.pathResolved = true
    });
    return path;
  }
}
Aryk commented 4 years ago

Just curious if this feature or something similar made its way into the library. I see there's been no activity for 6 months.

davidminor commented 4 years ago

We've been using a custom build and it fell off my radar. Probably won't have time to pick it back up for awhile.

Aryk commented 4 years ago

Got it, I implemented caching of the signed s3 urls on the server side. I have it set to cache for 5 days whereas the urls are good for 7 days. The mobile app itself reloads data from the server everytime it loads a page so I don't think there is a chance for the url to get stale. Anyways, thanks for your response.

obax commented 3 years ago

Hi guys, we are using a patched version of the library to use the feature. Is there any chance that this could make its way in the mainline code anytime soon? @davidminor Thanks for all the work guys!