Skip to content

feat: refactor useInView to utilize useOnInView and useCallback #742

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.

Sign up for GitHub

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

Open
thebuilder wants to merge 1 commit into
base: main
Choose a base branch
from
+70 −110
Open

feat: refactor useInView to utilize useOnInView and useCallback #742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,7 @@ Provide these as the options argument in the `useInView` hook or as props on the
| **initialInView** | `boolean` | `false` | Set the initial value of the `inView` boolean. This can be used if you expect the element to be in the viewport to start with, and you want to trigger something when it leaves. |
| **fallbackInView** | `boolean` | `undefined` | If the `IntersectionObserver` API isn't available in the client, the default behavior is to throw an Error. You can set a specific fallback behavior, and the `inView` value will be set to this instead of failing. To set a global default, you can set it with the `defaultFallbackInView()` |

`useOnInView` accepts the same options as `useInView` except `onChange`,
`initialInView`, and `fallbackInView`.
`useOnInView` accepts the same options as `useInView` except `onChange`.
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation now states that useOnInView accepts the same options as useInView except onChange, but the type definition at line 93 in src/index.tsx shows that IntersectionEffectOptions is now identical to IntersectionOptions, which includes the onChange property.

This creates a discrepancy between the documentation and the actual type definition. Either the type should exclude onChange (as it did before), or the documentation should be updated to reflect that onChange is technically allowed in the options (though the first parameter onIntersectionChange should be used instead).

Copilot uses AI. Check for mistakes.

### InView Props

Expand Down
7 changes: 6 additions & 1 deletion src/__tests__/useOnInView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { render } from "@testing-library/react";
import { useCallback, useEffect, useState } from "react";
import * as React from "react";
import type { IntersectionEffectOptions } from "..";
import { supportsRefCleanup } from "../reactVersion";
import { intersectionMockInstance, mockAllIsIntersecting } from "../test-utils";
import { useOnInView } from "../useOnInView";

