fix(injector): invoke config blocks for module after all providers#7147
fix(injector): invoke config blocks for module after all providers#7147caitp wants to merge 1 commit intoangular:masterfrom
Conversation
There was a problem hiding this comment.
This test actually will not fail without your fix. :)
You should move the config blocks to be before the provider registration
There was a problem hiding this comment.
Misunderstood what you were saying originally, yes you're right, the test is broken because of that :) fixes
This change ensures that a module's config blocks are always invoked after all of its providers are
registered.
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 angular#7139
|
LGTM. I am wondering it there should be a note that to migrate from the previous behavior a |
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. |
|
@caitp ok, makes sense |
|
I think the easiest way to migrate is using a 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 :) |
|
Please do not add this code on how to migrate. Putting nothing it better |
|
It would work, you just wouldn't be able to inject |
|
Triaging for beta-6, because it would be good to experiment and figure out if it upsets people or not. |
|
@IgorMinar there's a problem with that, it pretty much breaks ngMock to do it that way, because 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. |
|
ok, so let's get this is as is. |
5 participants
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:
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