search

inspect-js / object-inspect

string representations of objects in node and the browser
MIT License
142 stars 36 forks source link

Test whether AssertionError is deemed an Error #12

Closed mk-pmb closed 7 years ago

mk-pmb commented 7 years ago

My fix in inspectobj-pmb wil be to delegate error identification to the is-error package so I can maintain it in one central place.

PS: You'll probably want to add spaces in ln. 40 and 44. I've just deleted the lines, easier for my rebase later.

ljharb commented 7 years ago

fwiw it's fine for newly added lines to not have trailing whitespace; I always remove trailing whitespace on lines I've touched for other reasons. The preferred style isn't "consistent", it's "don't make whitespace-only changes".

mk-pmb commented 7 years ago

No problem, PR was just meant to make adding easier in case you want a test for this. You can always roll your own of course, or just not test it.

ljharb commented 7 years ago

AssertionErrors are already instanceof Errors; it's part of node core. Thus, it's node's job to test it, not object-inspect's.

As I pointed out above, a reliable "is error" module is impossible in JavaScript at the moment, pending https://github.com/ljharb/proposal-error-stacks

mk-pmb commented 7 years ago

a reliable "is error" module is impossible in JavaScript at the moment,

Yeah, it's a pity we can't have the perfect thing (yet). I prefer to use the best available thing in the meantime, and hope that community effort can collect edge case fixes to work to get it even closer to perfect.

mk-pmb commented 7 years ago

Your proposal looks good though! :+1: Thanks for telling me, I'll subscribe it so I can use it in is-error asap.