const { useCallback, useEffect, useState } = React;
const OnInViewChangedComponent = ({
options,
unmount,
Expand Down Expand Up @@ -309,6 +311,9 @@ const MultipleCallbacksComponent = ({
const mergedRefs = useCallback(
(node: Element | null) => {
const cleanup = [ref1(node), ref2(node), ref3(node)];
if (!supportsRefCleanup) {
return;
}
return () =>
cleanup.forEach((fn) => {
fn?.();
Expand Down
5 changes: 1 addition & 4 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,4 @@ export type InViewHookResponse = [
entry?: IntersectionObserverEntry;
};

export type IntersectionEffectOptions = Omit<
IntersectionOptions,
"onChange" | "fallbackInView" | "initialInView"
>;
export type IntersectionEffectOptions = IntersectionOptions;
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IntersectionEffectOptions type now includes the onChange property from IntersectionOptions, but useOnInView already has onIntersectionChange as its first parameter. This creates ambiguity - users could potentially pass both onIntersectionChange as the first argument and onChange in the options object.

The original design where IntersectionEffectOptions omitted onChange was intentional to prevent this confusion. Consider reverting to Omit<IntersectionOptions, "onChange"> or adding runtime logic to handle/validate when both are provided.

Suggested change
export type IntersectionEffectOptions = IntersectionOptions;
export type IntersectionEffectOptions = Omit<IntersectionOptions, "onChange">;

Copilot uses AI. Check for mistakes.
5 changes: 5 additions & 0 deletions src/reactVersion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as React from "react";

const major = Number.parseInt(React.version?.split(".")[0] ?? "", 10);
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version parsing logic assumes that React.version is always a string in the format "major.minor.patch", but if React.version is undefined or an empty string, split(".")[0] will return an empty string, which when passed to Number.parseInt will result in NaN. While the code handles NaN correctly by checking Number.isFinite, this could be more defensive.

Consider adding a check to ensure React.version exists before attempting to parse it, or provide a more explicit default value for better clarity.

Suggested change
const major = Number.parseInt(React.version?.split(".")[0] ?? "", 10);
const reactVersion = React.version;
const major =
typeof reactVersion === "string" && reactVersion.length > 0
? Number.parseInt(reactVersion.split(".")[0], 10)
: NaN;

Copilot uses AI. Check for mistakes.
// NaN => unknown version; default to false to avoid returning ref cleanup on <19.
export const supportsRefCleanup = Number.isFinite(major) && major >= 19;
140 changes: 43 additions & 97 deletions src/useInView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from "react";
import type { IntersectionOptions, InViewHookResponse } from "./index";
import { observe } from "./observe";
import { supportsRefCleanup } from "./reactVersion";
import { useOnInView } from "./useOnInView";

type State = {
inView: boolean;
Expand Down Expand Up @@ -33,116 +34,61 @@ type State = {
* };
* ```
*/
export function useInView({
threshold,
delay,
trackVisibility,
rootMargin,
root,
triggerOnce,
skip,
initialInView,
fallbackInView,
onChange,
}: IntersectionOptions = {}): InViewHookResponse {
const [ref, setRef] = React.useState<Element | null>(null);
const callback = React.useRef<IntersectionOptions["onChange"]>(onChange);
const lastInViewRef = React.useRef<boolean | undefined>(initialInView);
export function useInView(
options: IntersectionOptions = {},
): InViewHookResponse {
const [state, setState] = React.useState<State>({
inView: !!initialInView,
inView: !!options.initialInView,
entry: undefined,
});
const optionsRef = React.useRef(options);
optionsRef.current = options;
const entryTargetRef = React.useRef<Element | undefined>(undefined);

// Store the onChange callback in a `ref`, so we can access the latest instance
// inside the `useEffect`, but without triggering a rerender.
callback.current = onChange;

// biome-ignore lint/correctness/useExhaustiveDependencies: threshold is not correctly detected as a dependency
React.useEffect(
() => {
if (lastInViewRef.current === undefined) {
lastInViewRef.current = initialInView;
}
// Ensure we have node ref, and that we shouldn't skip observing
if (skip || !ref) return;

let unobserve: (() => void) | undefined;
unobserve = observe(
ref,
(inView, entry) => {
const previousInView = lastInViewRef.current;
lastInViewRef.current = inView;

// Ignore the very first `false` notification so consumers only hear about actual state changes.
if (previousInView === undefined && !inView) {
return;
}
const inViewRef = useOnInView((inView, entry) => {
entryTargetRef.current = entry.target;
setState({ inView, entry });
if (optionsRef.current.onChange) {
optionsRef.current.onChange(inView, entry);
}
}, options);
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useOnInView hook is called with options directly on line 54, but options is a new object reference on every render when passed inline (e.g., useInView({ threshold: 0.5 })). This will cause useOnInView to return a new ref callback on every render because its dependency array (lines 147-157 in useOnInView.tsx) will see different values.

While the optionsRef pattern is used to avoid issues with onChange callback changes, the same approach is not applied to the options passed to useOnInView. This could lead to unnecessary re-creation of IntersectionObservers and potential performance issues. Consider memoizing the options or using a similar ref pattern for the options passed to useOnInView.

Suggested change
}, options);
}, optionsRef.current);

Copilot uses AI. Check for mistakes.

const refCallback = React.useCallback(
(node: Element | null) => {
const resetIfNeeded = () => {
const {
skip,
triggerOnce,
initialInView: latestInitialInView,
} = optionsRef.current;
if (!skip && !triggerOnce && entryTargetRef.current) {
setState({
inView,
entry,
inView: !!latestInitialInView,
entry: undefined,
});
if (callback.current) callback.current(inView, entry);
entryTargetRef.current = undefined;
}
};
const cleanup = inViewRef(node);

if (entry.isIntersecting && triggerOnce && unobserve) {
// If it should only trigger once, unobserve the element after it's inView
unobserve();
unobserve = undefined;
}
},
{
root,
rootMargin,
threshold,
// @ts-expect-error
trackVisibility,
delay,
},
fallbackInView,
);
if (!node) {
resetIfNeeded();
return;
}

if (!supportsRefCleanup) {
return;
}
Comment on lines +79 to +81
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In React versions prior to 19, this callback returns undefined for non-null nodes, which means the cleanup logic in resetIfNeeded() will never be called when the element is unmounted in React 17/18. This is because React 17/18 doesn't call ref callbacks with null on unmount, and the cleanup function is not returned.

The original implementation handled this by using a useEffect that tracked the ref changes and cleaned up appropriately. With this new approach, the reset logic may not execute properly in React 17/18 when components unmount, potentially leaving stale state.

Copilot uses AI. Check for mistakes.

return () => {
if (unobserve) {
unobserve();
}
cleanup?.();
resetIfNeeded();
};
},
// We break the rule here, because we aren't including the actual `threshold` variable
// eslint-disable-next-line react-hooks/exhaustive-deps
[
// If the threshold is an array, convert it to a string, so it won't change between renders.
Array.isArray(threshold) ? threshold.toString() : threshold,
ref,
root,
rootMargin,
triggerOnce,
skip,
trackVisibility,
fallbackInView,
delay,
],
[inViewRef],
);
Comment on lines +88 to 89
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refCallback only includes inViewRef in its dependency array, but it also reads from optionsRef.current (lines 59-63) and entryTargetRef.current (line 64). While these are refs and their .current values can change without triggering re-creation of the callback, this could potentially cause issues.

Specifically, if options like skip, triggerOnce, or initialInView change between renders, the resetIfNeeded logic will use the latest values from optionsRef.current, but the inViewRef callback won't be updated because it was created with the old options. This could lead to inconsistent behavior where the reset logic uses different option values than the observation logic.

Copilot uses AI. Check for mistakes.

const entryTarget = state.entry?.target;
const previousEntryTarget = React.useRef<Element | undefined>(undefined);
if (
!ref &&
entryTarget &&
!triggerOnce &&
!skip &&
previousEntryTarget.current !== entryTarget
) {
// If we don't have a node ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`)
// This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView
previousEntryTarget.current = entryTarget;
setState({
inView: !!initialInView,
entry: undefined,
});
lastInViewRef.current = initialInView;
}

const result = [setRef, state.inView, state.entry] as InViewHookResponse;
const result = [refCallback, state.inView, state.entry] as InViewHookResponse;

// Support object destructuring, by adding the specific values.
result.ref = result[0];
Expand Down
20 changes: 14 additions & 6 deletions src/useOnInView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
IntersectionEffectOptions,
} from "./index";
import { observe } from "./observe";
import { supportsRefCleanup } from "./reactVersion";

const useSyncEffect =
(
Expand Down Expand Up @@ -55,18 +56,21 @@ export const useOnInView = <TElement extends Element>(
delay,
triggerOnce,
skip,
initialInView,
fallbackInView,
Comment on lines +59 to +60
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useOnInView hook now supports initialInView and fallbackInView options (lines 59-60), but there are no tests in the test file to verify this new functionality works correctly.

Tests should be added to ensure:

  1. The callback behaves correctly when initialInView: true is set
  2. The callback behaves correctly when initialInView: false is set
  3. The fallbackInView option is properly passed through to the observe function
  4. The initial state tracking in lastInViewRef works as expected with these options

Copilot uses AI. Check for mistakes.
}: IntersectionEffectOptions = {},
) => {
const onIntersectionChangeRef = React.useRef(onIntersectionChange);
const initialInViewValue = initialInView ? true : undefined;
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initialInViewValue is set to true when initialInView is truthy, but undefined otherwise. However, this doesn't correctly handle the case when initialInView is explicitly set to false.

When initialInView is false, the intention is to start with inView: false, but initialInViewValue will be set to undefined instead of false. This means the initial state and behavior won't match the user's expectation.

Consider changing the logic to: const initialInViewValue = initialInView ?? undefined; to properly preserve the false value when explicitly set.

Suggested change
const initialInViewValue = initialInView ? true : undefined;
const initialInViewValue = initialInView ?? undefined;

Copilot uses AI. Check for mistakes.
const observedElementRef = React.useRef<TElement | null>(null);
const observerCleanupRef = React.useRef<(() => void) | undefined>(undefined);
const lastInViewRef = React.useRef<boolean | undefined>(undefined);
const lastInViewRef = React.useRef<boolean | undefined>(initialInViewValue);

useSyncEffect(() => {
onIntersectionChangeRef.current = onIntersectionChange;
}, [onIntersectionChange]);

// biome-ignore lint/correctness/useExhaustiveDependencies: Threshold arrays are normalized inside the callback
// biome-ignore lint/correctness/useExhaustiveDependencies: threshold array handled inside
return React.useCallback(
(element: TElement | undefined | null) => {
// React <19 never calls ref callbacks with `null` during unmount, so we
Expand All @@ -80,19 +84,20 @@ export const useOnInView = <TElement extends Element>(
};

if (element === observedElementRef.current) {
return observerCleanupRef.current;
return supportsRefCleanup ? observerCleanupRef.current : undefined;
}

if (!element || skip) {
cleanupExisting();
observedElementRef.current = null;
lastInViewRef.current = undefined;
return;
lastInViewRef.current = initialInViewValue;
return undefined;
}

cleanupExisting();

observedElementRef.current = element;
lastInViewRef.current = initialInViewValue;
Comment on lines +93 to +100
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting lastInViewRef.current to initialInViewValue when an element is being observed (line 100) may not be correct. The lastInViewRef is used to track the previous intersection state to determine if a state change has occurred. By resetting it to initialInViewValue on every new element observation, you're potentially losing the actual last intersection state.

This could cause the logic at line 110 ("Ignore the very first false notification") to behave incorrectly when re-observing elements or when the ref changes to a different element. The lastInViewRef should only be initialized once, not reset every time an element is observed.

Suggested change
lastInViewRef.current = initialInViewValue;
return undefined;
}
cleanupExisting();
observedElementRef.current = element;
lastInViewRef.current = initialInViewValue;
lastInViewRef.current = undefined;
return undefined;
}
cleanupExisting();
observedElementRef.current = element;
lastInViewRef.current = undefined;

Copilot uses AI. Check for mistakes.
let destroyed = false;

const destroyObserver = observe(
Expand Down Expand Up @@ -121,6 +126,7 @@ export const useOnInView = <TElement extends Element>(
trackVisibility,
delay,
} as IntersectionObserverInit,
fallbackInView,
);

function stopObserving() {
Expand All @@ -136,7 +142,7 @@ export const useOnInView = <TElement extends Element>(

observerCleanupRef.current = stopObserving;

return observerCleanupRef.current;
return supportsRefCleanup ? observerCleanupRef.current : undefined;
},
[
Array.isArray(threshold) ? threshold.toString() : threshold,
Expand All @@ -146,6 +152,8 @@ export const useOnInView = <TElement extends Element>(
delay,
triggerOnce,
skip,
initialInViewValue,
fallbackInView,
],
);
};
Loading