support `document` containers in cleanup
What:
When container is set to document, the cleanup step fails:
TypeError: Cannot read properties of null (reading 'removeChild')
This is because the code assumes document.body exists:
if (container.parentNode === document.body) {
document.body.removeChild(container);
}
In reality, both container.parentNode and document.body are null.
Why:
The tests erroneously fail.
How:
The attempt to remove the container from the body now only occurs when the body exists.
Checklist:
- [ ] Documentation added to the docs site N/A
- [ ] Tests
- [ ] TypeScript definitions updated N/A
- [x] Ready to be merged
resolves #1329
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 6cb208752b112caa11a6fb41ef0a2dd624bd4630:
| Sandbox | Source |
|---|---|
| react-testing-library-examples | Configuration |
Please add a test.
I don't understand why document.body would be null? Isn't that always defined?
I don't know the setup, but is null when container is document. Locally, I fixed this by setting it to document.createElement('body').
I unfortunately do not have bandwidth to write a test for this, but the reproduction steps should be simple enough.
I ran into this specifically when writing a test for a NextJS root layout, which attempts to mount an HTML and body element. The docs say to use container: document when rendering your own HTML element, which fixed the previous error during test but introduced the OP error during cleanup.
@quisido thanks for taking the time to add this but we are not merging changes without proper coverage for them.
@quisido thanks for taking the time to add this but we are not merging changes without proper coverage for them.
Understandable. The PR is welcome to be updated. I opened an issue for it for tracking as well. I can try updating tests as time permits, but I wanted the problem and solution to both be documented.
I tried writing a test for this but it passes in our tests because document.body is not null. I'm trying to figure out what's the difference in the setup.