isEqualNode waiting for polyfill?

[AutoSponge]

I noticed the following commented code:

    // XXX optimization: if the nodes are equal, don't morph them
    /*
    if (fromNode.isEqualNode(toNode)) {
      return fromNode;
    }
    */

This seems entirely reasonable. If this is waiting on a polyfill for <IE9, I suggest just returning false. It won't be any slower or faster in IE but it could speed up other browsers. If there's another reason, then by all means, explain :)

[patrick-steele-idem]

It's worth benchmarking but the recursive nature of isEqualNode concerns me:

Each child of A equals the child of B at the identical index.

https://dom.spec.whatwg.org/#concept-node-equals

Essentially, calling isEqualNode would do a deep comparison and then morphdom would end up repeating the comparison if there were any differences at any level in the tree. If there was a way to limit the comparison to only be shallow comparison of the top-level element node (and not compare children) then that would be nice...

[AutoSponge]

Since jsperf is down, I'll create a repo and work out an example. I have a couple of hypothesis.

I expect using isEqualNode will add more time when changes are present. I expect isEqualNode will reduce time when changes are not present. If I'm correct, this might continue to see isEqualNode not being used inside morphdom. Instead morphdom would assume you have changes and using vdom, dirty flags, or isEqualNode would all provide the same function before invoking morphdom.

UPDATE: so far, the tests show that isEqualNode doesn't take enough time to make the process slower when there's a diff but clearly speeds things up when there is no diff. So far, I only tested in chrome and the dom representations may need more permutations to be conclusive. But this seems promising.

[AutoSponge]

I tested doing this:

    function morphEl(fromEl, toEl, alreadyVisited, childrenOnly) {
        var toElKey = getNodeKey(toEl);
        if (toElKey) {
            // If an element with an ID is being morphed then it is will be in the final
            // DOM so clear it out of the saved elements collection
            delete savedEls[toElKey];
        }
// NEW CODE HERE
        if (fromEl.isEqualNode(toEl)) {
            return;
        }

This greatly speeds up the benchmarks (basically smashing the previous todoMVC scores). However, the tests fail because onBeforeElUpdated and onElUpdated were not called for the fromEl AND all of its children. My question is, if the node wasn't discarded (it found a match) but wasn't morphed (because it didn't need to be) should it emit?

[patrick-steele-idem]

@AutoSponge I know of at least one use case (Marko Widgets) where is a possibility, albeit a very, very small possibility, that not emitting onBeforeElUpdated and onElUpdated for all elements could cause problems. morphdom already provides the hooks to take advantage of isEqualNode so maybe we just keep that outside the library:

morphdom(fromNode, toNode, {
    onBeforeElUpdated: function(fromEl, toEl) {
        if (fromEl.isEqualNode(toEl)) {
           return false; // Skip this entire sub-tree
       }
    }
});

Does that work for you?

I'm using morphdom with Marko Widgets and Marko Widgets is already using an optimization to compare the widget state associated with a DOM node to completely skip updating parts of the DOM associated with a widget whose state did not change. For Marko Widgets, enabling isEqualNode would likely slow things down with no performance advantage in most situations.

[AutoSponge]

@patrick-steele-idem I'm fine with that. Thanks for the example.