[New] compare .toString, .valueOf, and [Symbol.toPrimitive] completions

[arasmussen]

If val or other are objects:

• Compares val.toString() and other.toString()
• Compares val.valueOf() and other.valueOf()
• Uses es-to-primitive to perform the above checks, with two separate hints (String and Number)
• Also considers that toString and valueOf can throw, and makes sure both or neither arguments throw

Test plan:

• Adds test coverage for all of the above
• tests + lint pass, test coverage does not regress

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
es-to-primitive	1.2.1	None	+0	40.4 kB	ljharb

[arasmussen]

@ljharb hmm how do you want to proceed here? If we're adding toString comparisons then technically this test should be flipped. Alternatively, if you want to maintain backwards compatibility, we could skip the toString comparison if we detect typeof val === 'function'?

> const foo = Object(function(){});
undefined
> const bar = Object(function () {});
undefined
> foo.toString()
'function(){}'
> bar.toString()
'function () {}'

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Files	Coverage Δ
why.js	99.54% <100.00%> (+0.08%)	⬆️

📢 Thoughts on this report? Let us know!.

[arasmussen]

@ljharb updated PR per your feedback. pretty sure the one test failure is flaky but don't think I'm able to re-run.

[arasmussen]

@ljharb thanks for the feedback! ready for another pass, hopefully last one 😄

[ljharb]

why this line? surely either a function or an object could both have toStrings or valueOfs that need comparing?

[arasmussen]

otherwise functions with same arity/name/body are equal despite whitespace between signature and body will fail because:

> const foo = Object(function(){});
undefined
> const bar = Object(function () {});
undefined
> foo.toString()
'function(){}'
> bar.toString()
'function () {}'

our options are to include this line, or delete that test. including this line is more backwards compatible.

[ljharb]

aha, hmm. that means this new functionality wouldn't apply to functions, which is weird. at the least, let's document that in the readme?

an alternative, tho, would be to skip the check for a function whenever the function's toString is === to Function.prototype.toString?

[ljharb]

Looks great overall; I've rebased and squashed it.