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

Angular lacks support for recursive directives #8536

Closed
robstolarz opened this issue Aug 8, 2014 · 36 comments
Closed

Angular lacks support for recursive directives #8536

robstolarz opened this issue Aug 8, 2014 · 36 comments

Comments

@robstolarz
Copy link

This is a pretty major issue in terms of being able to do things "the Angular way."
My model is a tree. Trees require recursion. I'm making an app that depends on a directive that allows movement of nodes inside a tree. In order to be able to move nodes inside this tree, I need to place "drop zones" which will detect elements that are dropped onto them. 1
Unfortunately, with the "Angular way" of implementing this (2 is a JSFiddle that uses similar concepts; recursive directive with transcluded template for each node of the tree) the transcluded template is rendered totally unaware of where it is located relative to where its data comes from, because the transcluded template has to be removed from its parent element before compilation.
If it's removed, it loses its scope; if it's not removed, 3 happens. (Open a console, you'll see the infinite recursion. It's the same as 2 but with the compile function commented out.) Angular tries to compile the directive while it's already compiling the directive, which fails hopelessly.

Now, there are workarounds, which is why it is possible that angular-ui-tree 4 exists. However, the only way to use it is through a recursive template (5 is a good starting point) which misses out on all the cool directive support and parametrization and is generally not the intended use of templates. The implementation here becomes hairier than it needs to be because of the lack of support for recursive directives.

@Izhaki
Copy link
Contributor

Izhaki commented Aug 8, 2014

I've been writing tree directives in angular lately and you can pretty much do whatever you want, given you understand Angular well enough.

Anyhow, if I understand your issues correctly, I believe the following blog (Building Nested Recursive Directives in Angular) is demonstrating how to do exactly what you want. Make sure you also read the comments.

@caitp
Copy link
Contributor

caitp commented Aug 8, 2014

this is a valid issue, recursive templates are not really possible in angular without hacks currently. It's not that the hacks aren't possible, but they really, really suck, both in terms of code smell and performance, and should not be necessary.

Unfortunately, it's not a super simple problem to solve on top of the current compiler.

@robstolarz
Copy link
Author

@Izhaki scope is not respected by the transcluded HTML in link 2 in my original post, which is the real thorn in my side... and I adamantly refuse to put my entire view template inside my tree directive :P

@caitp At least I know I'm not crazy... perhaps the compiler should check the current stack and see if it's recursively compiling a directive that it is already trying to compile? Then it would defer compilation..?

@caitp
Copy link
Contributor

caitp commented Aug 8, 2014

@caitp At least I know I'm not crazy... perhaps the compiler should check the current stack and see if it's recursively compiling a directive that it is already trying to compile? Then it would defer compilation..?

It's not quite that simple --- if we defer compilation, we need to have machinery for doing the compilation later, and it's not clear what would trigger that.

It could be element transclusion, since the most common use case for this would be ng-if or ng-repeat, but it's not a trivial problem on top of the current compiler. If this gets fixed, we need to fix it in a way that doesn't break the status quo of angular apps. It's complicated =(

@robstolarz
Copy link
Author

@caitp what about adding a new rule to the compiler, like ng-recursive or something? Just tossing ideas out here.

@petebacondarwin
Copy link
Contributor

@callmeStriking - I believe that your scope issue may be resolved in a later version of Angular - your link is to a fiddle that is using 1.2.0-rc2. We did some work recently on tightening up what scopes are passed to nested transclude directives.

@robstolarz
Copy link
Author

@petebacondarwin Close, but no cigar. The failing case (3, on Plnkr) uses Angular 1.3.0-beta.5
Edit: unless there's a newer Angular than that and I've misunderstood something

Edit 2: Tried again. 1 After #, the local index of the element should be shown. Instead, I got nothin'. (Still 1.3.0-beta.5. Inside the link part of the compile function, I do have access to the scope but the elements will never be aware of that). Even if adjusting the scope did work, it still wouldn't fix the fact that this is a hack...

@jeme
Copy link

jeme commented Aug 11, 2014

