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

feat($compile): added new method strictComponentBindingsEnabled #16129

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
8 changes: 8 additions & 0 deletions docs/content/error/$compile/missingattr.ngdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
@ngdoc error
@name $compile:missingattr
@fullName Missing required attribute
@description

This error may occur only when `$compileProvider.strictComponentBindingsEnabled` is set to `true`.
Then all attributes mentioned in `bindings` without `?` must be set. If one or more aren't set,
the first one will throw an error.
41 changes: 41 additions & 0 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,32 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
return debugInfoEnabled;
};

/**
* @ngdoc method
* @name $compileProvider#strictComponentBindingsEnabled
*
* @param {boolean=} enabled update the strictComponentBindingsEnabled state if provided, otherwise just return the
* current strictComponentBindingsEnabled state
* @returns {*} current value if used as getter or itself (chaining) if used as setter
*
* @kind function
*
* @description
* Call this method to enable/disable strict component bindings check. If enabled, the compiler will enforce that
* for all bindings of a component that are not set as optional with `?`, an attribute needs to be provided
* on the component's HTML tag.
*
* The default value is false.
*/
var strictComponentBindingsEnabled = false;
this.strictComponentBindingsEnabled = function(enabled) {
if (isDefined(enabled)) {
strictComponentBindingsEnabled = enabled;
return this;
}
return strictComponentBindingsEnabled;
};

var TTL = 10;
/**
* @ngdoc method
Expand Down Expand Up @@ -3413,12 +3439,20 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
}

function strictBindingsCheck(attrName, directiveName) {
if (strictComponentBindingsEnabled) {
throw $compileMinErr('missingattr',
'Attribute \'{0}\' of \'{1}\' is non-optional and must be set!',
attrName, directiveName);
}
}

// Set up $watches for isolate scope and controller bindings.
function initializeDirectiveBindings(scope, attrs, destination, bindings, directive) {
var removeWatchCollection = [];
var initialChanges = {};
var changes;

forEach(bindings, function initializeBinding(definition, scopeName) {
var attrName = definition.attrName,
optional = definition.optional,
Expand All @@ -3430,7 +3464,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

case '@':
if (!optional && !hasOwnProperty.call(attrs, attrName)) {
strictBindingsCheck(attrName, directive.name);
destination[scopeName] = attrs[attrName] = undefined;

}
removeWatch = attrs.$observe(attrName, function(value) {
if (isString(value) || isBoolean(value)) {
Expand All @@ -3457,6 +3493,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
case '=':
if (!hasOwnProperty.call(attrs, attrName)) {
if (optional) break;
strictBindingsCheck(attrName, directive.name);
attrs[attrName] = undefined;
}
if (optional && !attrs[attrName]) break;
Expand Down Expand Up @@ -3501,6 +3538,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
case '<':
if (!hasOwnProperty.call(attrs, attrName)) {
if (optional) break;
strictBindingsCheck(attrName, directive.name);
attrs[attrName] = undefined;
}
if (optional && !attrs[attrName]) break;
Expand All @@ -3526,6 +3564,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
break;

case '&':
if (!optional && !hasOwnProperty.call(attrs, attrName)) {
strictBindingsCheck(attrName, directive.name);
}
// Don't assign Object.prototype method to scope
parentGet = attrs.hasOwnProperty(attrName) ? $parse(attrs[attrName]) : noop;

Expand Down
223 changes: 223 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,15 @@ describe('$compile', function() {
inject();
});

it('should allow strictComponentBindingsEnabled to be configured', function() {
module(function($compileProvider) {
expect($compileProvider.strictComponentBindingsEnabled()).toBe(false); // the default
$compileProvider.strictComponentBindingsEnabled(true);
expect($compileProvider.strictComponentBindingsEnabled()).toBe(true);
});
inject();
});

it('should allow onChangesTtl to be configured', function() {
module(function($compileProvider) {
expect($compileProvider.onChangesTtl()).toBe(10); // the default
Expand Down Expand Up @@ -2546,6 +2555,16 @@ describe('$compile', function() {
template: '<span></span>'
};
});
directive('prototypeMethodNameAsScopeVarD', function() {
return {
scope: {
'constructor': '<?',
'valueOf': '<'
},
restrict: 'AE',
template: '<span></span>'
};
});
directive('watchAsScopeVar', function() {
return {
scope: {
Expand Down Expand Up @@ -2854,6 +2873,57 @@ describe('$compile', function() {
})
);

it('should throw an error for undefined non-optional "=" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-a></div>'
)($rootScope);
};
expect(func).toThrowMinErr('$compile',
'missingattr',
'Attribute \'valueOf\' of \'prototypeMethodNameAs' +
'ScopeVarA\' is non-optional and must be set!');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, also verify that it doesn't fail when the attribute is present.

});

it('should not throw an error for set non-optional "=" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-a constructor="constructor" value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should not throw an error for undefined optional "=" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-a value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should handle "@" bindings with same method names in Object.prototype correctly when not present', inject(
function($rootScope, $compile) {
var func = function() {
Expand Down Expand Up @@ -2891,6 +2961,57 @@ describe('$compile', function() {
})
);

it('should throw an error for undefined non-optional "@" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-b></div>'
)($rootScope);
};
expect(func).toThrowMinErr('$compile',
'missingattr',
'Attribute \'valueOf\' of \'prototypeMethodNameAs' +
'ScopeVarB\' is non-optional and must be set!');
});
});

it('should not throw an error for set non-optional "@" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-b constructor="constructor" value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should not throw an error for undefined optional "@" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-b value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should handle "&" bindings with same method names in Object.prototype correctly when not present', inject(
function($rootScope, $compile) {
var func = function() {
Expand Down Expand Up @@ -2923,6 +3044,108 @@ describe('$compile', function() {
})
);

it('should throw an error for undefined non-optional "&" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-c></div>'
)($rootScope);
};
expect(func).toThrowMinErr('$compile',
'missingattr',
'Attribute \'valueOf\' of \'prototypeMethodNameAs' +
'ScopeVarC\' is non-optional and must be set!');
});
});

it('should not throw an error for set non-optional "&" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-c constructor="constructor" value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should not throw an error for undefined optional "&" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-c value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should throw an error for undefined non-optional "<" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-d></div>'
)($rootScope);
};
expect(func).toThrowMinErr('$compile',
'missingattr',
'Attribute \'valueOf\' of \'prototypeMethodNameAs' +
'ScopeVarD\' is non-optional and must be set!');
});
});

it('should not throw an error for set non-optional "<" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-d constructor="constructor" value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should not throw an error for undefined optional "<" bindings when ' +
'strictComponentBindingsEnabled is true', function() {
module(function($compileProvider) {
$compileProvider.strictComponentBindingsEnabled(true);
});
inject(
function($rootScope, $compile) {
var func = function() {
element = $compile(
'<div prototype-method-name-as-scope-var-d value-of="valueOf"></div>'
)($rootScope);
};
expect(func).not.toThrow();
});
});

it('should not throw exception when using "watch" as binding in Firefox', inject(
function($rootScope, $compile) {
$rootScope.watch = 'watch';
Expand Down