Merge branch 'main' of https://github.com/zino-hofmann/graphql-flutter …

…into feat-add-devtools-extension

packages/graphql/lib/client.dart

@@ -9,4 +9,5 @@ export 'package:graphql/src/graphql_client.dart';
 
 export 'package:graphql/src/links/links.dart';
 
-export 'package:graphql/src/utilities/helpers.dart' show gql;
+export 'package:graphql/src/utilities/helpers.dart'
+    show gql, optimizedDeepEquals;

packages/graphql/lib/src/cache/cache.dart

@@ -204,9 +204,23 @@ class GraphQLCache extends NormalizingDataProxy {
   ///
   /// This allows for hierarchical optimism that is automatically cleaned up
   /// without having to tightly couple optimistic changes
+  ///
+  /// This is called on every network result as cleanup
   void removeOptimisticPatch(String removeId) {
+    final patchesToRemove = optimisticPatches
+        .where(
+          (patch) =>
+              patch.id == removeId || _parentPatchId(patch.id) == removeId,
+        )
+        .toList();
+
+    if (patchesToRemove.isEmpty) {
+      return;
+    }
+    // Only remove + mark broadcast requested if something was actually removed.
+    // This is to prevent unnecessary rebroadcasts
     optimisticPatches.removeWhere(
-      (patch) => patch.id == removeId || _parentPatchId(patch.id) == removeId,
+      (patch) => patchesToRemove.contains(patch),
     );
     broadcastRequested = true;
   }

packages/graphql/lib/src/cache/fragment.dart

@@ -1,10 +1,10 @@
 import 'dart:convert';
+import "package:graphql/client.dart";
 import "package:meta/meta.dart";
 import "package:collection/collection.dart";
 
 import "package:gql/ast.dart";
 import 'package:gql/language.dart';
-import "package:gql_exec/gql_exec.dart";
 import 'package:normalize/utils.dart';
 
 /// A fragment in a [document], optionally defined by [fragmentName]
@@ -32,9 +32,7 @@ class Fragment {
   bool operator ==(Object o) =>
       identical(this, o) ||
       (o is Fragment &&
-          const ListEquality<Object?>(
-            DeepCollectionEquality(),
-          ).equals(
+          gqlDeepEquals(
             o._getChildren(),
             _getChildren(),
           ));
@@ -89,9 +87,7 @@ class FragmentRequest {
   bool operator ==(Object o) =>
       identical(this, o) ||
       (o is FragmentRequest &&
-          const ListEquality<Object?>(
-            DeepCollectionEquality(),
-          ).equals(
+          gqlDeepEquals(
             o._getChildren(),
             _getChildren(),
           ));

packages/graphql/lib/src/core/_base_options.dart

@@ -104,9 +104,7 @@ abstract class BaseOptions<TParsed extends Object?> {
       identical(this, other) ||
       (other is BaseOptions &&
           runtimeType == other.runtimeType &&
-          const ListEquality<Object?>(
-            DeepCollectionEquality(),
-          ).equals(
+          gqlDeepEquals(
             other.properties,
             properties,
           ));

packages/graphql/lib/src/core/observable_query.dart

@@ -93,8 +93,19 @@ class ObservableQuery<TParsed> {
   /// call [queryManager.maybeRebroadcastQueries] after all other [_onDataCallbacks]
   ///
   /// Automatically appended as an [OnData]
-  FutureOr<void> _maybeRebroadcast(QueryResult? result) =>
-      queryManager.maybeRebroadcastQueries(exclude: this);
+  FutureOr<void> _maybeRebroadcast(QueryResult? result) {
+    if (_onDataCallbacks.isEmpty &&
+        result?.hasException == true &&
+        result?.data == null) {
+      // We don't need to rebroadcast if there was an exception and there was no
+      // data. It's valid GQL to have data _and_ exception. If options.carryForwardDataOnException
+      // are true, this condition may never get hit.
+      // If there are onDataCallbacks, it's possible they modify cache and are
+      // depending on maybeRebroadcastQueries being called.
+      return false;
+    }
+    return queryManager.maybeRebroadcastQueries(exclude: this);
+  }
 
   /// The most recently seen result from this operation's stream
   QueryResult<TParsed>? latestResult;
@@ -242,12 +253,13 @@ class ObservableQuery<TParsed> {
   }
 
   /// Add a [result] to the [stream] unless it was created
-  /// before [lasestResult].
+  /// before [latestResult].
   ///
   /// Copies the [QueryResult.source] from the [latestResult]
   /// if it is set to `null`.
   ///
-  /// Called internally by the [QueryManager]
+  /// Called internally by the [QueryManager]. Do not call this directly except
+  /// for [QueryResult.loading]
   void addResult(QueryResult<TParsed> result, {bool fromRebroadcast = false}) {
     // don't overwrite results due to some async/optimism issue
     if (latestResult != null &&

packages/graphql/lib/src/core/policies.dart

@@ -307,9 +307,7 @@ class DefaultPolicies {
   bool operator ==(Object o) =>
       identical(this, o) ||
       (o is DefaultPolicies &&
-          const ListEquality<Object?>(
-            DeepCollectionEquality(),
-          ).equals(
+          gqlDeepEquals(
             o._getChildren(),
             _getChildren(),
           ));

packages/graphql/lib/src/core/query_manager.dart

@@ -1,5 +1,6 @@
 import 'dart:async';
 
+import 'package:graphql/src/utilities/helpers.dart';
 import 'package:graphql/src/utilities/response.dart';
 import 'package:meta/meta.dart';
 import 'package:collection/collection.dart';
@@ -19,18 +20,29 @@ import 'package:graphql/src/scheduler/scheduler.dart';
 
 import 'package:graphql/src/core/_query_write_handling.dart';
 
-bool Function(dynamic a, dynamic b) _deepEquals =
-    const DeepCollectionEquality().equals;
+typedef DeepEqualsFn = bool Function(dynamic a, dynamic b);
+
+/// The equality function used for comparing cached and new data.
+///
+/// You can alternatively provide [optimizedDeepEquals] for a faster
+/// equality check. Or provide your own via [GqlClient] constructor.
+DeepEqualsFn gqlDeepEquals = const DeepCollectionEquality().equals;
 
 class QueryManager {
   QueryManager({
     required this.link,
     required this.cache,
     this.alwaysRebroadcast = false,
+    DeepEqualsFn? deepEquals,
+    bool deduplicatePollers = false,
   }) {
     scheduler = QueryScheduler(
       queryManager: this,
+      deduplicatePollers: deduplicatePollers,
     );
+    if (deepEquals != null) {
+      gqlDeepEquals = deepEquals;
+    }
   }
 
   final Link link;
@@ -311,14 +323,26 @@ class QueryManager {
       // we attempt to resolve the from the cache
       if (shouldRespondEagerlyFromCache(options.fetchPolicy) &&
           !queryResult.isOptimistic) {
-        final data = cache.readQuery(request, optimistic: false);
-        // we only push an eager query with data
-        if (data != null) {
-          queryResult = QueryResult(
-            options: options,
-            data: data,
+        final latestResult = _getQueryResultByRequest<TParsed>(request);
+        if (latestResult != null && latestResult.data != null) {
+          // we have a result already cached + deserialized for this request
+          // so we reuse it.
+          // latest result won't be for loading, it must contain data
+          queryResult = latestResult.copyWith(
             source: QueryResultSource.cache,
           );
+        } else {
+          // otherwise, we try to find the query in cache (which will require
+          // deserialization)
+          final data = cache.readQuery(request, optimistic: false);
+          // we only push an eager query with data
+          if (data != null) {
+            queryResult = QueryResult(
+              options: options,
+              data: data,
+              source: QueryResultSource.cache,
+            );
+          }
         }
 
         if (options.fetchPolicy == FetchPolicy.cacheOnly &&
@@ -358,6 +382,18 @@ class QueryManager {
     return queryResult;
   }
 
+  /// If a request already has a result associated with it in cache (as
+  /// determined by [ObservableQuery.latestResult]), we can return it without
+  /// needing to denormalize + parse again.
+  QueryResult<TParsed>? _getQueryResultByRequest<TParsed>(Request request) {
+    for (final query in queries.values) {
+      if (query.options.asRequest == request) {
+        return query.latestResult as QueryResult<TParsed>?;
+      }
+    }
+    return null;
+  }
+
   /// Refetch the [ObservableQuery] referenced by [queryId],
   /// overriding any present non-network-only [FetchPolicy].
   Future<QueryResult<TParsed>?> refetchQuery<TParsed>(String queryId) {
@@ -383,11 +419,11 @@ class QueryManager {
     return results;
   }
 
-  ObservableQuery<TParsed>? getQuery<TParsed>(String? queryId) {
-    if (!queries.containsKey(queryId)) {
+  ObservableQuery<TParsed>? getQuery<TParsed>(final String? queryId) {
+    if (!queries.containsKey(queryId) || queryId == null) {
       return null;
     }
-    final query = queries[queryId!];
+    final query = queries[queryId];
     if (query is ObservableQuery<TParsed>) {
       return query;
     }
@@ -402,16 +438,17 @@ class QueryManager {
   void addQueryResult<TParsed>(
     Request request,
     String? queryId,
-    QueryResult<TParsed> queryResult,
-  ) {
+    QueryResult<TParsed> queryResult, {
+    bool fromRebroadcast = false,
+  }) {
     final observableQuery = getQuery<TParsed>(queryId);
 
     if (observableQuery != null && !observableQuery.controller.isClosed) {
-      observableQuery.addResult(queryResult);
+      observableQuery.addResult(queryResult, fromRebroadcast: fromRebroadcast);
     }
   }
 
-  /// Create an optimstic result for the query specified by `queryId`, if it exists
+  /// Create an optimistic result for the query specified by `queryId`, if it exists
   QueryResult<TParsed> _getOptimisticQueryResult<TParsed>(
     Request request, {
     required String queryId,
@@ -463,27 +500,55 @@ class QueryManager {
       return false;
     }
 
-    final shouldBroadast = cache.shouldBroadcast(claimExecution: true);
+    final shouldBroadcast = cache.shouldBroadcast(claimExecution: true);
 
-    if (!shouldBroadast && !force) {
+    if (!shouldBroadcast && !force) {
       return false;
     }
 
-    for (var query in queries.values) {
-      if (query != exclude && query.isRebroadcastSafe) {
+    // If two ObservableQueries are backed by the same [Request], we only need
+    // to [readQuery] for it once.
+    final Map<Request, QueryResult<Object?>> diffQueryResultCache = {};
+    final Map<Request, bool> ignoreQueryResults = {};
+    for (final query in queries.values) {
+      final Request request = query.options.asRequest;
+      final cachedQueryResult = diffQueryResultCache[request];
+      if (query == exclude || !query.isRebroadcastSafe) {
+        continue;
+      }
+      if (cachedQueryResult != null) {
+        // We've already done the diff and denormalized, emit to the observable
+        addQueryResult(
+          request,
+          query.queryId,
+          cachedQueryResult,
+          fromRebroadcast: true,
+        );
+      } else if (ignoreQueryResults.containsKey(request)) {
+        // We've already seen this one and don't need to notify
+        continue;
+      } else {
+        // We haven't seen this one yet, denormalize from cache and diff
         final cachedData = cache.readQuery(
           query.options.asRequest,
           optimistic: query.options.policies.mergeOptimisticData,
         );
         if (_cachedDataHasChangedFor(query, cachedData)) {
-          query.addResult(
-            mapFetchResultToQueryResult(
-              Response(data: cachedData, response: {}),
-              query.options,
-              source: QueryResultSource.cache,
-            ),
+          // The data has changed
+          final queryResult = QueryResult(
+            data: cachedData,
+            options: query.options,
+            source: QueryResultSource.cache,
+          );
+          diffQueryResultCache[request] = queryResult;
+          addQueryResult(
+            request,
+            query.queryId,
+            queryResult,
             fromRebroadcast: true,
           );
+        } else {
+          ignoreQueryResults[request] = true;
         }
       }
     }
@@ -496,7 +561,8 @@ class QueryManager {
   ) =>
       cachedData != null &&
       query.latestResult != null &&
-      (alwaysRebroadcast || !_deepEquals(query.latestResult!.data, cachedData));
+      (alwaysRebroadcast ||
+          !gqlDeepEquals(query.latestResult!.data, cachedData));
 
   void setQuery(ObservableQuery<Object?> observableQuery) {
     queries[observableQuery.queryId] = observableQuery;