feat(graphql): add asyncDeepEquals to the graphqlClient

[Neelansh-ns]

Fixes / Enhancements

• Added asyncDeepEquals param to the GraphqlClient as the DeepCollectionEquality is an expensive operation and was running synchronously. This in turn would result in a jank whenever there is a response received and the equality check happens in the _cachedDataHasChangedFor function.
• To avoid the jank the developer can pass the async function for equality check, like compute() to move the operation to a different isolate.
• This also partially fixes Jank on every query #1196

Before

After

[vincenzopalazzo]

While Ci is running, can you please adjust the commit message according with https://github.com/zino-hofmann/graphql-flutter/blob/main/docs/dev/MAINTAINERS.md

Thanks!

[Neelansh-ns]

While Ci is running, can you please adjust the commit message according with https://github.com/zino-hofmann/graphql-flutter/blob/main/docs/dev/MAINTAINERS.md

Thanks!

Done, please review.

[vincenzopalazzo]

@Neelansh-ns before jumping in the review, any comment on the copilot review?

Thanks

[Neelansh-ns]

@Neelansh-ns before jumping in the review, any comment on the copilot review?

Thanks

Accepted one suggestion, the other can be ignored. Added the reasoning in the reply to the suggestion.

[vincenzopalazzo]

Cool, now the commit needs to be squashed by preserving the Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> inside the commit body

[Neelansh-ns]

Cool, now the commit needs to be squashed by preserving the Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> inside the commit body

Done!

[Neelansh-ns]

Hi @vincenzopalazzo, kindly review and let me know your thoughts. If all looks good, can we aim for pushing it to pub?

[vincenzopalazzo]

LGTM

[Copilot]

Pull Request Overview

This PR adds asynchronous deep equality comparison to the GraphQL client to address UI jank issues caused by expensive synchronous deep equality checks. The implementation allows developers to move the equality operations to a separate isolate using compute() to improve performance during cache rebroadcast operations.

• Introduces AsyncDeepEqualsFn type and asyncDeepEquals parameter to GraphQLClient constructor
• Converts maybeRebroadcastQueries to async maybeRebroadcastQueriesAsync throughout the codebase
• Updates all rebroadcast operations to use the new async equality checking mechanism

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
packages/graphql/lib/src/graphql_client.dart	Adds asyncDeepEquals parameter to constructor and copyWith method, updates rebroadcast calls to async versions
packages/graphql/lib/src/core/query_manager.dart	Implements AsyncDeepEqualsFn typedef, converts equality checking to async, and updates all rebroadcast operations
packages/graphql/lib/src/core/observable_query.dart	Updates method call references and comments to reflect async rebroadcast changes

[Copilot]

Using .ignore() on a Future can suppress important error information. Consider using unawaited() from dart:async instead, or properly handle the Future with await or error handling.

Suggested change

	queryManager.maybeRebroadcastQueriesAsync().ignore();
	unawaited(queryManager.maybeRebroadcastQueriesAsync());

[Copilot]

This async method call is not awaited or handled. Consider using unawaited() to explicitly indicate the Future is intentionally not awaited, or handle it appropriately.

Suggested change

	queryManager.maybeRebroadcastQueriesAsync();
	queryManager.maybeRebroadcastQueriesAsync().ignore();

[Copilot]

This async method call within a map operation is not awaited. This could lead to race conditions or unexpected behavior. Consider using unawaited() or restructuring to properly handle the async operation.

Suggested change

	maybeRebroadcastQueriesAsync();
	unawaited(maybeRebroadcastQueriesAsync());

[Copilot]

This async method call is not awaited. Consider using unawaited() to explicitly indicate the Future is intentionally not awaited, or handle it appropriately.

Suggested change

	maybeRebroadcastQueriesAsync();
	unawaited(maybeRebroadcastQueriesAsync());

[Copilot]

This async method call is not awaited. Consider using unawaited() to explicitly indicate the Future is intentionally not awaited, or handle it appropriately.

Suggested change

	maybeRebroadcastQueriesAsync();
	unawaited(maybeRebroadcastQueriesAsync());

[Copilot]

The async method call within .then() is not awaited. This could lead to unhandled promise rejections. Consider using unawaited() or restructuring to properly handle the async operation.

Suggested change

	networkResult.then((value) => maybeRebroadcastQueriesAsync());
	networkResult.then((value) => unawaited(maybeRebroadcastQueriesAsync()));

[Copilot]

This async method call is not awaited. Consider using unawaited() to explicitly indicate the Future is intentionally not awaited, or handle it appropriately.

Suggested change

	maybeRebroadcastQueriesAsync();
	unawaited(maybeRebroadcastQueriesAsync());

[Copilot]

This async method call is not awaited. Consider using unawaited() to explicitly indicate the Future is intentionally not awaited, or handle it appropriately.

Suggested change

	maybeRebroadcastQueriesAsync();
	unawaited(maybeRebroadcastQueriesAsync());

[vincenzopalazzo]

@Neelansh-ns what do you think about the copilot suggestion? if do you think are important can you folloup on it?

[kvenn]

Does making maybeRebroadcastQueries async introduce any sort of race condition? I could have sworn I tried this back in the day and ran into issues where the state in the app was incorrect because maybeRebroadcastQueries was no longer synchronous / executed serially. And that added to the fact that the equality function itself was not async so it still took up the 150ms of time (just deferred which allowed flutter to optimize slightly). I may be mistaken. But if that is the case, you could still have it be async and throw it into a queue.

Also with the optimized deep equals, a medium sized system (10 observables) amounted to around 30ms per rebroadcast. So about 2 skipped frames.

You could maybe make the equality function, itself, async to maximize benefit and decrease jank.

Maybe something like this:

Example `optimizedDeepEqualsAsync`

// Deep equality check that yields to the event loop between recursive calls
Future<bool> optimizedDeepEqualsAsync(Object? a, Object? b) async {
  // Yield once to avoid blocking
  await Future<void>.delayed(Duration.zero);

  if (identical(a, b)) return true;
  if (a == b) return true;

  if (a is Map) {
    if (b is! Map || a.length != b.length) return false;
    for (var key in a.keys) {
      if (!b.containsKey(key)) return false;
      // Recursive async call
      if (!await optimizedDeepEqualsAsync(a[key], b[key])) return false;
    }
    return true;
  }

  if (a is List) {
    if (b is! List || a.length != b.length) return false;
    for (var i = 0; i < a.length; i++) {
      // Recursive async call
      if (!await optimizedDeepEqualsAsync(a[i], b[i])) return false;
    }
    return true;
  }

  return false;
}

---

To avoid the jank the developer can pass the async function for equality check, like compute() to move the operation to a different isolate.

Just a quick thought on this. I'd highly recommend using a shared isolate if you go this route. I tried this one with compute, too and found it would increase the equality check time by about 50-100ms (for the spawn cost).

[vincenzopalazzo]

@kvenn let me know if you preferer revert this commit

[kvenn]

I wouldn't want to hold off progress. It's been a while since I was in this code so I definitely could be wrong! For safer backwards compatibility though you could conditionally call the async or sync versions. As in, if the async version is set, use that one - otherwise use the sync version.

I'll defer to @Neelansh-ns, as they have tested this more recently.

[vincenzopalazzo]

Tracking the issue with #1501