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

fix: create results from options #1074

Closed budde377 closed 2 years ago

budde377 commented 2 years ago

This PR changes how we create QueryResults. By always creating them from a given BaseOption we're effectively preserving type information. Losing type context has been the cause of two issues, latest #1072.

Since this PR changes the signature of the QueryResult which would warrant a major release if we had a better split of releases. This will cause issues for our beta users. The instruction should be to keep the packages in sync. Again, this is a major argument for persuing split releases.

codecov[bot] commented 2 years ago

Codecov Report

Merging #1074 (2416e37) into main (a11c564) will increase coverage by 1.86%. The diff coverage is 78.12%.

:exclamation: Current head 2416e37 differs from pull request most recent head 9760559. Consider uploading reports for the commit 9760559 to get more accurate results

@@            Coverage Diff             @@
##             main    #1074      +/-   ##
==========================================
+ Coverage   55.89%   57.76%   +1.86%     
==========================================
  Files          41       41              
  Lines        1526     1520       -6     
==========================================
+ Hits          853      878      +25     
+ Misses        673      642      -31     
Impacted Files Coverage Δ
...ackages/graphql/lib/src/core/observable_query.dart 40.00% <0.00%> (-1.41%) :arrow_down:
packages/graphql/lib/src/utilities/response.dart 46.15% <ø> (-3.85%) :arrow_down:
...ql_flutter/lib/src/widgets/hooks/subscription.dart 0.00% <0.00%> (ø)
packages/graphql/lib/src/core/query_result.dart 50.00% <57.14%> (+2.94%) :arrow_up:
packages/graphql/lib/src/cache/hive_store.dart 94.73% <83.33%> (-5.27%) :arrow_down:
packages/graphql/lib/src/core/_base_options.dart 93.02% <100.00%> (+3.02%) :arrow_up:
packages/graphql/lib/src/core/query_manager.dart 76.35% <100.00%> (+5.83%) :arrow_up:
packages/graphql_flutter/lib/src/hive_init.dart 87.50% <100.00%> (ø)
... and 2 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 a11c564...9760559. Read the comment docs.

budde377 commented 2 years ago

@vincenzopalazzo, with the breaking change mentioned above the current CI will always fail because some are using melos bootstrap and others aren't. What are your thoughts on this?

vincenzopalazzo commented 2 years ago

@vincenzopalazzo, with the breaking change mentioned above the current CI will always fail because some are using melos bootstrap and others aren't. What are your thoughts on this?

I will look into the error today, sorry for delay!

apackin commented 2 years ago

We're running into a type issue that I believe is from this change

Screen Shot 2022-03-31 at 5 59 25 PM

vincenzopalazzo commented 2 years ago

@apackin

The version that you are using? In addition, a new issue with a good description of the problem can be very helpful, debugging in a screenshot is always frustrating for me, sorry!