Standardize DOM node type checking

[gibson042]

trafficstars

Description

Our style guide says that we identify DOM elements with (Boolean) object.nodeType, but that's not exactly true. We actually use it to identify DOM nodes, but even setting aside semantics, there is a case where an object with a truthy nodeType property can avoid the DOM node path: setting data.

This means that .data can succeed or fail based on some rather trivial differences, which makes me uncomfortable. We should have a single "is DOM node" test, applied consistently everywhere. nodeType will always have certain characteristics on real DOM nodes (existence, "number" type, integer, positive), but we can use any subset thereof—I'm leaning towards typeof obj.nodeType === "number" or obj && "nodeType" in obj, because any given kind of input seems unlikely to change classification. And whatever we pick, the style guide will also need an update.

A macro would be really nice, because it seems silly not to inline such trivial code, but I think we can continue to live without something like sweet.js for now.

Link to test case

https://jsbin.com/kafudoseno/edit?html,js,console

[timmywil]

This seems like bikeshedding to me. I've never had the need to have a nodeType property on an object unless I was mocking something in tests. That said, if the change is small, I don't mind.

[tumulr]

If we just only check for the nodeType to be number, it will allow all the other types of nodes too right(Attribute node -> nodeType = 2). Will that not cause an issue?

@gibson042 What are your thoughts on it?

[gibson042]

I'm not talking about element tests (which are always nodeType === 1), but about node tests, for which we have no standard (and in practice use boolean-cast nodeType almost exclusively, the main exception being acceptData).