Merge branch 'ntepluhina-update-guidelines-for-apollo-3' into 'master'

Updated GraphQL frontend development guidelines for Apollo 3

See merge request gitlab-org/gitlab!80602

doc/development/fe_guide/graphql.md

@@ -107,9 +107,9 @@ Default client accepts two parameters: `resolvers` and `config`.
 
 ### Multiple client queries for the same object
 
-If you are make multiple queries to the same Apollo client object you might encounter the following error: "Store error: the application attempted to write an object with no provided ID but the store already contains an ID of SomeEntity". [This error only should occur when you have made a query with an ID field for a portion, then made another that returns what would be the same object, but is missing the ID field.](https://github.com/apollographql/apollo-client/issues/2510#issue-271829009)
+If you are making multiple queries to the same Apollo client object you might encounter the following error: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. We are already checking `ID` presence for every GraphQL type that has an `ID`, so this shouldn't be the case. Most likely, the `SomeEntity` type doesn't have an `ID` property, and to fix this warning we need to define a custom merge function.
 
-This is being tracked in [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/326101) and the documentation will be updated when this issue is resolved.
+We have some client-wide types with `merge: true` defined in the default client as [typePolicies](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/lib/graphql.js) (this means that Apollo will merge existing and incoming responses in the case of subsequent queries). Please consider adding `SomeEntity` there or defining a custom merge function for it.
 
 ## GraphQL Queries
 
@@ -667,9 +667,7 @@ apollo: {
 ```
 
 When we want to move to the next page, we use an Apollo `fetchMore` method, passing a
-new cursor (and, optionally, new variables) there. In the `updateQuery` hook, we have
-to return a result we want to see in the Apollo cache after fetching the next page.
-[`Immer`s `produce`](#immutability-and-cache-updates)-function can help us with the immutability here:
+new cursor (and, optionally, new variables) there.
 
 ```javascript
 fetchNextPage(endCursor) {
@@ -679,24 +677,114 @@ fetchNextPage(endCursor) {
       first: 10,
       after: endCursor,
     },
-    updateQuery(previousResult, { fetchMoreResult }) {
-      // Here we can implement the logic of adding new designs to existing ones
-      // (for example, if we use infinite scroll) or replacing old result
-      // with the new one if we use numbered pages
-
-      const { designs: previousDesigns } = previousResult.project.issue.designCollection;
-      const { designs: newDesigns } = fetchMoreResult.project.issue.designCollection
-
-      return produce(previousResult, draftData => {
-        // `produce` gives us a working copy, `draftData`, that we can modify
-        // as we please and from it will produce the next immutable result for us
-        draftData.project.issue.designCollection.designs = [...previousDesigns, ...newDesigns];
-      });
-    },
   });
 }
 ```
 
+##### Defining field merge policy
+
+We would also need to define a field policy to specify how do we want to merge the existing results with the incoming results. For example, if we have `Previous/Next` buttons, it makes sense to replace the existing result with the incoming one:
+
+```javascript
+const apolloProvider = new VueApollo({
+  defaultClient: createDefaultClient(
+    {},
+    {
+      cacheConfig: {
+        typePolicies: {
+          DesignCollection: {
+            fields: {
+              designs: {
+                merge(existing, incoming) {
+                  if (!incoming) return existing;
+                  if (!existing) return incoming;
+
+                  // We want to save only incoming nodes and replace existing ones
+                  return incoming
+                }
+              }
+            }
+          }
+        }
+      },
+    },
+  ),
+});
+```
+
+When we have an infinite scroll, it would make sense to add the incoming `designs` nodes to existing ones instead of replacing. In this case, merge function would be slightly different:
+
+```javascript
+const apolloProvider = new VueApollo({
+  defaultClient: createDefaultClient(
+    {},
+    {
+      cacheConfig: {
+        typePolicies: {
+          DesignCollection: {
+            fields: {
+              designs: {
+                merge(existing, incoming) {
+                  if (!incoming) return existing;
+                  if (!existing) return incoming;
+
+                  const { nodes, ...rest } = incoming;
+                  // We only need to merge the nodes array.
+                  // The rest of the fields (pagination) should always be overwritten by incoming
+                  let result = rest;
+                  result.nodes = [...existing.nodes, ...nodes];
+                  return result;
+                }
+              }
+            }
+          }
+        }
+      },
+    },
+  ),
+});
+```
+
+`apollo-client` [provides](https://github.com/apollographql/apollo-client/blob/212b1e686359a3489b48d7e5d38a256312f81fde/src/utilities/policies/pagination.ts)
+a few field policies to be used with paginated queries. Here's another way to achieve infinite
+scroll pagination with the `concatPagination` policy:
+
+```javascript
+import { concatPagination } from '@apollo/client/utilities';
+import Vue from 'vue';
+import VueApollo from 'vue-apollo';
+import createDefaultClient from '~/lib/graphql';
+
+Vue.use(VueApollo);
+
+export default new VueApollo({
+  defaultClient: createDefaultClient(
+    {},
+    {
+      cacheConfig: {
+        typePolicies: {
+          Project: {
+            fields: {
+              dastSiteProfiles: {
+                keyArgs: ['fullPath'], // You might need to set the keyArgs option to enforce the cache's integrity
+              },
+            },
+          },
+          DastSiteProfileConnection: {
+            fields: {
+              nodes: concatPagination(),
+            },
+          },
+        },
+      },
+    },
+  ),
+});
+```
+
+This is similar to the `DesignCollection` example above as new page results are appended to the
+previous ones.
+
 #### Using a recursive query in components
 
 When it is necessary to fetch all paginated data initially an Apollo query can do the trick for us.
@@ -816,7 +904,7 @@ const data = store.readQuery({
 });
 ```
 
-Read more about the `@connection` directive in [Apollo's documentation](https://www.apollographql.com/docs/react/v2/caching/cache-interaction/#the-connection-directive).
+Read more about the `@connection` directive in [Apollo's documentation](https://www.apollographql.com/docs/react/caching/advanced-topics/#the-connection-directive).
 
 ### Managing performance
 
@@ -1017,22 +1105,13 @@ apollo: {
           issuableId: convertToGraphQLId(this.issuableClass, this.issuableId),
         };
       },
-      // Describe how subscription should update the query
-      updateQuery(prev, { subscriptionData }) {
-        if (prev && subscriptionData?.data?.issuableAssigneesUpdated) {
-          const data = produce(prev, (draftData) => {
-            draftData.workspace.issuable.assignees.nodes =
-              subscriptionData.data.issuableAssigneesUpdated.assignees.nodes;
-          });
-          return data;
-        }
-        return prev;
-      },
     },
   },
 },
 ```
 
+We would need also to define a field policy similarly like we do it for the [paginated queries](#defining-field-merge-policy)
+
 ### Best Practices
 
 #### When to use (and not use) `update` hook in mutations
@@ -1808,6 +1887,16 @@ relative to `app/graphql/queries` folder: for example, if we need a
 `app/graphql/queries/repository/files.query.graphql` query, the path is
 `repository/files`.
 
+## Troubleshooting
+
+### Mocked client returns empty objects instead of mock response
+
+If your unit test is failing because response contains empty objects instead of mock data, you would need to add `__typename` field to the mocked response. This happens because mocked client (unlike the real one) does not populate the response with typenames and in some cases we need to do it manually so the client is able to recognize a GraphQL type.
+
+### Warning about losing cache data
+
+Sometimes you can see a warning in the console: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. Please check section about [multiple queries](#multiple-client-queries-for-the-same-object) to resolve an issue.
+
   ```yaml
   - current_route_path = request.fullpath.match(/-\/tree\/[^\/]+\/(.+$)/).to_a[1]
   - add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" })