RFC: Matchers about earl HOT 10 CLOSED

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')

from earl.

@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

from earl.

@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.

from earl.

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

from earl.

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

from earl.

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

from earl.

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.

from earl.

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

from earl.

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.

from earl.

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

from earl.