From 263031c238ab03adbebc61ed6842697b7d0238f9 Mon Sep 17 00:00:00 2001
From: robertmirro <rmirro@evolenthealth.com>
Date: Wed, 30 Mar 2016 17:30:42 -0400
Subject: [PATCH] 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 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 #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
---
 src/loader.js      |  7 ++++---
 test/loaderSpec.js | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/loader.js b/src/loader.js
index 336517783889..7266a066f3ce 100644
--- a/src/loader.js
+++ b/src/loader.js
@@ -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
@@ -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;
           };
         }
diff --git a/test/loaderSpec.js b/test/loaderSpec.js
index 317717f65e17..6c766996b7a1 100644
--- a/test/loaderSpec.js
+++ b/test/loaderSpec.js
@@ -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'])],
@@ -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(['theModule']);
+    }).not.toThrow();
+  });
+
+
   it('should allow module redefinition', function() {
     expect(window.angular.module('a', [])).not.toBe(window.angular.module('a', []));
   });