_.isEqual for DOM elements always returns true #945

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@rudasn

DOM element comparisons always return true as the result variable is never changed to false (in eq method).

_.isEqual(document.createElement('div'), document.createElement('div')); // => true

Added a simple test on the eq method to check for the equality of two DOM elements.

@braddunbar
Collaborator

Hi @rudasn! Thanks for the patch. It seems like a rather weak test though as it means objects with a nodeName property could return false negatives. Why not just use el1 === el2?

@rudasn

How about using _.isElement instead? It returns !!(obj && obj.nodeType === 1).

I didn't want to use a === b as I thought there was a good reason such a test wasn't included in the first place.

@braddunbar
Collaborator

Hmm...I don't quite understand. What's your use case for comparing DOM elements?

@rudasn

It's not that I want to compare DOM elements directly, but I want to be able to do something like this and get a correct response:

var x = {
        'a': document.createElement('div'),
        'b': 'B property'
    },
    y = {
        'a': document.createElement('div'),
        'b': 'B property'
    }

_.isEqual(x, y); // true

Apparently that should be false.

@braddunbar
Collaborator

Yep, this is the expected behavior. _.isEqual is for a deep comparison of two objects and two newly created DOM elements have matching properties. If you change something about one element, then the comparison would be false.

var a = document.createElement('p');
a.id = _.uniqueId('el');
var b = document.createElement('p');
b.id = _.uniqueId('el');
_.isEqual(a, b); // false

If you want identity comparison you should use === instead of _.isEqual or add a differing property to the elements.

@braddunbar braddunbar closed this Jan 17, 2013
@jdalton

Inspecting nodes is a trap. In IE8 _.isEqual(document.createElement('div'), document.createElement('div')) will throw an error. Inspection of nodes is tricky and costly (lots of properties and child nodes to iterate over), which is why Lo-Dash detects nodes (cross-browser) and just === compares them (no crawling).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment