-
Notifications
You must be signed in to change notification settings - Fork 21
onSnapshot support #19
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
Changes from all commits
1fade31
57643cc
65a9ddf
27e7f8f
bba6e25
f3df3bd
c8557f5
e4169d4
bbe1b38
d55bbd4
4cee9bd
55e64c9
8d40b88
746b287
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ | |
node_modules | ||
coverage | ||
browser | ||
.vscode |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ var EventEmitter = require('events').EventEmitter; | |
|
||
function FlushQueue () { | ||
this.events = []; | ||
this.postFlushListeners = []; | ||
} | ||
|
||
FlushQueue.prototype.push = function () { | ||
|
@@ -23,6 +24,14 @@ FlushQueue.prototype.push = function () { | |
})); | ||
}; | ||
|
||
FlushQueue.prototype.onPostFlush = function(subscriber) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing tests for this change. Flush triggers subscribed listener, flush does not trigger unsubscribed listener, flush does not trigger subscribed listener during flush, unsubscribe is either idempotent or throws error on second call (not currently true!)... anything I forgot? |
||
this.postFlushListeners.push(subscriber); | ||
var self = this; | ||
return function() { | ||
self.postFlushListeners.pop(subscriber); | ||
}; | ||
}; | ||
|
||
FlushQueue.prototype.flushing = false; | ||
|
||
FlushQueue.prototype.flush = function (delay) { | ||
|
@@ -33,6 +42,9 @@ FlushQueue.prototype.flush = function (delay) { | |
} | ||
function process () { | ||
self.flushing = true; | ||
_.forEach(self.postFlushListeners, function (subscriber) { | ||
self.push(subscriber); | ||
}); | ||
while (self.events.length) { | ||
self.events[0].run(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -419,4 +419,99 @@ describe('MockFirestoreCollection', function () { | |
]); | ||
}); | ||
}); | ||
|
||
describe('#onSnapshot', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to fully test the allowed API usages here (unfortunately, there are lots of those). The API doc says we can call
|
||
it('returns value after collection is updated', function (done) { | ||
zeevl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var callCount = 0; | ||
collection.onSnapshot(function(snap) { | ||
callCount += 1; | ||
var names = []; | ||
snap.docs.forEach(function(doc) { | ||
names.push(doc.data().name); | ||
}); | ||
|
||
if (callCount === 2) { | ||
expect(names).to.contain('A'); | ||
expect(names).not.to.contain('a'); | ||
done(); | ||
} | ||
}); | ||
collection.doc('a').update({name: 'A'}, {setMerge: true}); | ||
collection.flush(); | ||
}); | ||
|
||
it('calls callback after multiple updates', function (done) { | ||
zeevl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var callCount = 0; | ||
collection.onSnapshot(function(snap) { | ||
callCount += 1; | ||
var names = []; | ||
snap.docs.forEach(function(doc) { | ||
names.push(doc.data().name); | ||
}); | ||
|
||
if (callCount === 2) { | ||
expect(names).to.contain('A'); | ||
expect(names).not.to.contain('a'); | ||
} | ||
|
||
if (callCount === 3) { | ||
expect(names).to.contain('AA'); | ||
expect(names).not.to.contain('A'); | ||
done(); | ||
} | ||
}); | ||
|
||
collection.doc('a').update({name: 'A'}, {setMerge: true}); | ||
collection.flush(); | ||
collection.doc('a').update({name: 'AA'}, {setMerge: true}); | ||
collection.flush(); | ||
}); | ||
|
||
it('should unsubscribe', function (done) { | ||
var callCount = 0; | ||
var unsubscribe = collection.onSnapshot(function(snap) { | ||
zeevl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
callCount += 1; | ||
}); | ||
|
||
collection.doc('a').update({name: 'A'}, {setMerge: true}); | ||
collection.flush(); | ||
|
||
process.nextTick(function() { | ||
expect(callCount).to.equal(2); | ||
|
||
collection.doc('a').update({name: 'AA'}, {setMerge: true}); | ||
unsubscribe(); | ||
|
||
collection.flush(); | ||
|
||
process.nextTick(function() { | ||
expect(callCount).to.equal(2); | ||
done(); | ||
}); | ||
}); | ||
|
||
|
||
}); | ||
|
||
it('Calls onError if error', function (done) { | ||
var error = new Error("An error occured."); | ||
collection.errs.onSnapshot = error; | ||
var callCount = 0; | ||
collection.onSnapshot(function(snap) { | ||
throw new Error("This should not be called."); | ||
}, function(err) { | ||
// onSnapshot always returns when first called and then | ||
// after data changes so we get 2 calls here. | ||
if (callCount == 0) { | ||
callCount++; | ||
return; | ||
} | ||
expect(err).to.equal(error); | ||
done(); | ||
}); | ||
collection.doc('a').update({name: 'A'}, {setMerge: true}); | ||
collection.flush(); | ||
}); | ||
|
||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two branches are equivalent now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you're referring to here..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 71 and line 73 do the same thing, which means the if-else is meaningless.