Chai: respondTo throws TypeError on arrow functions

[shvaikalesh]

// index.js
const fn = () => {}
fn.hello = () => console.log("hello, world!")

// test.js
fn.should.respondTo("hello") // doesn't work, but I feel like it should
fn.should.itself.respondTo("hello") // kinda workaround

[keithamus]

Well, I think we're doing the right thing here. A new fn does not have .__proto__.hello, so should.respondTo('hello') _should_ fail. We could provide extra information in the error message to allude to this, but fn.hello is firmly a "static method" and so the behaviour feels correct.

To carry the code on from where you are...

// index.js
const fn = () => {}
fn.prototype.hello = () => console.log("hello, world!") // notice .prototype

// test.js
fn.should.respondTo("hello") // this will now work
fn.should.itself.respondTo("hello") // this wont

[shvaikalesh]

Also, (() => {}).should.respondTo('hello') fails with TypeError, because arrows lack .prototype.

@keithamus "static method" point makes perfect sense, thanks!

[keithamus]

Also, (() => {}).should.respondTo('hello') fails with TypeError

This is the kind of think we should really catch and provide useful info. I feel like we need to have some kind of audit of usability in these methods, to make sure we don't expose underlying system errors and instead provide helpful descriptions of what went wrong.

[meeber]

I'm in favor of Chai detecting problems and providing descriptive error messages. In this case, it could be a documented prerequisite that target must be a function with a .prototype property, at least when the itself flag isn't set. If that prerequisite isn't met, then an AssertionError with a descriptive message is thrown by Chai; the AssertionError should respect the user's stacktrace settings and any custom message they defined for the assertion. Note that prerequisites such as these must always be met, even if the assertion is negated.

However, as noted here, we shouldn't use try/catch to catch and re-throw an error with a friendly message if there's any chance that the block of code being wrapped in try will execute code written by the user; errors generated by the user's code should retain the original error object, message, and stack trace.

[keithamus]

Agreed, but minor nitpick: target is either a function with a .prototype _or_ an object with a .__proto__ (I suppose Object.create(null) may also fail here?).

[meeber]

Ah yeah non-function objects. Looks like the current implementation of respondTo allows the method to be owned or inherited when the target is an object. This was one of the trickier ones to document in #920 due to divergent behavior based on the target's type, my least favorite type of assertion.

[shvaikalesh]

The current code has an assumption that typeof == 'function' detects constructors, which was kinda OK in ES5, but is kinda weird now. We have bound functions, methods, arrows, built-in methods, some built-in constructors (hey, Proxy) that have no .prototype. I still think that itself + arrows look weird, and assertion should work with .prototype-less functions as it does with plain objects.

We use itself when we have to point Chai to static methods and ignore instance methods, but mentioned functions can't possible have instance methods.

[meeber]

As a library maintainer, and as a developer who in general wants to avoid assertions that have complex and/or divergent behavior based on some non-flag factor, such as the type of the target, my preference would be for .respondTo to have consistent behavior regardless of whether the target is a function declaration, an arrow function, or a non-function object.

However, I don't personally use .respondTo (and didn't even know it existed until I was working on #920), so I don't have a bunch of insight into what the expectation of the average user is when using this assertion. What do people naturally think .respondTo does if they haven't read the docs? Do they expect this kind of divergent behavior based on target type? I'd be interested in hearing the point of view of actual users of this assertion.

[kenaniah]

For what it's worth (and from someone with a background in Ruby), my natural inclination is that both assertions in the original example should work. I don't see this as a problem given that explicit tests for static methods are still available via itself.

My humble take on the principal of least surprise here is that respondTo should always work if the method name is indeed callable, regardless of context.

[keithamus]

I think we'll deprecate respondTo, and probably reinvestigate a better way to do this. We have marked this on our roadmap, so we'll close this for now.