onSnapshot support #19
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.
|
Hey @zeevl, thanks for the pull request. Your name is on the PR but @BrianChapman's and @Venryx's names are still attached to all of the commits, so I think we should have no trouble with attribution. I'm just returning from a vacation and need a minute to look these over. |
package-lock.json was deleted and re-created to resolve a merge conflict. # Conflicts: # package-lock.json
|
After merging in master, I am seeing 5 failing unit tests: Does this work on your branch? I'm running a newer version of |
|
Honestly I haven't been able to get tests to run -- gulp keeps crashing with node. What versions are you using? I've tried with both node 10.x and 12.x and they both crash with node stracktraces, which seem to be incompatibility errors. Maybe time to upgrade to gulp 4? |
|
Gulp 3 is incompatible with Node 10+, but if you merge |
|
Perfect, thanks! Tests fixed. |
…into issue-81-onsnapshot � Conflicts: � package-lock.json
src/firestore-query.js
Outdated
| @@ -67,37 +67,11 @@ MockFirestoreQuery.prototype.get = function () { | |||
| var self = this; | |||
| return new Promise(function (resolve, reject) { | |||
| self._defer('get', _.toArray(arguments), function () { | |||
| var results = {}; | |||
| var results = self._results(); | |||
| var limit = 0; | |||
| } | ||
| }); | ||
| } | ||
|
|
||
| resolve(new QuerySnapshot(self.parent === null ? self : self.parent.collection(self.id), results)); |
There was a problem hiding this comment.
I think these two branches are equivalent now.
There was a problem hiding this comment.
Not sure what you're referring to here..
There was a problem hiding this comment.
Line 71 and line 73 do the same thing, which means the if-else is meaningless.
|
Thanks for the quality review @dmurvihill! |
…into issue-81-onsnapshot
# Conflicts: # package-lock.json # src/firestore-query.js
|
Merged. Look for a release in the next few days. Thanks @BrianChapman, @Venryx, and @zeevl for getting this over the line. |
Hey, first, thanks so much for offering to take over support of this project!
This brings over support for
onSnapshotthat current exists in soumak77#158.I added
metadatato theMockFirestoreDocumentSnapshotso we could use this in our project. However, I'm not sure what the proper etiquette is for this type of PR... other people's work on the upstream (dead) project, only slightly modified? They certainly deserve the credit here, haha.Anyways, let me know if you want something done differently here.