Skip to content

Commit fdac6ee

Browse files
committed
perf($compile): validate directive.restrict property on directive init
This allows the removal of try/catch from addDirective to avoid V8 deopt. Previously the directive.restrict property was not validated. This would potentially cause exceptions on each compilation of the directive requiring a try/catch and potentially causing repeated errors. New validation when directive.restrict is specified: * must be a string * must contain at least one valid character (E, A, C, M) Cases which previously silently failed (now throw an error): * values with an indexOf method (such as strings, arrays) which returned returned -1 for all valid restrict characters Cases which previously worked unintentionally (now throw an error): * arrays with single-character strings of valid restrict characters Closes angular#13263
1 parent cfc8b41 commit fdac6ee

File tree

3 files changed

+76
-17
lines changed

3 files changed

+76
-17
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
@ngdoc error
2+
@name $compile:badrestrict
3+
@fullName Invalid Directive Restrict
4+
@description
5+
6+
This error occurs when the restrict property of a directive is not valid.
7+
8+
The directive restrict property must be a string including one of more of the following characters:
9+
* E (element)
10+
* A (attribute)
11+
* C (class)
12+
* M (comment)
13+
14+
For example:
15+
```javascript
16+
restrict: 'E'
17+
restrict: 'EAC'
18+
```

src/ng/compile.js

+26-17
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,17 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
969969
return require;
970970
}
971971

972+
function getDirectiveRestrict(restrict, name) {
973+
if (restrict && !(isString(restrict) && /[EACM]/.test(restrict))) {
974+
throw $compileMinErr('badrestrict',
975+
"Restrict property '{0}' of directive '{1}' is invalid",
976+
restrict,
977+
name);
978+
}
979+
980+
return restrict || 'EA';
981+
}
982+
972983
/**
973984
* @ngdoc method
974985
* @name $compileProvider#directive
@@ -1006,7 +1017,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
10061017
directive.index = index;
10071018
directive.name = directive.name || name;
10081019
directive.require = getDirectiveRequire(directive);
1009-
directive.restrict = directive.restrict || 'EA';
1020+
directive.restrict = getDirectiveRestrict(directive.restrict, name);
10101021
directive.$$moduleName = directiveFactory.$$moduleName;
10111022
directives.push(directive);
10121023
} catch (e) {
@@ -2691,24 +2702,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
26912702
if (hasDirectives.hasOwnProperty(name)) {
26922703
for (var directive, directives = $injector.get(name + Suffix),
26932704
i = 0, ii = directives.length; i < ii; i++) {
2694-
try {
2695-
directive = directives[i];
2696-
if ((isUndefined(maxPriority) || maxPriority > directive.priority) &&
2697-
directive.restrict.indexOf(location) !== -1) {
2698-
if (startAttrName) {
2699-
directive = inherit(directive, {$$start: startAttrName, $$end: endAttrName});
2700-
}
2701-
if (!directive.$$bindings) {
2702-
var bindings = directive.$$bindings =
2703-
parseDirectiveBindings(directive, directive.name);
2704-
if (isObject(bindings.isolateScope)) {
2705-
directive.$$isolateBindings = bindings.isolateScope;
2706-
}
2705+
directive = directives[i];
2706+
if ((isUndefined(maxPriority) || maxPriority > directive.priority) &&
2707+
directive.restrict.indexOf(location) !== -1) {
2708+
if (startAttrName) {
2709+
directive = inherit(directive, {$$start: startAttrName, $$end: endAttrName});
2710+
}
2711+
if (!directive.$$bindings) {
2712+
var bindings = directive.$$bindings =
2713+
parseDirectiveBindings(directive, directive.name);
2714+
if (isObject(bindings.isolateScope)) {
2715+
directive.$$isolateBindings = bindings.isolateScope;
27072716
}
2708-
tDirectives.push(directive);
2709-
match = directive;
27102717
}
2711-
} catch (e) { $exceptionHandler(e); }
2718+
tDirectives.push(directive);
2719+
match = directive;
2720+
}
27122721
}
27132722
}
27142723
return match;

test/ng/compileSpec.js

+32
Original file line numberDiff line numberDiff line change
@@ -5558,6 +5558,38 @@ describe('$compile', function() {
55585558
});
55595559

55605560

5561+
it('should throw badrestrict on first compilation when restrict is invalid', function() {
5562+
module(function($compileProvider, $exceptionHandlerProvider) {
5563+
$compileProvider.directive('invalidRestrictBadString', valueFn({restrict: '"'}));
5564+
$compileProvider.directive('invalidRestrictTrue', valueFn({restrict: true}));
5565+
$compileProvider.directive('invalidRestrictObject', valueFn({restrict: {}}));
5566+
$compileProvider.directive('invalidRestrictNumber', valueFn({restrict: 42}));
5567+
5568+
// We need to test with the exceptionHandler not rethrowing...
5569+
$exceptionHandlerProvider.mode('log');
5570+
});
5571+
5572+
inject(function($exceptionHandler, $compile, $rootScope) {
5573+
$compile('<div invalid-restrict-true>')($rootScope);
5574+
expect($exceptionHandler.errors.length).toBe(1);
5575+
expect($exceptionHandler.errors[0]).toMatch(/\$compile.*badrestrict.*'true'/);
5576+
5577+
$compile('<div invalid-restrict-bad-string>')($rootScope);
5578+
$compile('<div invalid-restrict-bad-string>')($rootScope);
5579+
expect($exceptionHandler.errors.length).toBe(2);
5580+
expect($exceptionHandler.errors[1]).toMatch(/\$compile.*badrestrict.*'\"'/);
5581+
5582+
$compile('<div invalid-restrict-bad-string invalid-restrict-object>')($rootScope);
5583+
expect($exceptionHandler.errors.length).toBe(3);
5584+
expect($exceptionHandler.errors[2]).toMatch(/\$compile.*badrestrict.*'{}'/);
5585+
5586+
$compile('<div invalid-restrict-object invalid-restrict-number>')($rootScope);
5587+
expect($exceptionHandler.errors.length).toBe(4);
5588+
expect($exceptionHandler.errors[3]).toMatch(/\$compile.*badrestrict.*'42'/);
5589+
});
5590+
});
5591+
5592+
55615593
it('should throw noident when missing controllerAs directive property', function() {
55625594
module(function($compileProvider) {
55635595
$compileProvider.directive('noIdent', valueFn({

0 commit comments

Comments
 (0)