feat(client): null safe by default

BREAKING CHANGE: Null Safety
zino-hofmann · Mar 6, 2021 · 802185a · 802185a
1 parent 9194596
commit 802185a
Show file tree

Hide file tree

Showing 17 changed files with 70 additions and 163 deletions.
diff --git a/packages/graphql/lib/src/cache/_normalizing_data_proxy.dart b/packages/graphql/lib/src/cache/_normalizing_data_proxy.dart
@@ -85,7 +85,7 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         typePolicies: typePolicies,
         dataIdFromObject: dataIdFromObject,
         returnPartialData: returnPartialData,
-        addTypename: addTypename ?? false,
+        addTypename: addTypename,
         // if there is partial data, we cannot read and return null
         handleException: true,
         // provided from request
@@ -104,7 +104,7 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         typePolicies: typePolicies,
         dataIdFromObject: dataIdFromObject,
         returnPartialData: returnPartialData,
-        addTypename: addTypename ?? false,
+        addTypename: addTypename,
         // if there is partial data, we cannot read and return null
         handleException: true,
         // provided from request
@@ -127,7 +127,7 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         typePolicies: typePolicies,
         dataIdFromObject: dataIdFromObject,
         acceptPartialData: acceptPartialData,
-        addTypename: addTypename ?? false,
+        addTypename: addTypename,
         // provided from request
         document: request.operation.document,
         operationName: request.operation.operationName,
@@ -163,7 +163,7 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         typePolicies: typePolicies,
         dataIdFromObject: dataIdFromObject,
         acceptPartialData: acceptPartialData,
-        addTypename: addTypename ?? false,
+        addTypename: addTypename,
         // provided from request
         document: request.fragment.document,
         idFields: request.idFields,