For reference: https://github.com/dotJEM/angular-tree

I know that that solution uses two directives, but that brings some advantages with it, and when we have accepted that, then what are we really missing?...

@robstolarz
Copy link
Author

@jeme What advantages?

@jeme
Copy link

jeme commented Aug 11, 2014

@callmeStriking In this case it's a common directive for all recursion, having the two seperates directives means you can embed recursions in recursions of different types.

Not that that may be a very common usecase, regardless of that I think the Syntax is elegant enough to be acceptable.

Obivously you can do that if you go and make a new recursive directive again, and again, and again with different names... but why should we do that if we can make one simple solution all can just use in their directive templates?

@robstolarz
Copy link
Author

@jeme Why not be able to create your own, native recursion? Also, does your solution mangle the scope like above ones do?

@Izhaki
Copy link
Contributor

Izhaki commented Aug 11, 2014

@callmeStriking

Although this has nothing to do with Angular, I feel obliged to share my experience and suggest you are approaching this problem with the wrong strategy.

Indeed, most drag-and-drop (DnD) frameworks use 'drop zones', but if you really come to think of what should be the correct abstraction for the problem, you should distinguish between:

  • Drop-in: that's the 'traditional' drop zone, where the drop target is a container. Users will drop into that container.
  • Drop-in-between: where the user wishes to drop between two containers.

A tree (or a table) DnD is a classic drop-in-between. As such, instead of throwing more drop zones than tree nodes, you should define the whole tree as a single drop zone, and work out the 'above' and 'below' containers of the drop, then work out the legal drop positions based on that.

Lastly, do remember that users may want to drop below the last node, where, in theory at least, there can be as many drop positions as the levels of the last node (so if your illustration would only go as low as item E, users can drop below A, C and E). Implementing this type of drop will be substantially easier if the whole tree is the drop zone.

@robstolarz
Copy link
Author

@Izhaki That makes quite a bit of sense, but: drop-in-between, in this case, can't be done using existing hacks because using them mangles the scope.

@Izhaki
Copy link
Contributor

Izhaki commented Aug 11, 2014

Is this plunk of any help?

It uses the contents of the tree directive as a template:

<tree ng-init="nodes = children">
  <ul>
    <li ng-repeat="node in nodes">{{node.label}}
        <children ng-if="node.children" ng-init="nodes = node.children"></children>
    </li>
  </ul>
</tree>

With the directives simply being:

.directive( 'tree', function() {

  return {
    restrict: 'E',

    compile: function( tElement ) {
      tElement.template = tElement.html();
      return function() {};
    },

    controller: function( $scope, $element ) {
      this.getTemplate = function() {
        return $element.template;
      }
    },
  }
})

.directive( 'children', ['$compile', function( $compile ) {
  return {
    require: '^tree',
    restrict: 'E',

    link: function( scope, element, attrs, treeCtrl ) {
      var iTemplate = treeCtrl.getTemplate();
      element.html( iTemplate );
      $compile( element.contents() )( scope );      
    }
  }
}])

@jeme
Copy link

jeme commented Aug 12, 2014

@callmeStriking I am not saying the compiler couldn't do it, but if it adds allot of complexity, then I think it's worth looking to other solutions first... And why not solve this with a directive we can use on par with e.g. ng-repeat if we can?... (I bet you never thought "why can't the compiler repeat my directive")...

The solution doesn't mangle the scope, each node (defined by start-with or connect) will create a child scope. And then it will put some values on that (much like ng-repeat does it)...

On a further ittiration of things, we/I might add the ability to "name" the main value like in ng-repeat, something like dx-start-with="x as y"

It basically does the same thing as recursive templates, or close to what @Izhaki's example does.

I am sure there are room for improvements... But I haven't found a tree structure I can't render with it yet.

Edit: Just adding a sample use directly here for previously linked module:https://github.com/dotJEM/angular-tree


<ul dx-start-with="rootNode">
  <li ng-repeat="node in $dxPrior.nodes">
    {{ node.name }}
    <ul dx-connect="node" />
  </li>
