RFC: Matchers

[krzkaczor]

This issue facilitates discussions about matchers design

Problem of having multiple ways to match the same thing

toEqual is quite a beast and thus many matchers can be implemented just by using it:

1)
expect(true).toEqual(true)
// instead of
expect(true).toBeTrue()

// same goes for other primitive values like (false/null/undefined)

2)
expect("abc").toEqual(any(String))
//instead of
expect("abc").toBeInstanceOf(String)

I had this problem on my mind for some time already and I initially thought that avoiding this duplication entirely is a good thing but I believe that having some level of consistency with chai can beneficial. For example, I think 1) is fine (it's clear and shorter 🤔)

Any thoughts on this one?

Essentials matchers to implement in the first batch

• toBeTruthy
• toBeFalsy
• toThrow (match message and match error type as separate matches? to read: Using expect(...).to.throw(...) with custom assertion function chaijs/chai#655)

Asymmetric:

• any (or anyInstanceOf? much longer, much more explicit... i need to admit that any was confusing for me when I first saw it)
• anyString / anyStringMatching / anyStringContaining
• anyNumber

[sz-piotr]

Regarding multiple ways of doing the same thing.

I consider this an antipattern leading to unnecessary headaches. It forces the programmer to make more decisions that he actually needs. It also requires him to visit the documentation to see if maybe there is something actually different between the options. It also makes the library author have to pick a version for each example, leading to additional overhead and making documentation less straightforward.

Regarding essential matchers.

I am a big fan of appropriate and concise naming. Therefore I would prefer overloading some matchers to make it easier to reason.

• .toThrow('message')
• .toThrow(/message/)
• .toThrow(ErrorClass)
• .toThrow(ErrorClass, 'message')
• .toThrow(ErrorClass, /message/)
• .toBeA(MyClass) - uses instanceof
• .toBeA('string') - uses typeof

Knowing this it is easier to resolve the assymetric ones. Just make it consistent with symmetric.

• expect.anything()
• expext.a(MyClass)
• expect.a('string')

[krzkaczor]

@sz-piotr thanks for your input!

I consider this an antipattern leading to unnecessary headaches.

Yeah, I see your point. Agreed, lets start without these additional matchers.

I like having a instead of bunch of more specific asymmetric matchers, one thing though, I wanted to make it support expect.a(String) or expect.a(Number) out of the box. This can be a simple "if" that smooths out rough edges of JS and makes it work as expected in the first place. This goes to .toBeA(String) matcher as well.

Another thing that I wanted to discuss is what about matchers for promises? These days I don't use toBeResolved (or similar) at all because it's cleaner to use await/async, so why dont just drop it an explain in docs what to use instead? This leaves us with only matchers needed for rejections.

[sz-piotr]

Yeah, I also think that you only need matchers for promise rejections. Not sure about .a(String), since this violates the rule about one way to do things. The proper way should be .a('string'). Potentially if you pass String you could even log a warning with a link to MDN

[krzkaczor]

@sz-piotr I wanted to drop .a('string') entirely. Doesnt it exist only because of instanceOf oddities in JS?

[sz-piotr]

@krzkaczor do here is how I see it. There are two operators in JS. typeof and instanceOf. One is used for primitives like string or number and always returns 'object' for other values (sadly including null). The other is used for class instances like Error and returns false for strings and numbers.

What we want to do is combine those two operators under the name of a. I propose to use one if you pass a string the other if you pass a class. You want only to support only classes (and weird things like String which are almost classes).

If we go your route which is quite nice in its simplicity we run into the following problems.

• how to check for null and undefined (maybe the solution is to never match them)
• all primitives require custom code and introduction of new ones can be problematic (see BigInt), because you might not be able to match them
• we loose the simple mental model of using typeof and instanceof

Despite those problems the more I think about it the more I am convinced it is the way to go. Also if it does not use typeof or instanceof directly it is ok to use stuff like Array.isArray() under the hood

[krzkaczor]

how to check for null and undefined (maybe the solution is to never match them)

yeah i think the right way is to enforce it on type level and call it a day... (no need even for runtime exception then)

all primitives require custom code and introduction of new ones can be problematic (see BigInt), because you might not be able to match them

That's quite a rare situation and if your run up-to-date version of earl you should be fine 🤔

we loose the simple mental model of using typeof and instanceof

I never thought it's simple :D simple would be only having 1 way of doing things which works always properly (which we try to have here) 😆

Despite those problems the more I think about it the more I am convinced it is the way to go.

I am glad you like it! And yes Arrays also can be supported this way which is kinda neat...

I am working on it here: #10

[krzkaczor]

I had this crazy realization yesterday: what if we push using asymmetric matchers to the limit basically turning this library in a pattern matching assertion library 🤔 Then we could drop expect(x).beEqual entirely and replace with something simpler.

For example:

// this almost already works already
match(x, {
	name: a(String),
	age: numberCloseTo(20, { delta: 2 }),
})

// match strict mock to fully verified mock
match(mock, aVerifiedMock())

// assert mocks basically by creating a pattern out of fresh mock
match(looseMock, mock().calledOnceWith(1, 2, 3).calledTwiceWith(1))

// for waffle
match(tx, aRevertedTx('revert reason'))

Am I crazy that I kinda like it? Also, it allows leveraging typescript assertion functions.

It also solves discussions about "should this be a regular matcher or asymmetric matcher".

Finally, it doesn't require changing the API of expect by plugins which make for simpler integration.

@quezak @sz-piotr

EDIT:
Just to clarify I don't think that it's a way to go forward for this library (for sure not now). But it's an interesting insight.

[quezak]

Then we could drop expect(x).beEqual entirely and replace with something simpler.

I think "meh", this actually looks less intuitive than separate named checks for different occasions

[krzkaczor]

I just merged #18 which introduces some similar concepts to previous @sz-piotr work.

Matchers became simply validators, asymmetric matchers are just matchers and there are modifiers like not.

This PR also implemented simply toThrow.

[krzkaczor]

I am closing this one as initial implementation and API direction is done. Thanks everyone for your involvment!