diff --git a/packages/graphql/lib/src/cache/fragment.dart b/packages/graphql/lib/src/cache/fragment.dart
@@ -21,7 +21,7 @@ class Fragment {
   const Fragment({
     required this.document,
     this.fragmentName,
-  }) : assert(document != null);
+  });
 
   List<Object?> _getChildren() => [
         document,
@@ -77,8 +77,7 @@ class FragmentRequest {
     required this.fragment,
     required this.idFields,
     this.variables = const <String, dynamic>{},
-  })  : assert(fragment != null),
-        assert(idFields != null);
+  });
 
   List<Object> _getChildren() => [
         fragment,

diff --git a/packages/graphql/lib/src/core/_base_options.dart b/packages/graphql/lib/src/core/_base_options.dart
@@ -1,5 +1,4 @@
 import 'package:graphql/src/core/_data_class.dart';
-import 'package:meta/meta.dart';
 
 import 'package:gql/ast.dart';
 import 'package:gql_exec/gql_exec.dart';
@@ -27,7 +26,7 @@ abstract class BaseOptions extends MutableDataClass {
         context = context ?? Context();
 
   /// Document containing at least one [OperationDefinitionNode]
-  DocumentNode? document;
+  DocumentNode document;
 
   /// Name of the executable definition
   ///
@@ -57,7 +56,7 @@ abstract class BaseOptions extends MutableDataClass {
   /// Resolve these options into a request
   Request get asRequest => Request(
         operation: Operation(
-          document: document!,
+          document: document,
           operationName: operationName,
         ),
         variables: variables,
@@ -76,7 +75,7 @@ abstract class BaseOptions extends MutableDataClass {
 
   OperationType get type {
     final definitions =
-        document!.definitions.whereType<OperationDefinitionNode>().toList();
+        document.definitions.whereType<OperationDefinitionNode>().toList();
     if (operationName != null) {
       definitions.removeWhere(
         (node) => node.name!.value != operationName,

diff --git a/packages/graphql/lib/src/core/_query_write_handling.dart b/packages/graphql/lib/src/core/_query_write_handling.dart
@@ -1,5 +1,4 @@
 import 'package:graphql/client.dart';
-import 'package:meta/meta.dart';
 
 import 'package:gql_exec/gql_exec.dart';
 

diff --git a/packages/graphql/lib/src/core/fetch_more.dart b/packages/graphql/lib/src/core/fetch_more.dart
@@ -1,7 +1,6 @@
 import 'dart:async';
 
 import 'package:graphql/client.dart';
-import 'package:meta/meta.dart';
 
 import 'package:graphql/src/core/query_manager.dart';
 import 'package:graphql/src/core/query_options.dart';
@@ -26,17 +25,10 @@ Future<QueryResult> fetchMoreImplementation(
   String? queryId,
 }) async {
   // fetch more and udpate
-  assert(fetchMoreOptions.updateQuery != null);
 
-  final document = (fetchMoreOptions.document ?? originalOptions.document!);
+  final document = (fetchMoreOptions.document ?? originalOptions.document);
   final request = originalOptions.asRequest;
 
-  assert(
-    document != null,
-    'Either fetchMoreOptions.document '
-    'or the previous QueryOptions must be supplied!',
-  );
-
   final combinedOptions = QueryOptions(
     fetchPolicy: FetchPolicy.noCache,
     errorPolicy: originalOptions.errorPolicy,
@@ -56,12 +48,6 @@ Future<QueryResult> fetchMoreImplementation(
       fetchMoreResult.data,
     )!;
 
-    assert(
-      data != null,
-      'updateQuery result cannot be null:\n'
-      '  previousResultData: ${previousResult.data},\n'
-      ' fetchMoreResultData: ${fetchMoreResult.data}',
-    );
     fetchMoreResult.data = data;
 
     if (originalOptions.fetchPolicy != FetchPolicy.noCache) {

diff --git a/packages/graphql/lib/src/core/mutation_options.dart b/packages/graphql/lib/src/core/mutation_options.dart
@@ -1,4 +1,5 @@
 // ignore_for_file: deprecated_member_use_from_same_package
+import 'dart:async';
 import 'package:graphql/src/cache/cache.dart';
 import 'package:graphql/src/core/_base_options.dart';
 import 'package:graphql/src/core/observable_query.dart';
@@ -11,18 +12,16 @@ import 'package:graphql/src/core/query_result.dart';
 import 'package:graphql/src/utilities/helpers.dart';
 import 'package:graphql/src/core/policies.dart';
 
-typedef OnMutationCompleted = void Function(dynamic data);
-typedef OnMutationUpdate = void Function(
+typedef OnMutationCompleted = FutureOr<void> Function(dynamic data);
+typedef OnMutationUpdate = FutureOr<void> Function(
   GraphQLDataProxy cache,
   QueryResult? result,
 );
-typedef OnError = void Function(OperationException? error);
+typedef OnError = FutureOr<void> Function(OperationException? error);
 
 class MutationOptions extends BaseOptions {
   MutationOptions({
-    DocumentNode? document,
-    @Deprecated('Use `document` instead. Will be removed in 5.0.0')
-        DocumentNode? documentNode,
+    required DocumentNode document,
     String? operationName,
     Map<String, dynamic> variables = const {},
     FetchPolicy? fetchPolicy,
@@ -33,15 +32,11 @@ class MutationOptions extends BaseOptions {
     this.onCompleted,
     this.update,
     this.onError,
-  })  : assert(
-          (document ?? documentNode) != null,
-          'document must not be null',
-        ),
-        super(
+  }) : super(
           fetchPolicy: fetchPolicy,
           errorPolicy: errorPolicy,
           cacheRereadPolicy: cacheRereadPolicy,
-          document: document ?? documentNode,
+          document: document,
           operationName: operationName,
           variables: variables,
           context: context,
@@ -67,14 +62,12 @@ class MutationCallbackHandler {
     required this.options,
     required this.cache,
     required this.queryId,
-  })  : assert(cache != null),
-        assert(options != null),
-        assert(queryId != null);
+  });
 
   // callbacks will be called against each result in the stream,
   // which should then rebroadcast queries with the appropriate optimism
-  Iterable<OnData?> get callbacks =>
-      <OnData?>[onCompleted, update, onError].where(notNull);
+  Iterable<OnData> get callbacks =>
+      <OnData?>[onCompleted, update, onError].where(notNull).cast<OnData>();
 
   // Todo: probably move this to its own class
   OnData? get onCompleted {
@@ -132,7 +125,7 @@ class MutationCallbackHandler {
       final OnData optimisticUpdate = _optimisticUpdate;
 
       // wrap update logic to handle optimism
-      void updateOnData(QueryResult? result) {
+      FutureOr<void> updateOnData(QueryResult? result) {
         if (result!.isOptimistic) {
           return optimisticUpdate(result);
         } else {

diff --git a/packages/graphql/lib/src/core/observable_query.dart b/packages/graphql/lib/src/core/observable_query.dart
@@ -217,8 +217,6 @@ class ObservableQuery {
   ///
   /// To mitigate this, [FetchMoreOptions.partial] has been provided.
   Future<QueryResult> fetchMore(FetchMoreOptions fetchMoreOptions) async {
-    assert(fetchMoreOptions.updateQuery != null);
-
     addResult(QueryResult.loading(data: latestResult?.data));
 
     return fetchMoreImplementation(

diff --git a/packages/graphql/lib/src/core/policies.dart b/packages/graphql/lib/src/core/policies.dart
@@ -147,9 +147,7 @@ class Policies {
     FetchPolicy this.fetch,
     ErrorPolicy this.error,
     CacheRereadPolicy this.cacheReread,
-  )   : assert(fetch != null, 'fetch policy must be specified'),
-        assert(error != null, 'error policy must be specified'),
-        assert(cacheReread != null, 'cacheReread policy must be specified');
+  );
 
   Policies withOverrides([Policies? overrides]) => Policies.safe(
         overrides?.fetch ?? fetch!,

diff --git a/packages/graphql/lib/src/core/query_manager.dart b/packages/graphql/lib/src/core/query_manager.dart
@@ -68,7 +68,8 @@ class QueryManager {
     // Add optimistic or cache-based result to the stream if any
     if (options.optimisticResult != null) {
       // TODO optimisticResults for streams just skip the cache for now
-      yield QueryResult.optimistic(data: options.optimisticResult as Map<String, dynamic>?);
+      yield QueryResult.optimistic(
+          data: options.optimisticResult as Map<String, dynamic>?);
     } else if (options.fetchPolicy != FetchPolicy.noCache) {
       final cacheResult = cache.readQuery(
         request,
@@ -136,10 +137,10 @@ class QueryManager {
       queryId: '0',
     );
 
-    final Iterable<void Function(QueryResult)?> callbacks = mutationCallbacks.callbacks;
+    final callbacks = mutationCallbacks.callbacks;
 
     for (final callback in callbacks) {
-      await callback!(result);
+      await callback(result);
     }
 
     /// [fetchQuery] attempts to broadcast from the observable,
@@ -155,7 +156,7 @@ class QueryManager {
   ) async {
     final MultiSourceResult allResults =
         fetchQueryAsMultiSourceResult(queryId, options);
-    return (await (allResults.networkResult as FutureOr<QueryResult>?)) ?? allResults.eagerResult;
+    return allResults.networkResult ?? allResults.eagerResult;
   }
 
   /// Wrap both the `eagerResult` and `networkResult` future in a `MultiSourceResult`
@@ -186,7 +187,7 @@ class QueryManager {
 
   /// Resolve the query on the network,
   /// negotiating any necessary cache edits / optimistic cleanup
-  Future<QueryResult?> _resolveQueryOnNetwork(
+  Future<QueryResult> _resolveQueryOnNetwork(
     Request request,
     String queryId,
     BaseOptions options,

diff --git a/packages/graphql/lib/src/core/query_options.dart b/packages/graphql/lib/src/core/query_options.dart
@@ -1,7 +1,6 @@
 // ignore_for_file: deprecated_member_use_from_same_package
 import 'package:graphql/src/core/_base_options.dart';
 import 'package:graphql/src/utilities/helpers.dart';
-import 'package:meta/meta.dart';
 
 import 'package:gql/ast.dart';
 import 'package:gql_exec/gql_exec.dart';
@@ -12,9 +11,7 @@ import 'package:graphql/src/core/policies.dart';
 /// Query options.
 class QueryOptions extends BaseOptions {
   QueryOptions({
-    DocumentNode? document,
-    @Deprecated('Use `document` instead. Will be removed in 5.0.0')
-        DocumentNode? documentNode,
+    required DocumentNode document,
     String? operationName,
     Map<String, dynamic> variables = const {},
     FetchPolicy? fetchPolicy,
@@ -23,15 +20,11 @@ class QueryOptions extends BaseOptions {
     Object? optimisticResult,
     this.pollInterval,
     Context? context,
-  })  : assert(
-          (document ?? documentNode) != null,
-          'document must not be null',
-        ),
-        super(
+  }) : super(
           fetchPolicy: fetchPolicy,
           errorPolicy: errorPolicy,
           cacheRereadPolicy: cacheRereadPolicy,
-          document: document ?? documentNode,
+          document: document,
           operationName: operationName,
           variables: variables,
           context: context,
@@ -52,7 +45,7 @@ class QueryOptions extends BaseOptions {
         errorPolicy: errorPolicy,
         cacheRereadPolicy: cacheRereadPolicy,
         pollInterval: pollInterval,
-        fetchResults: fetchResults ?? true,
+        fetchResults: fetchResults,
         context: context,
         optimisticResult: optimisticResult,
       );
@@ -85,9 +78,7 @@ class SubscriptionOptions extends BaseOptions {
 
 class WatchQueryOptions extends QueryOptions {
   WatchQueryOptions({
-    required DocumentNode? document,
-    @Deprecated('Use `document` instead. Will be removed in 5.0.0')
-        DocumentNode? documentNode,
+    required DocumentNode document,
     String? operationName,
     Map<String, dynamic> variables = const {},
     FetchPolicy? fetchPolicy,
@@ -99,13 +90,9 @@ class WatchQueryOptions extends QueryOptions {
     this.carryForwardDataOnException = true,
     bool? eagerlyFetchResults,
     Context? context,
-  })  : assert(
-          (document ?? documentNode) != null,
-          'document must not be null',
-        ),
-        eagerlyFetchResults = eagerlyFetchResults ?? fetchResults,
+  })  : eagerlyFetchResults = eagerlyFetchResults ?? fetchResults,
         super(
-          document: document ?? documentNode,
+          document: document,
           operationName: operationName,
           variables: variables,
           fetchPolicy: fetchPolicy,
@@ -155,13 +142,10 @@ class WatchQueryOptions extends QueryOptions {
 /// To mitigate this, [FetchMoreOptions.partial] has been provided.
 class FetchMoreOptions {
   FetchMoreOptions({
-    DocumentNode? document,
-    @Deprecated('Use `document` instead. Will be removed in 5.0.0')
-        DocumentNode? documentNode,
+    required this.document,
     this.variables = const {},
     required this.updateQuery,
-  })  : assert(updateQuery != null),
-        this.document = document ?? documentNode;
+  });
 
   /// Automatically merge the results of [updateQuery] into `previousResultData`.
   ///