fix(injector): invoke config blocks for module after all providers

[caitp]

This change ensures that a module's config blocks are always invoked after all of its providers are registered.

NOTE: I'm not entirely sure if this is desirable yet, because the point mentioned in the BREAKING CHANGE section below may actually be something which is desirable. However, I do feel it is probably better to always invoke config blocks for a single module after all of its providers. The config blocks should not play a role in how providers are registered, in my own opinion.

BREAKING CHANGE:

Previously, config blocks would be able to control behaviour of provider registration, due to being invoked prior to provider registration. Now, provider registration always occurs prior to configuration for a given module, and therefore config blocks are not able to have any control over a providers
registration.

Example:

Previously, the following:

   angular.module('foo', [])
    .provider('$rootProvider', function() {
      this.$get = function() { ... }
    })
    .config(function($rootProvider) {
      $rootProvider.dependentMode = "B";
    })
    .provider('$dependentProvider', function($rootProvider) {
      if ($rootProvider.dependentMode === "A") {
        this.$get = function() {
          // Special mode!
        }
      } else {
        this.$get = function() {
          // something else
        }
      }
    });

would have "worked", meaning behaviour of the config block between the registration of "$rootProvider" and "$dependentProvider" would have actually accomplished something and changed the behaviour of the app. This is no longer possible within a single module.

Fixes #7139

[shahata]

This test actually will not fail without your fix. :)
You should move the config blocks to be before the provider registration

[caitp]

Misunderstood what you were saying originally, yes you're right, the test is broken because of that :) fixes

[lgalfaso]

LGTM. I am wondering it there should be a note that to migrate from the previous behavior a constant should be used and how that would be done

[caitp]

I am wondering it there should be a note that to migrate from the previous behavior a constant should be used and how that would be done

To be honest, if people were actually using the previous behaviour, in a way that this change would "break", they're really doing some things that are probably "wrong", like, putting together really inconsistent APIs that are kind of nightmarish.

I don't really expect that a lot of this is happening, but it should be possible to work-around it using module dependencies, so I could put a note about that in the commit message.

The thing is that such a note would be rather wordy and complicated, and I don't think it is likely to benefit very many people.

[lgalfaso]

@caitp ok, makes sense

[shahata]

I think the easiest way to migrate is using a config block with $provide.provider instead of the angular.module().provider:

Before:

angular.module('foo', [])
    .provider('$rootProvider', function() {
      this.$get = function() { ... }
    })
    .config(function($rootProvider) {
      $rootProvider.dependentMode = "B";
    })
    .provider('$dependentProvider', function($rootProvider) {
      if ($rootProvider.dependentMode === "A") {
        this.$get = function() {
          // Special mode!
        }
      } else {
        this.$get = function() {
          // something else
        }
      }
    });

After:

angular.module('foo', [])
    .provider('$rootProvider', function() {
      this.$get = function() { ... }
    })
    .config(function($rootProvider) {
      $rootProvider.dependentMode = "B";
    })
    .config(function($provide) {
      $provide.provider('$dependentProvider', function($rootProvider) {
        if ($rootProvider.dependentMode === "A") {
          this.$get = function() {
            // Special mode!
          }
        } else {
          this.$get = function() {
            // something else
          }
        }
      });
    });

And yes, this is definitely bad code :)

[lgalfaso]

Please do not add this code on how to migrate. Putting nothing it better

[caitp]

It would work, you just wouldn't be able to inject $dependentProvider into config blocks that occurred before the one that registered it. (which is essentially the current behaviour of the loader.js apis)

[caitp]

Triaging for beta-6, because it would be good to experiment and figure out if it upsets people or not.

[caitp]

@IgorMinar there's a problem with that, it pretty much breaks ngMock to do it that way, because ng registers things in a config block, but ngMock registers (most) things as regular providers. So what ends up happening is, the providers get set up in order (and therefore the ngMock stuff runs first), but the config blocks wait until the end, so the ng stuff added in config blocks overwrites the ngMock stuff.

I mean, there are ways around this, but it kind of sucks, so maybe it's better to just leave it the way it is for now. I think fixing that would be more of a breaking change than this is really worth, so IMO it's probably better to leave it.

[IgorMinar]

ok, so let's get this is as is.