Implement ObservableQuery#isDifferentFromLastResult. #4069
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit is a more conservative version of e66027c We still need to make a deep clone of observableQuery.lastResult in order to determine if future results are different (see #3992), but we can restrict the use of that snapshot to a single method, rather than replacing observableQuery.lastResult with the snapshot. Should help with #4054.
benjamn
force-pushed
the
issue-4054-ObservableQuery-isDifferentFromLastResult
branch
from
9db4980
to
510fc3c
Compare
benjamn
deleted the
issue-4054-ObservableQuery-isDifferentFromLastResult
branch
benjamn
added a commit
that referenced
this pull request
Feb 28, 2019
…bility. Part of the plan I outlined in this comment: #4464 (comment) If we could trust application code not to modify cache results, we wouldn't have to save deep snapshots of past results in order to implement isDifferentFromLastResult correctly (see #4069). Aside: why doesn't the cache just return defensive copies of all results? #4031 (comment) While you might agree that immutability is a worthwhile aspiration, it can be hard to maintain that discipline across your entire application over time, especially in a team of multiple developers. This commit implements a new freezeResults option for the InMemoryCache constructor, which (when true) causes all cache results to be frozen in development, so you can more easily detect accidental mutations. Note: mutating frozen objects only throws in strict mode, whereas it fails silently in non-strict code. ECMAScript module code automatically runs in strict mode, and most module transforms add "use strict" at the top of the generated code, so you're probably already using strict mode everywhere, though you might want to double-check. The beauty of this implementation is that it does not need to repeatedly freeze entire results, because it can shallow-freeze the root of each subtree when that object is first created. Thanks to result caching, those frozen objects can be shared between multiple different result trees without any additional freezing, and the entire result always ends up deeply frozen. The freezing happens only in non-production environments, so there is no runtime cost to using { freezeResults: true } in production. Please keep this in mind when benchmarking cache performance!
This was referenced Feb 28, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit is a more conservative version of e66027c.
We still need to make a deep clone of
observableQuery.lastResultin order to determine if future results are different (see #3992), but we should be able to restrict the use of that snapshot to a single method, rather than replacingobservableQuery.lastResultwith the snapshot.Should help with #4054 (cc @jsslai).