Skip to content

Commit

Permalink
Rename cache.batch options.transaction function to options.update.
Browse files Browse the repository at this point in the history
I've never felt like "transaction" was exactly the right word for what
cache.performTransaction and cache.batch are doing.

For example, the updates are not automatically undone on failure like the
word "transaction" suggests, unless you're recording against an optimistic
layer that you plan to remove later, via cache.recordOptimisticTransaction
and cache.removeOptimistic.

In reality, the primary purpose of cache.performTransaction and
cache.batch is to batch up broadcastWatches notifications for cache
updates that take place within the callback function.

I chose options.update because I think it's generic enough to avoid giving
any mistaken interpretations of the purpose or usage of the function,
unlike options.transaction.

Separately, I believe options.update better aligns with the naming of the
options.updateCache function for client.refetchQueries. Which may lead you
to ask... Why not call it updateCache, exactly like client.refetchQueries?

This is certainly a matter of subjective taste, but to me it feels
redundant (and not useful for readability) to have to type the extra
-Cache suffix every time you call cache.batch (which is more clearly a
cache-updating method than client.refetchQueries is):

  cache.batch({
    updateCache(cache) {...},
    ...
  })

This commit will allow the following code, which eliminates the redundancy
without sacrificing readability:

  cache.batch({
    update(cache) {...},
    ...
  })

For comparison, client.refetchQueries needs the extra specificity of
updateCache because it gives readers of the code a clue about what's being
updated (namely, the cache):

  client.refetchQueries({
    updateCache(cache) {...},
    ...
  })
  • Loading branch information
benjamn committed Apr 20, 2021
1 parent b19a89f commit 19d357f
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/cache/core/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
const optimisticId =
typeof options.optimistic === "string" ? options.optimistic :
options.optimistic === false ? null : void 0;
this.performTransaction(options.transaction, optimisticId);
this.performTransaction(options.update, optimisticId);
}

