Check structural equality in QueryInfo#setDiff. #6891
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
benjamn
force-pushed
the
check-structural-equality-in-QueryInfo-setDiff
branch
from
b210438 to
3d5aebd
Compare
benjamn
added a commit
that referenced
this pull request
Aug 28, 2020
This reverts commit 24a4396, which prevented some legitimate updates in https://studio.apollographql.com. We will have to keep investigating #6888 to find a better solution.
benjamn
added a commit
that referenced
this pull request
Apr 16, 2021
Revisiting PR #6891, which was reverted by commit c2ef68f. With the introduction of canonical cache results (#7439), the strict equality check in setDiff is about to become mostly synonymous with deep equality checking (but much faster to check), so we need to deal with the consequences of #6891 one way or another. As evidence that this change now (after #7439) has no observable impact, notice that we were able to switch back to using !equal(a, b) without needing to fix/update any failing tests, compared to the few tests that needed updating in #6891. However, by switching to deep equality checking, we allow ourselves the option to experiment with disabling (or periodically clearing) the ObjectCanon, which would presumably lead to broadcasting more !== but deeply equal results to cache.watch watchers. With deep equality checking in setDiff, those !== results will get past the filter in the same cases canonical objects would, so there's less of a discrepancy in client behavior with/without canonization enabled.
benjamn
added a commit
that referenced
this pull request
May 12, 2021
Revisiting PR #6891, which was reverted by commit c2ef68f. With the introduction of canonical cache results (#7439), the strict equality check in setDiff is about to become mostly synonymous with deep equality checking (but much faster to check), so we need to deal with the consequences of #6891 one way or another. As evidence that this change now (after #7439) has no observable impact, notice that we were able to switch back to using !equal(a, b) without needing to fix/update any failing tests, compared to the few tests that needed updating in #6891. However, by switching to deep equality checking, we allow ourselves the option to experiment with disabling (or periodically clearing) the ObjectCanon, which would presumably lead to broadcasting more !== but deeply equal results to cache.watch watchers. With deep equality checking in setDiff, those !== results will get past the filter in the same cases canonical objects would, so there's less of a discrepancy in client behavior with/without canonization enabled.
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.
Should help with #6888, which demonstrates that
===disagrees withequalin some important cases, even though the cache is often able to provide===results when nothing has changed.