Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Order by (and Issue 81 onsnapshot implementation) #145

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

BrianChapman
Copy link

Fixing the Orderby conflicted with changes in PR #130 (the onsnapshot implementation) so this is a merge of both changes. Probably can close pull #130 if this gets merged in.

…l the onSnapshot callbacks no mater where the flush is called from.
* issue-81-onsnapshot:
  Move onSnapshot handers to the queue so that a call to flush will call the onSnapshot callbacks no mater where the flush is called from.
  onSnapshot should call callback if no data changed and includeMetadataChanges is true.
  Add onSnapshot functionality to doc, collection and query
* issue-81-onsnapshot:
  onSnapshot should call immediatly and then after data changes.
@Venryx
Copy link

Venryx commented Oct 24, 2019

Oops. It looks like I based my fixed pull-request (#158) on your old onSnapshot pull-request (#130), not this newer one.

I'd rebase my changes on this pull-request, except it looks like @soumak77 is not currently active on evaluating/merging pull-requests.

Anyway, this pull-request adds basic onSnapshot support + orderBy for it, whereas mine (#158) adds basic onSnapshot support + fixes for two broken cases.

I don't need orderBy currently so am too lazy to update my branch, but if @soumak77 returns to take a look at the pull-requests, I can do the rebasing procedure mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants