Added (fixed) support for onSnapshot (issue 81)#158
Open
Venryx wants to merge 5 commits intosoumak77:masterfrom
Open
Added (fixed) support for onSnapshot (issue 81)#158Venryx wants to merge 5 commits intosoumak77:masterfrom
Venryx wants to merge 5 commits intosoumak77:masterfrom
Conversation
…aChanges is true.
…l the onSnapshot callbacks no mater where the flush is called from.
…tial callback-call. First case where it would fail: the initial-data was null and it would think "no change", despite it needing to always trigger initially. Second case where it would fail: calling onSnapshot on a document instead of a query.
This was referenced Oct 24, 2019
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The pull-request by BrianChapman (#130) mostly added support for onSnapshot, but had an issue where the onSnapshot callback sometimes wasn't called with the initial contents (if value was null at the time, or if called on a dec-ref instead of a query).
This pull-request is an updated version of his, fixing those issues.
EDIT: Apparently, BrianChapman made a newer pull-request (#145) with a fix for orderBy for it -- meaning we have two divergent pull-requests adding different fixes. If @soumak77 returns, one of us can integrate the other's fixes into a single onSnapshot merge. (too lazy atm since @soumak77 seems inactive currently)