Skip to content

Commit

Permalink
Avoid fallible previousResult === newData.result check.
Browse files Browse the repository at this point in the history
Should help fix #3992.

As #3992 (and specifically the reproduction that @OurMajesty provided at
https://codesandbox.io/s/q7rkm6y1ow) demonstrates, since the InMemoryCache
may return the same object for multiple reads (when the data have not
changed), the previousResult is often exactly (===) the same object as
newData.result, which was causing the code removed by this commit to
return early instead of broadcasting the watch.

This early return was unsafe, because the contents of the object may have
changed since they were previously broadcast, so we actually do need to
report those modifications, even though the object reference is the same.

In other words, `previousResult === newData.result` does not imply
"nothing has changed," as I mistakenly assumed in this discussion:
#3394 (comment)

In the longer term, I would like to eliminate the previousResult logic
entirely, since it seems redundant now that we can precisely track changes
to the store, but for now it's important for backwards compatibility.
Specifically, previousResult still provides a way to prefer the previous
object if it is structurally identical to the new object, though that
preference is moot when `previousResult === newData.result`.

The previousResult object should never be used as a basis for skipping
broadcasts. Only the application developer can decide whether === object
identity means it's safe to skip rendering, likely in the context of a
larger design which treats cache results as immutable data. The job of the
InMemoryCache is to make no unsound assumptions, and broadcast results
whenever they may have changed.
  • Loading branch information
benjamn committed Oct 20, 2018
1 parent 1f4abc3 commit d6a673f
Showing 1 changed file with 3 additions and 12 deletions.
15 changes: 3 additions & 12 deletions packages/apollo-cache-inmemory/src/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,20 +339,11 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
// This method is wrapped in the constructor so that it will be called only
// if the data that would be broadcast has changed.
private maybeBroadcastWatch(c: Cache.WatchOptions) {
const previousResult = c.previousResult && c.previousResult();

const newData = this.diff({
c.callback(this.diff({
query: c.query,
variables: c.variables,
previousResult,
previousResult: c.previousResult && c.previousResult(),
optimistic: c.optimistic,
});

if (previousResult &&
previousResult === newData.result) {
return;
}

c.callback(newData);
}));
}
}

0 comments on commit d6a673f

Please sign in to comment.