Unpredictable results when scope.$id is less than the length of your expandedNodesMap in the treeTransclude directive

[mjohnson1122]

So I have a rather large tree (thousands of nodes) which defaults to expanding all branches. My expandedNodesMap has over 500 nodes in it. If I'm using this relatively early in the lifetime of my app, the scope.$id in the treeTransclude directive is less than the length of my expandedNodesMap.

Thus, when I get to the following code in that directive:

angular.forEach(scope.expandedNodesMap, function (node, id)  {
    if (scope.options.equality(node, scope.node)) {
        scope.expandedNodesMap[scope.$id] = scope.node;
        scope.expandedNodesMap[id] = undefined;
    }
});

it completely overwrites an existing node in the map because scope.$id is < length of the map. This causes some very unpredictable results.

[yoavaa]

1. what is a "length of a map"? a map has a size, not length.
2. $id are not pure numbers, rather alpha nomeric. Scope $id are always going up, generated by the ng-repeat directive. How can you get the same $id value for different scopes?

[mjohnson1122]

1. Size is right, my bad.
2. It's not the same $id value for different scopes. It's an $id value of an already existing key of the expandedNodesMap.. My apologies... I'm not explaining it well. Here's what I gather from looking at the tree code. Taking a smaller example, if I had an expandedNodesMap as follows with original size 3 with keys 1-3 pointing to nodes 1-3.

Now, after the tree transclude directive runs perhaps my expandedNodesMap looks as follows:
1 undefined
2 undefined
3 undefined
8 node1
12 node2
15 node3

so now, based on the code, I can see that the scope $ids of my nodes are 8, 12, & 15. No problem there.

Now imagine my original expandedNodesMap had size 20 with keys 1-20 pointing to nodes 1-20 respectively. What would happen in the treeTransclude directive, if node 1 had a generated scope $id of 8? Now instead of marking key 1 undefined and creating a new item with key of 8, you would instead be overwriting the existing node 8 with node 1. Does that make sense?

So key1 would be marked undefined but instead of creating a new node 1 with key 8 you would be overwriting the existing node 8 with node 1. Does that explain it any better?

[kaape]

I've experienced (probably) the same behaviour with big trees (>500 nodes) as @mjohnson1122 describes. When the whole tree is expanded, by inserting every node in the expanded-nodes array, the result isn't as expected. Sometimes the whole tree is expanded and sometimes only parts of it. Sometimes the root node level isn't expanded at all (but the child nodes are, if we expand the root node by clicking it).

To reproduce this issue I've created a plunkr with trees of various size:
https://plnkr.co/edit/jKXECgeNQsnL3UN5oA6Z?p=preview

You should see the effect in the tree with size [1,5,10,15]. The first expand result differs from further ones. Try clicking the buttons "Expand nodes" and "Collapse nodes" multiple times.

Hopefully this will help to debug the issue.

[mjohnson1122]

Yes, that's it exactly. We experienced the same behavior. Our trees, which should have been fully expanded by default, weren't displaying as such. Some nodes would be expanded, some wouldn't. And trying to manually expand or collapse particular nodes, would also result in some weird behavior. In one example, trying to expand a node that should have already been expanded resulted in a completely different set of nodes expanding and then collapsing before the node I clicked on finally expanded. Other times, trying to expand a node, resulted in nothing happening at all. All of this stems from when a scope.$id for a particular node is the same as an existing key in the expandedNodesMap. When the expandedNodesMap is first populated, the key to each node is the index of a for loop. So when the map is modified to use the scope.$id as the key, if the scope.$id matches one of the original for loop indices, you will encounter these types of problems.

[mjohnson1122]

Actually, in looking at the pull requests out there, it looks as if #124 would address this very issue.

[yoavaa]

@kaape, I have checked your plunkr. The issue there is caused by the data having multiple nodes with the same index and the quality function comparing nodes only based on index.

In the second tree, the nodes

1 -> 11 (first child)    -> 121 (11th child)

and

1 -> 12 (second child)   -> 121 (first child)

have the same index numbers.

[kaape]

@yoavaa Sorry, my bad. I've updated the plunkr to prevent duplicate node indexes. Although the behaviour is slightly different, the issue is still occurring. Not all nodes are expanded on first try (at least for the third tree).

[yoavaa]

try now with the latest pushed version of the library. I cannot reproduce in tests but I think I have implemented a fix that will solve this issue. Works for me in plunkr

[mjohnson1122]

Yes, this fixes the problem. Thanks,

[th0r]

Guys, could you publish this fix to npm please?

[th0r]

@yoavaa ping

[yoavaa]

done

[th0r]

done

Sorry, but can't see it in npm - last version is still 0.2.26.

[yoavaa]

now should work.

On Wed, Jul 6, 2016 at 2:39 PM Yuriy Grunin notifications@github.com
wrote:

done

Sorry, but I can't see it in npm - last version is still 0.2.26.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#210 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABDlJ_8VnF2c2j9M-V3cWftCnpLUJTa8ks5qS5PsgaJpZM4IIqfI
.