refactor($compile): always call attr.$observe

attr.$observe used to call function only if there was interpolation
on that attribute. We now call the observation function all the time
but we only save the reference to it if interpolation is present.
This commit is contained in:
Misko Hevery 2012-06-04 15:06:02 -07:00 committed by Igor Minar
parent 2491319575
commit 9be82d942f
4 changed files with 39 additions and 44 deletions

View file

@ -221,9 +221,9 @@ function $CompileProvider($provide) {
this.$get = [
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
'$controller',
'$controller', '$rootScope',
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
$controller) {
$controller, $rootScope) {
var LOCAL_MODE = {
attribute: function(localName, mode, parentScope, scope, attr) {
@ -268,7 +268,6 @@ function $CompileProvider($provide) {
var Attributes = function(element, attr) {
this.$$element = element;
this.$$observers = {};
this.$attr = attr || {};
};
@ -286,7 +285,8 @@ function $CompileProvider($provide) {
* @param {string=} attrName Optional none normalized name. Defaults to key.
*/
$set: function(key, value, writeAttr, attrName) {
var booleanKey = getBooleanAttrName(this.$$element[0], key);
var booleanKey = getBooleanAttrName(this.$$element[0], key),
$$observers = this.$$observers;
if (booleanKey) {
this.$$element.prop(key, value);
@ -314,7 +314,7 @@ function $CompileProvider($provide) {
}
// fire observers
forEach(this.$$observers[key], function(fn) {
$$observers && forEach($$observers[key], function(fn) {
try {
fn(value);
} catch (e) {
@ -333,10 +333,17 @@ function $CompileProvider($provide) {
* @returns {function(*)} the `fn` Function passed in.
*/
$observe: function(key, fn) {
// keep only observers for interpolated attrs
if (this.$$observers[key]) {
this.$$observers[key].push(fn);
}
var attrs = this,
$$observers = (attrs.$$observers || (attrs.$$observers = {})),
listeners = ($$observers[key] || ($$observers[key] = []));
listeners.push(fn);
$rootScope.$evalAsync(function() {
if (!listeners.$$inter) {
// no one registered attribute interpolation function, so lets call it manually
fn(attrs[key]);
}
});
return fn;
}
};
@ -990,16 +997,16 @@ function $CompileProvider($provide) {
directives.push({
priority: 100,
compile: valueFn(function(scope, element, attr) {
var $$observers = (attr.$$observers || (attr.$$observers = {}));
if (name === 'class') {
// we need to interpolate classes again, in the case the element was replaced
// and therefore the two class attrs got merged - we want to interpolate the result
interpolateFn = $interpolate(attr[name], true);
}
// we define observers array only for interpolated attrs
// and ignore observers for non interpolated attrs to save some memory
attr.$$observers[name] = [];
attr[name] = undefined;
($$observers[name] || ($$observers[name] = [])).$$inter = true;
scope.$watch(interpolateFn, function(value) {
attr.$set(name, value);
});

View file

@ -284,7 +284,6 @@ forEach(BOOLEAN_ATTR, function(propName, attrName) {
priority: 100,
compile: function() {
return function(scope, element, attr) {
attr.$$observers[attrName] = [];
scope.$watch(attr[normalized], function(value) {
attr.$set(attrName, !!value);
});
@ -301,27 +300,15 @@ forEach(['src', 'href'], function(attrName) {
ngAttributeAliasDirectives[normalized] = function() {
return {
priority: 99, // it needs to run after the attributes are interpolated
compile: function() {
return function(scope, element, attr) {
var value = attr[normalized];
if (value == undefined) {
// undefined value means that the directive is being interpolated
// so just register observer
attr.$$observers[attrName] = [];
attr.$observe(normalized, function(value) {
attr.$set(attrName, value);
link: function(scope, element, attr) {
attr.$observe(normalized, function(value) {
attr.$set(attrName, value);
// on IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
// to set the property as well to achieve the desired effect
if (msie) element.prop(attrName, value);
});
} else {
// value present means that no interpolation, so copy to native attribute.
attr.$set(attrName, value);
element.prop(attrName, value);
}
};
// on IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
// to set the property as well to achieve the desired effect
if (msie) element.prop(attrName, value);
});
}
};
};

View file

@ -1239,7 +1239,6 @@ var ngValueDirective = function() {
};
} else {
return function(scope, elm, attr) {
attr.$$observers.value = [];
scope.$watch(attr.ngValue, function(value) {
attr.$set('value', value, false);
});

View file

@ -1249,15 +1249,15 @@ describe('$compile', function() {
describe('interpolation', function() {
var observeSpy, attrValueDuringLinking;
var observeSpy, directiveAttrs;
beforeEach(module(function() {
directive('observer', function() {
return function(scope, elm, attr) {
directiveAttrs = attr;
observeSpy = jasmine.createSpy('$observe attr');
expect(attr.$observe('someAttr', observeSpy)).toBe(observeSpy);
attrValueDuringLinking = attr.someAttr;
};
});
}));
@ -1295,19 +1295,21 @@ describe('$compile', function() {
it('should set interpolated attrs to undefined', inject(function($rootScope, $compile) {
attrValueDuringLinking = null;
$compile('<div some-attr="{{whatever}}" observer></div>')($rootScope);
expect(attrValueDuringLinking).toBeUndefined();
expect(directiveAttrs.someAttr).toBeUndefined();
}));
it('should not call observer of non-interpolated attr', inject(function($rootScope, $compile) {
$compile('<div some-attr="nonBound" observer></div>')($rootScope);
expect(attrValueDuringLinking).toBe('nonBound');
it('should call observer of non-interpolated attr through $evalAsync',
inject(function($rootScope, $compile) {
$compile('<div some-attr="nonBound" observer></div>')($rootScope);
expect(directiveAttrs.someAttr).toBe('nonBound');
$rootScope.$digest();
expect(observeSpy).not.toHaveBeenCalled();
}));
expect(observeSpy).not.toHaveBeenCalled();
$rootScope.$digest();
expect(observeSpy).toHaveBeenCalled();
})
);
it('should delegate exceptions to $exceptionHandler', function() {