feat: Introduce RouterLink component to prefetch links as they enter the viewport #11293

olerichter00 · 2024-12-17T14:43:37Z

This PR resolves ONYX-1461

Description

This PR introduces a RouterLink component that wraps Palette's Touchable and takes an extra to prop with a URL or path to link to.

The reason for introducing the new RouterLink component is to be able to prefetch (all) app links as they enter the viewport, similar to how it's implemented in Force (artsy/force#14520 & artsy/force#14454).

If prefetching is enabled, the component automatically prefetches the URL with Relay as soon as it enters the viewport.

The new RouterLink component will replace PrefetchFlatList, which is also capable of prefetching links as they enter the viewport, but is limited and cannot be used as universally as the new link component.

Next Steps:

Use the new RouterLink component in more places
Make routes ready to be prefetched by adding the main query to routes.ts

Screenshots / Videos

Bildschirmaufnahme.2025-01-09.um.10.54.00.mov

PR Checklist

I have tested my changes on iOS and Android.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

Prefetch links as they enter the viewport - ole

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

src/app/Components/ArtworkGrids/ArtworkGridItem.tsx

MounirDhahri · 2024-12-17T17:22:54Z

src/app/Components/RouterLink.tsx

+
+  return (
+    <ElementInView onVisible={handleVisible}>
+      <Touchable {...restProps} onPress={handlePress} />


I wonder here if Touchable would be our preferred way, sometimes we use TouchableOpacity and sometimes we use TouchableWithoutHighlight.
Anyway, that's a problem for later, we can start with this now and extend it later.

Maybe this is a call to use RouterTouchableOpacity 🤷

I used the Touchable component from "@artsy/palette-mobile" and figured this is probably the preferred one. It has a prop to adjust the opacity on press.

Yeah, RouterTouchableOpacity or RouterTouchable could also be good names for this component. I took the name RouterLink from Force but haven't thought much about naming yet. I'm very open for ideas for the naming.

ArtsyOpenSource · 2025-01-09T09:52:46Z

This PR contains the following changes:

Cross-platform user-facing changes (Prefetch links as they enter the viewport - ole - olerichter00)

Generated by 🚫 dangerJS against 3a7712b

olerichter00 · 2025-01-09T10:01:53Z

src/app/system/navigation/RouterLink.tsx

+  to: string | null | undefined
+}
+
+export const RouterLink: React.FC<RouterLinkProps & TouchableProps> = ({


I've named the component RouterLink, as it is called in Force (routerlink.ts). But I am open to other suggestions regarding the naming. Another name I find quite good for this component is AppLink.

MounirDhahri

Thanks for this addition. I left few suggestions - I would leave to use to decide if they're worth the effort

MounirDhahri · 2025-01-10T09:40:37Z

src/app/utils/queryPrefetching.ts

@@ -46,6 +46,7 @@ const prefetchRoute = async <TQuery extends OperationType>(
  const allVariables = { ...result.params, ...variables }

  return queries.map((query) => {
+    console.log("[queryPrefetching] Prefetching:", route, JSON.stringify(allVariables))


Thanks! This doesn't need to be here. Just used it for debugging.

MounirDhahri · 2025-01-10T09:40:47Z

src/app/utils/queryPrefetching.ts

@@ -8,7 +8,7 @@ import { fetchQuery, GraphQLTaggedNode } from "react-relay"
 import { OperationType, Variables, VariablesOf } from "relay-runtime"
 import { logPrefetching } from "./loggers"

-const DEFAULT_QUERIES_PER_INTERVAL = 60
+const DEFAULT_QUERIES_PER_INTERVAL = 180


MounirDhahri · 2025-01-10T09:46:34Z

src/app/system/navigation/RouterLink.tsx

+  const handlePress = (event: GestureResponderEvent) => {
+    onPress?.(event)
+
+    if (!to) return
+
+    if (passProps) {
+      navigate(to, { passProps })
+    } else {
+      navigate(to)
+    }
+  }


thought: I don't have strong opinions about your approach here but I would have approached this differently for the benefit of more flexibility.
I would have left handlePress={props.onPress} to allow us to have full flexibility about taps and to be able as well to disable router links without much code added and handle the navigate event from within the component that uses RouterLink. And, I would use to only for prefetching the url.

That's a very interesting thought! I approached it this way to simplify how we implement links (similar to the web) while still having the full flexibility of onPress.

Do you have an example from the app where we would not be able to use the new to prop?

If it's just an edge case, then the workaround could be to do the navigation in onPress and not use to and not prefetch

MounirDhahri · 2025-01-10T09:48:27Z

src/app/system/navigation/RouterLink.tsx

+  }
+
+  return (
+    <ElementInView onVisible={handleVisible}>


A lot of the computation gets triggered here. I would also suggest to wrap the component behind the same enableViewPortPrefetching feature flag.
Maybe something like
Wrapper = enableViewPortPrefetching ? ElementInView : Touchable (maybe not exactly like that)

This is a good idea and could be useful if the change affects performance too much 👍

MounirDhahri · 2025-01-10T09:48:42Z

src/app/system/navigation/RouterLink.tests.tsx

beautiful 👏

artsy-peril bot added the Jira Synced label Dec 17, 2024

olerichter00 marked this pull request as draft December 17, 2024 14:44

olerichter00 self-assigned this Dec 17, 2024

damassi reviewed Dec 17, 2024

View reviewed changes

src/app/Components/ArtworkGrids/ArtworkGridItem.tsx Outdated Show resolved Hide resolved

MounirDhahri reviewed Dec 17, 2024

View reviewed changes

olerichter00 force-pushed the olerichter00/ONYX-1461/prefetch-all-links-as-they-enter-the-viewport branch from f26a274 to 2cfaa76 Compare January 3, 2025 13:35

olerichter00 added 5 commits January 9, 2025 10:13

feat: Prefetch all links as they enter the viewport

cc4ad57

tests

cb968e2

optiona pass props

e19f6bf

remove ff

1c9d870

move component

d93009a

olerichter00 force-pushed the olerichter00/ONYX-1461/prefetch-all-links-as-they-enter-the-viewport branch from 1104fed to d93009a Compare January 9, 2025 09:14

olerichter00 changed the title ~~feat: Prefetch all links as they enter the viewport~~ feat: Introduce RouterLink component to prefetch links as they enter the viewport Jan 9, 2025

olerichter00 marked this pull request as ready for review January 9, 2025 09:50

refactor passProps logic

9e354b2

olerichter00 force-pushed the olerichter00/ONYX-1461/prefetch-all-links-as-they-enter-the-viewport branch from 346d855 to 9e354b2 Compare January 9, 2025 09:57

olerichter00 commented Jan 9, 2025

View reviewed changes

olerichter00 requested review from araujobarret, gkartalis, brainbicycle, MrSltun and dariakoko January 9, 2025 10:20

fix tests

3a7712b

olerichter00 force-pushed the olerichter00/ONYX-1461/prefetch-all-links-as-they-enter-the-viewport branch from fbbd649 to 3a7712b Compare January 9, 2025 14:46

MounirDhahri approved these changes Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce RouterLink component to prefetch links as they enter the viewport #11293

feat: Introduce RouterLink component to prefetch links as they enter the viewport #11293

olerichter00 commented Dec 17, 2024 •

edited

Loading

MounirDhahri Dec 17, 2024

MounirDhahri Dec 17, 2024

olerichter00 Dec 17, 2024

ArtsyOpenSource commented Jan 9, 2025 •

edited

Loading

olerichter00 Jan 9, 2025 •

edited

Loading

MounirDhahri left a comment

MounirDhahri Jan 10, 2025

olerichter00 Jan 10, 2025

MounirDhahri Jan 10, 2025

MounirDhahri Jan 10, 2025

olerichter00 Jan 10, 2025

MounirDhahri Jan 10, 2025

olerichter00 Jan 10, 2025

MounirDhahri Jan 10, 2025

feat: Introduce RouterLink component to prefetch links as they enter the viewport #11293

Are you sure you want to change the base?

feat: Introduce RouterLink component to prefetch links as they enter the viewport #11293

Conversation

olerichter00 commented Dec 17, 2024 • edited Loading

Description

Next Steps:

Screenshots / Videos

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtsyOpenSource commented Jan 9, 2025 • edited Loading

This PR contains the following changes:

olerichter00 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

MounirDhahri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olerichter00 commented Dec 17, 2024 •

edited

Loading

ArtsyOpenSource commented Jan 9, 2025 •

edited

Loading

olerichter00 Jan 9, 2025 •

edited

Loading