Jest-dom: Support async matchers?

[kentcdodds]

Just a thought... Perhaps we could take advantage of the Jest 23 release and support custom async matchers. Something like:

expect(node).toEventuallyHaveTextContent('foo')

That would eliminate the need for using wait in dom-testing-library in many cases... 🤔

[gnapse]

Yes, definitely.

Taking a look at that link to jest, shouldn't it be more something like this:

await expect(node).toHaveTextContent('foo')

In which case this is more of making some (or all) of our matchers supporting of async behavior? I'll take a look to how this is doable with the new jest in a few days (if no one else claims this first).

Also, wouldn't this make this lib depending on jest 23? In which case it'll force users to upgrade. Maybe it can be done detecting if jest has the feature? Any thoughts on that?

[kentcdodds]

I'd say make this change as a breaking change and require Jest 23.

[rickhanlonii]

Let me know if I can help on this, the docs here are admittedly not great. It could look something like:

expect.extend({
  async toHaveTextContent(node, content) {

    // Fill this logic in
    const pass = await nodeHasContent(node, content);

    if (pass) {
      return {
        message: () =>
          `expected ${node.nodeName} not to have content ${content}`,
        pass: true,
      };
    } else {
      return {
        message: () =>
          `expected ${node.nodeName} to have content:\n` +
          `  ${this.utiils.printExpected(content)}\n\n` + 
          'But it contained:\n' +
          `  ${this.utils.printReceived(node.textContent)}`,
        pass: false,
      };
    }
  },
});

There are various degrees you can go into for printing the message, most of our default matchers are here and everything exported from jest-matcher-utils are available on this.utils.

[gnapse]

I took a closer look to the documentation of this new jest feature, and to the example from @rickhanlonii above (Thanks BTW!)

Correct me if I'm wrong, but a custom matcher has to be either async or sync (depending on wether it returns a promise or not). So we cannot use the same matcher the way is used today, and make it also work as async.

This means that we then would have to provide a .toEventuallyXYZ for every .toXYZ that this library offers today. These versions of the matchers would then be callable prefixed with await, and the matchers existing today will continue to be called as they're called today.

I was expecting this to be avoided, and that we could get away with using the same custom matcher in both situations, but looks like it's not possible.

Thoughts?

Update: or following the example to which @kentcdodds linked above, where they showcase a hypothetical toHaveClassNameAsync, we could provide a .toXYZAsync for every .toXYZ.

[kentcdodds]

That's exactly what I was thinking. I think I like the Async suffix better than the Eventually version :+1:

[gnapse]

Yup, I was thinking that too, it pairs very well with the await in front of the line. Rings a bell with the async/await js feature.

[gnapse]

I was thinking that I'm not sure this would be as helpful as it sounds when you said that it could eliminate the need to use dom-testing-library's wait in many cases.

For instance, something like this:

await expect(getByText('Ok', { selector: 'button' })).toHaveClassAsync('disabled');

There's no way that .toHaveClassAsync would wait for the element to exist, in order to assert that it has the expected class. If at all, if could only wait for the element (which would have to exist at the moment expect is invoked) to eventually have that class name.

The problem is, all these matchers expect a html element, not a means to find a html element. So if at the time they're invoked, they do not receive a html element, they do not have a way to continually check if the html element came into being, as part of the thing that it's being awaited for.

Instead, users can do this:

expect(
  await waitForElement(() => getByText('Ok', { selector: 'button' }))
).toHaveClass('disabled');

Do you agree @kentcdodds, or maybe I'm missing something here. If you agree, do you still see value in making an async version of these custom matchers? I'm afraid it'll elicit the same invalid expectation that the matchers will wait for the html element to appear.

[kentcdodds]

You're right. I don't see much sense in it now. Unless we had a expect(container).toEventuallyHaveChildWithText('Stuff')

But....... Nah