react-native-cacheable-image icon indicating copy to clipboard operation
react-native-cacheable-image copied to clipboard

Broken cached image

Open bitcoinvsalts opened this issue 8 years ago • 26 comments

Any idea why do I get broken cached images sometimes?

broken_image

bitcoinvsalts avatar Nov 21 '16 17:11 bitcoinvsalts

Are the images within a loop or map function?

This may be a RN issue: https://github.com/facebook/react-native/issues/9893

nbolender avatar Nov 21 '16 17:11 nbolender

I am using a ListView and a renderRow function

bitcoinvsalts avatar Nov 21 '16 18:11 bitcoinvsalts

Are you using a unique key prop for the root component in your renderRow function? Just thinking it could be some sort of referencing issue.

Or maybe something corrupted the image when it was being cached.

nbolender avatar Nov 21 '16 18:11 nbolender

here is my render function:

enderRow = (data) => {
    console.log("--- _renderRow ---")
    const timeString = moment(data.timestamp).fromNow()
    return (
      <Post
        postTitle={data.title}
        posterName={data.username}
        postTime={timeString}
        postContent={data.text}
        imagePath={data.image}
        imageWidth={data.imageWidth}
        imageHeight={data.imageHeight}
      />
    )
  }

and

<CacheableImage
          source={{ uri:this.props.imagePath }}
          resizeMode='contain'
          style={{
            height: height,
            width: screenWidth,
            alignSelf: 'center',
            marginBottom: 10,
          }}
        />

bitcoinvsalts avatar Nov 21 '16 18:11 bitcoinvsalts

@jsappme I think your <Post> component needs a <Post key={{'#ID'}}> prop.

jayesbe avatar Nov 23 '16 10:11 jayesbe

it still breaks some of the images sometimes

bitcoinvsalts avatar Nov 24 '16 19:11 bitcoinvsalts

@jsappme Do any of the images break without CacheableImage ? meaning using regular Image ?

jayesbe avatar Nov 24 '16 20:11 jayesbe

no broken image with regular Image component

On Thu, Nov 24, 2016 at 2:16 PM, Jesse [email protected] wrote:

@jsappme https://github.com/jsappme Do any of the images break without CacheableImage ? meaning using regular Image ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jayesbe/react-native-cacheable-image/issues/27#issuecomment-262840003, or mute the thread https://github.com/notifications/unsubscribe-auth/AM0R9TZqZf5CkH98ZbY2rEep2Bd9mW5sks5rBfCcgaJpZM4K4fqf .

-- Hervé Fulchiron JSapp.me

bitcoinvsalts avatar Nov 24 '16 20:11 bitcoinvsalts

Hmm.. too many files maybe being written to storage at once ? maybe they need to be queued up for download.

jayesbe avatar Nov 24 '16 20:11 jayesbe

yes I was actually thinking about this probable cause.

Any simple way to queue up those downloads?

bitcoinvsalts avatar Nov 24 '16 21:11 bitcoinvsalts

I might have find a solution to download the images one by one for my case. I will let you know if it works

bitcoinvsalts avatar Nov 24 '16 22:11 bitcoinvsalts

I am still struggling with this issue. is there a way to know how much of the image is downloaded during the loading?

bitcoinvsalts avatar Nov 25 '16 18:11 bitcoinvsalts

I had to go back to the RN Image component for now :-(

bitcoinvsalts avatar Nov 25 '16 21:11 bitcoinvsalts

@jsappme I think it could be that the image is destroyed even before it's had a chance to finish downloading. I believe if that occurs the cached file just needs to be removed since it was incomplete. I will create a patch can you test it?

jayesbe avatar Nov 26 '16 09:11 jayesbe

@jsappme can you try the following please


diff --git a/image.js b/image.js
index c53e479..4b13ff3 100644
--- a/image.js
+++ b/image.js
@@ -18,11 +18,12 @@
         this.state = {
             isRemote: false,
             cachedImagePath: null,
+            downloadingImagePath: null,
             downloading: false,
             cacheable: true,
             jobId: null,
             networkAvailable: false
-        };
+        };  
     };
 
     componentWillReceiveProps(nextProps) {
@@ -89,23 +90,25 @@
                     begin: this.imageDownloadBegin,
                     progress: this.imageDownloadProgress
                 };
-
+                
+                this.setState({downloadingImagePath: filePath});
+                
                 // directory exists.. begin download
                 RNFS
                 .downloadFile(downloadOptions)
                 .promise
                 .then(() => {
-                    this.setState({cacheable: true, cachedImagePath: filePath});
+                    this.setState({cacheable: true, cachedImagePath: filePath, downloading: false, jobId: null, downloadingImagePath: null});
                 })
                 .catch((err) => {
                     // error occurred while downloading or download stopped.. remove file if created
                     this._deleteFilePath(filePath);
-                    this.setState({cacheable: false, cachedImagePath: null});
+                    this.setState({cacheable: false, cachedImagePath: null, downloading: false, jobId: null, downloadingImagePath: null});
                 });
             })
             .catch((err) => {
                 this._deleteFilePath(filePath);
-                this.setState({cacheable: false, cachedImagePath: null});
+                this.setState({cacheable: false, cachedImagePath: null, downloading: false, jobId: null, downloadingImagePath: null});
             })
         });
     }
@@ -156,19 +159,20 @@
 
     componentWillMount() {
         NetInfo.isConnected.addEventListener('change', this._handleConnectivityChange);
-        // initial
-        NetInfo.isConnected.fetch().then(isConnected => {
-            this.setState({networkAvailable: isConnected});
-		});
+        
+        if (this.props.checkNetwork) {
+            NetInfo.isConnected.fetch().done(this._handleConnectivityChange);
+        }
         
         this._processSource(this.props.source);
     }
-
+    
     componentWillUnmount() {
         NetInfo.isConnected.removeEventListener('change', this._handleConnectivityChange);
     
         if (this.state.downloading && this.state.jobId) {
             RNFS.stopDownload(this.state.jobId);
+            this._deleteFilePath(this.state.downloadingImagePath);
         }
     }
 

jayesbe avatar Nov 26 '16 10:11 jayesbe

Hi Jesse can you send me the new file to herve76 @ gmail.com?

bitcoinvsalts avatar Nov 27 '16 14:11 bitcoinvsalts

@jsappme if you can test the latest version.

jayesbe avatar Jan 30 '17 00:01 jayesbe

@jsappme I implemented a cache system similar to this module using react native fetch blob to download/save the images and I'm getting the exact same issue. Not sure why.

I'm also wondering if Image.prefetch could be used to do the job? And if yes how? Or I'm misunderstanding the semantic of Image.prefetch completely?

wcandillon avatar Feb 06 '17 05:02 wcandillon

I think you're right @jayesbe. In my tests with another component it seems to occur if I have the image inside a cell of a Flatlist which destroys the cells as they move outside of the render window. I'll try this component with that fix you've mentioned to see if it fixes the issue.

MossP avatar Jun 27 '17 13:06 MossP

~I very quickly ran into this error using this component. Not sure if it's related.~ May be my error

React Native : 0.45.1 Component Version : 1.5.1

Used inside a flatlist and scrolling quickly. If I throttle the connection then many images (~60%) never show even though I can see through the proxy that they have finished.

image uploaded from ios 1

MossP avatar Jun 27 '17 13:06 MossP

That previous error may have been due to some remaining elements of the last cache component clashing with this one. It seems to be much more stable now but I still get the frequent missed images as mentioned above.

I'd hazard a guess that the missing image are the ones that would have been incomplete, so now it's not showing them at all instead of showing half an image.

MossP avatar Jun 27 '17 13:06 MossP

@MossP have you tried the dev branch ?

The component has to be restructured so that data logic can be extracted into its own module that facilitates loading and cache file management. I haven't been able to get back to updating this component in some time.

jayesbe avatar Jun 27 '17 14:06 jayesbe

Ah, no, I'm back on another component now but I'll try to take a look soon. I just thought I would test as @jsappme had gone quiet.

MossP avatar Jun 27 '17 14:06 MossP

@jayesbe I just tried on the Dev branch. I get about 60% failure rate still when throttling the connection. I can see all images requests are complete via proxy but complete image was not received. App still shows activity indicator.

Even if I disable throttling and force close/restart App, some images do not recover although the requests to them are complete via the proxy and I can see the full image has come through.

MossP avatar Jun 27 '17 14:06 MossP

The same problem here. Could anyone fix this bug?

DavitVosk avatar Aug 07 '18 16:08 DavitVosk

Sorry guys.. I haven't been able to maintain this package as much as I need to.

I believe for the most part this package is now obsolete though may be worthwhile as a convenience.

RN now provides an ImageStore API that can be used to cache images. Adding this into the package would allow the ImageStore to maintain the cache rather than using react-native-fs. RNFS would still be used to download the file and convert to base64 encoded which would be required for ImageStore.

This requires dev.. react-native-cacheable-image doesn't work well with list components.. this is a known issue.. especially lists with a lot of elements.

jayesbe avatar Aug 08 '18 01:08 jayesbe