Skip to content

Conversation

@davidminor
Copy link

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.

Add an option to use a cache key that is different from the image
URI. This is useful for example if the image URI contains a parameter
that changes, but the image doesn't vary as a result, such as an
authentication token.

If the cache key is not present, the image URI will be used as the
cache key.
Create a single promise to handle the downloading of a cache
entry. Otherwise, multiple calls to get the path of a new cache
entry will result in multiple downloads, until one of the downloads
is successful.

If the download is unsuccesful, unset the promise so that the
next request will result in a new attempt to download the image.
@jeffreybello
Copy link

+1 to this.
This feature is sooooo good!

@davidminor
Copy link
Author

@wcandillon any chance of getting this merged soon?

@mo-hummus
Copy link

Hey man you are AMAZING!

@wcandillon
Copy link
Owner

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

@mo-hummus
Copy link

mo-hummus commented Dec 2, 2019

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

Aryk commented May 10, 2020

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

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

Aryk commented May 11, 2020

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

obax commented Dec 6, 2020

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!

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

Successfully merging this pull request may close these issues.

6 participants