Skip to content

Commit 31f190d

Browse files
IgorMinarIgorMinar
committed
fix($compile): fix (reverse) directive postLink fn execution order
previously the compile/link fns executed in this order controlled via priority: - CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow - PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow - link children - PostLinkPriorityHigh, PostLinkPriorityMedium, PostLinkPriorityLow This was changed to: - CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow - PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow - link children - PostLinkPriorityLow, PostLinkPriorityMedium , PostLinkPriorityHigh Using this order the child transclusion directive that gets replaced onto the current element get executed correctly (see issue angular#3558), and more generally, the order of execution of post linking function makes more sense. The incorrect order was an oversight that has gone unnoticed for many suns and moons. (FYI: postLink functions are the default linking functions) BREAKING CHANGE: the order of postLink fn is now mirror opposite of the order in which corresponding preLinking and compile functions execute. Very few directives in practice rely on order of postLinking function (unlike on the order of compile functions), so in the rare case of this change affecting an existing directive, it might be necessary to convert it to a preLinking function or give it negative priority (look at the diff of this commit to see how an internal attribute interpolation directive was adjusted). Closes angular#3558
1 parent fe21450 commit 31f190d

3 files changed

Lines changed: 123 additions & 30 deletions

File tree

‎src/ng/compile.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ function $CompileProvider($provide) {
10951095
childLinkFn && childLinkFn(scope, linkNode.childNodes, undefined, boundTranscludeFn);
10961096

10971097
// POSTLINKING
1098-
for(i = 0, ii = postLinkFns.length; i < ii; i++) {
1098+
for(i = postLinkFns.length - 1; i >= 0; i--) {
10991099
try {
11001100
linkFn = postLinkFns[i];
11011101
linkFn(scope, $element, attrs,
@@ -1328,7 +1328,7 @@ function $CompileProvider($provide) {
13281328
}
13291329

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

@@ -1346,6 +1346,7 @@ function $CompileProvider($provide) {
13461346
// register any observers
13471347
if (!interpolateFn) return;
13481348

1349+
// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the actual attr value
13491350
attr[name] = interpolateFn(scope);
13501351
($$observers[name] || ($$observers[name] = [])).$$inter = true;
13511352
(attr.$$observers && attr.$$observers[name].$$scope || scope).

‎src/ng/directive/input.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ var VALID_CLASS = 'ng-valid',
958958
</file>
959959
* </example>
960960
*
961-
*
961+
*
962962
*/
963963
var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$parse',
964964
function($scope, $exceptionHandler, $attr, $element, $parse) {

‎test/ng/compileSpec.js‎

Lines changed: 119 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,25 @@ describe('$compile', function() {
9696
directive('div', function(log) {
9797
return {
9898
restrict: 'ECA',
99-
link: log.fn('1')
99+
link: {
100+
pre: log.fn('pre1'),
101+
post: log.fn('post1')
102+
}
100103
};
101104
});
102105
directive('div', function(log) {
103106
return {
104107
restrict: 'ECA',
105-
link: log.fn('2')
108+
link: {
109+
pre: log.fn('pre2'),
110+
post: log.fn('post2')
111+
}
106112
};
107113
});
108114
});
109115
inject(function($compile, $rootScope, log) {
110116
element = $compile('<div></div>')($rootScope);
111-
expect(log).toEqual('1; 2');
117+
expect(log).toEqual('pre1; pre2; post2; post1');
112118
});
113119
});
114120
});
@@ -206,7 +212,7 @@ describe('$compile', function() {
206212
'<span greet="angular" log="L" x-high-log="H" data-medium-log="M"></span>')
207213
($rootScope);
208214
expect(element.text()).toEqual('Hello angular');
209-
expect(log).toEqual('H; M; L');
215+
expect(log).toEqual('L; M; H');
210216
}));
211217

212218

@@ -387,7 +393,7 @@ describe('$compile', function() {
387393
element = $compile(
388394
'<span log="L" x-high-log="H" data-medium-log="M"></span>')
389395
($rootScope);
390-
expect(log).toEqual('H; M; L');
396+
expect(log).toEqual('L; M; H');
391397
}));
392398
});
393399

