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

Introduce new cache.batch(options) method for InMemoryCache. #7819

Merged
merged 6 commits into from
Mar 11, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Mar 10, 2021

As new methods have been added to ApolloCache and InMemoryCache, we have found it convenient in many cases (e.g. cache.evict and cache.modify) to use a method signature that accepts named options, rather than positional parameters. Named options are easier to extend in the future, and less cumbersome when some of the options might be omitted.

This awkwardness of positional parameters affects the two existing transaction-related methods of InMemoryCache:

cache.performTransaction(cache => {...}, id?);
cache.recordOptimisticTransaction(cache => {...}, id);

Instead of adding new functionality to these methods by introducing new positional parameters, I would very much prefer to switch to named options, but I was previously concerned that introducing a new transaction method for the ApolloCache superclass would be a breaking change for anyone not using InMemoryCache.

As this PR demonstrates, it is possible to introduce an ApolloCache method called batch with a default implementation that just calls this.performTransaction, so any custom cache implementation (not based on InMemoryCache) immediately inherits a usable version of the batch method. The subclass may choose to override batch to take advantage of its named options, but that's not mandatory. Either way, ApolloClient (specifically, QueryManager#markMutationResult) can safely start calling cache.batch instead of calling cache.performTransaction as it does now.

Of course, InMemoryCache overrides batch, and relocates most of the code that used to live in performTransaction into the new batch method. It's a much more powerful and ergonomic API, already allowing new functionality like the onDirty callback, and an options.transaction function whose first parameter has the subclass type (e.g. InMemoryCache) rather than the ApolloCache superclass type.

This PR is a stepping stone to implementing 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.

I will finish implementing those features in subsequent PRs, but you might already be able to guess how onDirty will help.

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.

This looks great @benjamn!

Side note - I think taking a long look at our continued support of the ApolloCache abstraction is something we should add to our 4.0 planning.

benjamn added 4 commits March 11, 2021 11:38
This new method is similar to performTransaction, but takes an extensible
options object rather than positional arguments.

Among the new options is an onDirty callback function, which will be
invoked for each cache watcher affected by the batch transaction.
@benjamn benjamn force-pushed the new-InMemoryCache-batch-method branch from 2a75510 to 4c0d58f Compare March 11, 2021 16:38
@benjamn benjamn merged commit 70751df into release-3.4 Mar 11, 2021
@benjamn benjamn deleted the new-InMemoryCache-batch-method branch March 11, 2021 16:51
benjamn added a commit that referenced this pull request Mar 26, 2021
When an options.onDirty callback is provided to cache.batch (#7819), we
want to call it with only the Cache.WatchOptions objects that were
directly affected by options.transaction, so it's important to broadcast
watches before the transaction, to flush out any pending watches waiting
to be broadcast. See provided tests for an example where this matters.
benjamn added a commit that referenced this pull request Mar 26, 2021
When an `options.onDirty` callback is provided to `cache.batch` (#7819),
we want to call `onDirty` with only the `Cache.WatchOptions` objects
that were directly affected by `options.transaction`, so it's important to
broadcast watches _before_ the transaction, to flush out any pending
watches waiting to be broadcast.
@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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants