ProcessingInstruction Node fails check

[CodyReichert]

I have a ProcessingInstruction Node that gets passed to this library to check if it's a Node. However, this specific type of node is returning false - I believe it should return true.

Here's a representation of the Node I'm talking about (and MDN docs here):

ProcessingInstruction {ownerDocument: Document, target: "xml", tagName: "xml", data: "version="1.0" encoding="UTF-8"", nodeValue: "version="1.0" encoding="UTF-8"", …}
  columnNumber: 1,
  data: "version="1.0" encoding="UTF-8"",
  lineNumber: 1,
  nextSibling: Text {ownerDocument: Document, data: "↵", nodeValue: "↵", length: 1, 
  previousSibling: ProcessingInstruction, …},
  nodeValue: "version="1.0" encoding="UTF-8"",
  ownerDocument: Document {implementation: DOMImplementation, childNodes: NodeList, doctype: null, documentURI: undefined, firstChild: ProcessingInstruction, …},
  parentNode: Document {implementation: DOMImplementation, childNodes: NodeList, doctype: null, documentURI: undefined, firstChild: ProcessingInstruction, …},
  previousSibling : null,
  tagName: "xml",
  target: "xml",
  textContent: (...),
  __proto__:
    nodeType: 7,
    constructor: ƒ ProcessingInstruction(),
    textContent: (...),

Note that this type of Node has a nodeType, but not a nodeName. The actual XML would look something like this:

<?xml version="1.0" encoding="UTF-8"?>               // <-- This is the ProcessingInstruction

It fails because these types of Nodes have a nodeType property, but not a nodeName property:

function isNode (val) {
  return (!val || typeof val !== 'object')
    ? false
    // This check fails because `typeof window.Node === "function"`
    : (typeof window === 'object' && typeof window.Node === 'object')
      // Branch not taken, but would return true
      ? (val instanceof window.Node)

      // **HERE**: `nodeName` does not exist on ProcessingInstruction nodes
      : (typeof val.nodeType === 'number') &&
        (typeof val.nodeName === 'string')
}

Do you agree this should be returning True on these types of nodes? I'd be happy to submit a PR with a fix, just not sure of the best way to go about.

One options: the typeof window === 'object' && typeof window.Node === 'object' could have one more case to check || typeof window.Now === 'function'. This would return true, and val instanceof window.Node would return true as well.

Let me know what you think!

[CodyReichert]

Oh OK - MDN says that a ProcessingInstruction node has a node.target property, instead of a node.nodeName property. So, we could update the last check to check for that:

(typeof val.nodeType === 'number') &&
((typeof val.nodeName === 'string') || (typeof val.target === 'string'))

If that looks good I can submit a PR.

[CodyReichert]

Sorry for the noise, but it looks like my issue is probably coming from xmldom (which I'm using to parse a string to DOM), in that they are treating xml declarations as processing instructions: jindw/xmldom#209

is-dom is probably working correctly and I' was just looking in the wrong spot.

Let me know if you have any other info, though, closing this for now.

[TehShrike]

👍 Thanks for the notes - it wouldn't hurt to have a test for this - do you have a snippet of code to construct a ProcessingInstruction that could be used in a test?

[CodyReichert]

@TehShrike - no problem. I do, this is all it would take:

const pi = document.createProcessingInstruction('xml-stylesheet', 'href="mycss.css" type="text/css"');

I think that would pass the check in the library (but I'm not sure) - my issue seemed to have been xmldom serializing to the wrong type.