Regarding the deprecation of toBeInTheDOM we should either find better assertions or use toBeInTheDocument: https://github.com/gnapse/jest-dom/pull/40
Anyone wanna try that?
For dom-testing-library, most of the examples aren't actually attached to document.body so toBeInTheDocument wont work. We'll have to think of something better on a case-by-case basis I think.
I'll take this one too, so we can merge and release this prior or alongside to the corresponding change in kentcdodds/react-testing-library#140.
Indeed toBeInTheDocument is of no use in dom-testing-library's tests as it is today. But this is just in the tests, as the render helper does not attach the generated container to the document.
Is there a reason for this, or can I modify the helper to attach to the document? This may require a cleanup step though.
Alternatively, I can replace all uses of toBeInTheDOM() with toBeInstanceOf(HTMLElement). I already did it and it all worked. toBeTruthy() would also work.
Or perhaps it'd be more acceptable to take the time and always extract the container from what render returns, and replace all expect(query...).toBeInTheDOM() with expect(container).toContainElement(query...).
Which one would be preferable?
As for the README, in case option 1 above is not acceptable, would it be ok if in the README examples we assume the DOM being tested in each example (generally put in comments) is mounted in the document, and we replace all uses of toBeInTheDOM with toBeInTheDocument, just in the README? After all, the only reason not to use it is because of how the tests work, but the README is for general documentation.
I prefer: expect(container).toContainElement(query...) as it's more explicit. I think I'd prefer examples to show that as well.
I'm in doubt about this use of toBeInTheDOM. It seems to want to assert that there's no ... in the message of the error that's expected to be thrown, but that regex should be passed as argument to toThrowError instead of to toBeInTheDOM. And it's passing today because of course that toBeInTheDOM throws an error, albeit an unrelated one, and not the one that's being expected. Am I correct?
Feel free to rewrite that test. I never really liked it much.
Now I have another question:
query* and get* functions in dom-testing-library receive the container, rather than having it associated implicitly as is the case with react-testing-library. This makes it a bit redundant to have to put the container twice when using toContainElement:
expect(container).toContainElement(queryByTestId(container, 'ok-button'))
This happens quite a bit in the README examples. Nothing wrong about that per-se, just wanted to make sure is ok before taking the effort to change it.
Ah, that's fair, and kinda annoying. Maybe we should do toBeInstanceOf(HTMLElement) 馃
Honestly, for me the _real_ assertion is the get* methods which will throw a more helpful error message if it can't find the element anyway 馃檭
Ok, I'll see what I can do.
Now that the library is throwing warning about using toBeInTheDOM, I've moved to toBeInTheDocument as it suggests. However, the flow typings haven't been updated so I'm now getting a bunch of type errors. :(
Would you like to make that update? :)
I think I did this at some point. In any case, they're gone!
Most helpful comment
Honestly, for me the _real_ assertion is the
get*methods which will throw a more helpful error message if it can't find the element anyway 馃檭