Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(flutter): widgets make unnecessary requests when dependencies change #533

Conversation

fredericp
Copy link
Contributor

Similar to #352 but it looks like it's stalled. I think this PR is more complete.

Widgets can get its didChangeDependencies method called multiple times even when no real changes have happened to its own dependencies. As a result, our query and mutation widgets may perform many unnecessary network fetches. In my scenario I get hundreds of unnecessary requests.

See reddit discussion and flutter/flutter#39872 for more info.

The solution is to store the client in the widget state and check if it really changed before reinitializing the query.

@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #533 into beta will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             beta     #533   +/-   ##
=======================================
  Coverage   59.65%   59.65%           
=======================================
  Files          38       38           
  Lines        1465     1465           
=======================================
  Hits          874      874           
  Misses        591      591
Flag Coverage Δ
#graphql_client 60.46% <ø> (ø) ⬆️
#graphql_flutter 22.58% <ø> (ø) ⬆️

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 0b0a7d9...31936ff. Read the comment docs.

Copy link
Collaborator

@mainawycliffe mainawycliffe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@micimize
Copy link
Collaborator

@mainawycliffe this also looks good to me - if you've checked it out and tested it some we can merge it

@nivekz
Copy link
Contributor

nivekz commented Apr 11, 2020

May I request this PR be looked at again and hopefully be merged soon?

