Aljullu / react-lazy-load-image-component

React Component to lazy load images and components using a HOC to track window scroll position.
https://www.npmjs.com/package/react-lazy-load-image-component
MIT License
1.44k stars 109 forks source link

Double image download #50

Closed Asmadeus closed 4 years ago

Asmadeus commented 4 years ago

Bug description I use LazyLoadImage component. When I scroll page to image, and it is loading, on network tab I see that this image was loaded two times. Why does it happen and how can I fix it?

To Reproduce

<LazyLoadImage
  alt={alt}
  height={dimension}
  src={src}
  width={dimension}
/>

Technical details:

ascrazy commented 4 years ago

I'm experiencing the same behavior with Chrome, works well with FF at the same time.

Package version 1.3.2 Server Side Rendering? No Device Desktop Operating System Mac OS Browser Chrome Version 78.0.3904.87 (Official Build) (64-bit)

ascrazy commented 4 years ago

Code Sandbox

68294647-b6831500-00a1-11ea-8ac0-766f1747396f 68294527-77ed5a80-00a1-11ea-9d9f-7db2925a979d

Aljullu commented 4 years ago

Thanks for opening this issue @Asmadeus and for providing a screenshot and a sandbox @ascrazy.

I could reproduce it too with Google Chrome and GNOME Web (so probably it affects Safari too), while it seems to work fine in Firefox.

I will mark this issue as a bug.

mustafazaki commented 4 years ago

I faced this issue in normal jquery lazy load plugin. Chrome shows 2 image requests in devtool for lazy loaded image. If you uncheck this disable cache option in the network tab you will see a single request.

olaj commented 4 years ago

Ahh, that "flickering" that i have noticed is because of that. I can confirm that the "disable cache" thing helped.

zephraph commented 4 years ago

We're also seeing double rendering. From our perspective, it seems like the images are getting the src attribute injected on SSR when we don't expect it. I'm still investigating further.

Using the beta I'm not seeing that issue.

zephraph commented 4 years ago

Confirmed: There's an SSR issue in 1.3.2 which causes images to be SSR'd. On the beta version that issue is resolved.

S-Cardenas commented 4 years ago

I'm still seeing this issue on version 1.4.0. Can anyone confirm that it's fixed in the beta but still exists in 1.4.0?

zephraph commented 4 years ago

@S-Cardenas our issue might've been actually unrelated. I was struggling a bit with some deeply nested linking so if you're still seeing the issue persist it might be valid.

Could you try to add a minimal reproduction via codesandbox or something? I'd be happy to help look into it and maybe PR a fix.

Asmadeus commented 4 years ago

@zephraph @S-Cardenas This issue still exists in version 1.4.0.

I fixed this bug with custom LazyLoadImage component. In source code of LazyLoadImage component in render method there is checking for loaded property of state(https://github.com/Aljullu/react-lazy-load-image-component/blob/master/src/components/LazyLoadImage.jsx#L128).

Initially component renders LazyLoadComponent with placeholder. When you scroll to it LazyLoadComponent replaces placeholder with image, then image downloaded and loaded became true, so React renders in DOM new img tag(without LazyLoadComponent as wrapper) and in this time browser downloads "new" image(not all browsers).

I just remove this checking and always render LazyLoadComponent

const lazyLoadImage = this.getLazyLoadImage(image);

S-Cardenas commented 4 years ago

@Asmadeus could you show us sample code of what you did? I'm having a hard time envisioning what you did and implemented.

Any help is much appreciated.

zephraph commented 4 years ago

@S-Cardenas

Essentially, on https://github.com/Aljullu/react-lazy-load-image-component/blob/master/src/components/LazyLoadImage.jsx#L128

- const lazyLoadImage = loaded ? image : this.getLazyLoadImage(image);
+ const lazyLoadImage = this.getLazyLoadImage(image);
S-Cardenas commented 4 years ago

Thanks @zephraph. Can someone up a PR request for this?

Asmadeus commented 4 years ago

I’ve created PR #56

Aljullu commented 4 years ago

Thank you all for investigating this issue! Will merge the PR and release a beta version today.

Aljullu commented 4 years ago

This should be fixed in the last beta. You can install it with:

npm i --save react-lazy-load-image-component@1.4.1-beta.0