Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($compile): attribute bindings should not break due to terminal di…
Browse files Browse the repository at this point in the history
…rectives

Recently we changed the priority of attribute interpolation directive to -100
to ensure that it executes early in the post linking phase. This causes issues
with when terminal directives are placed on elements with attribute bindings
because the terminal directive will usually have 0 or higher priority which
results in attr interpolation directive not being applied to the element.

To fix this issue I'm switching the priority back to 100 and making moving the
binding setup into the pre-linking function.

This means that:

- terminal directives with priority lower than 100 will not affect the attribute
  binding
- if a directive wants to add or alter bindings it can do so in the pre-linking
  phase, as long as the priority of this directive is more than 100
- all post-linking functions will execute after the attribute binding has been
  set up
- all pre-linking functions with directive priority lower than 100 will execute
  after the attribute bindings have been setup

BREAKING CHANGE: the attribute interpolation (binding) executes as a directive
with priority 100 and the binding is set up in the pre-linking phase. It used
to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding
was setup in the post-linking phase.

Closes #4525
Closes #4528
Closes #4649
  • Loading branch information
IgorMinar committed Oct 25, 2013
1 parent 9bf9c23 commit 79223ea
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 32 deletions.
56 changes: 30 additions & 26 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1717,33 +1717,37 @@ function $CompileProvider($provide) {
}

directives.push({
priority: -100,
compile: valueFn(function attrInterpolateLinkFn(scope, element, attr) {
var $$observers = (attr.$$observers || (attr.$$observers = {}));

if (EVENT_HANDLER_ATTR_REGEXP.test(name)) {
throw $compileMinErr('nodomevents',
"Interpolations for HTML DOM event attributes are disallowed. Please use the " +
"ng- versions (such as ng-click instead of onclick) instead.");
}
priority: 100,
compile: function() {
return {
pre: function attrInterpolatePreLinkFn(scope, element, attr) {
var $$observers = (attr.$$observers || (attr.$$observers = {}));

if (EVENT_HANDLER_ATTR_REGEXP.test(name)) {
throw $compileMinErr('nodomevents',
"Interpolations for HTML DOM event attributes are disallowed. Please use the " +
"ng- versions (such as ng-click instead of onclick) instead.");
}

// we need to interpolate again, in case the attribute value has been updated
// (e.g. by another directive's compile function)
interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name));

// if attribute was updated so that there is no interpolation going on we don't want to
// register any observers
if (!interpolateFn) return;

// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the
// actual attr value
attr[name] = interpolateFn(scope);
($$observers[name] || ($$observers[name] = [])).$$inter = true;
(attr.$$observers && attr.$$observers[name].$$scope || scope).
$watch(interpolateFn, function interpolateFnWatchAction(value) {
attr.$set(name, value);
});
})
// we need to interpolate again, in case the attribute value has been updated
// (e.g. by another directive's compile function)
interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name));

// if attribute was updated so that there is no interpolation going on we don't want to
// register any observers
if (!interpolateFn) return;

// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the
// actual attr value
attr[name] = interpolateFn(scope);
($$observers[name] || ($$observers[name] = [])).$$inter = true;
(attr.$$observers && attr.$$observers[name].$$scope || scope).
$watch(interpolateFn, function interpolateFnWatchAction(value) {
attr.$set(name, value);
});
}
};
}
});
}

Expand Down
52 changes: 46 additions & 6 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ describe('$compile', function() {
);


it('should process attribute interpolation at the beginning of the post-linking phase', function() {
it('should process attribute interpolation in pre-linking phase at priority 100', function() {
module(function() {
directive('attrLog', function(log) {
return {
Expand All @@ -1600,22 +1600,36 @@ describe('$compile', function() {

return {
pre: function($scope, $element, $attrs) {
log('preLink=' + $attrs.myName);
log('preLinkP0=' + $attrs.myName);
},
post: function($scope, $element) {
post: function($scope, $element, $attrs) {
log('postLink=' + $attrs.myName);
}
}
}
}
})
});
});
module(function() {
directive('attrLogHighPriority', function(log) {
return {
priority: 101,
compile: function() {
return {
pre: function($scope, $element, $attrs) {
log('preLinkP101=' + $attrs.myName);
}
};
}
}
});
});
inject(function($rootScope, $compile, log) {
element = $compile('<div attr-log my-name="{{name}}"></div>')($rootScope);
element = $compile('<div attr-log-high-priority attr-log my-name="{{name}}"></div>')($rootScope);
$rootScope.name = 'angular';
$rootScope.$apply();
log('digest=' + element.attr('my-name'));
expect(log).toEqual('compile={{name}}; preLink={{name}}; postLink=; digest=angular');
expect(log).toEqual('compile={{name}}; preLinkP101={{name}}; preLinkP0=; postLink=; digest=angular');
});
});

Expand Down Expand Up @@ -1758,6 +1772,32 @@ describe('$compile', function() {
expect(element.text()).toBe('AHOJ|ahoj|AHOJ');
});
});


it('should make attributes observable for terminal directives', function() {
module(function() {
directive('myAttr', function(log) {
return {
terminal: true,
link: function(scope, element, attrs) {
attrs.$observe('myAttr', function(val) {
log(val);
});
}
}
});
});

inject(function($compile, $rootScope, log) {
element = $compile('<div my-attr="{{myVal}}"></div>')($rootScope);
expect(log).toEqual([]);

$rootScope.myVal = 'carrot';
$rootScope.$digest();

expect(log).toEqual(['carrot']);
});
})
});


Expand Down

0 comments on commit 79223ea

Please sign in to comment.