Chai: ok() is not a function

[slavafomin]

Hello!

Thank you for this great library!

I'm using a construct like this: expect(value).to.not.be.ok(), however, I'm getting an error cause ok is not a function.

When I use it like this: expect(value).to.not.be.ok, it works fine, but raises an error in my IDE, cause this construct is not a call and is not assigned to anything, i.e. redundant construct.

I think it makes sense to make such constructs at least look like a normal function call like in my firxt example: expect(value).to.not.be.ok().

What do you think? Thanks!

[meeber]

@slavafomin Thanks for the post! This has been kicked around before and was actually a feature in Chai 1.x but was later reverted due to issues. You can read about it here: https://github.com/chaijs/chai/issues/371

There is a solution for you though! Please check out the dirty-chai plugin :D

[slavafomin]

Thanks for the hint @meeber! I will definitely use dirty-chai then.

However, the idea to settle for property style definitions instead of function calls looks very weird to me, because it:

1. Throws errors in IDE
2. Throws errors when used with linters
3. It's illogical from the programming point of view, because you are accessing some property, but not doing anything with it's value. Actually it's a redundant operation, that should not cause any side-effects — very counter-intuitive!

[lucasfcosta]

Hi, @slavafomin thanks for your feedback! :smile:

I totally understand your concern, however if we needed to call methods instead of using getters we would need to use () after each language chain too.
IMO this is an issue related with the IDE, because it should take into account that JavaScript can set the getter functions used to retrieve properties, although it's not common, it's still possible. When it comes to Linters I think the same, the difference is the linters usually accept more config params so you can turn on/off the rules which you want. Maybe your IDE has some options to turn on/off this kind of notifications that can help you too. However I do understand and I totally agree with you that it would be good to support true() too so everyone gets happy and you won't get errors on your Linters/IDEs anymore.

I also took a look at #371 and I'm wondering: why can't we support both assertion styles? Both addProperty and addMethod return a new Assertion, so it would be just a matter of adding the same function both as a method and as a property (as the getter for it, in this case). Please correct me if I'm wrong, but this may solve the problem @slavafomin has just described. What do you guys think?

[meeber]

I think it's worth looking into again. Just to make sure we don't duplicate past efforts, here is a quote from @keithamus in the #371 thread that contains a ton of background information related to the original fix to this issue and the reason it got reverted:

• #41 was the initial proposal to add method syntax (.to.be.true()).
• #297 was the PR that was merged by me.
• #302 is where the original discovery that the two styles were incompatible with each other was.
• #306 is where we reverted the change.
• #321 and #326 are duplicates of this issue.

[meeber]

I just finished reviewing the history on this issue and it doesn't look good for supporting both syntax without introducing breaking changes, at least not with the previous approach of having the getter return a noop.

@lucasfcosta, would you mind elaborating on your idea:

Both addProperty and addMethod return a new Assertion, so it would be just a matter of adding the same function both as a method and as a property (as the getter for it, in this case).

[lucasfcosta]

Well, turns out I was wrong.

Unfortunately it's not possible to have both accessors and values for the same property.
I've just did some experimentation and research on it and this is what discovered:

Properties can have values or accessors, not both.

When trying to run a code which sets both acessors and values I've got this:

TypeError: Invalid property.  A property cannot both have accessors and be writable or have a value, #<Object>

So I guess there's nothing we can do about this.

---

EDIT: Hey @meeber, I was thinking about using Object.defineProperty to set both the accessor and the value for a property, so we would be able to call the function at the value when using () (invocation) and we would still be able to call the getter function (accessor) when retrieving a property value, but it's not possible due to what I said above.

More details related to this here (#642) and here (#562).

[meeber]

Ahh gotcha. Nice timing btw :D

[keithamus]

Just to clarify the matter: we still get lots of requests regarding this. I think eventually, when we tackle some of the things on the Roadmap - we'll have the flexibility of offering interfaces which don't use properties. But I think it'd be very turbulent to change things right now. For now, linters & IDEs can be tweaked, or you can use dirty-chai - as @meeber mentioned.

I'll close this because of all of the above discussion, which is to say "we're not saying wontfix, just wontfix-right-now".