-
Notifications
You must be signed in to change notification settings - Fork 27.5k
feat(injector): "strict-DI" mode which disables "automatic" function annotation / inferred dependencies #6719
Conversation
I like this a lot. |
So, per discussion today, we should do this in bootstrap and the ng-app directives instead of making this change in the module definition layer The problem becomes testing it, since angular-mocks doesn't really "bootstrap" anything, per se. I think keeping the module API as is (in the patch), but removing the public API from it (no extra param to |
* | ||
<example> | ||
<file name="index.html"> | ||
<div ng-app-strict="ngAppStrictDemo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petebacondarwin I'd like to use this in an example, but the docs preprocessor only understands the ng-app
attribute and not ng-app-strict
(if that does continue to be the correct name).
I was thinking it wouldn't be so bad to change the dgeni-packages template so that if ng-app-strict is present, it will include -strict
, but I dunno.
In any case, this makes things kind of awkward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not convinced that controlling this with a directive is the best API. If you are creating an application that is complex enough that you will need this then you are going to be writing a bunch of modules in JavaScript, in which case it is not a big deal to apply the flag to the module. For instance, you could have a method on the module API:
angular.module('myApp', ['ngRoute'])
.requireAnnotations()
.controller(...)
...
.factory(...);
This is also more self-documenting - I doubt that anyone would ever be able to guess what ng-app-strict
actually meant, without having to go to the docs.
Any, regarding the examples, we have a nice feature that asks Dgeni not to automatically apply an ng-app
to the index.html wrapper that gets generated, which allows us to apply it explicitly inside the example itself. We use this in the Concepts guide: https://github.com/angular/angular.js/blob/master/docs/content/guide/concepts.ngdoc#L35.
In this case the example will look like:
<example ng-app-included="true">
<file name="index.html">
<div ng-app-strict="ngAppStrictDemo">
...
</file>
</example>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, you could have a method on the module API:
This was discussed during the meeting, the problem with doing this at the module level is that it technically applies to all modules in the application, even if it's added by a third-party one. Doing this during bootstrapping means that it's in the application's control, regardless of what third party modules are doing.
I do agree that the directive name could (should) be better, but I didn't want to spend too much time thinking about what it should be.
Onto the docs thing (again), ah, I thought the docs processor was preprocessing it to find the attribute, but I guess not. So that should fix that up.
can we:
|
Docs page demo: http://ci.angularjs.org/job/angular.js-caitlin/274/artifact/build/docs/error/$injector/strctdi?p0=BadController I think it looks okay |
Looks good to me! |
@@ -0,0 +1,55 @@ | |||
@ngdoc error | |||
@name $injector:strctdi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strctdi
is a bit vague - I might think strct is to do with structure? Is there a problem with using strictdi
?
var attr, i, ii = ngAttrPrefixes.length, j, jj; | ||
for (i=0; i<ii; ++i) { | ||
attr = ngAttrPrefixes[i] + ngAttr; | ||
if (element.getAttribute) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we sniff whether getAttribute
is available outside this loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's less work to just defer to jqLite for this, since jquery/jqlite should have already been chosen by the time this is called. Testing locally it works, so it's probably okay.
lgtm. just squash it into a single commit please |
…annotation This modifies the injector to prevent automatic annotation from occurring for a given injector. This behaviour can be enabled when bootstrapping the application by using the attribute "ng-strict-di" on the root element (the element containing "ng-app"), or alternatively by passing an object with the property "strictDi" set to "true" in angular.bootstrap, when bootstrapping manually. JS example: angular.module("name", ["dependencies", "otherdeps"]) .provider("$willBreak", function() { this.$get = function($rootScope) { }; }) .run(["$willBreak", function($willBreak) { // This block will never run because the noMagic flag was set to true, // and the $willBreak '$get' function does not have an explicit // annotation. }]); angular.bootstrap(document, ["name"], { strictDi: true }); HTML: <html ng-app="name" ng-strict-di> <!-- ... --> </html> This will only affect functions with an arity greater than 0, and without an $inject property. Closes angular#6719 Closes angular#6717 Closes angular#4504 Closes angular#6069 Closes angular#3611
🎊 |
…annotation This modifies the injector to prevent automatic annotation from occurring for a given injector. This behaviour can be enabled when bootstrapping the application by using the attribute "ng-strict-di" on the root element (the element containing "ng-app"), or alternatively by passing an object with the property "strictDi" set to "true" in angular.bootstrap, when bootstrapping manually. JS example: angular.module("name", ["dependencies", "otherdeps"]) .provider("$willBreak", function() { this.$get = function($rootScope) { }; }) .run(["$willBreak", function($willBreak) { // This block will never run because the noMagic flag was set to true, // and the $willBreak '$get' function does not have an explicit // annotation. }]); angular.bootstrap(document, ["name"], { strictDi: true }); HTML: <html ng-app="name" ng-strict-di> <!-- ... --> </html> This will only affect functions with an arity greater than 0, and without an $inject property. Closes angular#6719 Closes angular#6717 Closes angular#4504 Closes angular#6069 Closes angular#3611
…annotation This modifies the injector to prevent automatic annotation from occurring for a given injector. This behaviour can be enabled when bootstrapping the application by using the attribute "ng-strict-di" on the root element (the element containing "ng-app"), or alternatively by passing an object with the property "strictDi" set to "true" in angular.bootstrap, when bootstrapping manually. JS example: angular.module("name", ["dependencies", "otherdeps"]) .provider("$willBreak", function() { this.$get = function($rootScope) { }; }) .run(["$willBreak", function($willBreak) { // This block will never run because the noMagic flag was set to true, // and the $willBreak '$get' function does not have an explicit // annotation. }]); angular.bootstrap(document, ["name"], { strictDi: true }); HTML: <html ng-app="name" ng-strict-di> <!-- ... --> </html> This will only affect functions with an arity greater than 0, and without an $inject property. Closes #6719 Closes #6717 Closes #4504 Closes #6069 Closes #3611
This modifies the injector to prevent automatic annotation (or inferring dependencies based on formal parameter names) from occurring for a given injector.
This behaviour can be enabled when bootstrapping the application by using the attribute
"ng-strict-di" on the root element (the element containing "ng-app"), or alternatively by passing
an object with the property "strictDi" set to "true" in angular.bootstrap, when bootstrapping
manually.
JS example:
HTML:
This will only affect functions with an arity greater than 0, and without an $inject property.