queries returned by render are not scoped
@testing-library/reactversion: 14.1.2- Testing Framework and version: Jest 29.7
- DOM Environment: jsdom 20.0.3
Relevant code or config:
it('uses a consistent scope', () => {
const MyComponent = () => {
useEffect(() => {
const separateElement = document.createElement('div');
separateElement.textContent = 'hello';
document.body.append(separateElement);
return () => separateElement.remove();
}, []);
return (
<div>
<div>hello</div>
</div>
);
};
const { container, getByText } = render(<MyComponent />);
within(container).getByText('hello'); // passes
getByText('hello'); // fails (finds 2 elements)
});
What happened:
The first test (within(container).getByText('hello')) passes, and the second (getByText('hello')) fails:
Found multiple elements with the text: hello
Here are the matching elements:
Ignored nodes: comments, script, style
<div>
hello
</div>
Ignored nodes: comments, script, style
<div>
hello
</div>
Problem description:
The container and queries returned by render should be consistent with each other: the queries should search within the returned container by default, to avoid pollution from other tests and libraries which attach elements to other parts of the DOM. For users who need to check the wider document, they can continue to use screen.
It is also worth noting that the documentation claims that the queries are "bound", which does not match the current behaviour (since they apply globally across the document).
Suggested solution:
It is possible to make a wrapper function in user-space which works around this, which should be easy to integrate into the core library:
export const renderScoped = (ui: ReactElement, options?: RenderOptions) => {
const rendered = render(ui, options);
return {
...rendered,
...within(rendered.container),
};
};
This will be a potentially breaking change for users who currently rely on the queries not being scoped, so probably needs to be a "4.2" release.
Hi @david-bezero, thanks for taking the time to open this one.
I agree that the docs section is a bit misleading but the query doesn't really apply globally.
If you'll pass a container to the render function or a baseElement, we'll use that and the query will be scoped to that.
At the moment, since you're not passing any of these, the query is bound to the body. If you'll create a new element and append it to the head, it won't be queried.
Having said that, I don't think this is a use-case that happens too often. The example you've attached (and thanks for attaching it) is really not a best practice as it has a side effect which changes the body of the component.
Do you have a better example to share?
Thanks again.
The example you've attached is really not a best practice as it has a side effect which changes the body of the component. Do you have a better example to share?
That example matches all our existing unit tests (and the tests in every other project I've seen / worked on!), so if it's not best practice I'd be interested to know what we should be doing differently.
That example matches all our existing unit tests (and the tests in every other project I've seen / worked on!), so if it's not best practice I'd be interested to know what we should be doing differently.
Do you mean that you have code inside your useEffect that adds elements to the DOM directly by creating an element? Because that's what I was referring to.
ah; I assumed you were referring to the sentence before. The useEffect is not too far from our current scenario, but it's coming from a library we're using which internally (and temporarily) attaches an item to the DOM for rendering (with display: none, but these selectors don't omit things based on that)
A slightly more accurate representation taking that into account would be:
it('uses a consistent scope', () => {
const MyComponent = () => {
useEffect(() => {
const separateElement = document.createElement('div');
separateElement.textContent = 'hello';
separateElement.style.display = 'none'; // not actually shown to the user
document.body.append(separateElement);
return () => separateElement.remove();
}, []);
return (
<div>
<div>hello</div>
</div>
);
};
const { container, getByText } = render(<MyComponent />);
within(container).getByText('hello'); // passes
getByText('hello'); // fails (finds 2 elements)
});
I believe the library has to attach the elements to be able to measure text (which doesn't work when detached).
Of course the other issue here is that multiple tests can interfere with each other if one fails to tear something down (causing the failure in another, "innocent", test). The most obvious scenario would be if a test fails to unmount its test component (and the developer hasn't set up an afterEach to call cleanup()), then the test passes but another may fail, and this may depend on the execution order of the tests. The rarer but still possible scenario is if a component being tested attaches an item to the DOM which it fails to clean up when unmounted, or delays removing the element due to an animation (e.g. a dialog or toast notification). Again whether this causes a failure would often depend on the execution order, making it appear "flakey").