wcandillon / react-native-expo-image-cache

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

defaultSource not working as expected #115

Closed jeanverster closed 5 years ago

jeanverster commented 5 years ago

Hi there, first off thanks for this lib. Image caching is a huge issue in RN apps and it's surprising that expo hasn't incorporated something into the core expo modules yet!

Anyways, I have a card component in which this package is being used to render the image.

My use case is the following, I have a remote image url which could be null, so when it is null I'd like to display a fallback image, and from what I can gather I need to use defaultSource for that? Thing is, it just doesn't seem to load the image. My code is as follows:

import { Image as CachedImage } from 'react-native-expo-image-cache';

...

<CachedImage
              style={{
                top: 0,
                left: 0,
                width: '100%',
                height: '100%',
                borderRadius: 4,
                overflow: 'visible',
                resizeMode: 'cover',
                position: 'absolute',
              }}
              resizeMode="cover"
              defaultSource={images.articlePlaceholder}
              uri={event && event.image && event.image.scaled}
            />

Where the placeholder image source is a local image, with require('../path-to-image')

It works fine when there is a url being passed to the uri prop, but not for the local defaultSource. Am I doing something wrong? Any help here would be much appreciated.

wcandillon commented 5 years ago

Could you send me a small snack of the issue? I'm need to bit more information to understand the use case.

jeanverster commented 5 years ago

Hi @wcandillon, herewith a snack showing the behaviour:

https://snack.expo.io/@jeanverster/defaultsource-bug

Just change the hasRemoteImage var to false to switch between uri / default source :)

I've also noticed something else, if I add some image to the preview prop, then the defaultSource image shows up, however it doesn't take on any of the styles I'm passing through - should the styles from the style prop not also apply to the image being used for defaultSource ?

wcandillon commented 5 years ago

Thks a lot. I took a look at the example and it made me wonder how this should work. Right now it looks like the default source is not showed because no preview is supplied. This would be easy to fix.

wcandillon commented 5 years ago

Thks for providing an example. I submitted a PR that display the defaultSource if no preview is submitted. let me know if this helps. I guess the next step would be to use defaultSource as well if the regular image download fails.

To be completely transparent my usage of this module is very narrow (I don't use default source for instance) so it's quite hard for me to support such use cases.

jeanverster commented 5 years ago

Hi @wcandillon, thanks for this change! The only other issue I've noticed is that the styles from the style prop don't seem to get passed to the image.. I've had a look at the computedStyle function but it seems to return an empty object for the second param regardless of what styles I pass in.. So all the styles that the image receives are StyleSheet.absoluteFill.. Would you mind explaining the purpose of the computedStyle function? I've tried to decode it but am struggling to understand it.. Also happy to open a separate issue if you think it's required 👍

wcandillon commented 5 years ago

Wow indeed this part of the code cannot be understood. Sorry about that. The goal of this function is to copy some styling props, see propsToCopy. I haven't actively used this module in a while, it probably could use a big clean up. Also back then using resume()/cancel() for image download didn't with Expo, it might work nicely now.