@@ -511,8 +517,7 @@ describe('$compile', function() {
511517
($rootScope);
512518
$rootScope.$digest();
513519
expect(element.text()).toEqual('Replace!');
514-
// HIGH goes after MEDIUM since it executes as part of replaced template
515-
expect(log).toEqual('MEDIUM; HIGH; LOG');
520+
expect(log).toEqual('LOG; HIGH; MEDIUM');
516521
}));
517522

518523

@@ -521,7 +526,7 @@ describe('$compile', function() {
521526
($rootScope);
522527
$rootScope.$digest();
523528
expect(element.text()).toEqual('Append!');
524-
expect(log).toEqual('HIGH; LOG; MEDIUM');
529+
expect(log).toEqual('LOG; HIGH; MEDIUM');
525530
}));
526531

527532

@@ -1079,7 +1084,7 @@ describe('$compile', function() {
10791084
expect(log).toEqual(
10801085
'first-C; FLUSH; second-C; last-C; third-C; ' +
10811086
'first-PreL; second-PreL; last-PreL; third-PreL; ' +
1082-
'third-PostL; first-PostL; second-PostL; last-PostL');
1087+
'third-PostL; last-PostL; second-PostL; first-PostL');
10831088

10841089
var span = element.find('span');
10851090
expect(span.attr('first')).toEqual('');
@@ -1104,7 +1109,7 @@ describe('$compile', function() {
11041109
expect(log).toEqual(
11051110
'iFirst-C; FLUSH; iSecond-C; iThird-C; iLast-C; ' +
11061111
'iFirst-PreL; iSecond-PreL; iThird-PreL; iLast-PreL; ' +
1107-
'iFirst-PostL; iSecond-PostL; iThird-PostL; iLast-PostL');
1112+
'iLast-PostL; iThird-PostL; iSecond-PostL; iFirst-PostL');
11081113

11091114
var div = element.find('div');
11101115
expect(div.attr('i-first')).toEqual('');
@@ -1130,7 +1135,7 @@ describe('$compile', function() {
11301135
expect(log).toEqual(
11311136
'first-C; FLUSH; second-C; last-C; third-C; ' +
11321137
'first-PreL; second-PreL; last-PreL; third-PreL; ' +
1133-
'third-PostL; first-PostL; second-PostL; last-PostL');
1138+
'third-PostL; last-PostL; second-PostL; first-PostL');
11341139

11351140
var span = element.find('span');
11361141
expect(span.attr('first')).toEqual('');
@@ -1156,7 +1161,7 @@ describe('$compile', function() {
11561161
expect(log).toEqual(
11571162
'iFirst-C; FLUSH; iSecond-C; iThird-C; iLast-C; ' +
11581163
'iFirst-PreL; iSecond-PreL; iThird-PreL; iLast-PreL; ' +
1159-
'iFirst-PostL; iSecond-PostL; iThird-PostL; iLast-PostL');
1164+
'iLast-PostL; iThird-PostL; iSecond-PostL; iFirst-PostL');
11601165

11611166
var div = element.find('div');
11621167
expect(div.attr('i-first')).toEqual('');
@@ -1344,10 +1349,10 @@ describe('$compile', function() {
13441349
scope: true,
13451350
restrict: 'CA',
13461351
compile: function() {
1347-
return function (scope, element) {
1352+
return {pre: function (scope, element) {
13481353
log(scope.$id);
13491354
expect(element.data('$scope')).toBe(scope);
1350-
};
1355+
}};
13511356
}
13521357
};
13531358
});
@@ -1409,25 +1414,25 @@ describe('$compile', function() {
14091414
directive('log', function(log) {
14101415
return {
14111416
restrict: 'CA',
1412-
link: function(scope) {
1417+
link: {pre: function(scope) {
14131418
log('log-' + scope.$id + '-' + scope.$parent.$id);
1414-
}
1419+
}}
14151420
};
14161421
});
14171422
}));
14181423

14191424

14201425
it('should allow creation of new scopes', inject(function($rootScope, $compile, log) {
14211426
element = $compile('<div><span scope><a log></a></span></div>')($rootScope);
1422-
expect(log).toEqual('LOG; log-002-001; 002');
1427+
expect(log).toEqual('002; log-002-001; LOG');
14231428
expect(element.find('span').hasClass('ng-scope')).toBe(true);
14241429
}));
14251430

14261431

14271432
it('should allow creation of new isolated scopes for directives', inject(
14281433
function($rootScope, $compile, log) {
14291434
element = $compile('<div><span iscope><a log></a></span></div>')($rootScope);
1430-
expect(log).toEqual('LOG; log-002-001; 002');
1435+
expect(log).toEqual('log-002-001; LOG; 002');
14311436
$rootScope.name = 'abc';
14321437
expect(iscope.$parent).toBe($rootScope);
14331438
expect(iscope.name).toBeUndefined();
@@ -1439,7 +1444,7 @@ describe('$compile', function() {
14391444
$httpBackend.expect('GET', 'tscope.html').respond('<a log>{{name}}; scopeId: {{$id}}</a>');
14401445
element = $compile('<div><span tscope></span></div>')($rootScope);
14411446
$httpBackend.flush();
1442-
expect(log).toEqual('LOG; log-002-001; 002');
1447+
expect(log).toEqual('log-002-001; LOG; 002');
14431448
$rootScope.name = 'Jozo';
14441449
$rootScope.$apply();
14451450
expect(element.text()).toBe('Jozo; scopeId: 002');
@@ -1453,7 +1458,7 @@ describe('$compile', function() {
14531458
respond('<p><a log>{{name}}; scopeId: {{$id}}</a></p>');
14541459
element = $compile('<div><span trscope></span></div>')($rootScope);
14551460
$httpBackend.flush();
1456-
expect(log).toEqual('LOG; log-002-001; 002');
1461+
expect(log).toEqual('log-002-001; LOG; 002');
14571462
$rootScope.name = 'Jozo';
14581463
$rootScope.$apply();
14591464
expect(element.text()).toBe('Jozo; scopeId: 002');
@@ -1467,7 +1472,7 @@ describe('$compile', function() {
14671472
respond('<p><a log>{{name}}; scopeId: {{$id}} |</a></p>');
14681473
element = $compile('<div><span ng-repeat="i in [1,2,3]" trscope></span></div>')($rootScope);
14691474
$httpBackend.flush();
1470-
expect(log).toEqual('LOG; log-003-002; 003; LOG; log-005-004; 005; LOG; log-007-006; 007');
1475+
expect(log).toEqual('log-003-002; LOG; 003; log-005-004; LOG; 005; log-007-006; LOG; 007');
14711476
$rootScope.name = 'Jozo';
14721477
$rootScope.$apply();
14731478
expect(element.text()).toBe('Jozo; scopeId: 003 |Jozo; scopeId: 005 |Jozo; scopeId: 007 |');
@@ -1481,7 +1486,7 @@ describe('$compile', function() {
14811486
$httpBackend.expect('GET', 'tiscope.html').respond('<a log></a>');
14821487
element = $compile('<div><span tiscope></span></div>')($rootScope);
14831488
$httpBackend.flush();
1484-
expect(log).toEqual('LOG; log-002-001; 002');
1489+
expect(log).toEqual('log-002-001; LOG; 002');
14851490
$rootScope.name = 'abc';
14861491
expect(iscope.$parent).toBe($rootScope);
14871492
expect(iscope.name).toBeUndefined();
@@ -1501,7 +1506,7 @@ describe('$compile', function() {
15011506
'</b>' +
15021507
'</div>'
15031508
)($rootScope);
1504-
expect(log).toEqual('LOG; log-003-002; 003; LOG; log-002-001; 002; LOG; log-004-001; 004');
1509+
expect(log).toEqual('002; 003; log-003-002; LOG; log-002-001; LOG; 004; log-004-001; LOG');
15051510
})
15061511
);
15071512

@@ -1571,7 +1576,38 @@ describe('$compile', function() {
15711576
$rootScope.$digest();
15721577
expect(element.text()).toEqual('text: angular');
15731578
expect(element.attr('name')).toEqual('attr: angular');
1574-
}));
1579+
})
1580+
);
1581+
1582+
1583+
it('should process attribute interpolation at the beginning of the post-linking phase', function() {
1584+
module(function() {
1585+
directive('attrLog', function(log) {
1586+
return {
1587+
compile: function($element, $attrs) {
1588+
log('compile=' + $attrs.myName);
1589+
1590+
return {
1591+
pre: function($scope, $element, $attrs) {
1592+
log('preLink=' + $attrs.myName);
1593+
},
1594+
post: function($scope, $element) {
1595+
log('postLink=' + $attrs.myName);
1596+
}
1597+
}
1598+
}
1599+
}
1600+
})
1601+
});
1602+
inject(function($rootScope, $compile, log) {
1603+
element = $compile('<div attr-log my-name="{{name}}"></div>')($rootScope);
1604+
$rootScope.name = 'angular';
1605+
$rootScope.$apply();
1606+
log('digest=' + element.attr('my-name'));
1607+
expect(log).toEqual('compile={{name}}; preLink={{name}}; postLink=; digest=angular');
1608+
});
1609+
});
1610+
15751611

15761612
describe('SCE values', function() {
15771613
it('should resolve compile and link both attribute and text bindings', inject(
@@ -1753,7 +1789,7 @@ describe('$compile', function() {
17531789
it('should compile from top to bottom but link from bottom up', inject(
17541790
function($compile, $rootScope, log) {
17551791
element = $compile('<a b><c></c></a>')($rootScope);
1756-
expect(log).toEqual('tA; tB; tC; preA; preB; preC; postC; postA; postB');
1792+
expect(log).toEqual('tA; tB; tC; preA; preB; preC; postC; postB; postA');
17571793
}
17581794
));
17591795

@@ -2230,7 +2266,7 @@ describe('$compile', function() {
22302266
});
22312267
inject(function(log, $compile, $rootScope) {
22322268
element = $compile('<div main dep other></div>')($rootScope);
2233-
expect(log).toEqual('main; dep:main; false');
2269+
expect(log).toEqual('false; dep:main; main');
22342270
});
22352271
});
22362272

@@ -2639,7 +2675,7 @@ describe('$compile', function() {
26392675
element = $compile('<div><div high-log trans="text" log>{{$parent.$id}}-{{$id}};</div></div>')
26402676
($rootScope);
26412677
$rootScope.$apply();
2642-
expect(log).toEqual('compile: <!-- trans: text -->; HIGH; link; LOG; LOG');
2678+
expect(log).toEqual('compile: <!-- trans: text -->; link; LOG; LOG; HIGH');
26432679
expect(element.text()).toEqual('001-002;001-003;');
26442680
});
26452681
});
@@ -2907,6 +2943,62 @@ describe('$compile', function() {
29072943
});
29082944

29092945

2946+
it('should make the result of a transclusion available to the parent *replace* directive in post-linking phase (template)',
2947+
function() {
2948+
module(function() {
2949+
directive('replacedTrans', function(log) {
2950+
return {
2951+
transclude: true,
2952+
replace: true,
2953+
template: '<div ng-transclude></div>',
2954+
link: {
2955+
pre: function($scope, $element) {
2956+
log('pre(' + $element.text() + ')');
2957+
},
2958+
post: function($scope, $element) {
2959+
log('post(' + $element.text() + ')');
2960+
}
2961+
}
2962+
};
2963+
});
2964+
});
2965+
inject(function(log, $rootScope, $compile) {
2966+
element = $compile('<div replaced-trans><span>unicorn!</span></div>')($rootScope);
2967+
$rootScope.$apply();
2968+
expect(log).toEqual('pre(); post(unicorn!)');
2969+
});
2970+
});
2971+
2972+
2973+
it('should make the result of a transclusion available to the parent *replace* directive in post-linking phase (templateUrl)',
2974+
function() {
2975+
module(function() {
2976+
directive('replacedTrans', function(log) {
2977+
return {
2978+
transclude: true,
2979+
replace: true,
2980+
templateUrl: 'trans.html',
2981+
link: {
2982+
pre: function($scope, $element) {
2983+
log('pre(' + $element.text() + ')');
2984+
},
2985+
post: function($scope, $element) {
2986+
log('post(' + $element.text() + ')');
2987+
}
2988+
}
2989+
};
2990+
});
2991+
});
2992+
inject(function(log, $rootScope, $compile, $templateCache) {
2993+
$templateCache.put('trans.html', '<div ng-transclude></div>');
2994+
2995+
element = $compile('<div replaced-trans><span>unicorn!</span></div>')($rootScope);
2996+
$rootScope.$apply();
2997+
expect(log).toEqual('pre(); post(unicorn!)');
2998+
});
2999+
});
3000+
3001+
29103002
it('should terminate compilation only for element trasclusion', function() {
29113003
module(function() {
29123004
directive('elementTrans', function(log) {

0 commit comments

Comments
 (0)