public abstract performTransaction(
Expand Down
2 changes: 1 addition & 1 deletion src/cache/core/types/Cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export namespace Cache {
export interface BatchOptions<C extends ApolloCache<any>> {
// Same as the first parameter of performTransaction, except the cache
// argument will have the subclass type rather than ApolloCache.
transaction(cache: C): void;
update(cache: C): void;

// Passing a string for this option creates a new optimistic layer, with the
// given string as its layer.id, just like passing a string for the
Expand Down
12 changes: 6 additions & 6 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,7 @@ describe('Cache', () => {
const dirtied = new Map<Cache.WatchOptions, Cache.DiffResult<any>>();

cache.batch({
transaction(cache) {
update(cache) {
cache.writeQuery({
query: aQuery,
data: {
Expand Down Expand Up @@ -1400,7 +1400,7 @@ describe('Cache', () => {
dirtied.clear();

cache.batch({
transaction(cache) {
update(cache) {
cache.writeQuery({
query: bQuery,
data: {
Expand Down Expand Up @@ -1471,7 +1471,7 @@ describe('Cache', () => {
const dirtied = new Map<Cache.WatchOptions, Cache.DiffResult<any>>();

cache.batch({
transaction(cache) {
update(cache) {
cache.modify({
fields: {
a(value, { INVALIDATE }) {
Expand Down Expand Up @@ -1545,7 +1545,7 @@ describe('Cache', () => {
const dirtied = new Map<Cache.WatchOptions, Cache.DiffResult<any>>();

cache.batch({
transaction(cache) {
update(cache) {
cache.modify({
fields: {
a(value) {
Expand All @@ -1568,7 +1568,7 @@ describe('Cache', () => {

expect(aInfo.diffs).toEqual([
// This diff resulted from the cache.modify call in the cache.batch
// transaction function.
// update function.
{
complete: true,
result: {
Expand All @@ -1579,7 +1579,7 @@ describe('Cache', () => {

expect(abInfo.diffs).toEqual([
// This diff resulted from the cache.modify call in the cache.batch
// transaction function.
// update function.
{
complete: true,
result: {
Expand Down
43 changes: 21 additions & 22 deletions src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {

public batch(options: Cache.BatchOptions<InMemoryCache>) {
const {
transaction,
update,
optimistic = true,
removeOptimistic,
onWatchUpdated,
Expand All @@ -317,7 +317,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
this.data = this.optimisticData = layer;
}
try {
transaction(this);
update(this);
} finally {
--this.txCount;
this.data = data;
Expand All @@ -328,16 +328,15 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
const alreadyDirty = new Set<Cache.WatchOptions>();

if (onWatchUpdated && !this.txCount) {
// If an options.onWatchUpdated callback is provided, we want to
// call it with only the Cache.WatchOptions objects affected by
// options.transaction, but there might be dirty watchers already
// waiting to be broadcast that have nothing to do with the
// transaction. To prevent including those watchers in the
// post-transaction broadcast, we perform this initial broadcast to
// collect the dirty watchers, so we can re-dirty them later, after
// the post-transaction broadcast, allowing them to receive their
// pending broadcasts the next time broadcastWatches is called, just
// as they would if we never called cache.batch.
// If an options.onWatchUpdated callback is provided, we want to call it
// with only the Cache.WatchOptions objects affected by options.update,
// but there might be dirty watchers already waiting to be broadcast that
// have nothing to do with the update. To prevent including those watchers
// in the post-update broadcast, we perform this initial broadcast to
// collect the dirty watchers, so we can re-dirty them later, after the
// post-update broadcast, allowing them to receive their pending
// broadcasts the next time broadcastWatches is called, just as they would
// if we never called cache.batch.
this.broadcastWatches({
...options,
onWatchUpdated(watch) {
Expand All @@ -354,14 +353,14 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
this.optimisticData = this.optimisticData.addLayer(optimistic, perform);
} else if (optimistic === false) {
// Ensure both this.data and this.optimisticData refer to the root
// (non-optimistic) layer of the cache during the transaction. Note
// that this.data could be a Layer if we are currently executing an
// optimistic transaction function, but otherwise will always be an
// EntityStore.Root instance.
// (non-optimistic) layer of the cache during the update. Note that
// this.data could be a Layer if we are currently executing an optimistic
// update function, but otherwise will always be an EntityStore.Root
// instance.
perform(this.data);
} else {
// Otherwise, leave this.data and this.optimisticData unchanged and
// run the transaction with broadcast batching.
// Otherwise, leave this.data and this.optimisticData unchanged and run
// the update with broadcast batching.
perform();
}

Expand All @@ -386,8 +385,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
return result;
}
});
// Silently re-dirty any watches that were already dirty before the
// transaction was performed, and were not broadcast just now.
// Silently re-dirty any watches that were already dirty before the update
// was performed, and were not broadcast just now.
if (alreadyDirty.size) {
alreadyDirty.forEach(watch => this.maybeBroadcastWatch.dirty(watch));
}
Expand All @@ -400,11 +399,11 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
}

public performTransaction(
transaction: (cache: InMemoryCache) => any,
update: (cache: InMemoryCache) => any,
optimisticId?: string | null,
) {
return this.batch({
transaction,
update,
optimistic: optimisticId || (optimisticId !== null),
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,11 +1095,11 @@ export class QueryManager<TStore> {

if (updateCache) {
this.cache.batch({
transaction: updateCache,
update: updateCache,

// Since you can perform any combination of cache reads and/or writes in
// the cache.batch transaction function, its optimistic option can be
// either a boolean or a string, representing three distinct modes of
// the cache.batch update function, its optimistic option can be either
// a boolean or a string, representing three distinct modes of
// operation:
//
// * false: read/write only the root layer
Expand All @@ -1110,7 +1110,7 @@ export class QueryManager<TStore> {
// temporarily created within cache.batch with that string as its ID. If
// we then pass that same string as the removeOptimistic option, we can
// make cache.batch immediately remove the optimistic layer after
// running the transaction, triggering only one broadcast.
// running the updateCache function, triggering only one broadcast.
//
// However, the refetchQueries method accepts only true or false for its
// optimistic option (not string). We interpret true to mean a temporary
Expand Down

0 comments on commit 19d357f

Please sign in to comment.