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

require: '^something' starts searching on the current node, not the parent. #8511

Closed
Izhaki opened this issue Aug 6, 2014 · 8 comments
Closed

Comments

@Izhaki
Copy link
Contributor

Izhaki commented Aug 6, 2014

The docs

The docs say:

^ - Locate the required controller by searching the element's parents.

Code example

Now consider this DOM (plunk):

<node ng-init='name = "First Level"'>
    <node ng-init='name = "Second Level"'>
    </node>
</node>

With this directive:

.directive( 'node', function() {
    return {
        require: '^node',
        restrict: 'E',
        scope: true,

        controller: function( $scope ) {
            this.getName = function() {
                return $scope.name;
            }
        },

        link: function( scope, element, attrs, ctrl ) {
            console.log( scope.name + ' => ' + ctrl.getName() );
        }
    }
});

If the docs are correct, this code should throw an error, as the top level node doesn't have a parent node. But instead the console output is:

Second Level => Second Level 
First Level => First Level 

The Angular code

This is the Angular code (jqLite):

function jqLiteInheritedData(element, name, value) {
  // if element is the document object work with the html element instead
  // this makes $(document).scope() possible
  if(element.nodeType == 9) {
    element = element.documentElement;
  }
  var names = isArray(name) ? name : [name];

  while (element) {
    for (var i = 0, ii = names.length; i < ii; i++) {
      if ((value = jqLite.data(element, names[i])) !== undefined) return value;
    }

    // If dealing with a document fragment node with a host element, and no parent, use the host
    // element as the parent. This enables directives within a Shadow DOM or polyfilled Shadow DOM
    // to lookup parent controllers.
    element = element.parentNode || (element.nodeType === 11 && element.host);
  }
}

And you can see that the InheritedData search starts on the current element, not on the parent.

Hopefully a bug

So either the docs are wrong, or the code is wrong.

I'm not sure why the search starts on the current element, or what will be the consequences of changing it to start on the parent. But I've already been involved with 3 directives that could benefit from '^' search starting on the parent, as often you wish to have an hierarchy of the same directive.

@Izhaki Izhaki changed the title require: '^something' starts searching on the current node. require: '^something' starts searching on the current node, not the parent. Aug 6, 2014
@caitp
Copy link
Contributor

caitp commented Aug 6, 2014

This is a well known issue, and there have been pull requests for fixing this.

My view: we shouldn't change the behaviour of ^, given that it's been consistent for a long time. My fix allows you to strengthen it with ^^, which will search parents not including the current node instead.

@Izhaki
Copy link
Contributor Author

Izhaki commented Aug 6, 2014

Yeah, changing the behaviour of ^ can break others code. But I doubt anyone has used ^, got a controller from the current element, and said: "That's exactly what I wanted!".

If you search for a parent, you should have used ^, if not, you shouldn't have. And if you've used ^ which returned a controller on the current directive - that's a programmer error.

I suspect that if you take the ^^ path, a few years down the line no one will use ^.

I'd go with fixing ^, unless someone can think of a good reason to want to search on the current OR parent directives.

@Izhaki Izhaki closed this as completed Aug 6, 2014
@Izhaki
Copy link
Contributor Author

Izhaki commented Aug 15, 2014

I stand corrected - it does makes sense to introduce a ^^ syntax for parents search, and keep ^ for current & parents.

Consider a tooltip directive that can have a config attribute:

<input tooltip="test" tooltip-config="{ position: 'below', align: 'middle' }"/>

It can be useful if the tooltip-config attribute can be defined either on the same element (for a single tooltip) or on some ancestor container so all descendant tooltips have the same configuration. Like so:

<body tooltip-config="{ position: 'below', align: 'middle' }">
....
    <input tooltip="Your first name"/>
    <input tooltip="Your family name"/
....
</body>

So having ^ looking at current and parents is useful.

@Izhaki
Copy link
Contributor Author

Izhaki commented Aug 15, 2014

PR for documentation fix: #8622

@tenphi
Copy link

tenphi commented Sep 5, 2014

+1 for ^^ syntax. It can be very useful when you have nested elements that have been injected by one directive. You just can't get parent controller in this case. I had to make a workaround using scope but it's ugly solution. Should I create a new issue? Cause this seems closed.

@caitp
Copy link
Contributor

caitp commented Sep 5, 2014

Yeah you know what? lets get this resolved: @IgorMinar / @jeffbcross / @btford what do you think about landing the fix for rc1 or rc2? It's been waiting for a while (#4540)

@btford
Copy link
Contributor

btford commented Sep 5, 2014

I'm +1 on this

@jeffbcross
Copy link
Contributor

Me, too. We need @IgorMinar's blessing of the API addition (or @tbosch in his absence).

caitp pushed a commit that referenced this issue Sep 26, 2014
Implement option to strengthen require '^' operator, by adding another '^'.

When a second '^' is used, the controller will only search parent nodes for the
matching controller, and will throw or return null if not found, depending on
whether or not the requirement is optional.

Closes #4518
Closes #4540
Closes #8240
Closes #8511
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants