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

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

Closed
jdpt0 opened this issue Jul 14, 2022 · 6 comments
Closed
Labels
bug Something isn't working

Comments

@jdpt0
Copy link

jdpt0 commented Jul 14, 2022

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>
  );
};
@jdpt0 jdpt0 changed the title useIntersectionObserver loses binding when ref conditionally rendered React 18: useIntersectionObserver loses binding when ref conditionally rendered Jul 14, 2022
@parkerault
Copy link

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
Copy link
Contributor

jbean96 commented Nov 9, 2022

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

@electroheadfx
Copy link

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
Copy link

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 juliencrn added the bug Something isn't working label Jan 8, 2024
@juliencrn
Copy link
Owner

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
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants