react-testing-library: TypeError: Cannot read property textContent of null when you expect a property on a missing element queried by data-testid

Thanks for filing this @sompylasar!

For more context, here’s what the error looks like when I make a typo in a similar test:

TypeError: Cannot read property 'textContent' of null

  18 | test('calling render with the same component on the same container does not remount', () => {
  19 |   const {container, queryByTestId} = render(<NumberDisplay number={1} />)
> 20 |   expect(queryByTestId('number-displays').textContent).toBe('1')
  21 | 
  22 |   // re-render the same component with different props
  23 |   // but pass the same container in the options argument.
  
  at Object.<anonymous>.test (src/__tests__/number-display.js:20:10)

The fact that Jest will call out the line number where the error happened makes the problem less concerning for me. I think that people will understand what went wrong.

And just to be clear for everyone, we’re not throwing any errors, this is a regular JavaScript error (trying to read textContent on null). I think the error output makes it clear where that’s happening. As to answering why that’s happening, I think that could be improved.

For the benefit of everyone else, I talked with @sompylasar about this already. He asked if we could throw an error in queryByTestId when the element wasn’t found and I explained that would break this use case:

expect(queryByTestId('foo')).not.toBeTruthy()

Where you’re trying to assert that something does not exist.

So one thing’s for sure: We will not be changing the current behavior.

However, it would be nice to improve the error message. One suggestion @sompylasar made is we could create a new function called queryByTestIdRequired, but first off I doubt many people will use it so we lose the benefits there, and secondly I’d prefer to avoid expanding the API unless it’s absolutely necessary.

Another idea I had is to create custom expect matchers that people can use in their apps that could throw more helpful error messages:

expect(queryByTestId('foo')).not.toBeInTheDOM()
expect(queryByTestId('number-display')).toHaveTextContent('1')

I don’t think I’ll work on these myself, but I would be happy to accept a contribution that adds this capability with a simple API 😃

Ok, @pbomb’s PR was merged. A release should be out soon and you’ll be able to use getByTestId 👍

Another suggestion: a generic matcher

.toMatchDOM(el => el.textContent === 'hello')

where el is guaranteed to exist or it fails; for safety and clarity, the exceptions from within the matching function can also be caught and enhanced with “the DOM element did not match” hint.

@sompylasar, if you’d like to make a pr to add yourself to the contributor table you’re more than welcome. You can follow the instructions in the CONTRIBUTING.md 👍

For the record, the link to PR: https://github.com/kentcdodds/react-testing-library/pull/10

@kentcdodds Ahh, that make sense to me. Will do the same and raise a PR. Thanks!

The reason to split it up is so people can just do this in their test files:

import 'react-testing-library/extend-expect'

It’s explicit, but it’s really easy. And it means that if someone wanted to use them differently, or only use one of them, they could:

import {toBeInTheDOM} from 'react-testing-library/dist/jest-extensions'

expect.extend({toBeInTheDOM})

Gives a bit more flexibility without sacrificing usability.

Another related issue with the queryByTestId is with the TypeScript definition. It’s declared that it can return HTMLElement | null which is true, but prevents me from being able to write the kind of simple code documented for this library. Just like this issue mentions the runtime error if the queried-for element is null, with TypeScript projects, these lines all become compile-time errors because of this possible scenario.

The rejected proposal of throwing an error would resolve this issue as well since it would no longer return null. Alternately, changing the type declaration in an incorrect way to specify that it doesn’t return null might also be a reasonable solution to this problem. I’m not worried about these runtime errors being thrown as this library should only be used in a testing environment where errors thrown are likely to result, correctly, in test failures.

What are your thoughts on this problem? I lean towards fudging the type declaration as the developer experience is really bad when null can be returned.

I’d rather it be called: .toSatisfyDOM

There’s a toSatisfy in jest-extend, so it would make sense to follow the naming convention there.