Some exclusions are incorrectly disabled when removingDuplicates

[edwingamboa]

When removing a duplicated node, the DependencyTree class checks for invalid exclusions. In the _removeDuplicate method, it is expected that the duplicated nodes have the same children in the same order.

However, we have detected a case in which the same nodes have different subtrees. To reproduce this behaviour, this CRCInit config, can be used. One problem wiht this, is that some exclusions are disabled since the "excluded" property of two different nodes can be compared assuming that the nodes are the same, if this propery is different in both nodes, the exclusion of one of the nodes will be disabled.

I modify the code of the _removeDuplicate method (in chrome devtools) to log the exact differences in the console (it would apply from line 134):

// calculate the intersection of all excluded nodes. Only nodes that are excluded in both subtrees are removed
      descendantsOfDuplicated.forEach(function (node, idx) {
        var duplicate = descendantsOfDuplicate[idx];

        // the exclude on duplicate node is invalid because the same node is not marked as excluded in duplicated subtree
        if (!node.excluded && duplicate.excluded) {
        console.log('\n\nduplicated----------', duplicated.data.webpackageId, duplicated.data.artifactId,
          'duplicate---------', duplicateCopy.data.webpackageId, duplicateCopy.data.artifactId);

          console.log('descs of duplicated----------')
          descendantsOfDuplicated.forEach(function (node) {
            console.log(node.data.webpackageId, node.data.artifactId)
          })
          console.log('descs of duplicate----------')
          descendantsOfDuplicate.forEach(function (node) {
            console.log(node.data.webpackageId, node.data.artifactId)
            duplicate.excluded = false;
        })
        }

        // the exclude on duplicated node is invalid because the same node is not marked as excluded in duplicate subtree
        if (node.excluded && !duplicate.excluded) {
        console.log('\n\nduplicated----------', duplicated.data.webpackageId, duplicated.data.artifactId,
          'duplicate---------', duplicateCopy.data.webpackageId, duplicateCopy.data.artifactId);

          console.log('descs of duplicated----------')
          descendantsOfDuplicated.forEach(function (node) {
            console.log(node.data.webpackageId, node.data.artifactId)
          })
          console.log('descs of duplicate----------')
          descendantsOfDuplicate.forEach(function (node) {
            console.log(node.data.webpackageId, node.data.artifactId)
          })
        node.excluded = false;
      }

[edwingamboa]

In cubx-dependency-resolver the issue was solved by traversing all nodes of a duplicate node since the sub-trees of two duplicated nodes can be different due to conflict resolution removals (See this). However, this means traversing node trees more times than before. A better solution would be great.

[pwrinc]

I think because we need to calculate the intersection of all descendants of the duplicates node and all descendants there is no better solution then traversing both (sub)trees and compare them against each other. So I would just migrate you solution from cubx-dependency-resolver to RTE