thebuilder / react-intersection-observer

React implementation of the Intersection Observer API to tell you when an element enters or leaves the viewport.
https://react-intersection-observer.vercel.app
MIT License
5.1k stars 186 forks source link

[Question] How would you handle conditional rendering with hooks? #162

Closed justinhelmer closed 5 years ago

justinhelmer commented 5 years ago

First, I'd like to say thank you for such a great tool. Very useful.

I ran into a situation I wanted to run by you, and see if you have any suggestions.

This is specifically when using functional components with conditional rendering.

Consider the following scenario:

export default () => {
  const ref = useRef();
  const inView = useInView(ref);
  const [isLoading, setIsLoading] = useState(true);

  useEffect(() => {
    setIsLoading(false);
  }, []);

  if (isLoading) {
    return <div>Loading...</div>;
  }

  return <div ref={ref}>in view - {inView ? 'yes' : 'no'}</div>
};

In the above example, ref.current won't exist for the initial render. When the effect is run in useInView, it will fail to attach the observer.

When the isLoading state changes, a re-render will happen. However, the useInView effect is only run if one of the options changes.

Even if we add ref.current to the dependency array in the effect, this doesn't help. The reason being, the dependencies are checked against at the time of render, whereas react refs get modified after rendering. See https://github.com/facebook/react/issues/14387#issuecomment-455415008.

If we remove the list of dependencies entirely, we can see the ref.current has the proper reference in useEffect on the subsequent render. This is because the hook execution is deferred until after render (and thus the reference has been updated). However this results in an infinite render loop for this scenario. This may be the reason you added dependencies to useEffect to begin with.


I apologize for not coming forward with a potential solution, but this one stumped me. useLayoutEffect doesn't help us here either, because the inView state is set asynchronously.

One "workaround" is to always include the ref in the rendered component, i.e.

if (isLoading) {
  return <div>Loading...<div ref={ref}></div>;
}

return <div ref={ref}>in view - {inView ? 'yes' : 'no'}</div>

However, this is not ideal and comes with its own set of pitfalls/challenges.

It may also be worth noting that this is a non-issue when using render props, since the ref is guaranteed to exist when the observer is attached.

thebuilder commented 5 years ago

That's interesting! Thanks for the detailed description. I guess issues like this is to be expected with hooks to start with.

thebuilder commented 5 years ago

The question is, how to handle when the ref changes, and should we handle it?

It needs to handle observering and unobservering whenever it changes.

thebuilder commented 5 years ago

I'm currently leaning towards throwing an error if the ref is not the set when you create the hook.

Looking at your example, i think it would make sense for you to create a <Loading> component and a <Result> component. You can then move the the useInView hook inside the Result.

The reason is you aren't actually using the hook before you load the content.

justinhelmer commented 5 years ago

makes sens @thebuilder. May result in a pretty superficial component, but it does seem like the right solution (and certainly the easiest). Feel free to close this issue.

thebuilder commented 5 years ago

I looked a bit more into how to solve this, and made a solution that recreates the observer when the ref changes. I do this by updating a useState with the current ref in a new useEffect call that always runs. This state can then be used in second useEffect to trigger a new render.

https://github.com/thebuilder/react-intersection-observer/blob/master/src/hooks.tsx#L25

  React.useEffect(() => {
    if (ref.current !== currentRef) {
      setCurrentRef(ref.current)
    }
  })

It seems like a more robust solution, that's less likely to cause surprises. Also had a coworker with a similar issue last week, so it seems like a common enough use case.

justinhelmer commented 5 years ago

elegant

raRaRa commented 5 years ago

I looked a bit more into how to solve this, and made a solution that recreates the observer when the ref changes. I do this by updating a useState with the current ref in a new useEffect call that always runs. This state can then be used in second useEffect to trigger a new render.

https://github.com/thebuilder/react-intersection-observer/blob/master/src/hooks.tsx#L25

  React.useEffect(() => {
    if (ref.current !== currentRef) {
      setCurrentRef(ref.current)
    }
  })

It seems like a more robust solution, that's less likely to cause surprises. Also had a coworker with a similar issue last week, so it seems like a common enough use case.

The useEffect will run on every re-render, wouldn't it be better if it only runs when ref.current changes? E.g.

  React.useEffect(() => {
    if (ref.current !== currentRef) {
      setCurrentRef(ref.current)
    }
  }, [ref.current])
thebuilder commented 5 years ago

@raRaRa That's the problem, the hook won't trigger when ref.current changes.

I made a blog post about how I solved this issue - Using a ref callback instead of useRef. https://medium.com/@teh_builder/ref-objects-inside-useeffect-hooks-eb7c15198780

raRaRa commented 5 years ago

Ah wow, good catch! Thanks :)

thebuilder commented 5 years ago

You also shouldn't really worry about running an inexpensive useEffect on every render - It is the recommended way, in order to avoid bugs like this.

justinhelmer commented 5 years ago

I had an issue with moving the ref to a child component, but it looks like you already thought of that and handled it with callback refs in the new 8.x version! 👏

theairbend3r commented 4 years ago

Sorry to post on a closed issue. I think I have an error directly related to this. I am unable to solve it. The question is on stackoverflow - React state is always one step behind while making predictions on uploaded image using Tensorflowjs.

I'm using useRef to pass the image to tfjs using useEffect. But instead of the prediction happening for the current image, I get the prediction for the previous image. Would greatly appreciate if you can have look.

gsevla commented 3 years ago

@raRaRa That's the problem, the hook won't trigger when ref.current changes.

I made a blog post about how I solved this issue - Using a ref callback instead of useRef. https://medium.com/@teh_builder/ref-objects-inside-useeffect-hooks-eb7c15198780

Hey man, thanks for the article!

I have an use case that I need to pass back some things back to the main component from the component that have ref.

Usually I could achieve that using the useImperativeHandle hook but I couldn't make it work with this solution.

I have something like:

const C1 = () => {
    const [ref, refReady] = useCallbackRef();

    useEffect(() => {
        if(refReady) {
            // break here because now ref is a function and not a object, so what should i do to call setActiveItem?

        }
    }, [refReady])

return (
    <C2Fw ref={ref} />
>
}

const C2 = ({innerRef}) => {
    const activeItem = useRef(); //tried using useState too

    const setActiveItem = (itemId) => {
        activeItem.current = itemId
    }

    useImperativeHandle(innerRef, () => ({
        setActiveItem,
        activeItem: activeItem.current
    }))
}

const C2Fw = forwardRef((props, ref) => (<C2 innerRef={ref} {...props} />));

My problem is that I need some values from inside C2 to use on C1 and make it render on changes.