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

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

Closed
magizh opened this issue Jul 2, 2014 · 7 comments
Closed

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

magizh opened this issue Jul 2, 2014 · 7 comments

Comments

@caitp
Copy link
Contributor

caitp commented Jul 2, 2014

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)

@Narretz Narretz added this to the Backlog milestone Jul 9, 2014
@Izhaki
Copy link
Contributor

Izhaki commented Jul 9, 2014

@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
Copy link
Contributor

caitp commented Jul 9, 2014

I never said they didn't get the correct scope

@Izhaki
Copy link
Contributor

Izhaki commented Jul 9, 2014

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
Copy link
Contributor

caitp commented Jul 9, 2014

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
Copy link
Contributor

Izhaki commented Jul 9, 2014

I see! That makes perfect sense now!

Thanks.

@btford btford removed the gh: issue label Aug 20, 2014
@magizh
Copy link
Author

magizh commented Oct 17, 2014

duplicate of #1307

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants