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

Queries in a re-mounted component revert to using "fetchPolicy" instead of "nextFetchPolicy" #7311

Closed
dannycochran opened this issue Nov 11, 2020 · 4 comments

Comments

@dannycochran
Copy link
Contributor

dannycochran commented Nov 11, 2020

Intended outcome:

  1. I am using the following for my Apollo Client:
  defaultOptions: {
    watchQuery: {
      fetchPolicy: 'cache-and-network',
      nextFetchPolicy: 'cache-first',
    }
  }
  1. I would like to avoid queries re-firing when returning to a previously mounted component, since the query has already fired once and has the data in cache.

  2. I would expect that after the first query completes in the browser lifecycle, "nextFetchPolicy" is used thereafter in that query until the user re-loads the page.

Actual outcome:

  1. I use the default settings from above.

  2. Queries that fired within a component use cache-and-network to start, but once the component is unmounted (via a route change), and then returned to, cache-and-network is used again instead of cache-first.

  3. This results in the query re-firing, even though the data is already available in cache.

How to reproduce the issue:

Minimal reproduction repository and instructions can be found here:

https://github.com/dannycochran/usequery

git clone https://github.com/dannycochran/usequery
cd usequery
npm install
npm run start
  1. Click "Click me to unmount to another route"
  2. Click "Click me to go back to home and see the query re-fire"
  3. You'll see the "loading.." indicator, which indicates the query has re-fired.

Versions

System:
OS: macOS 10.15.6
Binaries:
Node: 12.18.4 - /usr/local/bin/node
Yarn: 1.22.4 - ~/npm-global/bin/yarn
npm: 6.14.6 - /usr/local/bin/npm
Browsers:
Chrome: 86.0.4240.193
Safari: 14.0
npmPackages:
@apollo/client: ^3.2.5 => 3.2.5
apollo-server-express: ^2.19.0 => 2.19.0
npmGlobalPackages:
apollo: 2.27.0

Disclaimer

If this bug is in fact intended behavior, I'd like to discuss what a good work around is for the behavior I want. Currently I'm wrapping useQuery and doing this hacky workaround which uses the query as stringified JSON to keep track of whether the query has already fired once.

const firstFetchCompleteMap = new Map<string, boolean>();

const getNextFetchPolicy = (
  lastPolicy: WatchQueryFetchPolicy = 'cache-first',
  nextFetchPolicy?: WatchQueryFetchPolicy | ((lastPolicy: WatchQueryFetchPolicy) => WatchQueryFetchPolicy),
): WatchQueryFetchPolicy => {
  if (typeof nextFetchPolicy === 'string') {
    return nextFetchPolicy;
  }
  if (nextFetchPolicy) {
    return nextFetchPolicy(lastPolicy);
  }
  return 'cache-first';
};

const useQueryHacky = <TData = any, TVariables = OperationVariables>(
  query: DocumentNode,
  options: QueryHookOptions<TData, TVariables>,
) => {
  const client = useApolloClient();
  const defaultOptions = client.defaultOptions;
  const currentCachePolicy = options?.fetchPolicy ?? defaultOptions.watchQuery?.fetchPolicy;
  const nextFetchPolicy =
    options?.nextFetchPolicy ?? getNextFetchPolicy(currentCachePolicy, defaultOptions.watchQuery?.nextFetchPolicy);
  const { persistNextFetchPolicyAcrossMounts } = defaultOptions.watchQuery ?? {};
  const uniqueKey = useMemo(() => (persistNextFetchPolicyAcrossMounts ? JSON.stringify(query) : ''), [
    query,
    persistNextFetchPolicyAcrossMounts,
  ]);
  const firstFetchDone = firstFetchCompleteMap.get(uniqueKey);

  if (persistNextFetchPolicyAcrossMounts && firstFetchDone) {
    Object.assign(options, { fetchPolicy: nextFetchPolicy });
  }

  const response = apolloUseQuery(query, options);
  const previousLoading = usePrevious(response.loading);
  useEffect(() => {
    if (previousLoading === true && response.loading === false && !firstFetchDone) {
      firstFetchCompleteMap.set(uniqueKey, true);
    }
  }, [response.loading, uniqueKey, firstFetchDone, previousLoading]);

  return response;
};
@dylanwulf
Copy link
Contributor

dylanwulf commented Apr 13, 2021

Hi, I have a question about this proposal. I have a paginated query which should be refetched every time the component mounts. Currently, I am doing this with fetchPolicy="network-only" and nextFetchPolicy="cache-first". I need the nextFetchPolicy="cache-first", because without that it will re-fetch the first page in between each new page. With your proposed changes, how would I go about this? Would I have to call refetch every time my component mounts?

@dannycochran
Copy link
Contributor Author

@dylanwulf the way I have it instrumented, it's opt-in behavior. It's configurable under defaultOptions.watchQuery as a global setting to persist the fetch policy across all your queries, but you can also set it on a per-query basis just like you can now with fetchPolicy / nextFetchPolicy. So you could configure it globally if you wanted this behavior for most of your queries, then disable it for the queries where it doesn't make sense.

@dylanwulf
Copy link
Contributor

@dannycochran ah ok that sounds good, thank you for the quick response!

@bignimbus
Copy link
Contributor

It appears this has been addressed in the linked issues. Please feel free to ping me if you feel this should be re-opened. Thanks all!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants