Reinitializing observer doesn't unobserve previous element

[jeron-diovis]

If I understand intent correctly, observer should watch only for one node at a time, unobserving previous node if refCallback is re-invoked:

react-intersection-observer-hook/packages/react-intersection-observer-hook/src/use-intersection-observer.ts

Lines 59 to 62 in dcdd47c

	if (node) {
	observer?.unobserve(node);
	setEntry(undefined);
	}

But refCallback updates node reference before invoking reinitializeObserver:

react-intersection-observer-hook/packages/react-intersection-observer-hook/src/use-intersection-observer.ts

Lines 94 to 99 in dcdd47c

	nodeRef.current = node;
	reinitializeObserver();
	return () => {
	nodeRef.current = null;
	reinitializeObserver();

So unobserve just never ever is called.

Demo: https://codesandbox.io/p/sandbox/s7mwhj
Check out console output. See how it logs visibility changes for all nodes, despite the fact same refCallback is used for all them – so only the last one should be observed.

[onderonur]

Firstly, thanks for investigating this issue and creating a reproduction so clearly 🙏

I always aimed for refCallback to be used on a single element at a time. In the current structure, since refCallback cleanup is not called for the previous elements (since they are not unmounted etc.) observer continues to observe them. So, it's exactly like you described!

This can actually be useful for cases which you want to observe a list of elements but since entry is the latest element the observer is observing (or the latest one with the visibility change), it becomes useless unfortunately.

It can be used with some hacky ways but they would not be reliable.

I will keep this issue open for now and think for a pattern to make this a supported structure.

Instead of a single entry, keeping a list of entries as state might work. But there should be a way to distinguish entries in this case for app developers. Maybe some data attributes can be used.

Anyway, thanks again! I will try to look into this topic some time I can 🙌

[jeron-diovis]

@onderonur If your original intent indeed was to observe a single element at a time, then maybe it's better to start with just fixing this implicit multi-observe behavior. And clearly declare in docs that this hook is for observing a single element only. That's primary usecase anyway.

Extending it with multi-observe can be an extra feature for some future versions.

[onderonur]

Thanks again @jeron-diovis 🙏

I just published a fix for this with v4.0.1, and added a note to README 🚀