Skip to content

Commit

Permalink
Fixes #617 - $loaded() triggered too soon
Browse files Browse the repository at this point in the history
Because of promisifying the $$added method, the child_added events were actually being processed internally after the loaded trigger. This occurred because $q.when() actually uses nextTick() internally as well, meaning the child_added events occurred immediately, but then $$process was not called until the second tick occurred; a difficult use case to catch.

The $loaded() event, meanwhile, had already triggered on the first tick. Solution was to promisify all the events and then remove the batch() wrapper since they trigger on nextTick() anyway now. So all events are back to firing in order on the next tick.
  • Loading branch information
katowulf committed May 5, 2015
1 parent 58d32d8 commit 01cea9c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 26 deletions.
44 changes: 20 additions & 24 deletions src/FirebaseArray.js
Expand Up @@ -630,43 +630,39 @@
}

var def = $firebaseUtils.defer();
var created = $firebaseUtils.batch(function(snap, prevChild) {
var created = function(snap, prevChild) {
var rec = firebaseArray.$$added(snap, prevChild);
if( rec ) {
$firebaseUtils.resolve(rec).then(function(rec) {
if( rec ) {
firebaseArray.$$process('child_added', rec, prevChild);
}
});
}
});
var updated = $firebaseUtils.batch(function(snap) {
$firebaseUtils.whenUnwrapped(rec, function(rec) {
firebaseArray.$$process('child_added', rec, prevChild);
});
};
var updated = function(snap) {
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
if( rec ) {
var changed = firebaseArray.$$updated(snap);
if( changed ) {
var res = firebaseArray.$$updated(snap);
$firebaseUtils.whenUnwrapped(res, function() {
firebaseArray.$$process('child_changed', rec);
}
});
}
});
var moved = $firebaseUtils.batch(function(snap, prevChild) {
};
var moved = function(snap, prevChild) {
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
if( rec ) {
var confirmed = firebaseArray.$$moved(snap, prevChild);
if( confirmed ) {
var res = firebaseArray.$$moved(snap, prevChild);
$firebaseUtils.whenUnwrapped(res, function() {
firebaseArray.$$process('child_moved', rec, prevChild);
}
});
}
});
var removed = $firebaseUtils.batch(function(snap) {
};
var removed = function(snap) {
var rec = firebaseArray.$getRecord($firebaseUtils.getKey(snap));
if( rec ) {
var confirmed = firebaseArray.$$removed(snap);
if( confirmed ) {
var res = firebaseArray.$$removed(snap);
$firebaseUtils.whenUnwrapped(res, function() {
firebaseArray.$$process('child_removed', rec);
}
});
}
});
};

var isResolved = false;
var error = $firebaseUtils.batch(function(err) {
Expand Down
14 changes: 12 additions & 2 deletions src/utils.js
Expand Up @@ -61,7 +61,7 @@
batch: function(action, context) {
return function() {
var args = Array.prototype.slice.call(arguments, 0);
$rootScope.$evalAsync(function() {
utils.compile(function() {
action.apply(context, args);
});
};
Expand Down Expand Up @@ -182,6 +182,16 @@

resolve: $q.when,

whenUnwrapped: function(possiblePromise, callback) {
if( possiblePromise ) {
utils.resolve(possiblePromise).then(function(res) {
if( res ) {
callback(res);
}
});
}
},

//TODO: Remove false branch and use only angular implementation when we drop angular 1.2.x support.
promise: angular.isFunction($q) ? $q : Q,

Expand Down Expand Up @@ -210,7 +220,7 @@
},

compile: function(fn) {
return $timeout(fn||function() {});
return $rootScope.$evalAsync(fn||function() {});
},

deepCopy: function(obj) {
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/FirebaseArray.spec.js
Expand Up @@ -417,6 +417,14 @@ describe('$firebaseArray', function () {
expect(spy).toHaveBeenCalledWith(arr);
});

it('should have all data loaded when it resolves', function() {
var spy = jasmine.createSpy('resolve');
arr.$loaded().then(spy);
flushAll();
var list = spy.calls.argsFor(0)[0];
expect(list.length).toBe(5);
});

it('should reject when error fetching records', function() {
var whiteSpy = jasmine.createSpy('resolve');
var blackSpy = jasmine.createSpy('reject');
Expand Down

0 comments on commit 01cea9c

Please sign in to comment.