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

Calling refetch does not immediately change networkStatus #7842

Open
david-arteaga opened this issue Mar 14, 2021 · 3 comments
Open

Calling refetch does not immediately change networkStatus #7842

david-arteaga opened this issue Mar 14, 2021 · 3 comments

Comments

@david-arteaga
Copy link

Intended outcome:
The networkStatus value that comes from the useQuery hook should immediately change to NetworkStatus.refetch as soon as the refetch function is called.

Actual outcome:
There is some delay between when the refetch function is called and the networkStatus value is updated to NetworkStatus.refetch.

RPReplay_Final1615761941.MP4

How to reproduce the issue:
I've created a snack to show visualize the error:
https://snack.expo.io/@david-arteaga/apollo-client-pull-to-refresh

Initially, I am not using the networkStatus value to calculate if the query is refetching.
In this case there is no issue with the "pull to refresh" functionality.
Everything works as expected.

If you tap on the toggle and start using networkStatus === NetworkStatus.refetch to calculate if the query is refetching, you can see that the "pull to refresh" animation is janky and makes the content jump up and down.
I found the same effect can be achieved if when onRefresh is called, instead of updating the refreshing value on RefreshControl immediately, you add a setTimeout(() => setIsRefreshing(true)), even without a time, you get the same result.
So my guess is that a process tick is going by between the time the refetch function is called and the time networkStatus is updated.

This is what causes the RefreshControl to end up with a janky behavior, which is not ideal.

Versions

  System:
    OS: macOS 11.2.2
  Binaries:
    Node: 14.14.0 - ~/.nvm/versions/node/v14.14.0/bin/node
    Yarn: 1.22.4 - ~/.yarn/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v14.14.0/bin/npm
  Browsers:
    Chrome: 88.0.4324.192
    Firefox: 86.0
    Safari: 14.0.3
  npmPackages:
    @apollo/client: 3.3.11 => 3.3.11
@dylanwulf
Copy link
Contributor

I believe I'm also seeing this issue in my tests. Many of my tests look like this (using jest and react testing library):

fireEvent.click(refetchButton);
expect(screen.queryByRole('progressbar')).toBeInTheDocument();

but now that refetch doesn't immediately set the loading state/networkStatus, many of my tests are now broken.

@andreialecu
Copy link
Contributor

Also hitting this. Here's a workaround that is easy to copy/paste, based on @david-arteaga's repro:

import {ObservableQuery} from '@apollo/client';
import React from 'react';

type RefetchFnType<TData, TVariables> = ObservableQuery<
  TData,
  TVariables
>['refetch'];

export function useRefetch<TData, TVariables>(
  refetch: RefetchFnType<TData, TVariables>,
): [RefetchFnType<TData, TVariables>, boolean] {
  const [refetching, setRefetching] = React.useState(false);
  const refetchWithLoading: RefetchFnType<
    TData,
    TVariables
  > = React.useCallback(
    (...args) => {
      setRefetching(true);
      return refetch(...args).finally(() => {
        setRefetching(false);
      });
    },
    [refetch],
  );

  return [refetchWithLoading, refetching];
}

Used like:

  const {loading, data, refetch: _refetch, error} = useListNotificationsQuery({...});
  const [refetch, isRefetching] = useRefetch(_refetch);

  // call refetch() to start a refetch, and check isRefetching for its status

I believe this may also reduce the number of re-renders required, because notifyOnNetworkStatusChange is no longer necessary.

@Shaninnik
Copy link

Was pulling my hair out with janky refresh controls, took me some time to find the actual source of this behaviour and find this issue. @andreialecu solution works perfect.

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

No branches or pull requests

5 participants