Fetching data again and again if more queries without id executed

[vhenzl]

The problem started after upgrading from beta 45 to beta 48 and is related to missing id in queries.

Intended outcome:
I have two components, displaying a list of items, each has its own query. The queries are very similar and differ only in query arguments.

function MyRecentIncidents() {
  const {loading, error, data} = useQuery<MyRecentIncidentsQuery>(MY_RECENT_INCIDENTS);
  console.log('[Query 1] loading:', loading, 'error:', error, 'data:', data);
  return null;
}

const MY_RECENT_INCIDENTS = gql`
  query MyRecentIncidents {
    viewer {
      incidents(first: 5, createdByViewer: true) {
        nodes {
          id
        }
      }
    }
  }
`;

function RecentIncidents() {
  const {loading, error, data} = useQuery<RecentIncidentsQuery>(RECENT_INCIDENTS);
  console.log('[Query 2] loading:', loading, 'error:', error, 'data:', data);
  return null;
}

const RECENT_INCIDENTS = gql`
  query RecentIncidents {
    viewer {
      incidents(first: 5) {
        nodes {
          id
        }
      }
    }
  }
`;

const client = new ApolloClient({
  cache: new InMemoryCache(),
  uri: process.env.REACT_APP_API_GRAPHQL_URL,
});

export default function App() {
  return (
    <ApolloProvider client={client}>
      <MyRecentIncidents/>
      <RecentIncidents/>
    </ApolloProvider>
  );
}

It should fetch each query just once, render items and doesn't do anything more.

Actual outcome:
After initial queries complete, a new request to the server is started. Once completed, the other query starts. Then the first one. And again and again…

However, loading status doesn't change, the components don't rerender. It just fetches data in the background.

My observations:
The code above worked in beta 45, the problem occurs in beta 48+. I didn't try 46 and 47. It's still present in the just-released beta 49.

Querying id field on viewer like

const RECENT_INCIDENTS = gql`
  query RecentIncidents {
    viewer {
      id # <--- this fixes it
      incidents(first: 5) {
        nodes {
          id
        }
      }
    }
  }
`;

in both queries fixes the problem.

Versions

  System:
    OS: Windows 10 10.0.18363
  Binaries:
    Node: 12.13.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.21.1 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.12.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.18362.449.0
  npmPackages:
    @apollo/client: ^3.0.0-beta.48 => 3.0.0-beta.48 
    @apollo/link-context: ^2.0.0-beta.3 => 2.0.0-beta.3 
    @apollo/link-error: ^2.0.0-beta.3 => 2.0.0-beta.3 

[peterhanneman]

Ran into the same issue. I've narrowed it down to being introduced in v3.0.0-beta.46.

[benjamn]

@vhenzl Can you try this?

new InMemoryCache({
  typePolicies: {
    // Assuming the __typename for the viewer object is "Viewer":
    Viewer: {
      // This means the Viewer object is a singleton whose identity
      // is independent of any of its fields (other than __typename),
      // so the cache can assume that any two Viewer objects are merely
      // different views of the same object, and safely merge their
      // fields rather than just replacing one with the other, which is
      // what seems to be happening right now. This is different from
      // keyFields: false in that it allows the Viewer singleton object
      // to be normalized, like other entity objects, rather than treating
      // it as unidentified data.
      keyFields: [],
    },
  },
})

You could also write keyFields: ["id"] if there are going to be multiple different Viewer objects within the same application, and the cache will enforce that the id field is always present.

[vhenzl]

@benjamn Thanks for the answer. Unfortunately, I can't use the trick with keyFields: [] as viewer is User and can appear again anywhere in the tree. So the only way for me is enforcing id on User.

Is this new behaviour considered a bug or is it intended, expected?

If it's expected, my guess is the same problem can occur for any type anywhere in the query, not just for the top-level fields. Right?

Then the safest way how to prevent it would be to enforce querying id field for all types that have it. And possibly autogenerate the whole typePolicies object from the schema with a tool like @graphql-codegen. Would you agree?

[ambientlight]

most likely a bug, local mutations doing cache.modify will also cause backend queries dispatched (with id),
like:

export const toggleComments = (
    root: {},
    variables: {
        id: string,
        commentsOpenColumnIndex: number | null
    },
    { cache }: LocalResolverContext<NormalizedCacheObject, {}>
) => {
    const id = cache.identify({
        __typename: 'ReportType',
        id: variables.id
    });

    if(id) {
        cache.modify( {
            _commentsOpenColumnIndex: _currentValue => variables.commentsOpenColumnIndex
        }, id)
        return true
    } else {
        return false
    }
}

also working fine on 3.0.0-beta.45

[trevordebard]

This issue exists for me in beta 50. Adding ID to the graphql query also resolved the bug.

[benjamn]

Without a reproduction I can't be sure, but @apollo/[email protected] should fix the endless network request loop, thanks to #6448.

[peterhanneman]

@benjamn - Your efforts are greatly appreciated however I have tested both @apollo/[email protected] as well as the latest @apollo/[email protected] but both unfortunately still exhibit the same erroneous re-fetch behavior.

The proper fix in my instance has been to pass a custom function when instantiating the cache to parse the __typename and/or structure of the cache object to derive the primary key for the associated object. e.g. user_id from __typename = users.

function deriveCacheIdFromObject(obj) {
  switch(obj.__typename) { ... }
}

new InMemoryCache({
  addTypename: true,
  dataIdFromObject: deriveCacheIdFromObject,
}).restore(initialState || {})

The horrible bodge solution is simply to sweep the issue under the rug and continue using @apollo/[email protected].

Alternatively I wonder if simply aliasing every query to include a generic id value would also serve as a stopgap solution?