-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
queries returned by render are not scoped #1268
Comments
Hi @david-bezero, thanks for taking the time to open this one. |
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 |
ah; I assumed you were referring to the sentence before. The 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 |
@testing-library/react
version: 14.1.2Relevant code or config:
What happened:
The first test (
within(container).getByText('hello')
) passes, and the second (getByText('hello')
) fails: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 usescreen
.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:
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.
The text was updated successfully, but these errors were encountered: