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.

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

Window Focus Refetching #3312

Closed
redbar0n opened this issue Jan 6, 2021 · 15 comments
Closed

Window Focus Refetching #3312

redbar0n opened this issue Jan 6, 2021 · 15 comments
Labels

Comments

@redbar0n
Copy link

redbar0n commented Jan 6, 2021

Rising libraries such as SWR or React Query support focus refetching, meaning that the client will not poll if the user has switched to a different tab or window, and it will immediately fetch again if the user comes back to the tab. This saves a lot of wasted requests, while maintaining a great UX.

According to this and this then it seems like Relay Modern doesn't support Window Focus Refetching. I couldn't find official information/discussion about it from Relay, so I thought this issue would be a good place to have that, and for anyone else looking for this feature in Relay.

Above quote is from the similar request for Apollo Client. It has details and example code on how SWR and React Query implement window focus refetching, with a code sandbox example showing it, too. That might help for inspiration and/or discussion on a potential concrete implementation: apollographql/apollo-feature-requests#247

Does Window Focus Refetching make sense for Relay Modern? If so, is it already baked in (contrary to the above comparisons)? If not, is it on the roadmap? If not, why not?

@sibelius
Copy link
Contributor

sibelius commented Jan 6, 2021

can't you handle this in userland?

@redbar0n
Copy link
Author

redbar0n commented Jan 6, 2021

I'm sure you can. But I'm not sure you'd want to.

@sibelius
Copy link
Contributor

sibelius commented Jan 6, 2021

Relay has a polling option on cacheConfig

https://ollie.relph.me/relay-modern-easy-polling/

and this #2174

@redbar0n
Copy link
Author

redbar0n commented Jan 6, 2021

Yeah, but I don't think it's the same. The React Query vs Relay Modern comparison page (still just an unmerged PR) differentiates between them as "Polling/Intervals" vs. "Window Focus Refetching". Similarly on the URQL vs Relay page.

@sibelius
Copy link
Contributor

sibelius commented Jan 6, 2021

const useWindowFocusRelayFetching = (relay) => {
  const pollInterval = 5000;

  const isDocumentVisible = () =>
    typeof document === "undefined" ||
    document.visibilityState === undefined ||
    document.visibilityState === "visible";

  const launchQuery = () => {
    if (isDocumentVisible()) {
      console.log(`query ${debug}`);
      relay.refetch();
    }
  };

  useInterval(launchQuery, pollInterval);

  // TODO - stop interval based on visibility
  document.addEventListener("visibilitychange", () => startPollingSequence());
}

what about a hook for this?

@josephsavona
Copy link
Contributor

Hmm, my instinct is that window focus and network status integration are things that should happen in the user-provided network layer, not the framework. By default Relay only does work in response to user events or new data arriving via the network (the only exception I can think of is the cacheConfig polling option, which again still goes through the network layer to actually execute the poll), so the network layer is a single point to pause network queries for any criteria (lost window focus, lost network connection, etc).

Having a user-space helper library for composing an existing network fetch function w window focus / network connectivity integration would be pretty cool, and if it was well done, OSS, etc we would happily link to it from the docs.

@redbar0n
Copy link
Author

redbar0n commented Jan 6, 2021

If the cacheConfig polling option is on, and the user has switched to a different tab or window, would it continuously consume resources by polling the network layer? If so, maybe it would be better to cap it at the origin?

Immediately fetching again, if the user comes back to the tab, seems like it ought to be decided in userland and configurable in the network layer (since some apps might not want that, for UX/network purposes).

@josephsavona
Copy link
Contributor

I'd be inclined to make the cacheConfig polling option be something that the core framework doesn't know about at all, and leave it up to the user-provided network layer to optionally interpret that parameter. That would mean Relay itself wouldn't automatically issue any fetches for polling purposes, and the network layer could then choose to stop polling when focus/network was lost.

@zth
Copy link
Contributor

zth commented Jan 7, 2021

Yeah, this does seem like something that belongs in user land. The functionality would also differ over platforms (web vs React Native). With that said, something like a community driven "Relay utils" lib, with this and other useful hooks, would be really nice. What do you think about that type of solution @redbar0n ? That's an initiative we could put together as the community.

@redbar0n
Copy link
Author

redbar0n commented Jan 7, 2021

@zth Yeah, good point. That "Relay utils" library seems to already exist: react-relay-network-modern. I brought forward this discussion to them already, when it became apparent that's where it belonged, in this issue. As you can see, the library author isn't using Relay atm, so hasn't got the time to implement it, but he would happily accept a PR. I haven't got the experience with Relay and Network Layers to do it myself. But maybe @sibelius would be willing to submit his aforementioned code in a PR there? Possibly as a hook, but ideally as a simple config to the Network Layer.

@aleksandrlat
Copy link

@sibelius could you please answer what is relay.refetch in your comment above #3312 (comment)?
Does it refetch all queries in app?

@sibelius
Copy link
Contributor

if you use a createPaginationContainer or createRefetchContainer relay will inject a relay refetch prop on it

you could check the typescript types to understand more about it

@aleksandrlat
Copy link

Thank you @sibelius! Got it
I thought it is some global refetch that re-fetches all queries in app at once.

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 9, 2022
@sibelius
Copy link
Contributor

Migrate to relay hooks, if the issue still valid, open another issue

this should be a solution in user land

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants