fix($compile): correct controller instantiation for async directives

This fixes regression introduced by angular#3514 (5c56011) - this commit is being reverted here and a better fix is included. The regression caused the controller to be instantiated before the isolate scope was initialized. Closes angular#3493 Closes angular#3482 Closes angular#3537
IgorMinar · Aug 10, 2013 · d32aa8d · d32aa8d
1 parent 2430347
commit d32aa8d
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 11 deletions.
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -782,6 +782,13 @@ function $CompileProvider($provide) {
 
         directiveName = directive.name;
 
+        if (directiveValue = directive.controller) {
+          controllerDirectives = controllerDirectives || {};
+          assertNoDuplicate("'" + directiveName + "' controller",
+              controllerDirectives[directiveName], directive, $compileNode);
+          controllerDirectives[directiveName] = directive;
+        }
+
         if (directiveValue = directive.transclude) {
           assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
           transcludeDirective = directive;
@@ -870,13 +877,6 @@ function $CompileProvider($provide) {
           }
         }
 
-        if (!directive.templateUrl && directive.controller) {
-          controllerDirectives = controllerDirectives || {};
-          assertNoDuplicate("'" + directiveName + "' controller",
-              controllerDirectives[directiveName], directive, $compileNode);
-          controllerDirectives[directiveName] = directive;
-        }
-
         if (directive.terminal) {
           nodeLinkFn.terminal = true;
           terminalPriority = Math.max(terminalPriority, directive.priority);
@@ -1152,7 +1152,7 @@ function $CompileProvider($provide) {
           origAsyncDirective = directives.shift(),
           // The fact that we have to copy and patch the directive seems wrong!
           derivedSyncDirective = extend({}, origAsyncDirective, {
-            templateUrl: null, transclude: null, scope: null, replace: null
+            controller: null, templateUrl: null, transclude: null, scope: null, replace: null
           }),
           templateUrl = (isFunction(origAsyncDirective.templateUrl))
               ? origAsyncDirective.templateUrl($compileNode, tAttrs)
@@ -1208,9 +1208,10 @@ function $CompileProvider($provide) {
               replaceWith(linkRootElement, jqLite(beforeTemplateLinkNode), linkNode);
             }
 
-            afterTemplateNodeLinkFn(function() {
-              beforeTemplateNodeLinkFn(afterTemplateChildLinkFn, scope, linkNode, $rootElement, controller);
-            }, scope, linkNode, $rootElement, controller);
+            afterTemplateNodeLinkFn(
+              beforeTemplateNodeLinkFn(afterTemplateChildLinkFn, scope, linkNode, $rootElement, controller),
+              scope, linkNode, $rootElement, controller
+            );
           }
           linkQueue = null;
         }).

diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -2455,6 +2455,61 @@ describe('$compile', function() {
     });
 
 
+    it('should instantiate the controller after the isolate scope bindings are initialized (with template)', function () {
+      module(function () {
+        var Ctrl = function ($scope, log) {
+          log('myFoo=' + $scope.myFoo);
+        };
+
+        directive('myDirective', function () {
+          return {
+            scope: {
+              myFoo: "="
+            },
+            template: '<p>Hello</p>',
+            controller: Ctrl
+          };
+        });
+      });
+
+      inject(function ($templateCache, $compile, $rootScope, log) {
+        $rootScope.foo = "bar";
+
+        element = $compile('<div my-directive my-foo="foo"></div>')($rootScope);
+        $rootScope.$apply();
+        expect(log).toEqual('myFoo=bar');
+      });
+    });
+
+
+    it('should instantiate the controller after the isolate scope bindings are initialized (with templateUrl)', function () {
+      module(function () {
+        var Ctrl = function ($scope, log) {
+          log('myFoo=' + $scope.myFoo);
+        };
+
+        directive('myDirective', function () {
+          return {
+            scope: {
+              myFoo: "="
+            },
+            templateUrl: 'hello.html',
+            controller: Ctrl
+          };
+        });
+      });
+
+      inject(function ($templateCache, $compile, $rootScope, log) {
+        $templateCache.put('hello.html', '<p>Hello</p>');
+        $rootScope.foo = "bar";
+
+        element = $compile('<div my-directive my-foo="foo"></div>')($rootScope);
+        $rootScope.$apply();
+        expect(log).toEqual('myFoo=bar');
+      });
+    });
+
+
     it('should instantiate controllers in the parent->child->baby order when nested transluction, templateUrl and ' +
         'replacement are in the mix', function() {
       // similar to the test above, except that we have one more layer of nesting and nested transclusion