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: queryDeduplication from context #6261

Merged

Conversation

igaloly
Copy link
Contributor

@igaloly igaloly commented May 10, 2020

Overview
As mentioned in this article, https://www.apollographql.com/docs/react/networking/network-layer/#query-deduplication, if we want to override our default queryManager's queryDeduplication property, we should pass to request's context { queryDeduplication: boolean }.

The problem
It seems like getObservableFromLink did not handle the property from the context and just passed { forceFetch: [arg queryDeduplication / default queryDeduplication] }

The solution
Make getObservableFromLink's deduplication smarter

    if(typeof deduplication === 'undefined') { // else, is passed as an argument
      if(typeof context === 'object' && 'queryDeduplication' in context) {
        deduplication = context.queryDeduplication
      } else {
        deduplication = this.queryDeduplication
      }
    }

Issues
apollographql/apollo-link#517
#4150
https://github.com/apollographql/apollo-feature-requests/issues/40

Edit
It seems like tests are failing because of the use of private class properties.
Those properties were changes and that's why they need to be tested.

@igaloly igaloly changed the title Bugfix - queryDeduplication from context fix: queryDeduplication from context May 10, 2020
@Kalcode
Copy link

Kalcode commented Jun 3, 2020

Looking into passing this for context per query rather than disabling it client wide.

Do we know what the status on getting this merged?

Thanks

@Kalcode
Copy link

Kalcode commented Jun 3, 2020

Looking to disable deduplication per request on hooks and refetch queries.

Per documentation

Query deduplication can help reduce the number of queries that are sent over the wire. It is turned on by default, but can be turned off by passing queryDeduplication: false to the context on each requests or using the defaultOptions key on Apollo Client setup.

Do we know the status on when this will get merged?

Thanks

@benjamn benjamn force-pushed the bugfix/deduplication-from-request branch from 99b7b87 to 9b0271f Compare July 20, 2020 22:28
benjamn added a commit that referenced this pull request Jul 20, 2020
Use context.queryDeduplication if provided. Similar to #6261, but for AC2 instead of AC3.
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

Thanks for your patience @igaloly—this definitely fell through the cracks as we were working on AC3. I made a small change to the default logic (have a look if you're curious), and I was able to fix the tests by cherry-picking @Kujawadl's commit from #6526, so this seems ready to go.

@benjamn benjamn added this to the Release 3.0 milestone Jul 20, 2020
@benjamn benjamn merged commit 7913eec into apollographql:master Jul 20, 2020
benjamn added a commit that referenced this pull request Jul 20, 2020
@benjamn
Copy link
Member

benjamn commented Jul 23, 2020

Heads up: we're not quite ready to publish @apollo/client@3.1.0 yet, but I've published an @apollo/client@3.1.0-pre.0 release that includes this PR, if you want to try that in the meantime. 🙏

@benjamn
Copy link
Member

benjamn commented Jul 28, 2020

Following up: we just published @apollo/client@3.1.0 to npm!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants