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

Support returnPartialData for watched queries again. #4743

Merged
merged 4 commits into from
Apr 24, 2019

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Apr 23, 2019

PR #1370 inexplicably removed support for a useful feature: the ability to opt into receiving partial results from the cache for queries that are not fully satisfied by the cache, by passing the returnPartialData: true option to client.watchQuery.

This feature is especially important in conjunction with React (or any other view framework Apollo integrations that use client.watchQuery), since it can prevent rerendering undefined data just before fetching a larger query over the network.

A lot has changed since PR #1370 was merged in March 2017. The author and reviewer of that PR no longer work for Apollo, and I would like to believe that our current development culture would prevent us from ripping out such a useful feature without providing a meaningful replacement.

@benjamn benjamn added this to the Release 2.6.0 milestone Apr 23, 2019
@benjamn benjamn requested a review from hwillson April 23, 2019 16:16
@benjamn benjamn self-assigned this Apr 23, 2019
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Excellent - thanks for bringing this back @benjamn!

benjamn added a commit that referenced this pull request Apr 23, 2019
@benjamn benjamn force-pushed the support-returnPartialData-again branch from 7374bb6 to f823b5f Compare April 23, 2019 20:51
PR #1370 inexplicably removed support for a useful feature: the ability to
opt into receiving partial results from the cache for queries that are not
fully satisfied by the cache, by passing the returnPartialData:true option
to client.watchQuery.

This feature is especially important in conjunction with React (or any
other view framework Apollo integrations that use client.watchQuery),
since it can prevent rerendering undefined data just before fetching a
larger query over the network.

A lot has changed since PR #1370 was merged in March 2017. The author and
reviewer of that PR no longer work for Apollo, and I would like to believe
that our current development culture would prevent us from ripping out
such a useful feature without providing a meaningful replacement.
These tests were removed by PR #1370. A lot has changed since March 2017,
but fortunately the same general concepts still apply, and the tests pass
with minor adjustments.
@benjamn benjamn force-pushed the support-returnPartialData-again branch from f823b5f to 21916e0 Compare April 23, 2019 21:11
@benjamn benjamn requested a review from hwillson April 23, 2019 21:16
@@ -250,10 +250,6 @@ export class LocalState<TCacheShape> {
return forceResolvers;
}

public shouldForceResolver(field: FieldNode) {
return this.shouldForceResolvers(field);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@hwillson Do you think it's safe to remove this method if we're not using it internally?

Copy link
Member

Choose a reason for hiding this comment

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

@benjamn Yes, for sure!

@hwillson
Copy link
Member

I am sooooooo incredibly excited about this PR! Just to call this PR out for those that have notifications enabled on this repo - currently, if you run a query like:

const GET_MEMBER = gql`
  query GetMember($id: ID!) {
    member(id: $id) {
      id
      name
    }
  }
`;

in one component, the results are cached, then you run a superset query like the following in another component:

const GET_MEMBER_WITH_PLANS = gql`
  query GetMemberWithPlans($id: ID!) {
    member(id: $id) {
      id
      name
      plans {
        id
        title
        duration
      }
    }
  }
`;

Apollo Client will not re-use the partial data that was cached from the first query, when it preps and displays the second component. It can't find a cache hit for the full second query, so it fires the full query over the network. After this PR (and after we add in related react-apollo changes), if returnPartialData is set on the second component, the data available to that component will be automatically pre-loaded with the parts of the query that can be found in the cache, before the full query is fired over the network. So we'll be able to show partial data in our components, while the rest of the data is being loaded! 🎉

@DeciderWill
Copy link

DeciderWill commented May 30, 2019

Hi @benjamn - thank you for taking the time to add this.

We've had an issue with it however. We were unaware of the re-implementation and when updating to 2.6.0 we found a few of our query components were not returning any data and spent a bit of time debugging. Resulting behavior: re-renders a component 3 times with loading states false, false, and finally true. On true the data object is still an empty object. I can see the data has returned successfully and there are no errors. But it is not available on the data prop. Adding returnPartialData={true} resolved the issue. For context, we had two components rendering at the same time making the same query with a variable determining the type of data to be returned ie MetricItem being a union of CpuItem, NetworkItem and MemoryItem.

I'm unsure if this was an intended change that returnPartialData would need to be added and some existing queries would stop working without it?

Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants