Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

Commit 37ad35e

Browse files
committed
array.$loaded should wait for all promises to resolve. Closes #629.
When we added promise support for user-extendable functions in `$firebaseArray`, the unintended consequense was that the `$loaded` promise was resolving before updates from the server were populated to the array. The solution is to accumulate any user returned promises, and wait until they have all resolved.
1 parent feb8436 commit 37ad35e

File tree

3 files changed

+53
-24
lines changed

3 files changed

+53
-24
lines changed

src/FirebaseArray.js

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@
4747
* var list = new ExtendedArray(ref);
4848
* </code></pre>
4949
*/
50-
angular.module('firebase').factory('$firebaseArray', ["$log", "$firebaseUtils",
51-
function($log, $firebaseUtils) {
50+
angular.module('firebase').factory('$firebaseArray', ["$log", "$firebaseUtils", "$q",
51+
function($log, $firebaseUtils, $q) {
5252
/**
5353
* This constructor should probably never be called manually. It is used internally by
5454
* <code>$firebase.$asArray()</code>.
@@ -631,39 +631,46 @@
631631

632632
var def = $firebaseUtils.defer();
633633
var created = function(snap, prevChild) {
634-
var rec = firebaseArray.$$added(snap, prevChild);
635-
$firebaseUtils.whenUnwrapped(rec, function(rec) {
636-
firebaseArray.$$process('child_added', rec, prevChild);
634+
waitForResolution(firebaseArray.$$added(snap, prevChild), function(rec) {
635+
firebaseArray.$$process('child_added', rec, prevChild)
637636
});
638637
};
639638
var updated = function(snap) {
640639
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
641640
if( rec ) {
642-
var res = firebaseArray.$$updated(snap);
643-
$firebaseUtils.whenUnwrapped(res, function() {
641+
waitForResolution(firebaseArray.$$updated(snap), function() {
644642
firebaseArray.$$process('child_changed', rec);
645643
});
646644
}
647645
};
648646
var moved = function(snap, prevChild) {
649647
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
650648
if( rec ) {
651-
var res = firebaseArray.$$moved(snap, prevChild);
652-
$firebaseUtils.whenUnwrapped(res, function() {
649+
waitForResolution(firebaseArray.$$moved(snap, prevChild), function() {
653650
firebaseArray.$$process('child_moved', rec, prevChild);
654651
});
655652
}
656653
};
657654
var removed = function(snap) {
658655
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
659656
if( rec ) {
660-
var res = firebaseArray.$$removed(snap);
661-
$firebaseUtils.whenUnwrapped(res, function() {
662-
firebaseArray.$$process('child_removed', rec);
657+
waitForResolution(firebaseArray.$$removed(snap), function() {
658+
firebaseArray.$$process('child_removed', rec);
663659
});
664660
}
665661
};
666662

663+
function waitForResolution(maybePromise, callback) {
664+
var promise = $q.when(maybePromise);
665+
promise.then(function(result){
666+
if (result) callback(result);
667+
});
668+
if (!isResolved) {
669+
resolutionPromises.push(promise);
670+
}
671+
}
672+
673+
var resolutionPromises = [];
667674
var isResolved = false;
668675
var error = $firebaseUtils.batch(function(err) {
669676
_initComplete(err);
@@ -677,7 +684,11 @@
677684
destroy: destroy,
678685
isDestroyed: false,
679686
init: init,
680-
ready: function() { return def.promise; }
687+
ready: function() { return def.promise.then(function(result){
688+
return $q.all(resolutionPromises).then(function(){
689+
return result;
690+
});
691+
}); }
681692
};
682693

683694
return sync;

src/utils.js

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -182,16 +182,6 @@
182182

183183
resolve: $q.when,
184184

185-
whenUnwrapped: function(possiblePromise, callback) {
186-
if( possiblePromise ) {
187-
utils.resolve(possiblePromise).then(function(res) {
188-
if( res ) {
189-
callback(res);
190-
}
191-
});
192-
}
193-
},
194-
195185
//TODO: Remove false branch and use only angular implementation when we drop angular 1.2.x support.
196186
promise: angular.isFunction($q) ? $q : Q,
197187

tests/unit/FirebaseArray.spec.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,37 @@ describe('$firebaseArray', function () {
104104
queue[0]('James');
105105
$timeout.flush();
106106
expect(arr.length).toBe(1);
107-
expect(arr[0].name).toBe('James')
107+
expect(arr[0].name).toBe('James');
108108
});
109109

110+
it('should wait to resolve $loaded until $$added promise is resolved', function () {
111+
var queue = [];
112+
function addPromise(snap, prevChild){
113+
return new $utils.promise(
114+
function(resolve) {
115+
queue.push(resolve);
116+
}).then(function(name) {
117+
var data = $firebaseArray.prototype.$$added.call(arr, snap, prevChild);
118+
data.name = name;
119+
return data;
120+
});
121+
}
122+
var called = false;
123+
var ref = stubRef();
124+
arr = stubArray(null, $firebaseArray.$extend({$$added:addPromise}), ref);
125+
arr.$loaded().then(function(){
126+
expect(arr.length).toBe(1);
127+
called = true;
128+
});
129+
ref.set({'-Jwgx':{username:'James', email:'[email protected]'}});
130+
ref.flush();
131+
$timeout.flush();
132+
queue[0]('James');
133+
$timeout.flush();
134+
expect(called, 'called').toBe(true);
135+
});
136+
137+
110138
it('should reject promise on fail', function() {
111139
var successSpy = jasmine.createSpy('resolve spy');
112140
var errSpy = jasmine.createSpy('reject spy');

0 commit comments

Comments
 (0)