Permalink
Browse files

fix($compile): always error if two directives add isolate-scope and n…

…ew-scope

Previously, the compiler would throw an error if a directive requested new non-isolate scope
after a directive had requested isolate scope. But it would not error if a directive
requested an isolate scope after a directive had requested a new non-isolate scope.

Since it is invalid to have more than one directive request any kind of scope if one of
them has requested isolate scope, then the compiler should error whatever order the
directives are applied.

This fix addresses this situation by throwing error regardless of order of directives.

BREAKING CHANGE:

Requesting isolate scope and any other scope on a single element is an error.
Before this change, the compiler let two directives request a child scope
and an isolate scope if the compiler applied them in the order of non-isolate
scope directive followed by isolate scope directive.

Now the compiler will error regardless of the order.

If you find that your code is now throwing a `$compile:multidir` error,
check that you do not have directives on the same element that are trying
to request both an isolate and a non-isolate scope and fix your code.

Closes #4402
Closes #4421
  • Loading branch information...
1 parent 73e3e85 commit 2cde927e58c8d1588569d94a797e43cdfbcedaf9 @buunguyen buunguyen committed with petebacondarwin Oct 15, 2013
Showing with 30 additions and 3 deletions.
  1. +11 −3 src/ng/compile.js
  2. +19 −0 test/ng/compileSpec.js
View
@@ -1213,17 +1213,25 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
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)) {
+ // This directive is trying to add an isolated scope.
+ // Check that there is no scope of any kind already
+ assertNoDuplicate('new/isolated scope', newIsolateScopeDirective || newScopeDirective,
+ directive, $compileNode);
newIsolateScopeDirective = directive;
+ } else {
+ // This directive is trying to add a child scope.
+ // Check that there is no isolated scope already
+ assertNoDuplicate('new/isolated scope', newIsolateScopeDirective, directive,
+ $compileNode);
}
}
+
+ newScopeDirective = newScopeDirective || directive;
}
directiveName = directive.name;
@@ -1989,6 +1989,25 @@ describe('$compile', function() {
})
);
+ it('should not allow more than one isolate scope creation per element regardless of directive priority', function() {
+ module(function($compileProvider) {
+ $compileProvider.directive('highPriorityScope', function() {
+ return {
+ restrict: 'C',
+ priority: 1,
+ scope: true,
+ link: function() {}
+ };
+ });
+ });
+ inject(function($compile) {
+ expect(function(){
+ $compile('<div class="iscope-a; high-priority-scope"></div>');
+ }).toThrowMinErr('$compile', 'multidir', 'Multiple directives [highPriorityScope, iscopeA] asking for new/isolated scope on: ' +
+ '<div class="iscope-a; high-priority-scope">');
+ });
+ });
+
it('should create new scope even at the root of the template', inject(
function($rootScope, $compile, log) {

0 comments on commit 2cde927

Please sign in to comment.