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 options.reobserveQuery callback for mutations. #7827

Merged
merged 8 commits into from Mar 26, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Mar 11, 2021

Building on #7819, this PR implements the reobserveQuery option I described in #7060 (comment), which allows mutations to override the refetching logic for any watched queries invalidated by the mutation update function.

The word "invalidated" is important here, since it means reobserveQuery will fire for any queries affected by cache writes, even if the result of the query does not end up changing. This is why it was previously difficult to make use of the INVALIDATE sentinel object added in #6991, as reported in #7060. Using cache.modify and INVALIDATE in the mutation update function should work smoothly with reobserveQuery, hopefully resolving #7060.

Similar functionality is achievable using just cache.batch and onDirty, so reobserveQuery-style behavior does not absolutely require a mutation, but reobserveQuery should greatly reduce the need for manual query enumeration after mutations, a la refetchQueries. Also, since you can simply return a Promise from reobserveQuery to make the mutation await async work, the need to use awaitRefetchQueries should be (mostly) eliminated.

As foretold in #7819, this PR continues (finishes? 🤞) the implementation of the following item from the Apollo Client v3.4 ROADMAP.md section:

  • A new API for reobserving/refetching queries after a mutation, eliminating the need for updateQueries, refetchQueries, and awaitRefetchQueries in a lot of cases.

@benjamn benjamn force-pushed the mutation-reobserveQuery-using-batch branch from e4c9fad to f15f0e7 Compare March 11, 2021 19:43
Comment on lines +509 to +510
describe('refetching queries', () => {
itAsync('can pass reobserveQuery to useMutation', (resolve, reject) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the test that wasn't passing without the changes from #7887 (against which I just rebased this PR), since #7887 solves some problems with the intersection of result caching and optimistic updates, and React hooks are especially sensitive to those kind of problems.

@benjamn benjamn requested a review from brainkim March 23, 2021 22:17
@benjamn benjamn marked this pull request as ready for review March 23, 2021 22:19
@benjamn
Copy link
Member Author

benjamn commented Mar 23, 2021

As I mentioned in #7878 (comment), I'm now thinking about supporting a standalone client.reobserveQueries method, not requiring a mutation:

client.reobserveQueries(
  // This callback invalidates some fields in the cache.
  cache => {
    // Modify the cache somehow, using writeQuery, writeFragment,
    // evict, modify, etc. Anything that invalidates fields.
  },
  // This callback gets called for any ObservableQuery objects
  // affected by the invalidation.
  (observableQuery, diff) => {
    // Force any affected queries to be refetched from the network.
    return observableQuery.refetch();
  },
);

@benjamn benjamn force-pushed the mutation-reobserveQuery-using-batch branch from 141fcb3 to 0774b64 Compare March 23, 2021 23:31
@benjamn benjamn changed the base branch from EntityStore-Stump-layer to release-3.4 March 23, 2021 23:32
@benjamn benjamn force-pushed the mutation-reobserveQuery-using-batch branch from 0774b64 to 432e21c Compare March 25, 2021 22:26
@benjamn
Copy link
Member Author

benjamn commented Mar 25, 2021

@hwillson @jcreighton @brainkim I think this PR is ready for review now? I can tackle #7827 (comment) in a follow-up PR.

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.

Awesome @benjamn - 👍 from me! We'll want to add docs for this before we finalize 3.4 (but those can definitely be handled in a separate PR). Thanks!

src/react/types/types.ts Outdated Show resolved Hide resolved
@benjamn benjamn force-pushed the mutation-reobserveQuery-using-batch branch from 432e21c to ac87e61 Compare March 26, 2021 20:30
@benjamn benjamn merged commit fb0dc25 into release-3.4 Mar 26, 2021
@benjamn benjamn deleted the mutation-reobserveQuery-using-batch branch March 26, 2021 20:42
@benjamn
Copy link
Member Author

benjamn commented Mar 26, 2021

Wow, just noticed some of these commits are timestamped from August 2020. Guess I've been rebasing this branch locally for a while now!

@hwillson hwillson moved this from In progress to Done in Release 3.4 Mar 30, 2021
@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants