From ae8355a0875b8e533f0ed823279f63fb9d05f0d5 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@apollographql.com>
Date: Wed, 4 Aug 2021 13:21:23 -0400
Subject: [PATCH] Determine queryInfo.queryId in QueryInfo constructor.

This refactoring ensures `observableQuery.queryId` ends up the same as
`observableQuery.queryInfo.queryId`, which is relevant in cases when we
choose a specific query ID string before creating the `QueryInfo` and
`ObservableQuery` objects.

Ensuring that `queryInfo.queryId === queryInfo.observableQuery.queryId`
(another way of saying the same thing) also fixes the `invariant`
failures I deliberately introduced in the previous commit.

This inconsistency between IDs was the cause of issue #8586, a double
registration bug for mutation `refetchQueries` specified using the
legacy one-time `refetchQueries: [{ query, variables }]` style.

Another (recommended) way to work around that problem would be to
specify `refetchQueries: [query]` instead, to select the existing query
by its `DocumentNode` rather than creating and deleting a new one-time
legacy query.
---
 src/core/ObservableQuery.ts |  2 +-
 src/core/QueryInfo.ts       | 10 +++++++++-
 src/core/QueryManager.ts    |  4 ++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
index 0ca64231bb8..b51ddb7b6c5 100644
--- a/src/core/ObservableQuery.ts
+++ b/src/core/ObservableQuery.ts
@@ -129,7 +129,7 @@ export class ObservableQuery<
 
     // query information
     this.options = options;
-    this.queryId = queryManager.generateQueryId();
+    this.queryId = queryInfo.queryId || queryManager.generateQueryId();
 
     const opDef = getOperationDefinition(options.query);
     this.queryName = opDef && opDef.name && opDef.name.value;
diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts
index 6f89ea9ebab..ea23f374a79 100644
--- a/src/core/QueryInfo.ts
+++ b/src/core/QueryInfo.ts
@@ -17,6 +17,7 @@ import {
   isNetworkRequestInFlight,
 } from './networkStatus';
 import { ApolloError } from '../errors';
+import { QueryManager } from './QueryManager';
 
 export type QueryStoreValue = Pick<QueryInfo,
   | "variables"
@@ -85,7 +86,14 @@ export class QueryInfo {
   graphQLErrors?: ReadonlyArray<GraphQLError>;
   stopped = false;
 
-  constructor(private cache: ApolloCache<any>) {
+  private cache: ApolloCache<any>;
+
+  constructor(
+    queryManager: QueryManager<any>,
+    public readonly queryId = queryManager.generateQueryId(),
+  ) {
+    const cache = this.cache = queryManager.cache;
+
     // Track how often cache.evict is called, since we want eviction to
     // override the feud-stopping logic in the markResult method, by
     // causing shouldWrite to return true. Wrapping the cache.evict method
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
index 66f1fc49bc4..83036477cde 100644
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -619,7 +619,7 @@ export class QueryManager<TStore> {
       options.notifyOnNetworkStatusChange = false;
     }
 
-    const queryInfo = new QueryInfo(this.cache);
+    const queryInfo = new QueryInfo(this);
     const observable = new ObservableQuery<T, TVariables>({
       queryManager: this,
       queryInfo,
@@ -1470,7 +1470,7 @@ export class QueryManager<TStore> {
 
   private getQuery(queryId: string): QueryInfo {
     if (queryId && !this.queries.has(queryId)) {
-      this.queries.set(queryId, new QueryInfo(this.cache));
+      this.queries.set(queryId, new QueryInfo(this, queryId));
     }
     return this.queries.get(queryId)!;
   }