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

Commit 6a2ebdb

Browse files
robertmirroNarretz
robertmirro
authored andcommitted
fix(loader): module.decorator order of operations is now irrelevant
`module.decorator` is now processed via the configBlocks order of operations and: 1. no longer throws error if declared before the provider being decorated. 2. guarantees the correct provider will be decorated when multiple, same-name providers are defined. (1) Prior to this fix, declaring `module.decorator` before the provider that it decorates results in throwing an error: ```js angular .module('theApp', []) .decorator('theFactory', moduleDecoratorFn) .factory('theFactory', theFactoryFn); // Error: [$injector:modulerr] Failed to instantiate module theApp due to: // Error: [$injector:unpr] Unknown provider: theFactoryProvider ``` The result of this fix now allows for the declaration order above. (2) Prior to this fix, declaring `module.decorator` before the final, same-named provider results in that provider **not** being decorated as expected: **NOTE:** Angular does not use provider name spacing, so the final declared provider is selected if multiple, same-named providers are declared. ```js angular .module('theApp', []) .factory('theFactory', theFactoryFn) .decorator('theFactory', moduleDecoratorFn) .factory('theFactory', theOtherFactoryFn); ``` `theOtherFactoryFn` is selected as 'theFactory' provider, but prior to this fix it is **not** decorated via `moduleDecoratorFn`. This fix ensures that `theOtherFactoryFn` will be decorated as expected when using the declaration order above. Closes #12382 Closes #14348 BREAKING CHANGE: `module.decorator` declarations are now processed as part of the `module.config` queue and may result in providers being decorated in a different order if `module.config` blocks are also used to decorate providers via `$provide.decorator`. For example, consider the following declaration order in which 'theFactory' is decorated by both a `module.decorator` and a `$provide.decorator`: ```js angular .module('theApp', []) .factory('theFactory', theFactoryFn) .config(function($provide) { $provide.decorator('theFactory', provideDecoratorFn); }) .decorator('theFactory', moduleDecoratorFn); ``` Prior to this fix, 'theFactory' provider would be decorated in the following order: 1. moduleDecoratorFn 2. provideDecoratorFn The result of this fix changes the order in which 'theFactory' is decorated because now `module.decorator` declarations are processed in the same order as `module.config` declarations: 1. provideDecoratorFn 2. moduleDecoratorFn
1 parent c2033d7 commit 6a2ebdb

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

src/loader.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ function setupModuleLoader(window) {
203203
* @description
204204
* See {@link auto.$provide#decorator $provide.decorator()}.
205205
*/
206-
decorator: invokeLaterAndSetModuleName('$provide', 'decorator'),
206+
decorator: invokeLaterAndSetModuleName('$provide', 'decorator', configBlocks),
207207

208208
/**
209209
* @ngdoc method
@@ -349,10 +349,11 @@ function setupModuleLoader(window) {
349349
* @param {string} method
350350
* @returns {angular.Module}
351351
*/
352-
function invokeLaterAndSetModuleName(provider, method) {
352+
function invokeLaterAndSetModuleName(provider, method, queue) {
353+
if (!queue) queue = invokeQueue;
353354
return function(recipeName, factoryFunction) {
354355
if (factoryFunction && isFunction(factoryFunction)) factoryFunction.$$moduleName = name;
355-
invokeQueue.push([provider, method, arguments]);
356+
queue.push([provider, method, arguments]);
356357
return moduleInstance;
357358
};
358359
}

test/loaderSpec.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ describe('module loader', function() {
4848
expect(myModule.requires).toEqual(['other']);
4949
expect(myModule._invokeQueue).toEqual([
5050
['$provide', 'constant', jasmine.objectContaining(['abc', 123])],
51-
['$provide', 'decorator', jasmine.objectContaining(['dk', 'dv'])],
5251
['$provide', 'provider', jasmine.objectContaining(['sk', 'sv'])],
5352
['$provide', 'factory', jasmine.objectContaining(['fk', 'fv'])],
5453
['$provide', 'service', jasmine.objectContaining(['a', 'aa'])],
@@ -60,12 +59,24 @@ describe('module loader', function() {
6059
]);
6160
expect(myModule._configBlocks).toEqual([
6261
['$injector', 'invoke', jasmine.objectContaining(['config'])],
62+
['$provide', 'decorator', jasmine.objectContaining(['dk', 'dv'])],
6363
['$injector', 'invoke', jasmine.objectContaining(['init2'])]
6464
]);
6565
expect(myModule._runBlocks).toEqual(['runBlock']);
6666
});
6767

6868

69+
it("should not throw error when `module.decorator` is declared before provider that it decorates", function() {
70+
angular.module('theModule', []).
71+
decorator('theProvider', function($delegate) { return $delegate; }).
72+
factory('theProvider', function() { return {}; });
73+
74+
expect(function() {
75+
createInjector(['theModule']);
76+
}).not.toThrow();
77+
});
78+
79+
6980
it('should allow module redefinition', function() {
7081
expect(window.angular.module('a', [])).not.toBe(window.angular.module('a', []));
7182
});

0 commit comments

Comments
 (0)