Skip to content

fix: queryDeduplication from context#6261

Merged
benjamn merged 5 commits intomasterfrom
unknown repository
Jul 20, 2020
Merged

fix: queryDeduplication from context#6261
benjamn merged 5 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented May 10, 2020 •
edited by ghost
Loading

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.

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

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
Copy Markdown

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

This was referenced Jul 1, 2020
Closed
Merged
Open
benjamn added a commit that referenced this pull request Jul 20, 2020
@benjamn
Merge pull request #6526 from Kujawadl/version-2.6
173f4ce 
Use context.queryDeduplication if provided. Similar to #6261, but for AC2 instead of AC3.
benjamn
benjamn approved these changes Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
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
Mention PRs #6261 and #6526 in CHANGELOG.md.
a1855ff
@benjamn
Copy link
Copy Markdown
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
Copy Markdown
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.

Reviewers

No reviews

Assignees

No one assigned

Labels

🐞 bug

Projects

None yet

Milestone

Release 3.0

Development

Successfully merging this pull request may close these issues.

2 participants

@Kalcode @benjamn