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

Broken cached image #27

Open
bitcoinvsalts opened this issue Nov 21, 2016 · 26 comments
Open

Broken cached image #27

bitcoinvsalts opened this issue Nov 21, 2016 · 26 comments

Comments

@bitcoinvsalts
Copy link

Any idea why do I get broken cached images sometimes?

broken_image

@nbolender
Copy link
Contributor

Are the images within a loop or map function?

This may be a RN issue: facebook/react-native#9893

@bitcoinvsalts
Copy link
Author

I am using a ListView and a renderRow function

@nbolender
Copy link
Contributor

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.

@bitcoinvsalts
Copy link
Author

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,
          }}
        />

@jayesbe
Copy link
Owner

jayesbe commented Nov 23, 2016

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

@bitcoinvsalts
Copy link
Author

it still breaks some of the images sometimes

@jayesbe
Copy link
Owner

jayesbe commented Nov 24, 2016

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

@bitcoinvsalts
Copy link
Author

bitcoinvsalts commented Nov 24, 2016 via email

@jayesbe
Copy link
Owner

jayesbe commented Nov 24, 2016

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

@bitcoinvsalts
Copy link
Author

yes I was actually thinking about this probable cause.

Any simple way to queue up those downloads?

@bitcoinvsalts
Copy link
Author

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

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

@bitcoinvsalts
Copy link
Author

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

@jayesbe
Copy link
Owner

jayesbe commented Nov 26, 2016

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

jayesbe commented Nov 26, 2016

@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);
         }
     }
 

@bitcoinvsalts
Copy link
Author

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

@jayesbe
Copy link
Owner

jayesbe commented Jan 30, 2017

@jsappme if you can test the latest version.

@wcandillon
Copy link

@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?

@MossP
Copy link

MossP commented Jun 27, 2017

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

MossP commented Jun 27, 2017

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

MossP commented Jun 27, 2017

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.

@jayesbe
Copy link
Owner

jayesbe commented Jun 27, 2017

@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.

@MossP
Copy link

MossP commented Jun 27, 2017

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

MossP commented Jun 27, 2017

@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.

@DavitVosk
Copy link

The same problem here. Could anyone fix this bug?

@jayesbe
Copy link
Owner

jayesbe commented Aug 8, 2018

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.

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

6 participants