jest-dom icon indicating copy to clipboard operation
jest-dom copied to clipboard

Treat <br/> as a whitespace for toHaveTextContent()

Open srittau opened this issue 4 years ago • 6 comments

Describe the feature you'd like:

Suppose you want to match text from a paragraph containing <br/> elements for custom line break handling:

const MyGreatTextComponent = () => {
    return (
        <p>
            This is text
            <br/>
            with multiple lines.
            <br/>
            In reality, we'd have some dynamic data here.
        </p>
    )
}
test("", () => {
    render(<MyGreatTextComponent/>);
    expect(document.body).toHaveTextContent(/This is text with multiple lines/);
});

This test will fail, since the <br/> element will get stripped, without adding whitespace. The matcher sees This is textwith multiple lines.In reality [...]. In my opinion, it would be preferable, if <br/> elements would be considered whitespace for the .toHaveTextContent() matcher. As in the example, <br/> is usually used in place of a whitespace character and could be replaced with a space for the same semantic content.

Of course, changing the matcher to treat <br/> elements as space is a behavior change that could break existing tests.

Describe alternatives you've considered:

Currently, we are adding extra forced spaces {" "} before <br/> elements. That works, but is a neither obvious, nor useful for the implementation.

Teachability, Documentation, Adoption, Migration Strategy:

I am not too familiar with testing library's migration strategies, but in my opinion, this change would at least require a major version bump, due to the possibility of breaking existing tests relying on the current behavior. On the other hand, I believe this behavior would actually ease teachability, since it conforms closer to what a human user sees on the screen and is therefore closer to the expected behavior.

srittau avatar Nov 09 '20 13:11 srittau

Personally I don't think this requires a major version bump, as it fixes behavior that's surprising. I think it's more likely that a user would want to switch to what you're suggesting than keep the old behavior for back compatibility.

nickserv avatar Nov 10 '20 16:11 nickserv

Tried to repro this with:

  test('treat <br/> tags as a whitespce', () => {
    const {container} = render(`
		<p>
                   This is text
                   <br/>
                   with multiple lines
                   <br/>
                   In reality, we'd have some dynamic data here.
		</p>
	`)

    expect(container.querySelector('p')).toHaveTextContent(
      /This is text with multiple lines. In reality/,
    )
  })

The test passes as expected by treating the <br/> tags as whitespace. Am I missing anything?

kiranjd avatar Dec 19 '20 18:12 kiranjd

@kiranjd I failed to mention that I was using render() from @testing-library/react in my example. Maybe that's the difference?

srittau avatar Dec 21 '20 14:12 srittau

@srittau This is using the @testing-library/react package itself

kiranjd avatar Dec 21 '20 15:12 kiranjd

There's an extra ` around what you're rendering, so it's a string, not JSX. If you remove that it should fail like the original example.

nickserv avatar Dec 22 '20 04:12 nickserv

I faced same issue with zero width space &#8203;.

Now, I am using expect(actual).toHaveTextContent('\u{200B}').

moeyashi avatar Jul 08 '22 06:07 moeyashi