Skip to content

Commit

Permalink
fix(loader): module.decorator order of operations is now irrelevant
Browse files Browse the repository at this point in the history
`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 it is **not**
// decorated via `moduleDecoratorFn` as expected.
```

The result of this fix ensures that `theOtherFactoryFn` will be decorated as
expected when using the declaration order above.

Closes angular#12382

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
  • Loading branch information
robertmirro committed May 1, 2016
1 parent 1964620 commit d7de76d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/loader.js
Expand Up @@ -203,7 +203,7 @@ function setupModuleLoader(window) {
* @description
* See {@link auto.$provide#decorator $provide.decorator()}.
*/
decorator: invokeLaterAndSetModuleName('$provide', 'decorator'),
decorator: invokeLaterAndSetModuleName('$provide', 'decorator', configBlocks),

/**
* @ngdoc method
Expand Down Expand Up @@ -349,10 +349,11 @@ function setupModuleLoader(window) {
* @param {string} method
* @returns {angular.Module}
*/
function invokeLaterAndSetModuleName(provider, method) {
function invokeLaterAndSetModuleName(provider, method, queue) {
if (!queue) queue = invokeQueue;
return function(recipeName, factoryFunction) {
if (factoryFunction && isFunction(factoryFunction)) factoryFunction.$$moduleName = name;
invokeQueue.push([provider, method, arguments]);
queue.push([provider, method, arguments]);
return moduleInstance;
};
}
Expand Down
13 changes: 12 additions & 1 deletion test/loaderSpec.js
Expand Up @@ -48,7 +48,6 @@ describe('module loader', function() {
expect(myModule.requires).toEqual(['other']);
expect(myModule._invokeQueue).toEqual([
['$provide', 'constant', jasmine.objectContaining(['abc', 123])],
['$provide', 'decorator', jasmine.objectContaining(['dk', 'dv'])],
['$provide', 'provider', jasmine.objectContaining(['sk', 'sv'])],
['$provide', 'factory', jasmine.objectContaining(['fk', 'fv'])],
['$provide', 'service', jasmine.objectContaining(['a', 'aa'])],
Expand All @@ -60,12 +59,24 @@ describe('module loader', function() {
]);
expect(myModule._configBlocks).toEqual([
['$injector', 'invoke', jasmine.objectContaining(['config'])],
['$provide', 'decorator', jasmine.objectContaining(['dk', 'dv'])],
['$injector', 'invoke', jasmine.objectContaining(['init2'])]
]);
expect(myModule._runBlocks).toEqual(['runBlock']);
});


it("should not throw error when `module.decorator` is declared before provider that it decorates", function() {
angular.module('theModule', []).
decorator('theProvider', function($delegate) { return $delegate; }).
factory('theProvider', function() { return {}; });

expect(function() {
createInjector(['myModule']);
}).not.toThrow();
});


it('should allow module redefinition', function() {
expect(window.angular.module('a', [])).not.toBe(window.angular.module('a', []));
});
Expand Down

0 comments on commit d7de76d

Please sign in to comment.