zino-hofmann / graphql-flutter

A GraphQL client for Flutter, bringing all the features from a modern GraphQL client to one easy to use package.
https://zino-hofmann.github.io/graphql-flutter
MIT License
3.25k stars 620 forks source link

feat: Add possibility to update the client components #1081

Closed vincenzopalazzo closed 2 years ago

vincenzopalazzo commented 2 years ago

This is a very opinionated API, but it is required because it hides the complexity of the client under the hood.

The user should never know how we use the component and how the client is built.

The user needs to know that we are like Harry Potter and the stuff happens like magic.

Changelog-Add: Add API to update the client component like Link, cache etc.

Signed-off-by: Vincenzo Palazzo vincenzopalazzodev@gmail.com

codecov[bot] commented 2 years ago

Codecov Report

Merging #1081 (556a668) into main (09cc1cc) will increase coverage by 0.67%. The diff coverage is 83.33%.

:exclamation: Current head 556a668 differs from pull request most recent head 1c37a09. Consider uploading reports for the commit 1c37a09 to get more accurate results

@@            Coverage Diff             @@
##             main    #1081      +/-   ##
==========================================
+ Coverage   59.92%   60.59%   +0.67%     
==========================================
  Files          41       28      -13     
  Lines        1522     1317     -205     
==========================================
- Hits          912      798     -114     
+ Misses        610      519      -91     
Impacted Files Coverage Δ
packages/graphql/lib/src/graphql_client.dart 82.92% <83.33%> (+0.06%) :arrow_up:
...ackages/graphql_flutter/lib/src/widgets/query.dart
..._flutter/lib/src/widgets/hooks/graphql_client.dart
...raphql_flutter/lib/src/widgets/cache_provider.dart
...s/graphql_flutter/lib/src/widgets/hooks/query.dart
...ages/graphql_flutter/lib/src/widgets/mutation.dart
...ql_flutter/lib/src/widgets/result_accumulator.dart
...hql_flutter/lib/src/widgets/hooks/watch_query.dart
packages/graphql_flutter/lib/src/hive_init.dart
...phql_flutter/lib/src/widgets/graphql_provider.dart
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 09cc1cc...1c37a09. Read the comment docs.

budde377 commented 2 years ago

This seems like an alright change, but I'm not sure I understand the issue we're trying to solve. Generally, I'd recommend that you simply create a new instance of the client, preserving any cache that you'd like. This has the benefit of updating widgets using the client (when using flutter) because the client instance is a new one.

Do you mind elaborating a bit on the problem motivating this? :)

vincenzopalazzo commented 2 years ago

Do you mind elaborating a bit on the problem motivating this? :)

I don't think that this API is required for any operation, and I share your opinion on flutter. However, there is some use cases like the command line application where this API makes the code more readable, but I have no other good reason for this change, sorry!

I open this PR because some user points out that they want to update the client, and the only solution was to recreate it.

I'm fine with closing or merging this PR, and sorry if I have no good reason for this change.

budde377 commented 2 years ago

What if we preserve immutability by adding a copyWith method which creates a new instance?

final newClient = client.copyWith(link: SomeNewLink());
vincenzopalazzo commented 2 years ago

What if we preserve immutability by adding a copyWith method which creates a new instance?

I like the API, thanks I will make the update

vincenzopalazzo commented 2 years ago

This PR should be ready, thanks for the improvement @budde377