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

fix($compile): check clashing directives before compilation #4217

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 50 additions & 21 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,55 @@ function $CompileProvider($provide) {
linkFn,
directiveValue;

for(var i = 0, ii = directives.length; i < ii; i++) {
directive = directives[i];
if (terminalPriority > directive.priority) {
break; // prevent further processing of directives
}

directiveName = directive.name;

if (directiveValue = directive.scope) {
assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);

// skip the check for directives with async templates, we'll check the derived sync directive when
// the template arrives
if (!directive.templateUrl && isObject(directiveValue)) {
newIsolateScopeDirective = directive;
}
}

if (!directive.templateUrl && directive.controller) {
directiveValue = directive.controller;
controllerDirectives = controllerDirectives || {};
assertNoDuplicate("'" + directiveName + "' controller",
controllerDirectives[directiveName], directive, $compileNode);
controllerDirectives[directiveName] = directive;
}

if (directiveValue = directive.transclude) {
terminalPriority = directive.priority;
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
transcludeDirective = directive;
}

if (directive.template) {
assertNoDuplicate('template', templateDirective, directive, $compileNode);
templateDirective = directive;
}

if (directive.templateUrl) {
assertNoDuplicate('template', templateDirective, directive, $compileNode);
templateDirective = directive;
}

if (directive.terminal) {
terminalPriority = Math.max(terminalPriority, directive.priority);
}
}

terminalPriority = -Number.MAX_VALUE;

// executes all directives on the current element
for(var i = 0, ii = directives.length; i < ii; i++) {
directive = directives[i];
Expand All @@ -789,31 +838,17 @@ function $CompileProvider($provide) {
if (directiveValue = directive.scope) {
newScopeDirective = newScopeDirective || directive;

// skip the check for directives with async templates, we'll check the derived sync directive when
// the template arrives
if (!directive.templateUrl) {
assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive, $compileNode);
if (isObject(directiveValue)) {
safeAddClass($compileNode, 'ng-isolate-scope');
newIsolateScopeDirective = directive;
}
safeAddClass($compileNode, 'ng-scope');
}
}

directiveName = directive.name;

if (!directive.templateUrl && directive.controller) {
directiveValue = directive.controller;
controllerDirectives = controllerDirectives || {};
assertNoDuplicate("'" + directiveName + "' controller",
controllerDirectives[directiveName], directive, $compileNode);
controllerDirectives[directiveName] = directive;
}

if (directiveValue = directive.transclude) {
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
transcludeDirective = directive;
terminalPriority = directive.priority;
if (directiveValue == 'element') {
$template = groupScan(compileNode, attrStart, attrEnd)
Expand All @@ -832,9 +867,6 @@ function $CompileProvider($provide) {
}

if (directive.template) {
assertNoDuplicate('template', templateDirective, directive, $compileNode);
templateDirective = directive;

directiveValue = (isFunction(directive.template))
? directive.template($compileNode, templateAttrs)
: directive.template;
Expand All @@ -843,6 +875,7 @@ function $CompileProvider($provide) {

if (directive.replace) {
replaceDirective = directive;

$template = jqLite('<div>' +
trim(directiveValue) +
'</div>').contents();
Expand Down Expand Up @@ -877,9 +910,6 @@ function $CompileProvider($provide) {
}

if (directive.templateUrl) {
assertNoDuplicate('template', templateDirective, directive, $compileNode);
templateDirective = directive;

if (directive.replace) {
replaceDirective = directive;
}
Expand All @@ -904,7 +934,6 @@ function $CompileProvider($provide) {
nodeLinkFn.terminal = true;
terminalPriority = Math.max(terminalPriority, directive.priority);
}

}

nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope;
Expand Down
27 changes: 24 additions & 3 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ describe('$compile', function() {
expect(function(){
$compile('<div class="iscope-a; scope-b"></div>');
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [iscopeA, scopeB] asking for new/isolated scope on: ' +
'<div class="iscope-a; scope-b ng-isolate-scope ng-scope">');
'<div class="iscope-a; scope-b">');
})
);

Expand Down Expand Up @@ -2700,7 +2700,7 @@ describe('$compile', function() {
});


it('should only allow one transclude per element', function() {
it('should only allow one content transclusion per element', function() {
module(function() {
directive('first', valueFn({
scope: {},
Expand All @@ -2716,7 +2716,28 @@ describe('$compile', function() {
expect(function() {
$compile('<div class="first second"></div>');
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
'<div class="first second ng-isolate-scope ng-scope">');
'<div class="first second">');
});
});


it('should only allow one element transclusion per element', function() {
module(function() {
directive('first', valueFn({
scope: {},
restrict: 'CA',
transclude: 'element'
}));
directive('second', valueFn({
restrict: 'CA',
transclude: 'element'
}));
});
inject(function($compile) {
expect(function() {
$compile('<div class="first second"></div>');
}).toThrowMinErr('$compile', 'multidir', 'Multiple directives [first, second] asking for transclusion on: ' +
'<div class="first second">');
});
});

Expand Down