Warning

[khrizt]

Hi,

Thanks for this great library. Right now I'm adding it to our react native app and sometimes I see this warning:

Warning: Can only update a mounted or mounting component. This usually means you called setState, replaceState or forceUpdate on an unmounted component. This is a no-op.

on line 151 of imageCacheHoc.js. At least that's what it says :). Am I doing something wrong or is there a way to fix that? Thanks

[billmalarky]

Since this.fileSystem.getLocalFilePathFromUrl() is an async function and setState() doesn't get called until after this.fileSystem.getLocalFilePathFromUrl() resolves, that warning will throw if the CacheableImage component is mounted and then quickly unmounted before the remote image can be downloaded.

This is a warning, so the issue will not show up in production, and as such there's no need for you to consider yourself "blocked" by this. That said, there's got to be a way to check if a component is currently mounted before calling setState(). I'll investigate this and let you know.

Thanks for bringing this to my attention.

[billmalarky]

This appears to be the solution.

https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html

[khrizt]

Oh, great! Do you want a PR or are you gonna get at it yourself?

[billmalarky]

Patching it right now ;-)

Thanks for offering though!

[khrizt]

By the way, today I was trying to put an ActivityIndicator as placeholder but with an enclosing View of the same width and height than the Image I'm trying to load. Is that possible somehow? I tried during some time to access to the style property with no luck, I also tried to pass a placeHolderStyle property and assign it to the placeholder component but that also didn't work.

[billmalarky]

Okay this is resolved.

ActivityIndicator as placeholder but with an enclosing View of the same width and height than the Image I'm trying to load. Is that possible somehow?

I can't think of a way offhand, but is there a way to use the children prop?

Something like

const defaultPlaceholderObject = {
  component: View,
  props: {
    style: styles.viewStyleThatMatchesImageWidthAndHeight,
    children: // is there a way to pass activity indicator component into here.
  }
};

[khrizt]

Great, I'll try that. And thanks a lot for the quick fix!!

[khrizt]

Finally I got it by doing a wrapper ti imageCacheHoc and passing the same style to the Image and the placeholder!

Thanks again

[billmalarky]

Great!

Mind pasting example code of your solution so I can help if someone else asks how to accomplish this?

[khrizt]

Of course, sorry. This is what I did:

const CacheableImagePlaceholder = ({ style }) => (
  <View style={[style, { alignItems: 'center', justifyContent: 'center' }]}>
    <ActivityIndicator color={colors.grey} />
  </View>
);

CacheableImagePlaceholder.propTypes = {
  ...View.propTypes,
};

const ImageCacheHoc = imageCacheHoc(Image, {
  defaultPlaceholder: {
    component: CacheableImagePlaceholder,
    props: {},
  },
});

ImageCacheHoc.propTypes = {
  ...Image.propTypes,
};

const CacheableImage = ({ source, resizeMode, style }) => (
  <ImageCacheHoc
    source={source}
    style={style}
    resizeMode={resizeMode}
    placeholder={{
      component: CacheableImagePlaceholder,
      props: {
        style,
      },
    }}
  />
);

CacheableImage.propTypes = {
  ...Image.propTypes,
};

So this way you only have to change your Image by this component and it works with the placeholder without needing anything.

But I have another problem, when testing the release version of the app it crashes when starting with this error:

com.facebook.react.common.JavascriptException: undefined is not an object (evaluating 's.View.propTypes.style')

It comes from this line:

    static propTypes = {
      fileHostWhitelist: PropTypes.array,
      source: PropTypes.object.isRequired,
      permanent: PropTypes.bool,
      // style: View.propTypes.style,  <---- THIS LINE
      placeholder: PropTypes.shape({
        component: PropTypes.func,
        props: PropTypes.object
      })
    };

But I haven't found the reason for the crash, but If I comment it it works. I'll investigate some more, but if you could help me, that'd be great.

[billmalarky]

Ah excellent solution and I'll keep it in mind for anyone else who asks.

As for that other issue, looks like View.propTypes has been deprecated in favor of ViewPropTypes (guess FB hasn't updated react docs appropriately since I pulled View.propTypes from their docs).

facebook/react-native@53905a5

If that resolves it and you make a PR I can merge it in on my phone otherwise it will have to wait until next wednesday for me patch.

If you make a PR make sure to run the tests "npm test" and lint "npm run lint" so build doesn't get broke when I merge.

Happy Thanksgiving!