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

Commit b1ec35b

Browse files
committed
fix/perf: combine value and metaVar listener
checking metaVars and scopeValue separately causes the listener function to be called twice during a $digest cycle if both the metaVars and value change. This definitely causes a performance hit, and might be leading to subtle bugs
1 parent 0f131a8 commit b1ec35b

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

src/FirebaseObject.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -354,20 +354,16 @@
354354

355355
// $watch will not check any vars prefixed with $, so we
356356
// manually check $priority and $value using this method
357-
function checkMetaVars() {
358-
var dat = parsed(scope);
359-
if( dat.$value !== rec.$value || dat.$priority !== rec.$priority ) {
360-
scopeUpdated();
361-
}
357+
function watchExp(){
358+
var obj = parsed(scope);
359+
return [obj, obj.$priority, obj.$value];
362360
}
363361

364-
self.subs.push(scope.$watch(checkMetaVars));
365-
366362
setScope(rec);
367363
self.subs.push(scope.$on('$destroy', self.unbind.bind(self)));
368364

369365
// monitor scope for any changes
370-
self.subs.push(scope.$watch(varName, scopeUpdated, true));
366+
self.subs.push(scope.$watch(watchExp, scopeUpdated, true));
371367

372368
// monitor the object for changes
373369
self.subs.push(rec.$watch(recUpdated));

tests/unit/FirebaseObject.spec.js

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,7 @@ describe('$FirebaseObject', function() {
331331
$scope.test.$priority = 999;
332332
});
333333
$interval.flush(500);
334-
$timeout.flush(); // for $interval
335-
$timeout.flush(); // for $watch
334+
$timeout.flush();
336335
expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({'.priority': 999}));
337336
});
338337

@@ -348,11 +347,39 @@ describe('$FirebaseObject', function() {
348347
$scope.test.$value = 'bar';
349348
});
350349
$interval.flush(500);
351-
$timeout.flush(); // for $interval
352-
$timeout.flush(); // for $watch
350+
$timeout.flush();
353351
expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({'.value': 'bar'}));
354352
});
355353

354+
it('should only call $$scopeUpdated once if both metaVars and properties change in the same $digest',function(){
355+
var $scope = $rootScope.$new();
356+
var fb = new MockFirebase();
357+
fb.autoFlush(true);
358+
fb.setWithPriority({text:'hello'},3);
359+
var $fb = new $firebase(fb);
360+
var obj = $fb.$asObject();
361+
flushAll();
362+
flushAll();
363+
obj.$bindTo($scope, 'test');
364+
$scope.$apply();
365+
expect($scope.test).toEqual({text:'hello', $id: obj.$id, $priority: 3});
366+
var callCount = 0;
367+
var old$scopeUpdated = obj.$$scopeUpdated;
368+
obj.$$scopeUpdated = function(){
369+
callCount++;
370+
return old$scopeUpdated.apply(this,arguments);
371+
};
372+
$scope.$apply(function(){
373+
$scope.test.text='goodbye';
374+
$scope.test.$priority=4;
375+
});
376+
flushAll();
377+
flushAll();
378+
flushAll();
379+
flushAll();
380+
expect(callCount).toEqual(1);
381+
});
382+
356383
it('should throw error if double bound', function() {
357384
var $scope = $rootScope.$new();
358385
var aSpy = jasmine.createSpy('firstBind');

0 commit comments

Comments
 (0)