Skip to content

Commit 4f74b36

Browse files
jerelmillerjerelmiller
committed
Use an early return
1 parent 19c0c74 commit 4f74b36

1 file changed

Lines changed: 92 additions & 91 deletions

File tree

‎src/core/QueryInfo.ts‎

Lines changed: 92 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -309,100 +309,101 @@ export class QueryInfo<
309309
return result;
310310
}
311311

312-
if (shouldWriteResult(result, errorPolicy)) {
313-
// Using a transaction here so we have a chance to read the result
314-
// back from the cache before the watch callback fires as a result
315-
// of writeQuery, so we can store the new diff quietly and ignore
316-
// it when we receive it redundantly from the watch callback.
317-
this.cache.batch({
318-
onWatchUpdated: (
319-
// all additional options on ObservableQuery.CacheWatchOptions are
320-
// optional so we can use the type here
321-
watch: ObservableQuery.CacheWatchOptions,
322-
diff
323-
) => {
324-
if (watch.watcher === this.observableQuery) {
325-
// see comment on `lastOwnDiff` for explanation
326-
watch.lastOwnDiff = diff;
327-
}
328-
},
329-
update: (cache) => {
330-
const shouldWrite = this.shouldWrite(result, variables);
331-
332-
// If result is the same as the last result we received from
333-
// the network (and the variables match too), avoid writing
334-
// result into the cache again. The wisdom of skipping this
335-
// cache write is far from obvious, since any cache write
336-
// could be the one that puts the cache back into a desired
337-
// state, fixing corruption or missing data. However, if we
338-
// always write every network result into the cache, we enable
339-
// feuds between queries competing to update the same data in
340-
// incompatible ways, which can lead to an endless cycle of
341-
// cache broadcasts and useless network requests. As with any
342-
// feud, eventually one side must step back from the brink,
343-
// letting the other side(s) have the last word(s). There may
344-
// be other points where we could break this cycle, such as
345-
// silencing the broadcast for cache.writeQuery (not a good
346-
// idea, since it just delays the feud a bit) or somehow
347-
// avoiding the network request that just happened (also bad,
348-
// because the server could return useful new data). All
349-
// options considered, skipping this cache write seems to be
350-
// the least damaging place to break the cycle, because it
351-
// reflects the intuition that we recently wrote this exact
352-
// result into the cache, so the cache *should* already/still
353-
// contain this data. If some other query has clobbered that
354-
// data in the meantime, that's too bad, but there will be no
355-
// winners if every query blindly reverts to its own version
356-
// of the data. This approach also gives the network a chance
357-
// to return new data, which will be written into the cache as
358-
// usual, notifying only those queries that are directly
359-
// affected by the cache updates, as usual. In the future, an
360-
// even more sophisticated cache could perhaps prevent or
361-
// mitigate the clobbering somehow, but that would make this
362-
// particular cache write even less important, and thus
363-
// skipping it would be even safer than it is today.
364-
if (shouldWrite) {
365-
cache.writeQuery({
366-
query,
367-
data: result.data as Unmasked<any>,
368-
variables,
369-
overwrite: cacheWriteBehavior === CacheWriteBehavior.OVERWRITE,
370-
extensions: result.extensions,
371-
});
372-
373-
this.lastWrite = {
374-
result,
375-
variables,
376-
dmCount: destructiveMethodCounts.get(this.cache),
377-
};
378-
}
379-
380-
const isNetworkOnly =
381-
fetchPolicy === "network-only" &&
382-
networkStatus !== NetworkStatus.refetch;
383-
384-
const { dataState, result: diffResult } = this.getDiff(
385-
{
386-
...diffOptions,
387-
// Never deliver partial data for network-only requests
388-
returnPartialData: returnPartialData && !isNetworkOnly,
389-
},
390-
this.getIncrementalInfo(result, { isNetworkOnly })
391-
);
392-
393-
if (
394-
dataState === "complete" ||
395-
(returnPartialData && dataState === "partial" && shouldWrite) ||
396-
(this.hasNext && dataState === "streaming")
397-
) {
398-
result = { ...result, data: diffResult, dataState };
399-
}
400-
},
401-
});
402-
} else {
312+
if (!shouldWriteResult(result, errorPolicy)) {
403313
this.lastWrite = void 0;
314+
return result;
404315
}
405316

317+
// Using a transaction here so we have a chance to read the result
318+
// back from the cache before the watch callback fires as a result
319+
// of writeQuery, so we can store the new diff quietly and ignore
320+
// it when we receive it redundantly from the watch callback.
321+
this.cache.batch({
322+
onWatchUpdated: (
323+
// all additional options on ObservableQuery.CacheWatchOptions are
324+
// optional so we can use the type here
325+
watch: ObservableQuery.CacheWatchOptions,
326+
diff
327+
) => {
328+
if (watch.watcher === this.observableQuery) {
329+
// see comment on `lastOwnDiff` for explanation
330+
watch.lastOwnDiff = diff;
331+
}
332+
},
333+
update: (cache) => {
334+
const shouldWrite = this.shouldWrite(result, variables);
335+
336+
// If result is the same as the last result we received from
337+
// the network (and the variables match too), avoid writing
338+
// result into the cache again. The wisdom of skipping this
339+
// cache write is far from obvious, since any cache write
340+
// could be the one that puts the cache back into a desired
341+
// state, fixing corruption or missing data. However, if we
342+
// always write every network result into the cache, we enable
343+
// feuds between queries competing to update the same data in
344+
// incompatible ways, which can lead to an endless cycle of
345+
// cache broadcasts and useless network requests. As with any
346+
// feud, eventually one side must step back from the brink,
347+
// letting the other side(s) have the last word(s). There may
348+
// be other points where we could break this cycle, such as
349+
// silencing the broadcast for cache.writeQuery (not a good
350+
// idea, since it just delays the feud a bit) or somehow
351+
// avoiding the network request that just happened (also bad,
352+
// because the server could return useful new data). All
353+
// options considered, skipping this cache write seems to be
354+
// the least damaging place to break the cycle, because it
355+
// reflects the intuition that we recently wrote this exact
356+
// result into the cache, so the cache *should* already/still
357+
// contain this data. If some other query has clobbered that
358+
// data in the meantime, that's too bad, but there will be no
359+
// winners if every query blindly reverts to its own version
360+
// of the data. This approach also gives the network a chance
361+
// to return new data, which will be written into the cache as
362+
// usual, notifying only those queries that are directly
363+
// affected by the cache updates, as usual. In the future, an
364+
// even more sophisticated cache could perhaps prevent or
365+
// mitigate the clobbering somehow, but that would make this
366+
// particular cache write even less important, and thus
367+
// skipping it would be even safer than it is today.
368+
if (shouldWrite) {
369+
cache.writeQuery({
370+
query,
371+
data: result.data as Unmasked<any>,
372+
variables,
373+
overwrite: cacheWriteBehavior === CacheWriteBehavior.OVERWRITE,
374+
extensions: result.extensions,
375+
});
376+
377+
this.lastWrite = {
378+
result,
379+
variables,
380+
dmCount: destructiveMethodCounts.get(this.cache),
381+
};
382+
}
383+
384+
const isNetworkOnly =
385+
fetchPolicy === "network-only" &&
386+
networkStatus !== NetworkStatus.refetch;
387+
388+
const { dataState, result: diffResult } = this.getDiff(
389+
{
390+
...diffOptions,
391+
// Never deliver partial data for network-only requests
392+
returnPartialData: returnPartialData && !isNetworkOnly,
393+
},
394+
this.getIncrementalInfo(result, { isNetworkOnly })
395+
);
396+
397+
if (
398+
dataState === "complete" ||
399+
(returnPartialData && dataState === "partial" && shouldWrite) ||
400+
(this.hasNext && dataState === "streaming")
401+
) {
402+
result = { ...result, data: diffResult, dataState };
403+
}
404+
},
405+
});
406+
406407
return result;
407408
}
408409

0 commit comments

Comments
 (0)