angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.9k stars 27.54k forks source link

Error when controller is specified at the same level of directive #8043

Closed magizh closed 9 years ago

magizh commented 10 years ago

When I declare the controller at the same level of directive like this

http://plnkr.co/edit/ANkGH7r4GXyKLAFCTUWW?p=preview

I get a error while accessing the object passed to the directive in its controller.

TypeError: Cannot read property 'name' of undefined at new controller (http://run.plnkr.co/2tcxCoWpBWVvSdXs/script.js:22:37) at invoke (http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:3979:17) at Object.instantiate (http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:3990:23) at http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:7281:28 at http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:6662:34 at forEach (http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:327:20) at nodeLinkFn (http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:6649:11) at http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:6907:13 at http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:8021:11 at wrappedCallback (http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.10/angular.js:11410:81)

caitp commented 10 years ago

The main issue here is that isolate scope properties are not "ready" at the time of controller instantiation, they're actually ready shortly after that.

Something like this will usually work better for you: http://plnkr.co/edit/bCniFefoBkHnhRTYvk3q?p=preview

(My PR to allow isolate scope bindings directly into the controller will probably make this easier for people too)

Izhaki commented 10 years ago

@caitp, when you say 'the isolate scope properties are not "ready"', is this because controllers are called before the preLink phase where binding occurs?

The following code suggests that controller do get the isolated scope alright:

    if (newIsolateScopeDirective) {
      var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/;
      var $linkNode = jqLite(linkNode);

      isolateScope = scope.$new(true);

      if (templateDirective && (templateDirective === newIsolateScopeDirective ||
          templateDirective === newIsolateScopeDirective.$$originalDirective)) {
        $linkNode.data('$isolateScope', isolateScope) ;
      } else {
        $linkNode.data('$isolateScopeNoTemplate', isolateScope);
      }

      safeAddClass($linkNode, 'ng-isolate-scope');

      forEach(newIsolateScopeDirective.scope, function(definition, scopeName) {
        var match = definition.match(LOCAL_REGEXP) || [],
            attrName = match[3] || scopeName,
            optional = (match[2] == '?'),
            mode = match[1], // @, =, or &
            lastValue,
            parentGet, parentSet, compare;

        isolateScope.$$isolateBindings[scopeName] = mode + attrName;

        switch (mode) {

          case '@':
            attrs.$observe(attrName, function(value) {
              isolateScope[scopeName] = value;
            });
            attrs.$$observers[attrName].$$scope = scope;
            if( attrs[attrName] ) {
              // If the attribute has been provided then we trigger an interpolation to ensure
              // the value is there for use in the link fn
              isolateScope[scopeName] = $interpolate(attrs[attrName])(scope);
            }
            break;

          case '=':
            if (optional && !attrs[attrName]) {
              return;
            }
            parentGet = $parse(attrs[attrName]);
            if (parentGet.literal) {
              compare = equals;
            } else {
              compare = function(a,b) { return a === b; };
            }
            parentSet = parentGet.assign || function() {
              // reset the change, or we will throw this exception on every $digest
              lastValue = isolateScope[scopeName] = parentGet(scope);
              throw $compileMinErr('nonassign',
                  "Expression '{0}' used with directive '{1}' is non-assignable!",
                  attrs[attrName], newIsolateScopeDirective.name);
            };
            lastValue = isolateScope[scopeName] = parentGet(scope);
            isolateScope.$watch(function parentValueWatch() {
              var parentValue = parentGet(scope);
              if (!compare(parentValue, isolateScope[scopeName])) {
                // we are out of sync and need to copy
                if (!compare(parentValue, lastValue)) {
                  // parent changed and it has precedence
                  isolateScope[scopeName] = parentValue;
                } else {
                  // if the parent can be assigned then do so
                  parentSet(scope, parentValue = isolateScope[scopeName]);
                }
              }
              return lastValue = parentValue;
            }, null, parentGet.literal);
            break;

          case '&':
            parentGet = $parse(attrs[attrName]);
            isolateScope[scopeName] = function(locals) {
              return parentGet(scope, locals);
            };
            break;

          default:
            throw $compileMinErr('iscp',
                "Invalid isolate scope definition for directive '{0}'." +
                " Definition: {... {1}: '{2}' ...}",
                newIsolateScopeDirective.name, scopeName, definition);
        }
      });
    }
    transcludeFn = boundTranscludeFn && controllersBoundTransclude;
    if (controllerDirectives) {
      forEach(controllerDirectives, function(directive) {
        var locals = {
          $scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
          $element: $element,
          $attrs: attrs,
          $transclude: transcludeFn
        }, controllerInstance;

        controller = directive.controller;
        if (controller == '@') {
          controller = attrs[directive.name];
        }

        controllerInstance = $controller(controller, locals);
        // For directives with element transclusion the element is a comment,
        // but jQuery .data doesn't support attaching data to comment nodes as it's hard to
        // clean up (http://bugs.jquery.com/ticket/8335).
        // Instead, we save the controllers for the element in a local hash and attach to .data
        // later, once we have the actual element.
        elementControllers[directive.name] = controllerInstance;
        if (!hasElementTranscludeDirective) {
          $element.data('$' + directive.name + 'Controller', controllerInstance);
        }

        if (directive.controllerAs) {
          locals.$scope[directive.controllerAs] = controllerInstance;
        }
      });
    }
caitp commented 10 years ago

I never said they didn't get the correct scope

Izhaki commented 10 years ago

I never suggested you did. Was just wondering what did you mean by saying:

The main issue here is that isolate scope properties are not "ready" at the time of controller instantiation...

Specifically, what did you mean by "ready"?

caitp commented 10 years ago

the custom directive is set up before ngController (the reasons why are silly, it's a combination of the priority and alphabetical order), so we set up a controller which uses properties set up in a controller which doesn't exist yet (and are thus undefined).

The properties aren't ready yet, ergo, reference error

Izhaki commented 10 years ago

I see! That makes perfect sense now!

Thanks.

magizh commented 9 years ago

duplicate of https://github.com/angular/angular.js/issues/1307