We have a production app that is severely affected by this bug (ie #575) and we have independently arrived at basically the same fix as this PR. So we are glad to find that this PR exists (otherwise we would be creating one 😄). I guess potentially many others are on the same boat. To have this PR merged and released soon can save us from all forking for a fix.

@mainawycliffe mainawycliffe merged commit ea69eea into zino-hofmann:beta Apr 11, 2020
@HofmannZ
Copy link
Member

🎉 This PR is included in version 3.1.0-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@joseignaciopergolesi
Copy link

Sorry, could you show an example of how to make a query and not to do it three times?
I still can't do it with version 3.1.0-beta.2

@joseignaciopergolesi
Copy link

It doesn't work for me using the latest beta version of the package. Could you upload an example of how to make a query using the wiget Query and not make three requests to the server?
Thank you

@micimize
Copy link
Collaborator

@joseignaciopergolesi If the basic example produces unnecessary requests, please open a new issue extrapolating

@nivekz
Copy link
Contributor

nivekz commented Apr 14, 2020

@micimize @mainawycliffe I have a follow-up issue (currently as a comment in the original issue: #575 (comment)) and a proposed PR #601, and wonder if you could please take a look?

micimize added a commit that referenced this pull request May 9, 2020
…m client (for now), retrieve subscription changes from #533
@HofmannZ
Copy link
Member

🎉 This PR is included in version 3.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

HofmannZ pushed a commit that referenced this pull request Oct 7, 2020
# [4.0.0-beta.1](v3.1.0...v4.0.0-beta.1) (2020-10-07)

### Bug Fixes

* **examples:** ignore missing token ([ffd3294](ffd3294))
* query test ([f54c6ae](f54c6ae))
* **ci:** loosen path version ([645d462](645d462))
* **client:** gql_http_link==0.3.2 for custom toJsons closing [#734](#734) ([98b8cf7](98b8cf7))
* **client:** mutation not firing observer callbacks ([75393c2](75393c2))
* **client:** only queries are refetch safe ([1e93376](1e93376))
* **docs:** typo in docstring, add todo to sanitizeVariables ([9c84cb1](9c84cb1))
* **examples:** cleanup bloc example ([82724f0](82724f0))
* **examples:** flutter bloc pubspec ([61582b3](61582b3))
* **examples:** starwars example cache ([22db4f7](22db4f7))
* **examples:** starwars example works again ([7514b93](7514b93))
* **examples:** update ios files for graphql_flutter/example ([5b6e3d0](5b6e3d0))
* **graphql:** default-yet-overrideable variable ([6ba687e](6ba687e))
* **graphql:** don't close mutations after callbacks ([2ba6c74](2ba6c74))
* **graphql:** dumb ?? documentNode bug ([ba7b641](ba7b641))
* **graphql:** fix rebroadcasting by refactoring onData callbacks into a simpler async function ([9a5fff1](9a5fff1))
* **graphql:** keep deprecated QueryResult api and mark it as such ([2b447a0](2b447a0))
* **graphql:** sanitize multipart files for cache ([4ceb800](4ceb800))
* **graphql:** simplified AuthLink ([0b3fbd9](0b3fbd9))
* **tests:** update tests ([bba4a7a](bba4a7a))

### Features

* cache now flags itself for broadcasting ([84cba43](84cba43))
* client.fetchMore utility for leveraging the fetch more logic results without using ObservableQuery ([814ccb3](814ccb3))
* documentNode -> document, dependency issues, reexport links from client (for now), retrieve subscription changes from [#533](#533) ([4fb205c](4fb205c))
* **graphql_flutter:** add ResultAccumulator, fix Subscription ([7e1edee](7e1edee))
* drop Link layer in favor of package:gql_link and package:gql_exec ([2e491a7](2e491a7))
* **client:** cache proxy methods on cache, resetStore with optional refetchQueries ([ba7134a](ba7134a))
* **client:** expose store, cleanup ([6fc5e7e](6fc5e7e))
* **client:** refetchSafeQueries, clarify rebroadcast calls in docs ([e45b240](e45b240))
* **docs:** v4 changelog ([38cfd9b](38cfd9b))
* **examples:** reorg graphql example so pub displays code ([bc32bdd](bc32bdd))
* **examples:** starwars hivestore usage ([2f874ec](2f874ec))
* **graphql:** add isMutation etc helpers to Options types ([04e7888](04e7888))
* **graphql:** complete caching overhaul ([e9b5660](e9b5660))
* **graphql:** HiveStore api improvements, fetchmore fixes ([2d1a7f2](2d1a7f2))
* **graphql:** HiveStore.open ([6db4677](6db4677))
* **graphql:** multipart file support ([c2733ca](c2733ca))
* **graphql:** re-add documentNode asdeprecated ([20d0176](20d0176))
* **graphql:** Robust ObservableQuery docs ([1e893b5](1e893b5))
* **graphql:** update old websocket_link ([496d994](496d994))
* **graphql:** use new cache correctly everywhere else ([f64a6c8](f64a6c8))
* HiveStore ([2c3c66c](2c3c66c))
* more work on gql links ([0d7ef7a](0d7ef7a))
* move to DocumentNode-only documents ([7499323](7499323))
* starting on gql links ([d9452bc](d9452bc))
* **graphql:** work on making subscriptions more of a first-class citizen ([6d0b045](6d0b045))
* **graphql_flutter:** initHiveForFlutter ([1118cc7](1118cc7))
* **graphql_flutter:** work on making subscriptions more of a first-class citizen ([a0e0d5c](a0e0d5c))
* **tests:** test subscriptions ([2a3e6a1](2a3e6a1))

### BREAKING CHANGES

* the deprecated string documents are no longer supported
* Link layer is now implemented via package:gql_link and package:gql_exec
HofmannZ pushed a commit that referenced this pull request Jan 31, 2021
# [4.0.0](v3.1.0...v4.0.0) (2021-01-31)

### Bug Fixes

* **examples:** starwars example cache ([22db4f7](22db4f7))
* fix ObservableQuery.lifecycle for cache only results ([f44b479](f44b479))
* **client:** mutation not firing observer callbacks ([75393c2](75393c2))
* query test ([f54c6ae](f54c6ae))
* **ci:** loosen path version ([645d462](645d462))
* **client:** add CacheMissException for when write/read results in null ([a0a967f](a0a967f))
* **client:** fetchMore partial handling ([10ec576](10ec576))
* **client:** gql_http_link==0.3.2 for custom toJsons closing [#734](#734) ([98b8cf7](98b8cf7))
* **client:** only queries are refetch safe ([1e93376](1e93376))
* **client:** refetch overrides fetchPolicy ([891bc2b](891bc2b))
* **client:** skip cache writes on null data, thus fixing [#405](#405) ([7472bb9](7472bb9))
* **client:** wrap all subscription errors in QueryResults ([aae61ca](aae61ca))
* **docs:** typo in docstring, add todo to sanitizeVariables ([9c84cb1](9c84cb1))
* **examples:** cleanup bloc example ([82724f0](82724f0))
* **examples:** flutter bloc pubspec ([61582b3](61582b3))
* **examples:** ignore missing token ([ffd3294](ffd3294))
* **examples:** starwars example works again ([7514b93](7514b93))
* **examples:** update ios files for graphql_flutter/example ([5b6e3d0](5b6e3d0))
* **graphql:** default-yet-overrideable variable ([6ba687e](6ba687e))
* **graphql:** don't close mutations after callbacks ([2ba6c74](2ba6c74))
* **graphql:** dumb ?? documentNode bug ([ba7b641](ba7b641))
* **graphql:** fix rebroadcasting by refactoring onData callbacks into a simpler async function ([9a5fff1](9a5fff1))
* **graphql:** keep deprecated QueryResult api and mark it as such ([2b447a0](2b447a0))
* **graphql:** sanitize multipart files for cache ([4ceb800](4ceb800))
* **graphql:** simplified AuthLink ([0b3fbd9](0b3fbd9))
* **tests:** update tests ([bba4a7a](bba4a7a))

### Features

* cache now flags itself for broadcasting ([84cba43](84cba43))
* client.fetchMore utility for leveraging the fetch more logic results without using ObservableQuery ([814ccb3](814ccb3))
* documentNode -> document, dependency issues, reexport links from client (for now), retrieve subscription changes from [#533](#533) ([4fb205c](4fb205c))
* **graphql:** HiveStore.open ([6db4677](6db4677))
* drop Link layer in favor of package:gql_link and package:gql_exec ([2e491a7](2e491a7))
* **client:** add context to QueryResult ([fbc5a2d](fbc5a2d))
* **client:** cache proxy methods on cache, resetStore with optional refetchQueries ([ba7134a](ba7134a))
* **client:** cache writes are now strict, and throw PartialDataException (from normalize), ([616b5ed](616b5ed))
* **client:** CacheRereadPolicy, watchMutation workaround ([32e02da](32e02da))
* **client:** carry forward data on exception ([ccf3b9c](ccf3b9c))
* **client:** expose store, cleanup ([6fc5e7e](6fc5e7e))
* **client:** only rebroadcast on deep equals ([ee64e99](ee64e99))
* **client:** partialDataPolicy for configuring rejections ([0a7cd28](0a7cd28))
* **client:** QueryResult.unexecuted ([13e3257](13e3257))
* **client:** refetchSafeQueries, clarify rebroadcast calls in docs ([e45b240](e45b240))
* **docs:** v4 changelog ([38cfd9b](38cfd9b))
* **examples:** reorg graphql example so pub displays code ([bc32bdd](bc32bdd))
* **examples:** starwars hivestore usage ([2f874ec](2f874ec))
* **graphql:** add isMutation etc helpers to Options types ([04e7888](04e7888))
* **graphql:** complete caching overhaul ([e9b5660](e9b5660))
* **graphql:** HiveStore api improvements, fetchmore fixes ([2d1a7f2](2d1a7f2))
* **graphql:** multipart file support ([c2733ca](c2733ca))
* **graphql:** re-add documentNode asdeprecated ([20d0176](20d0176))
* more work on gql links ([0d7ef7a](0d7ef7a))
* **graphql:** Robust ObservableQuery docs ([1e893b5](1e893b5))
* **graphql:** update old websocket_link ([496d994](496d994))
* **graphql:** upgrade normalize to 0.4.2 ([4655e7d](4655e7d))
* **graphql:** use new cache correctly everywhere else ([f64a6c8](f64a6c8))
* starting on gql links ([d9452bc](d9452bc))
* **graphql:** work on making subscriptions more of a first-class citizen ([6d0b045](6d0b045))
* **graphql_flutter:** add ResultAccumulator, fix Subscription ([7e1edee](7e1edee))
* **graphql_flutter:** initHiveForFlutter ([1118cc7](1118cc7))
* HiveStore ([2c3c66c](2c3c66c))
* move to DocumentNode-only documents ([7499323](7499323))
* **graphql_flutter:** work on making subscriptions more of a first-class citizen ([a0e0d5c](a0e0d5c))
* **tests:** test subscriptions ([2a3e6a1](2a3e6a1))

### BREAKING CHANGES

* **client:** By fixing the defaults for mutations, the old behavior
is now lost
* the deprecated string documents are no longer supported
* Link layer is now implemented via package:gql_link and package:gql_exec
micimize pushed a commit that referenced this pull request Jan 31, 2021
# [4.0.0](v3.1.0...v4.0.0) (2021-01-31)

### Bug Fixes

* **examples:** starwars example cache ([22db4f7](22db4f7))
* fix ObservableQuery.lifecycle for cache only results ([f44b479](f44b479))
* **client:** mutation not firing observer callbacks ([75393c2](75393c2))
* query test ([f54c6ae](f54c6ae))
* **ci:** loosen path version ([645d462](645d462))
* **client:** add CacheMissException for when write/read results in null ([a0a967f](a0a967f))
* **client:** fetchMore partial handling ([10ec576](10ec576))
* **client:** gql_http_link==0.3.2 for custom toJsons closing [#734](#734) ([98b8cf7](98b8cf7))
* **client:** only queries are refetch safe ([1e93376](1e93376))
* **client:** refetch overrides fetchPolicy ([891bc2b](891bc2b))
* **client:** skip cache writes on null data, thus fixing [#405](#405) ([7472bb9](7472bb9))
* **client:** wrap all subscription errors in QueryResults ([aae61ca](aae61ca))
* **docs:** typo in docstring, add todo to sanitizeVariables ([9c84cb1](9c84cb1))
* **examples:** cleanup bloc example ([82724f0](82724f0))
* **examples:** flutter bloc pubspec ([61582b3](61582b3))
* **examples:** ignore missing token ([ffd3294](ffd3294))
* **examples:** starwars example works again ([7514b93](7514b93))
* **examples:** update ios files for graphql_flutter/example ([5b6e3d0](5b6e3d0))
* **graphql:** default-yet-overrideable variable ([6ba687e](6ba687e))
* **graphql:** don't close mutations after callbacks ([2ba6c74](2ba6c74))
* **graphql:** dumb ?? documentNode bug ([ba7b641](ba7b641))
* **graphql:** fix rebroadcasting by refactoring onData callbacks into a simpler async function ([9a5fff1](9a5fff1))
* **graphql:** keep deprecated QueryResult api and mark it as such ([2b447a0](2b447a0))
* **graphql:** sanitize multipart files for cache ([4ceb800](4ceb800))
* **graphql:** simplified AuthLink ([0b3fbd9](0b3fbd9))
* **tests:** update tests ([bba4a7a](bba4a7a))

### Features

* cache now flags itself for broadcasting ([84cba43](84cba43))
* client.fetchMore utility for leveraging the fetch more logic results without using ObservableQuery ([814ccb3](814ccb3))
* documentNode -> document, dependency issues, reexport links from client (for now), retrieve subscription changes from [#533](#533) ([4fb205c](4fb205c))
* **graphql:** HiveStore.open ([6db4677](6db4677))
* drop Link layer in favor of package:gql_link and package:gql_exec ([2e491a7](2e491a7))
* **client:** add context to QueryResult ([fbc5a2d](fbc5a2d))
* **client:** cache proxy methods on cache, resetStore with optional refetchQueries ([ba7134a](ba7134a))
* **client:** cache writes are now strict, and throw PartialDataException (from normalize), ([616b5ed](616b5ed))
* **client:** CacheRereadPolicy, watchMutation workaround ([32e02da](32e02da))
* **client:** carry forward data on exception ([ccf3b9c](ccf3b9c))
* **client:** expose store, cleanup ([6fc5e7e](6fc5e7e))
* **client:** only rebroadcast on deep equals ([ee64e99](ee64e99))
* **client:** partialDataPolicy for configuring rejections ([0a7cd28](0a7cd28))
* **client:** QueryResult.unexecuted ([13e3257](13e3257))
* **client:** refetchSafeQueries, clarify rebroadcast calls in docs ([e45b240](e45b240))
* **docs:** v4 changelog ([38cfd9b](38cfd9b))
* **examples:** reorg graphql example so pub displays code ([bc32bdd](bc32bdd))
* **examples:** starwars hivestore usage ([2f874ec](2f874ec))
* **graphql:** add isMutation etc helpers to Options types ([04e7888](04e7888))
* **graphql:** complete caching overhaul ([e9b5660](e9b5660))
* **graphql:** HiveStore api improvements, fetchmore fixes ([2d1a7f2](2d1a7f2))
* **graphql:** multipart file support ([c2733ca](c2733ca))
* **graphql:** re-add documentNode asdeprecated ([20d0176](20d0176))
* more work on gql links ([0d7ef7a](0d7ef7a))
* **graphql:** Robust ObservableQuery docs ([1e893b5](1e893b5))
* **graphql:** update old websocket_link ([496d994](496d994))
* **graphql:** upgrade normalize to 0.4.2 ([4655e7d](4655e7d))
* **graphql:** use new cache correctly everywhere else ([f64a6c8](f64a6c8))
* starting on gql links ([d9452bc](d9452bc))
* **graphql:** work on making subscriptions more of a first-class citizen ([6d0b045](6d0b045))
* **graphql_flutter:** add ResultAccumulator, fix Subscription ([7e1edee](7e1edee))
* **graphql_flutter:** initHiveForFlutter ([1118cc7](1118cc7))
* HiveStore ([2c3c66c](2c3c66c))
* move to DocumentNode-only documents ([7499323](7499323))
* **graphql_flutter:** work on making subscriptions more of a first-class citizen ([a0e0d5c](a0e0d5c))
* **tests:** test subscriptions ([2a3e6a1](2a3e6a1))

### BREAKING CHANGES

* **client:** By fixing the defaults for mutations, the old behavior
is now lost
* the deprecated string documents are no longer supported
* Link layer is now implemented via package:gql_link and package:gql_exec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants