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

Error fallback image with loadEagerly? #22

Open
aralroca opened this issue Apr 18, 2019 · 6 comments
Open

Error fallback image with loadEagerly? #22

aralroca opened this issue Apr 18, 2019 · 6 comments

Comments

@aralroca
Copy link

I need an image that should be rendered asap (I'm using SSR with next.js), and currently I'm using the loadEagerly prop in this image... However, I found interesting that if for some reason this image doesn't exist, display the fallback image... Looks that with loadEagerly is only rendering the actual.

There is some another option to do it?

@fpapado
Copy link
Owner

fpapado commented Apr 18, 2019

Hi @aralroca!

Thanks for opening this issue.
loadEagerly is indeed meant for cases where you want to make it as if the "actual" img was always there. For example, it's useful for hero images and other things you know are important.

An important property there, is that it works immediately and without Javascript (the browser only ever sees an img from the SSR). Between the time the browser gets the SSR'd HTML and the client-side JS has run, then we are essentially in the "no Javascript" zone. So the current behaviour is benefficial for those cases, but you're right that it does not allow us to observe the loading states. The code just short-circuits to the LoadSuccess state;

I think that observing the loading states with loadEagerly can give a false sense of it working. If the client-side scripts are slow to load after SSR'd HTML has come in (because of network, size, etc.), then we would not catch the loading error cases (the image would have already failed before the scripts). In other words, there could be a race condition on something that is ultimately hard to control. I would not want to expose that from the library that way :)

Having said that, this does make me consider that in cases where we render on the client with loadEagerly, we could be skipping to the Loading state, and observing the load/error. It offers perhaps a more consistent experience. I'll give this some thought! I would also be happy to hear more about your specific use case, as an extra data point.

@aralroca
Copy link
Author

Hi @fpapado, thank you for the answer.

In my case, I'm doing an e-commerce with Next.js 8.1.0. With next.js, the first loaded page is loaded using SSR, and the rest of pages (using navigation) are with CSR. So the same page sometimes will be rendered from SSR and other times in CSR.

In all the app I'm loading the images with the lazy way (widgets and lists) using your library. And everywhere is working fine, loading the fallback image if this image is broken (using the error prop).

However, in the only place that I want to load the image asap, is in the product page. In this case I used the prop loadEagerly to display this image asap. But how I commented, some products can have the image broken, and I want to display the fallback image in these cases. And the problem that I have using loadEagerly is that is not rendering the error fallback.

I try it to do a workaround, doing something similar to:

<LazyImage
  loadEagerly
  src={src}
  alt=""
  actual={({ imageProps }) => (
    <img
         onError={onErrorImage}
         {...imageProps}
      />
    )}
/>

and:

function onErrorImage(e) {
    e.target.src = fallbackSrc
 }

And this works in some cases :) . With CSR this workaround works 100% well. Nevertheless, I realized that sometimes is not firing the onError event when it comes from SSR... This I'm not sure if is something that I'm missing, or some issue with Next.js (reported here: vercel/next.js#7047)

@aralroca
Copy link
Author

aralroca commented Apr 18, 2019

I just realized that I can pass the loadEagerly as loadEagerly={!process.browser}to only use it in SSR, serve the image asap, and then, check the error using the error prop after the rehydration.

Doing this I have a similar result than the workaround. Rendering the fallback image if is broken...

However in the Rehydration moment is blinking... If the image is broken, maybe make sense because is replacing the broken image to the fallback image. But if the image is not broken, is doing some rare blinking effect.

cases

@fpapado
Copy link
Owner

fpapado commented Apr 18, 2019

I think the workaround that you mention with onError is the best way to handle loadEagerly + error handling at the moment.

The second scenario with loadEagerly={!process.browser} will have quirks, because the image will be re-rendered when the client sripts rehydrate. This is why you get the "blinking" behaviour even if there is no error - it's the image entering the "Loading" state again.

The reason the workaround only works sometimes, as far as I can tell, is that the event could fire before the client scripts have downloaded/parsed/executed. I don't think it's Next.js specific, but rather how hydration and events work. I could be wrong though!

A potential solution

This discussion does give me an idea:

  • If the image has loadEagerly, then we move to a new state, Refreshing
  • In that Referesing state, we would keep rendering the actual, and also kick off an observed image load
  • When the new image comes in, or fails, we move to Loaded or Error states, as before

In theory, this means:

  • If the image has been fetched by the browser already, then it's in cache, and there should be no blinking (no Loading state) when we "swap"
  • If the image was broken, and the new load is broken, then error is used

I have some free time over the next few days, and was looking into tackling some leftover issues here. I'll let you know if I manage to get a beta version out. No guarantees that it'll work, but we can try it and get some feedback :)

@aralroca
Copy link
Author

@fpapado currently I'm using the first workaround that I commented, with onError event. However, to fix the SSR problem that I told (when the event is fired before hydration) I did another workaround:

 useEffect(() => {
    if (imageRef && imageRef.current) {
      const { complete, naturalHeight } = imageRef.current
      const errorLoadingImgBeforeHydration = complete && naturalHeight === 0

      if (errorLoadingImgBeforeHydration) {
        imageRef.current.src = fallbackSrc
      }
    }
  }, [])

I expect that always that image is completed with naturalHeight as 0, is broken.

It's not an elegant solution. However, it works.

@aralroca
Copy link
Author

aralroca commented Apr 18, 2019

Better detailed:

hook:

import { useRef, useCallback, useEffect } from 'react'
import noop from 'lodash/noop'

/**
 * Using loadEagerly, with SSR the image is displayed ASAP,
 * in the first render.
 *
 * With loadEagerly activated, the "error" prop of LazyImage
 * component is not rendering. So, in order to workaround the fallback image,
 * I need to do two things:
 *
 * * Catch the event onError and change the src with the fallback image
 * * Check in the "didMount" (useEffect) that if is completed and doesn't
 *   have height, this means that there is an error, and how is completed
 *   before the hydration, the onError event was not fired by React. So in
 *   this case also the src shoud be changed to the fallback image.
 */
export default function useLoadEagerlyFallbackImage(fallbackSrc) {
  const ref = useRef(null)

  /**
   * Error happened / catched after hydration
   */
  const onError = useCallback(
    e => { e.target.src = fallbackSrc }, [fallbackSrc],
  )

  /**
   * Error happened before hydration, but catched after hydration
   */
  useEffect(() => {
    if (ref && ref.current) {
      const { complete, naturalHeight } = ref.current
      const errorLoadingImgBeforeHydration = complete && naturalHeight === 0

      if (errorLoadingImgBeforeHydration) {
        ref.current.src = fallbackSrc
      }
    }
  }, [fallbackSrc])

  return { ref, onError }
}

Usage:

const loadEagerlyProps = useLoadEagerlyFallbackImage(fallbackSrc)

return (
  <LazyImage
    loadEagerly
    src={src}
    alt=""
    actual={({ imageProps }) => (
     <img {...imageProps} {...loadEagerlyProps} />
     )}
  />
)

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

2 participants