Remove outerHTML fallback from inspect

[shvaikalesh]

Supersedes #829. We still need to do some work with inspect (according to extremely useful #457 👍 ) though: it seems we can use Node's util.inspect for every type expect for HTMLElement. Is anyone working on loupe or inspect atm or I can help?

[shvaikalesh]

Because of Custom Elements V1, HTMLElement is reported as function in newest Chrome and Firefox.

[lucasfcosta]

What if HTMLElement is undefined?
The MDN docs aren't very clear about which browsers support this or not, there's a lot of ? on the compatibility table.

[meeber]

LGTM (and I appreciate separate commits for each distinct change being made).

[lucasfcosta]

Hi @shvaikalesh, thanks for the work! This seems awesome (as it always does on your PRs) 😄

Since the outerHTML prop is supported since FF 11 and IE 4 (according to MDN) I totally agree with that change.

Regarding the checks for the HTMLElement, I'm not sure we should totally remove that. I've taken a look at the MDN page for those and it isn't clear which browsers support it and which don't. However, if you have any more info on this I'd be happy to learn about it 😄

Also, is there any special reason for us to be using those additional checks for isArray? It has been supported for a long long time and we still have those checks, but I can't think of any special cases that makes those additional checks useful. If you guys do, please tell me, otherwise I agree with this change too.

[shvaikalesh]

@lucasfcosta HTMLElement and many other DOM interfaces were not available in IE8, but work fine in IE9 (SauceLabs).

Doing

Array.isArray(obj) || toString.call(obj) === '[object Array]'

is kinda dangerous, it gives false positive for { [Symbol.toStringTag]: 'Array' }.
Maybe it is ponyfill (for IE8) done wrong, or support for Object.create([]) (which is confusing)

[lucasfcosta]

@shvaikalesh thanks for the explanation, it looks clear to me now! So, if HTMLElement is supported on every browser after IE8 I think we can use only object instanceof HTMLElement when checking if something is a DOM element (instead of also checking if typeof HTMLElement !== 'undefined'), because as #124 says, we don't support <= IE 8. What do you think?

Also, I feel like Object.create([]) should be supported, but I'm not sure about it, I think that if someone is using the Array prototype for another object, the user expects it to be treated like an array. What do you guys think?

Also, thanks again for this work, it's looking great! I'm impressed by your knowledge about browser supports 😄

[shvaikalesh]

We need typeof HTMLElement !== 'undefined' for Node.js.

Displaying Object.create([]) as [] leads to a confusion, because former does not have internal [[DefineOwnProperty]] method, thus have no special behavior for setting indexed keys and .length. If subclass of Array is wanted, extends should be used for correct implementation.

I think util.inspect does pretty good job displaying many types (Map and Set included #662), and we should move towards using it for everything except html elements.

[keithamus]

We have type-detect for exactly this kind of stuff. If type-detect isn't doing something sufficiently, we should move the convo there.

[lucasfcosta]

@shvaikalesh ah, that makes sense!
Also, as @keithamus said, I think we could use type-detect for the array detection in order to keep the behavior consistent across all of the Chai ecosystem.

[meeber]

Based on the discussion, it looks like we just need to switch the isArray check to use type-detect instead, and then we're good to go. The HTMLElement stuff looks correct as-is with feature detection in case of non-browser environment, and instanceof which is identical to how type-detect does it.

[keithamus]

Hey @shvaikalesh. @lucasfcosta and I are clearing house on all these PRs. We have https://github.com/chaijs/chai/projects/2 which we're going to use to track some features we want, but we're going to significantly refactor all of chai over the next few weeks.

We'll be sure to come back to this. I'll drop you some messages when loupe is in a releasable state and we can make sure inspection of HTML entities works in a robust way.