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.

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

Jump to bottom

feat: add prefetch="viewport" support to <Link> #6433

Merged
merged 10 commits into from
Jun 20, 2023
Merged

feat: add prefetch="viewport" support to <Link> #6433

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e24f65d
feat: initial viewport prefetching
mcansh May 19, 2023
331e8b3
feat: viewport prefetching
mcansh May 19, 2023
4520f0f
chore: remove debug console
mcansh May 19, 2023
acd4645
Create tidy-bears-turn.md
mcansh May 19, 2023
ee74d1b
chore: remove prefetch links when scrolled out of vp
mcansh May 19, 2023
fab2cce
Update changeset
brophdawg11 Jun 15, 2023
44f28f3
Only attach ref when prefetch=viewport is specified
brophdawg11 Jun 15, 2023
c3e9ded
Docs
brophdawg11 Jun 15, 2023
219057f
Add E2E test
brophdawg11 Jun 15, 2023
748d6a0
Revert "Only attach ref when prefetch=viewport is specified"
brophdawg11 Jun 15, 2023
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
6 changes: 6 additions & 0 deletions .changeset/tidy-bears-turn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"remix": patch
"@remix-run/react": patch
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
---

adds support for viewport prefetching to `<Link>`. this allows you to prefetch a page when it gets into view via an [intersection observer](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserver)
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
50 changes: 41 additions & 9 deletions packages/remix-react/components.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export function RemixRouteError({ id }: { id: string }) {
* - "render": Fetched when the link is rendered
* - "none": Never fetched
*/
type PrefetchBehavior = "intent" | "render" | "none";
type PrefetchBehavior = "intent" | "render" | "none" | "viewport";

export interface RemixLinkProps extends LinkProps {
prefetch?: PrefetchBehavior;
Expand All @@ -219,19 +219,35 @@ interface PrefetchHandlers {
onTouchStart?: TouchEventHandler;
}

function usePrefetchBehavior(
function usePrefetchBehavior<T extends HTMLAnchorElement>(
prefetch: PrefetchBehavior,
theirElementProps: PrefetchHandlers
): [boolean, Required<PrefetchHandlers>] {
): [boolean, React.RefObject<T>, Required<PrefetchHandlers>] {
let [maybePrefetch, setMaybePrefetch] = React.useState(false);
let [shouldPrefetch, setShouldPrefetch] = React.useState(false);
let { onFocus, onBlur, onMouseEnter, onMouseLeave, onTouchStart } =
theirElementProps;

let ref = React.useRef<T>(null);

React.useEffect(() => {
if (prefetch === "render") {
setShouldPrefetch(true);
}

if (prefetch === "viewport") {
let callback: IntersectionObserverCallback = (entries) => {
entries.forEach((entry) => {
setShouldPrefetch(entry.isIntersecting);
});
};
Comment on lines +239 to +243
Copy link
Member

Choose a reason for hiding this comment

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

Should the observer be disconnected after prefetch is set? That way the IntersectionObserver doesn't need to keep observing if ref.current is in the viewport

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mostly depends on if we should clean up link preloads when the link leaves the viewport. another thing would be that if it re-enters the vp, should it refetch the loader?

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing/re-adding link tags if an element scrolled out of and back into the viewport would be consistent with our prefetch="intent" behavior

let observer = new IntersectionObserver(callback, { threshold: 0.5 });
if (ref.current) observer.observe(ref.current);

return () => {
observer.disconnect();
};
}
}, [prefetch]);

let setIntent = () => {
Expand Down Expand Up @@ -260,6 +276,7 @@ function usePrefetchBehavior(

return [
shouldPrefetch,
ref,
{
onFocus: composeEventHandlers(onFocus, setIntent),
onBlur: composeEventHandlers(onBlur, cancelIntent),
Expand All @@ -282,17 +299,18 @@ let NavLink = React.forwardRef<HTMLAnchorElement, RemixNavLinkProps>(
let isAbsolute = typeof to === "string" && ABSOLUTE_URL_REGEX.test(to);

let href = useHref(to);
let [shouldPrefetch, prefetchHandlers] = usePrefetchBehavior(
let [shouldPrefetch, ref, prefetchHandlers] = usePrefetchBehavior(
prefetch,
props
);

return (
<>
<RouterNavLink
ref={forwardedRef}
to={to}
{...props}
{...prefetchHandlers}
ref={mergeRefs(forwardedRef, ref)}
to={to}
/>
{shouldPrefetch && !isAbsolute ? (
<PrefetchPageLinks page={href} />
Expand All @@ -315,18 +333,18 @@ let Link = React.forwardRef<HTMLAnchorElement, RemixLinkProps>(
let isAbsolute = typeof to === "string" && ABSOLUTE_URL_REGEX.test(to);

let href = useHref(to);
let [shouldPrefetch, prefetchHandlers] = usePrefetchBehavior(
let [shouldPrefetch, ref, prefetchHandlers] = usePrefetchBehavior(
prefetch,
props
);

return (
<>
<RouterLink
ref={forwardedRef}
to={to}
{...props}
{...prefetchHandlers}
ref={mergeRefs(forwardedRef, ref)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any internal react overhead in attaching a ref? This feels potentially troublesome since we now attach a ref to every Link, even if the user didn't forward a ref and if they're not using prefetch="viewport". Does a page with hundreds of links get immediately "slower" with no way for the user to opt out of the refs?

I took a stab at an opt-in ref approach in 44f28f3

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no perf hit per @jacob-ebey so going to back out that commit

to={to}
/>
{shouldPrefetch && !isAbsolute ? (
<PrefetchPageLinks page={href} />
Expand Down Expand Up @@ -1805,3 +1823,17 @@ export const LiveReload =
/>
);
};

function mergeRefs<T = any>(
...refs: Array<React.MutableRefObject<T> | React.LegacyRef<T>>
): React.RefCallback<T> {
return (value) => {
refs.forEach((ref) => {
if (typeof ref === "function") {
ref(value);
} else if (ref != null) {
(ref as React.MutableRefObject<T | null>).current = value;
}
});
};
}