</ul>

@btford btford added this to the Backlog milestone Aug 18, 2014
@Jon889
Copy link

Jon889 commented Sep 13, 2014

I have a directive called card it needs to display sub cards, so in the template file I just have <card ng-repeat="subcard in subcards" carddata="subcard"></card> and it gets stuck in an infinite loop, despite the sub card not having any sub cards itself. It's semi understandable that this causes problems because it's feasible the element directive should be done before any attribute directives. However with this: <div ng-repeat="subcard in subcards"><card carddata="subcard></card></div> angular should not even touch the card directive if subcards.length === 0

@robstolarz
Copy link
Author

Angular has to compile the directives by traversing the tree fully. It doesn't matter if there aren't any objects because the compilation doesn't check the number of objects, it's filled in on render. The unfortunate thing about this is, recursive objects can't be compiled, and that object is recursive no matter what. There's no way to make elements render like this out-of-the-box (or without compile function hacks).

@Jon889
Copy link

Jon889 commented Sep 14, 2014

Why does it compile directives by traversing the tree fully then? Why not compile it on demand, which I assume what the various hacks/workarounds do. It seems a bit of waste to be compiling something that possibly isn't needed. Or adding an option you can set in the directive to have to it compiled lazily (effectively building the hacks into angular)

@robstolarz
Copy link
Author

@Jon889 It's more efficient for usecases that support it. Recompiling everything on-demand could cause code complexity, and would likely be slower. The hacks do indeed compile on-demand... although they work, they cause code smell and really just aren't "the Angular way."
I really hope this feature lands in Angular 2.0, or something. Arrays aren't the only data structure.

@caitp
Copy link
Contributor

caitp commented Sep 14, 2014

It's not that there is "no way" to do it, but it requires some refactoring, and is complicated (nobody likes adding complicated stuff to the compiler)

@jeme
Copy link

jeme commented Sep 16, 2014

@callmeStriking I won't consider the concept @Izhaki posts, nor will I consider https://github.com/dotJEM/angular-tree a hack... Doing that means that ng-include is a hack IMO...

angular-tree pretty much just does the same as:

<script type="text/ng-template" id="node_template.html">
    {{node.label}} 
    <ul><li ng-repeat="node in node.nodes" ng-include = "'node_template.html'" ></li></ul>
</script>
<div ng-include="'node_template.html'"></div>

But in a single go (dx-start-with first functions as the template collector, then as a regular node)
Obviously this means lazy evaluation, which blows the compilers optimizations.

I wondered if there was a way to use transclusion (like ng-repeat does it) or other means of optimizations where the template is already compiled, but just requires linking down the tree.

But I have not been able to implement it in such a way yet.

@Jon889
Copy link

Jon889 commented Sep 16, 2014

With the directive ng-repeat the repeated element is compiled only once and then linked for each repetition, so why isn't each directive compiled only once across the entire page? And then linked for each instance?

@nhhockeyplayer
Copy link

Folks, is there a consensus on this ? Whats appropriate to commit to. Whats available for commercial ? Specifically for implementing a fast super node/branched tree that will support different KINDS of nodes and branches and click drag drop open crud... etc... thank you

@robstolarz
Copy link
Author

Angular 1 will not have native support for recursion. Use any of the previously mentioned solutions to do it, or use a framework more suitable for your project.

On Mar 20, 2015, at 11:15 AM, nhhockeyplayer notifications@github.com wrote:

Folks, is there a consensus on this ? Whats appropriate to commit to. Whats available for commercial ?


Reply to this email directly or view it on GitHub.

@nhhockeyplayer
Copy link

Well I have implemented several trees in the past recursive ones. Doesn't what this gentleman is doing jar anyone's mind as to the possibilities here ?

https://www.youtube.com/watch?v=vJqJ-0U74IY

I am thinking not outside of the box (angular) but thinking deeper inside the box (javascript).
Whats wrong with taking what angular offers and souping it up with some snazzy javascript in between the seams. ?

??

