toHaveTextContent should ignore whitespace

[gnapse]

• jest-dom version: 1.0.0

What you did:

//  <span>
//    Step
//      1
//    of 4
//  </span>

expect(container.querySelector('span')).toHaveTextContent('Step 1 of 4')

What happened:

    expect(element).toHaveTextContent()

    Expected element to have text content:
      Step 1 of 4
    Received:

          Step
            1
          of 4

Problem description:

This was initially reported in testing-library/react-testing-library#53, and it was really about a bug in testing-library/dom-testing-library#19. However, this also applies to a piece of code in jest-dom. The custom matchet .toHaveTextContent should also ignore a node's whitespace in its text content before determining if the text matches the expectation.

Suggested solution:

Normalize the text of a node before using to match it with the expected text. Similar to what was done in testing-library/dom-testing-library#19

[kentcdodds]

Actually, dom-testing-library actually exposes its matches function, we should probably use that too 👍

[gnapse]

That fix was not done in matches because that one is used in other contexts. But perhaps we wanted all other contexts to be affected too? In that case we need to revisit my fix to be done in that way instead.

[kentcdodds]

No, I was just suggesting while you're working on this you could do that as well. Perhaps we should also expose the getText method from dom-testing-library as well 🤔

[gnapse]

@kentcdodds wouldn't there be a problem if two packages depend on each other?

[kentcdodds]

I think that's actually allowed. But I'm thinking that dom-testing-library should no longer depend on jest-dom. I think we should make a major version bump to make that happen. I think folks should install jest-dom separately.

[smacpherson64]

-- Removed idea about using element.innerText as it has a few quirks and better options exist.

[smacpherson64]

From @noinkling:
#56 (comment)

This change [#56] breaks testing for text within child elements (nested text), because you switched to using getNodeText from dom-testing-library, which only considers immediate text node children.

Example:

<div class="error">
  <ul>
    <li>Email format is invalid</li>
  </ul>
</div>

I just want to write something like expect(errorBox).toHaveTextContent('email format'), instead of tightly coupling my tests to whatever the internal structure may be. This worked fine in the previous version because it used .textContent (which is what I would expect a method named "toHaveTextContent" to use).

Since it now fails, the best I can come up with is something like expect(errorBox.textContent).toMatch(/email format/i), which isn't nearly as nice.

[smacpherson64]

From @kentcdodds:
#56 (comment)

expect(errorBox).toHaveTextContent('email format')

I don't think that would have worked before. The regex is how I would have done that before and now 🤔

[smacpherson64]

Hrm, Line 5: of get-node-text filters out all non TEXT_NODES. So it would only currently get one level deep of text. If we switch the && to || I think it would get all sub text as well:

Example:
https://codesandbox.io/s/5xl195pynx
The console.log shows the examples

@kentcdodds is getNodeText meant to get the full text (including children) or just one level deep?

This however, would only normalize one level deep though, so it wouldn't solve the issue.

[smacpherson64]

The normalization was moved to: https://github.com/kentcdodds/dom-testing-library/blob/863741a204bca67c63ca3944770727e5afb071d2/src/matches.js#L39-L46

So getNodeText no longer normalizes the text. (testing-library/dom-testing-library#31)

Hrm, Maybe we could use fuzzyMatches in conjunction with getNodeText!

[noinkling]

Why not just normalize the .textContent?

@kentcdodds Well toHaveTextContent also supports a regex, if that's specifically the part you're worried about (maybe there's an argument for not doing partial or case-insensitive matches when using a string). But it still won't work on nested text anymore regardless.

[smacpherson64]

That's a really good point haha!

[gnapse]

🎉 This issue has been resolved in version 2.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[noinkling]

Thanks for the fix 🙂

[malixsys]

As @smacpherson64 predicted, this broke a lot of our tests... Is that allowed under SemVer?

            expect(signInSection).toHaveTextContent(
                '[DICT] Welcome,Kermit the Frog'
            );

[gnapse]

@malixsys I'm so sorry.

I'm not sure how you're specifying jest-dom's version number, but this was indeed published as a breaking change, and it's the breaking change that switched jest-dom from v1.x to v2. I think that would not violate SemVer terms.

Maybe I'm missing something. Can you clarify what version you're using that broke your tests, and under which version they used to work? Your code snippet above does not give much information.

[malixsys]

@gnapse Ah yes, indeed! Sorry, thought this was in 2.0.1! No worries, 2.x has been put in the code debt column for now... :)