fix(angularAMD): Capture and use providerInjector (config-time $injector) instead of run-time $injector.

[christopherthielen]

This allows .config blocks to inject Providers, as opposed to services. Closes #28

[christopherthielen]

Updated plunkr from issue #21 which shows the RestangularProvider successfully injected into the .config block.

http://plnkr.co/edit/m7BHsCazf1VRtzJgn45B

[marcoslin]

The changes causes unit test to fail:

Error: [$injector:unpr] Unknown provider: $rootScope
http://errors.angularjs.org/1.2.15-build.2399+sha.ca4ddfa/$injector/unpr?p0=%24rootScope at
.../angularAMD/master/bower_components/angular/angular.js:3596

Any idea why? You can run grunt test-unit to invoke the unit test.

[marcoslin]

Ok, the summary of the problem is that while RestangularProvider needs a .config block injector, most of other simple packages uses normal injector. This solution breaks something as simple as ngload'ing test/unit/lib/service.js.

This just mean that I need cache both injector and somehow figure out a way to detect which one should be used, or just simply let user choose.

No sure if you know this but any idea why RestangularProvider requires injector from the .config block?

[marcoslin]

I came up with the following that seems to work:

var cachedProvider;
if ( provider === "$inject" && method === "provider" ) {
    cachedProvider = providerInjector;
} else {
    cachedProvider = app_cached_providers[provider];
};

[marcoslin]

I just saw your new commit. Do we really need to store and loop through the configArgs? I don't think the problem is related to the need to detect .config block as existing code work just fine.

What seem to breaks is the use of angular.provide that, for whatever reason, requires the caching of the injector from .config (or so it seems). I wanna do a bit more test to make sure I fully understand this.

Any new ideas/comments please do post it here.

[christopherthielen]

Hi Marcos,

Thanks for looking at my pull request. I reverted my last commit because my editor changed all the white spaces. I've re-committed and pushed a cleaner delta.

Angular .config and .provider blocks can only have providers and constants injected. Run-time blocks (All other types, which includes .controller, .service, etc) can only have instances (such as $rootScope) injected. See https://docs.angularjs.org/guide/module:

Configuration blocks - get executed during the provider registrations and configuration phase. Only providers and constants can be injected into configuration blocks. This is to prevent accidental instantiation of services before they have been fully configured.

Run blocks - get executed after the injector is created and are used to kickstart the application. Only instances and constants can be injected into run blocks. This is to prevent further system configuration during application run time.

Your services.js does not work when used outside of angularAMD because it tries to inject $rootScope into the .config block. Have a look at this plunk, where I load angular and services.js in an otherwise blank plunkr, which demonstrates the failure: http://plnkr.co/edit/y8VntcgNk2sH6eJReslG

"[$injector:unpr] Unknown provider: $rootScope http://errors.angularjs.org/1.2.16/$injector/unpr?p0=%24rootScope"

That's why I changed the test to inject a Provider instead. I chose $filterProvider because it can easily be configured to return a testable string during the run-phase.

There should be no user choice in the matter regarding which injector to provide. Only .config blocks get the providerInjector (and they cannot get the runtime $injector). All other blocks get the runtime $injector (and they cannot be given the providerInjector).

The way my PR detects if the block in the queue being executed is a .config block, or any other block, is by saving a reference to config arguments (.config(arguments)), then checking if the block being matches the saved arguments. This is a little hokey, but it works.

[christopherthielen]

Tests pass locally, but travis fails. I think this is from build log line 867, it looks like the build directory isn't getting cleaned properly?

npm ERR! error rolling back Error: ENOTEMPTY, rmdir '/home/travis/build/marcoslin/angularAMD/node_modules/grunt-bower-task/node_modules/bower/node_modules/glob' npm ERR! error rolling back bower@1.2.8 { [Error: ENOTEMPTY, rmdir '/home/travis/build/marcoslin/angularAMD/node_modules/grunt-bower-task/node_modules/bower/node_modules/glob'] npm ERR! error rolling back errno: 53, npm ERR! error rolling back code: 'ENOTEMPTY', npm ERR! error rolling back path: '/home/travis/build/marcoslin/angularAMD/node_modules/grunt-bower-task/node_modules/bower/node_modules/glob' } npm ERR! Error: ENOENT, lstat '/home/travis/build/marcoslin/angularAMD/node_modules/grunt-bower-task/node_modules/bower/node_modules/chalk/node_modules/has-color/index.js'

[christopherthielen]

Regarding storing and looping over .config arguments, I think it might be OK to use only provider === '$injector' && method === 'invoke'.

My thought is that it's 100% accurate to capture the arguments when .config is run and compare later. It's future proof. For instance, what if angular team adds a run-time function to the module that just calls $injector.invoke or changes .run() to do just that.

[marcoslin]

If we can do with just provider === '$injector' && method === 'invoke' is preferred as we remove the inner look to check for the .config block. I am trying to pitch angularAMD for large project and that loop you added might have performance implications.

I am doing more tests on my end to fully understand what is going on here and will share my finding here with you.

[marcoslin]

Ok, I think I finally nailed it. It is actually very simple, all we need is:

var cachedProvider;
if ( provider === "$injector" && method === "invoke" ) {
    cachedProvider = config_injector;
} else {
    cachedProvider = app_cached_providers[provider];
};

For the sake of naming, can you use config_injector? There is no need to detect the .config block as it is defined for you by the variables above.

I updated the unit-test in the "devel" branch adding a custom provider. It is failing now awaiting your pull request.

Thanks!!!

[christopherthielen]

New PR coming to devel branch.