juliencrn / usehooks-ts

React hook library, ready to use, written in Typescript.
https://usehooks-ts.com
MIT License
6.49k stars 421 forks source link

React 18: useIntersectionObserver loses binding when ref conditionally rendered #182

Closed jdpt0 closed 9 months ago

jdpt0 commented 2 years ago

In the below example I'm passing in an infinite react-query hook into this component to load more results once the bottom of the list has been reached. With useIntersectionObserver this happens the first time, but after that, the binding is lost and the Load more button has to be clicked to initiate the fetching of the query. Tested on React 17 and works as expected.

A better pattern might be to pass a ref from the hook to bind to a specific component, as is done here.


export const InfiniteList = ({
  query,
  children,
  className,
  style,
}: {
  query: UseInfiniteQueryResult;
  children: ReactNode;
  className?: string;
  style?: CSSProperties;
}): ReactElement => {
  const ref = useRef<HTMLButtonElement | null>(null);

  const entry = useIntersectionObserver(ref, {});
  const isIntersecting = !!entry?.isIntersecting;

  useEffect(() => {
    if (
      isIntersecting &&
      query.data &&
      !query.isLoading &&
      !query.isFetchingNextPage
    )
      query.fetchNextPage();
  }, [query, isIntersecting]);

  const renderLoadButton = () => {
    if (!query || !query.hasNextPage) return null;
    if (query.isFetchingNextPage) return <Spinner />;
    return (
      <div className={styles.loadMoreContainer}>
        <Button size={"small"} ref={ref} onClick={() => query.fetchNextPage()}>
          Load more
        </Button>
      </div>
    );
  };

  return (
    <div className={classNames("overflow-auto", className)} style={style}>
      {children}
      <div className={styles.loadMoreContainer}>{renderLoadButton()}</div>
    </div>
  );
};
parkerault commented 2 years ago

This is just because the dependency array in the useEffect in useIntersectionObserver is using the ref instead of ref.current. I just ran into this and changed the dependency array and it works as expected now. I will submit a PR in a bit.

jbean96 commented 1 year ago

This still seems to be a problem for me even after the fix that changes the dep array for useIntersectionObserver to elementRef?.current.

electroheadfx commented 1 year ago

I have too this issue, seem a problem related with react 18 (and Next ? I use SSR), not sure. The ref is lost. It works on a page and not in anothers, I need to investigate more to understand the issue ...

scottrippey commented 1 year ago

I think I see the issue. The hook has a deps array with elementRef.current. The problem is due to React's lifecycle, which happens in this order:

  1. Render
  2. Update ref.current to the new DOM nodes
  3. Execute useEffect'

So, when we use elementRef.current inside the deps array (during render time), it's always going to be the previous value, not the new value.
This means that if we hide an element, then we show it, the dependency array will still contain null, and won't re-run the useEffect. This can also happen when the ref changes to a new DOM node.

Other libs (eg. react-intersection-observer linked above) solve this by exporting a ref callback, so that they're notified every time the ref is updated. This is a very different API than the "passing in a ref" approach, but it works well.

juliencrn commented 9 months ago

Hi, thanks for the feedback! I've pushed a PR that should fix it, feel free to give a code review on it. See #464

jp-software-development commented 5 months ago

@juliencrn I can confirm that is still not working as expected in 3.1.0.

If the observed element is conditionally rendered (rendered -> not rendered -> rendered) it simply stops working. Even the onResize option fnc stops firing. Please consider fixing this again :) Thank you