I havent seen anything yet that is causing me to jump up and start doing back flips.

I did a struts-tiles tree years back that I recall qualifies similarly structurally to whats available this moment in angular and javascript that would provide a sound commercial grade load and erect tree-widget based on some json structure and could be implemented to do-all...click, click-open, click-drag, click-close... on nodes and branches. And add/delete too.

I am still looking to do it myself but wondering if anyone has done this already.

@robstolarz
Copy link
Author

Few people have done anything of merit that you can use commercially. Good luck with your custom solution, hope you open-source it.

@schmod
Copy link
Contributor

schmod commented Jul 8, 2015

Is there any solution on the roadmap for this? It's pretty glaring that this is still an issue.

@lgalfaso
Copy link
Contributor

lgalfaso commented Jul 8, 2015

To some extend, this will be solved once #12078 lands, and there is a reasonable chance that it will make it for 1.5. Given that this is a breaking change, it is not possible for it to land before.

@schmod
Copy link
Contributor

schmod commented Jul 8, 2015

That's fantastic, and I doubt that there's a solution to this problem that wouldn't involve a breaking change. This issue has been a thorn in my side for way too long -- angular's supposed to be good at building things like tree-views.

@jeme
Copy link

jeme commented Jul 9, 2015

Just to add an update on this from the https://github.com/dotJEM/angular-tree perspective.

Which have been moved to use Transclude instead of compile, which speeds it up a high degree.
So this no longer compiles on demand.

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

@schmod
Copy link
Contributor

schmod commented May 2, 2016

So, does Angular 1.5 now provide a reasonable path for creating trees without using $compile?

My intuition seems to suggest that this is possible, but I'm not quite sure what the recommended path for doing it would be.

@lgalfaso
Copy link
Contributor

lgalfaso commented May 2, 2016

@schmod it depends on the specific case, but most simple solutions work. Eg if the recursive reference in the directive template is inside an ngIf or ngSwitch that controls if the recursion should happen, then that is enough.

@lgalfaso lgalfaso closed this as completed May 2, 2016
@jeme
Copy link

jeme commented May 3, 2016

@schmod that depends on preference, but I would think the following example is reasonable and very simple? https://plnkr.co/edit/5Qv1pN4ISbRxvGnsUVgN?p=preview

app.controller('appController', function() {

  this.data = { 
    name: 'TEST',
    nodes: [
      { name: 'Child1', 
        nodes: [
          { name: 'GrandChild',
            nodes: [
              { name: 'GrandGrandChild' }
            ]

          }
        ]
      },
      { name: 'Child1', 
        nodes: [
          { name: 'GrandChild' }
        ]
      },
      { name: 'Child1', 
        nodes: [
          { name: 'GrandChild' }
        ]
      }
    ]
  };
});

app.directive('recurseMe', function() {
  return {
    scope: { 
      tree: '='
    },
    template: '<ul>'
            + '<li ng-repeat="node in tree.nodes">'
            + 'NAME: {{ node.name }}'
            + '<recurse-me tree="node" />'
            + '</li>'
            + '<ul>'
  }
});

@petebacondarwin
Copy link
Contributor

@jeme To get this sort of thing to work we must squeeze some asynchronicity into the template. Some people have used ngInclude to inject the child node template; others, as mentioned by @lgalfaso, use ngIf, etc; still others have actually added an explicit $timeout(..., 0) to get the same result.

The key is that we need the child nodes to be rendered in a subsequent digest and not the current one.
The way it is working in your example the recurseMe directive is being rendered recursively synchronously in the same digest - this causes a new digest loop (or possibly multiple digest loops) for each recursion. Eventually that will hit the $scope TTL limit.

@lgalfaso
Copy link
Contributor

lgalfaso commented May 7, 2016

@petebacondarwin the example that @jeme posted works well as the recursion is inside an ngRepeat. Am I missing something else from your answer?

@petebacondarwin
Copy link
Contributor

Sorry I meant to reply to @school not @jeme. Yes, ngRepeat also provides this asynchronicity

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

10 participants