From 9cba9d850a633afc9ffbc1556ddc88cd01390e2b Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 24 Jul 2014 08:20:21 -0700 Subject: [PATCH] feat($compile): change directive's restrict setting to default to EA (element/attribute) Previously we defaulted just to A because of IE8 which had a hard time with applying css styles to HTMLUnknownElements. This is no longer the case with IE9, so we should make restrict default to EA. Doing so will make it easier to create components and avoid matching errors when creating new directives BREAKING CHANGE: directives now match elements by default unless specific restriction rules are set via `restrict` property. This means that if a directive 'myFoo' previously didn't specify matching restrictrion, it will now match both the attribute and element form. Before:
<---- my-foo attribute matched the directive <---- no match After:
<---- my-foo attribute matched the directive <---- my-foo element matched the directive It is not expected that this will be a problem in practice because of widespread use of prefixes that make "" like elements unlikely. Closes #8321 --- src/ng/compile.js | 4 +-- src/ng/directive/attrs.js | 1 + src/ng/directive/input.js | 9 +++++ src/ng/directive/ngBind.js | 1 + src/ng/directive/ngController.js | 1 + src/ng/directive/ngEventDirs.js | 1 + src/ng/directive/ngRepeat.js | 1 + src/ng/directive/ngShowHide.js | 2 ++ src/ng/directive/select.js | 6 +++- test/ng/compileSpec.js | 57 ++++++++++++++++++++++---------- 10 files changed, 63 insertions(+), 20 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 66677a3b5a80..5a8103d7f024 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -213,7 +213,7 @@ * String of subset of `EACM` which restricts the directive to a specific directive * declaration style. If omitted, the default (attributes only) is used. * - * * `E` - Element name: `` + * * `E` - Element name (default): `` * * `A` - Attribute (default): `
` * * `C` - Class: `
` * * `M` - Comment: `` @@ -581,7 +581,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { directive.index = index; directive.name = directive.name || name; directive.require = directive.require || (directive.controller && directive.name); - directive.restrict = directive.restrict || 'A'; + directive.restrict = directive.restrict || 'EA'; directives.push(directive); } catch (e) { $exceptionHandler(e); diff --git a/src/ng/directive/attrs.js b/src/ng/directive/attrs.js index 6b230a75150c..8047e35d0960 100644 --- a/src/ng/directive/attrs.js +++ b/src/ng/directive/attrs.js @@ -351,6 +351,7 @@ forEach(BOOLEAN_ATTR, function(propName, attrName) { var normalized = directiveNormalize('ng-' + attrName); ngAttributeAliasDirectives[normalized] = function() { return { + restrict: 'A', priority: 100, link: function(scope, element, attr) { scope.$watch(attr[normalized], function ngBooleanAttrWatchAction(value) { diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index fb3ee0577946..f1421da5a934 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -2150,6 +2150,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ */ var ngModelDirective = function() { return { + restrict: 'A', require: ['ngModel', '^?form', '^?ngModelOptions'], controller: NgModelController, link: { @@ -2251,6 +2252,7 @@ var ngModelDirective = function() { * */ var ngChangeDirective = valueFn({ + restrict: 'A', require: 'ngModel', link: function(scope, element, attr, ctrl) { ctrl.$viewChangeListeners.push(function() { @@ -2262,6 +2264,7 @@ var ngChangeDirective = valueFn({ var requiredDirective = function() { return { + restrict: 'A', require: '?ngModel', link: function(scope, elm, attr, ctrl) { if (!ctrl) return; @@ -2281,6 +2284,7 @@ var requiredDirective = function() { var patternDirective = function() { return { + restrict: 'A', require: '?ngModel', link: function(scope, elm, attr, ctrl) { if (!ctrl) return; @@ -2311,6 +2315,7 @@ var patternDirective = function() { var maxlengthDirective = function() { return { + restrict: 'A', require: '?ngModel', link: function(scope, elm, attr, ctrl) { if (!ctrl) return; @@ -2329,6 +2334,7 @@ var maxlengthDirective = function() { var minlengthDirective = function() { return { + restrict: 'A', require: '?ngModel', link: function(scope, elm, attr, ctrl) { if (!ctrl) return; @@ -2430,6 +2436,7 @@ var minlengthDirective = function() { */ var ngListDirective = function() { return { + restrict: 'A', require: 'ngModel', link: function(scope, element, attr, ctrl) { // We want to control whitespace trimming so we use this convoluted approach @@ -2526,6 +2533,7 @@ var CONSTANT_VALUE_REGEXP = /^(true|false|\d+)$/; */ var ngValueDirective = function() { return { + restrict: 'A', priority: 100, compile: function(tpl, tplAttr) { if (CONSTANT_VALUE_REGEXP.test(tplAttr.ngValue)) { @@ -2688,6 +2696,7 @@ var ngValueDirective = function() { */ var ngModelOptionsDirective = function() { return { + restrict: 'A', controller: ['$scope', '$attrs', function($scope, $attrs) { var that = this; this.$options = $scope.$eval($attrs.ngModelOptions); diff --git a/src/ng/directive/ngBind.js b/src/ng/directive/ngBind.js index 8c5a4dba0805..a0cf67665d9a 100644 --- a/src/ng/directive/ngBind.js +++ b/src/ng/directive/ngBind.js @@ -178,6 +178,7 @@ var ngBindTemplateDirective = ['$interpolate', function($interpolate) { */ var ngBindHtmlDirective = ['$sce', '$parse', function($sce, $parse) { return { + restrict: 'A', compile: function (tElement, tAttrs) { tElement.addClass('ng-binding'); diff --git a/src/ng/directive/ngController.js b/src/ng/directive/ngController.js index eb4fc8bdd4ee..452f88f260c1 100644 --- a/src/ng/directive/ngController.js +++ b/src/ng/directive/ngController.js @@ -221,6 +221,7 @@ */ var ngControllerDirective = [function() { return { + restrict: 'A', scope: true, controller: '@', priority: 500 diff --git a/src/ng/directive/ngEventDirs.js b/src/ng/directive/ngEventDirs.js index ed8ddd6ba167..4b1088c6d89f 100644 --- a/src/ng/directive/ngEventDirs.js +++ b/src/ng/directive/ngEventDirs.js @@ -43,6 +43,7 @@ forEach( var directiveName = directiveNormalize('ng-' + name); ngEventDirectives[directiveName] = ['$parse', function($parse) { return { + restrict: 'A', compile: function($element, attr) { var fn = $parse(attr[directiveName]); return function ngEventHandler(scope, element) { diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 5807311a3769..2db6671ee6f8 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -212,6 +212,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { var NG_REMOVED = '$$NG_REMOVED'; var ngRepeatMinErr = minErr('ngRepeat'); return { + restrict: 'A', multiElement: true, transclude: 'element', priority: 1000, diff --git a/src/ng/directive/ngShowHide.js b/src/ng/directive/ngShowHide.js index a327843bb23f..3513b78e40d2 100644 --- a/src/ng/directive/ngShowHide.js +++ b/src/ng/directive/ngShowHide.js @@ -157,6 +157,7 @@ */ var ngShowDirective = ['$animate', function($animate) { return { + restrict: 'A', multiElement: true, link: function(scope, element, attr) { scope.$watch(attr.ngShow, function ngShowWatchAction(value){ @@ -311,6 +312,7 @@ var ngShowDirective = ['$animate', function($animate) { */ var ngHideDirective = ['$animate', function($animate) { return { + restrict: 'A', multiElement: true, link: function(scope, element, attr) { scope.$watch(attr.ngHide, function ngHideWatchAction(value){ diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index d9af96f91485..886aae20306a 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -135,7 +135,11 @@ var ngOptionsMinErr = minErr('ngOptions'); */ -var ngOptionsDirective = valueFn({ terminal: true }); +var ngOptionsDirective = valueFn({ + restrict: 'A', + terminal: true +}); + // jshint maxlen: false var selectDirective = ['$compile', '$parse', function($compile, $parse) { //000011111111110000000000022222222220000000000000000000003333333333000000000000004444444444444440000000005555555555555550000000666666666666666000000000000000777777777700000000000000000008888888888 diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index aedff6e4d795..fb5dc525ded6 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -255,14 +255,6 @@ describe('$compile', function() { })); - it('should allow directives in comments', inject( - function($compile, $rootScope, log) { - element = $compile('
01
')($rootScope); - expect(log).toEqual('angular'); - } - )); - - it('should receive scope, element, and attributes', function() { var injector; module(function() { @@ -437,20 +429,21 @@ describe('$compile', function() { describe('restrict', function() { - it('should allow restriction of attributes', function() { - module(function() { - forEach({div:'E', attr:'A', clazz:'C', all:'EAC'}, function(restrict, name) { - directive(name, function(log) { + it('should allow restriction of availability', function () { + module(function () { + forEach({div: 'E', attr: 'A', clazz: 'C', comment: 'M', all: 'EACM'}, + function (restrict, name) { + directive(name, function (log) { return { restrict: restrict, - compile: valueFn(function(scope, element, attr) { + compile: valueFn(function (scope, element, attr) { log(name); }) }; }); }); }); - inject(function($rootScope, $compile, log) { + inject(function ($rootScope, $compile, log) { dealoc($compile('')($rootScope)); expect(log).toEqual(''); log.reset(); @@ -459,7 +452,7 @@ describe('$compile', function() { expect(log).toEqual('div'); log.reset(); - dealoc($compile('')($rootScope)); + dealoc($compile('')($rootScope)); expect(log).toEqual(''); log.reset(); @@ -475,8 +468,38 @@ describe('$compile', function() { expect(log).toEqual('clazz'); log.reset(); - dealoc($compile('')($rootScope)); - expect(log).toEqual('all; all; all'); + dealoc($compile('')($rootScope)); + expect(log).toEqual('comment'); + log.reset(); + + dealoc($compile('')($rootScope)); + expect(log).toEqual('all; all; all; all'); + }); + }); + + + it('should use EA rule as the default', function () { + module(function () { + directive('defaultDir', function (log) { + return { + compile: function () { + log('defaultDir'); + } + }; + }); + }); + inject(function ($rootScope, $compile, log) { + dealoc($compile('')($rootScope)); + expect(log).toEqual('defaultDir'); + log.reset(); + + dealoc($compile('')($rootScope)); + expect(log).toEqual('defaultDir'); + log.reset(); + + dealoc($compile('')($rootScope)); + expect(log).toEqual(''); + log.reset(); }); }); });