Skip to content

Commit 70c0922

Browse files
IgorMinarjamesdaily
authored andcommitted
fix($compile): attribute bindings should not break due to terminal directives
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 angular#4525 Closes angular#4528 Closes angular#4649
1 parent da86dfa commit 70c0922

File tree

2 files changed

+76
-32
lines changed

2 files changed

+76
-32
lines changed

src/ng/compile.js

+30-26
Original file line numberDiff line numberDiff line change
@@ -1717,33 +1717,37 @@ function $CompileProvider($provide) {
17171717
}
17181718

17191719
directives.push({
1720-
priority: -100,
1721-
compile: valueFn(function attrInterpolateLinkFn(scope, element, attr) {
1722-
var $$observers = (attr.$$observers || (attr.$$observers = {}));
1723-
1724-
if (EVENT_HANDLER_ATTR_REGEXP.test(name)) {
1725-
throw $compileMinErr('nodomevents',
1726-
"Interpolations for HTML DOM event attributes are disallowed. Please use the " +
1727-
"ng- versions (such as ng-click instead of onclick) instead.");
1728-
}
1720+
priority: 100,
1721+
compile: function() {
1722+
return {
1723+
pre: function attrInterpolatePreLinkFn(scope, element, attr) {
1724+
var $$observers = (attr.$$observers || (attr.$$observers = {}));
1725+
1726+
if (EVENT_HANDLER_ATTR_REGEXP.test(name)) {
1727+
throw $compileMinErr('nodomevents',
1728+
"Interpolations for HTML DOM event attributes are disallowed. Please use the " +
1729+
"ng- versions (such as ng-click instead of onclick) instead.");
1730+
}
17291731

1730-
// we need to interpolate again, in case the attribute value has been updated
1731-
// (e.g. by another directive's compile function)
1732-
interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name));
1733-
1734-
// if attribute was updated so that there is no interpolation going on we don't want to
1735-
// register any observers
1736-
if (!interpolateFn) return;
1737-
1738-
// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the
1739-
// actual attr value
1740-
attr[name] = interpolateFn(scope);
1741-
($$observers[name] || ($$observers[name] = [])).$$inter = true;
1742-
(attr.$$observers && attr.$$observers[name].$$scope || scope).
1743-
$watch(interpolateFn, function interpolateFnWatchAction(value) {
1744-
attr.$set(name, value);
1745-
});
1746-
})
1732+
// we need to interpolate again, in case the attribute value has been updated
1733+
// (e.g. by another directive's compile function)
1734+
interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name));
1735+
1736+
// if attribute was updated so that there is no interpolation going on we don't want to
1737+
// register any observers
1738+
if (!interpolateFn) return;
1739+
1740+
// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the
1741+
// actual attr value
1742+
attr[name] = interpolateFn(scope);
1743+
($$observers[name] || ($$observers[name] = [])).$$inter = true;
1744+
(attr.$$observers && attr.$$observers[name].$$scope || scope).
1745+
$watch(interpolateFn, function interpolateFnWatchAction(value) {
1746+
attr.$set(name, value);
1747+
});
1748+
}
1749+
};
1750+
}
17471751
});
17481752
}
17491753

test/ng/compileSpec.js

+46-6
Original file line numberDiff line numberDiff line change
@@ -1591,7 +1591,7 @@ describe('$compile', function() {
15911591
);
15921592

15931593

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

16011601
return {
16021602
pre: function($scope, $element, $attrs) {
1603-
log('preLink=' + $attrs.myName);
1603+
log('preLinkP0=' + $attrs.myName);
16041604
},
1605-
post: function($scope, $element) {
1605+
post: function($scope, $element, $attrs) {
16061606
log('postLink=' + $attrs.myName);
16071607
}
16081608
}
16091609
}
16101610
}
1611-
})
1611+
});
1612+
});
1613+
module(function() {
1614+
directive('attrLogHighPriority', function(log) {
1615+
return {
1616+
priority: 101,
1617+
compile: function() {
1618+
return {
1619+
pre: function($scope, $element, $attrs) {
1620+
log('preLinkP101=' + $attrs.myName);
1621+
}
1622+
};
1623+
}
1624+
}
1625+
});
16121626
});
16131627
inject(function($rootScope, $compile, log) {
1614-
element = $compile('<div attr-log my-name="{{name}}"></div>')($rootScope);
1628+
element = $compile('<div attr-log-high-priority attr-log my-name="{{name}}"></div>')($rootScope);
16151629
$rootScope.name = 'angular';
16161630
$rootScope.$apply();
16171631
log('digest=' + element.attr('my-name'));
1618-
expect(log).toEqual('compile={{name}}; preLink={{name}}; postLink=; digest=angular');
1632+
expect(log).toEqual('compile={{name}}; preLinkP101={{name}}; preLinkP0=; postLink=; digest=angular');
16191633
});
16201634
});
16211635

@@ -1758,6 +1772,32 @@ describe('$compile', function() {
17581772
expect(element.text()).toBe('AHOJ|ahoj|AHOJ');
17591773
});
17601774
});
1775+
1776+
1777+
it('should make attributes observable for terminal directives', function() {
1778+
module(function() {
1779+
directive('myAttr', function(log) {
1780+
return {
1781+
terminal: true,
1782+
link: function(scope, element, attrs) {
1783+
attrs.$observe('myAttr', function(val) {
1784+
log(val);
1785+
});
1786+
}
1787+
}
1788+
});
1789+
});
1790+
1791+
inject(function($compile, $rootScope, log) {
1792+
element = $compile('<div my-attr="{{myVal}}"></div>')($rootScope);
1793+
expect(log).toEqual([]);
1794+
1795+
$rootScope.myVal = 'carrot';
1796+
$rootScope.$digest();
1797+
1798+
expect(log).toEqual(['carrot']);
1799+
});
1800+
})
17611801
});
17621802

17631803

0 commit comments

Comments
 (0)