-
Notifications
You must be signed in to change notification settings - Fork 189
use of React 19 ref callbacks for IntersectionObserver tracking #718
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
base: main
Are you sure you want to change the base?
Conversation
|
|
|
Thanks alot for putting in the work on these features. React 19 Ref Fallback support
|
|
Am I correct in understanding, that it only removes the extra re-renders if using the |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thank you for your quick reply
Correct
Good catch - changing the dependencies will destroy the observer and create it once again.
I can think of these three options:
import { version } from "react"
import { useInView18 } from "./useInView18";
import { useInView19 } from "./useInView19";
export const useInView: typeof useInView18 = version.startsWith("19") ? useInView19 : useInView18;I also added some docs and tests for |
|
Found another optimization: Currently the same element would be added multiple time to the same observer I fixed |
|
Thanks a lot for doing all this. I think the conditional version switch, is the safest bet right now. It's a small hook, so overhead is minimal. I'm a bit tied up at work, so haven't had time to properly dig into and test the changes. |
|
Cool I am looking forward to it I guess a You probably know that React fiber allows to pause and resume renderings Unfortunately this comes with a cost - the following is no longer allowed: const onGetsIntoViewRef = React.useRef(onGetsIntoView);
onGetsIntoViewRef.current = onGetsIntoView;https://react.dev/reference/react/useRef#caveats
There was the Which was merged as experimental There is a polyfill which uses I changed |
README.md
Outdated
| console.log('Element is in view', element); | ||
|
|
||
| // Optionally return a cleanup function | ||
| return (entry) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be a usecase for the cleanup function for the consumer? You shouldn't use it to track if elements are leaving, where it's better to observe the callback entry/inView value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The advantage of a cleanup is that you have access to the closure of the viewport entering
Some example cases:
- On View: Start tracking the size with a ResizeObserver (and stop it once it's out of view)
- Start a poll Timer to refresh Data (and stop it once out of view)
It's also the pattern for useEffect useLayoutEffect and now with React 19 also for refs when using useCallback
README.md
Outdated
| const inViewRef = useOnInViewChanged( | ||
| (element, entry) => { | ||
| // Do something with the element that came into view | ||
| console.log('Element is in view', element); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the consumer needs the element, they should be able to get it from entry.target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the idea - element is gone
This has only one downside - entry might be undefined if initialInView is true:
const inViewRef = useOnInViewChanged(
(enterEntry) => {
// Do something with the element that came into view
console.log('Element is in view', enterEntry?.element);
// Optionally return a cleanup function
return (exitEntry) => {
console.log('Element moved out of view or unmounted');
};
},
options // Optional IntersectionObserver options
);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initialInView wouldn't make sense when used with useOnViewChanged anyway - It's for avoiding flashing content on the initial render.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe the name useOnInViewChanged misleading and should be useOnInViewEntered or useOnInViewEffect
useInView uses useOnInViewChanged and therefore has to pass over the initialInView option - otherwise it is not possible to update the state on out of view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe changing the api of useOnInViewChanged slightly might get rid of the undefined entry case and handle the initialInVIew better
I'll let you know if it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, and I agree with the name. I have been considering building that hook before, but got stuck on the finding the right name. I might be more into useOnInView.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I refactored the code
useInViewsame api like beforeuseOnInViewno longer acceptsinitialInViewuseOnInViewaccepts now atriggeroption (which is set toenterby default but can be changed toleave):
const trackingRef = useOnInView((entry) => {
console.log("Element left the view", entry.target);
return () => {
console.log("Element entered the view");
};
}, {
trigger: "leave",
});that made it way easier to use useOnInView inside useInView for the initialInView case
it also solved the non existing entry case
what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - Did you try it with multiple thresholds? Would it just trigger multiple times? Should be fine, as long as it can then be read from the entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good catch - I found a missing cleanup edge case - now it's fixed and tested:
test("should track thresholds when crossing multiple in a single update", () => {
// Using multiple specific thresholds
const { getByTestId } = render(
<ThresholdTriggerComponent options={{ threshold: [0.2, 0.4, 0.6, 0.8] }} />,
);
const element = getByTestId("threshold-trigger");
// Initially not in view
expect(element.getAttribute("data-trigger-count")).toBe("0");
// Jump straight to 0.7 (crosses 0.2, 0.4, 0.6 thresholds)
// The IntersectionObserver will still only call the callback once
// with the highest threshold that was crossed
mockAllIsIntersecting(0.7);
expect(element.getAttribute("data-trigger-count")).toBe("1");
expect(element.getAttribute("data-cleanup-count")).toBe("0");
expect(element.getAttribute("data-last-ratio")).toBe("0.60");
// Go out of view
mockAllIsIntersecting(0);
expect(element.getAttribute("data-cleanup-count")).toBe("1");
// Change to 0.5 (crosses 0.2, 0.4 thresholds)
mockAllIsIntersecting(0.5);
expect(element.getAttribute("data-trigger-count")).toBe("2");
expect(element.getAttribute("data-last-ratio")).toBe("0.40");
// Jump to full visibility - should cleanup the 0.5 callback
mockAllIsIntersecting(1.0);
expect(element.getAttribute("data-trigger-count")).toBe("3");
expect(element.getAttribute("data-cleanup-count")).toBe("2");
expect(element.getAttribute("data-last-ratio")).toBe("0.80");
});…alInView option to trigger
# Conflicts: # package.json # pnpm-lock.yaml
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @thebuilder @jantimon, thanks for being careful about supporting this without breaking React 18, it's critical for several projects we're working on.
Some projects can't upgrade to React 19 anytime soon due to legacy dependencies that may never support it. Since React 19 is still recent, many packages lack support, so upgrading isn't an option yet.
If React 19 becomes the only target, a major version bump would likely be needed to avoid breaking existing setups.
Appreciate the careful consideration!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the input. And I agree - We shouldn't just break React 18. I supported React 15 and 16 until last year.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Files not reviewed (2)
- biome.json: Language not supported
- package.json: Language not supported
Comments suppressed due to low confidence (3)
src/useInView.tsx:72
- [nitpick] The use of '!initialInView' to determine the current in-view state can be confusing. Consider renaming the variable or adding a comment to clarify that the state toggles based on the initial value and the trigger configuration.
const inView = !initialInView;
src/observe.ts:121
- The explicit type cast here might be masking potential type issues. Consider refactoring the types or adjusting the function signature to avoid the need for casting.
callbacks.push(callback as ObserverInstanceCallback);
src/useOnInView.tsx:123
- [nitpick] Consider rephrasing this comment to explain the rationale behind converting the threshold to a string for dependency stability, ensuring it's clear why this deviation is necessary.
// We break the rule here, because we aren't including the actual `threshold` variable
|
just fixed another bug on this branch the following code executes all callbacks for the given dom element: elements.get(entry.target)?.forEach((callback) => {
callback(inView, entry);
});however if callback calls |
|
great efforts and looking forward for this being merged 👍 |

Hello! I have created a PR that uses React 19's new ref callback cleanup functionality to simplify the implementation of this library for better performance
Background
React 19 introduced cleanup functions for ref callbacks. This allows us to handle both attaching and detaching observers in one place without needing separate useEffect hooks and state management
What's Changed
useInViewto use ref callback cleanup instead ofuseEffectuseOnInViewChangedhook which doesn't trigger re-renders (great for analytics/impression tracking)IntersectionObserversupportSize Improvements
The changes result in slightly smaller bundle size although it exposes an additional hook:
Breaking Changes
This quite an update and I hope you are fine with these two rather opinionated changes:
IntersectionObserverto be available (has been supported in all major browsers for 5+ years now)Why I Made These Changes
The new implementation is not only smaller but also has better performance since it:
useOnInViewChanged)All tests are passing with these changes. I did remove the tests that were specifically for the fallback functionality
What do you think? I'm happy to adjust the implementation if you have any concerns or ideas