Apollo-client: Infinite Loop using graphql.macro loader inline after upgradring from 3.0.2 to 3.1.2

[ivoiv]

Intended outcome:

After upgrading from @apollo/client 3.0.2 to 3.1.2 I'm suddenly getting infinites loops while using inline-loading with graphql.macro.

I understand this might be more of an issue with graphql.macro than with @apollo/client but I'm wondering why this was n not an issue in 3.0.2 and earlier version.

Was the query parameter memoized by @apollo/client before but no longer?

This causes an infinite loop

import { loader } from 'graphql.macro';

const { loading, data } = useQuery(loader('./CountriesQuery.gql'), { fetchPolicy: "network-only" });

This causes an infinite loop

import { loader } from 'graphql.macro';

function SomeComponent() {
   ...
   const query = loader('./CountriesQuery.gql')
   const { loading, data } = useQuery(query, { fetchPolicy: "network-only" });
   ...
   return...
}

This is fine

import { loader } from 'graphql.macro';

const query = loader('./CountriesQuery.gql')

function SomeComponent() {
   ...
   const { loading, data } = useQuery(query, { fetchPolicy: "network-only" });
   ...
   return...
}

This is fine

import { gql } from '@apollo/client';

const query = gql`
  query{
     countries{
       name
     }
  }
`
const { loading, data } = useQuery(query, { fetchPolicy: "network-only" });

-->

Actual outcome:

How to reproduce the issue:
Repo with the issue:
https://codesandbox.io/s/dark-lake-6o4um

Versions
System:
OS: macOS 10.15.5
Binaries:
Node: 12.18.2 - /usr/local/bin/node
Yarn: 1.13.0 - /usr/local/bin/yarn
npm: 6.14.5 - /usr/local/bin/npm
Browsers:
Chrome: 84.0.4147.105
Firefox: 78.0.2
Safari: 13.1.1
npmPackages:
@apollo/client: 3.1.2 => 3.1.2

[benjamn]

@ivoiv Given the version range and your use of network-only, I'm guessing this is a consequence of #6712. Can you try the following code?

useQuery(query, {
  fetchPolicy: "network-only",
  nextFetchPolicy: "cache-first",
})

More generally, I think it's actually a good idea to declare your queries outside of your components, so that you don't end up reparsing/recreating the query every time the component renders. The specific DocumentNode object reference matters for some of the internal caching that we do, so pumping fresh (!==) query objects into useQuery can have negative performance consequences.

[ivoiv]

@benjamn adding nextFetchPolicy: 'cache-first' does indeed solve the issue.

I agree that declaring the query inside the component is generally unnecessary, but this was an edge-case where we needed to load 'random' non-predefined queries during runtime.

While it makes sense that changing the object reference would trigger the query all over, I wonder why this wasn't the case pre-3.1.0.

[benjamn]

this was an edge-case where we needed to load 'random' non-predefined queries during runtime

Totally reasonable, then!

Using different queries each time is more of an issue for the performance of repeated cache reads. I don't think it's the reason for the infinite loop, since useQuery should be able to use the same query ID each time you call useQuery from the same component (and the query ID is not sensitive to the exact query object reference). Instead, I think the fetchPolicy change in v3.1.0 is the primary source of this issue, so I'm glad to hear that nextFetchPolicy helps.

[andrewnaeve]

@benjamn I'm also experiencing this with graphql-tag.macro, although I'm using like so

import gql from 'graphql-tag.macro';

export const GET_THING = gql`
  query {
    getThing(stuff: "thing" {
      id
    }
  }
`;

Is it safe to apply this nextFetchPolicy: 'cache-first' as a general fix in all areas with cache-and-network as a policy?

[ShawnToubeau]

Hey @benjamn I was running into the same issue with the infinite loop described above and using nextFetchPolicy: "cache-first" worked. My team and I followed the ApolloClient guide here to setup our project but because that option wasn't set in the first code snippet we spent a considerable amount of time debugging until we stumbled upon this issue. Would it be alright if we updated the code snipped to include that option and potentially save others from the same pitfall?
Thanks!