Description
Describe the feature you'd like:
I wanted to propose querying an array of containers rather than a single container. It would allow us to do queries like this:
getByText(getAllByRole('heading'), 'Page 2'));... which is more composable. Currently you can only query one container at a time, not an array of them.
As for a semi-practical example, let's say I'm on a page with the following (convoluted) JSX:
const pages = ['Page 1', 'Page 2'];
const App = () => {
const [title, setTitle] = useState(pages[0]);
return (
<main>
<h1>{title}</h1>
<nav aria-labelledby="inner">
<h2 id="inner">Inner nav</h2>
<ul>
{pages.map((pageTitle) => (
<li>
<a href="#url" onClick={() => setTitle(pageTitle)}>
{pageTitle}
</a>
</li>
))}
</ul>
</nav>
</main>
);
};I want to check that clicking Page 2 results in the page heading changing from Page 1 to Page 2.
However, since the same text is inside the heading and the nav (and potentially more in a real app), it's hard to select the proper element. With the new API, here's how we could test for that:
test('clicking navigation link results in page contents changing', () => {
const { getAllByRole, getByText, getByLabelText } = render(<App/>);
// Click the Page 2 link inside the inner nav.
// We could also just do `fireEvent.click(getByText('Page 2'))` but
// this is an example, and the intent there is less clear anyway
const navs = getAllByRole('navigation');
// !!! New API !!!
const toc = within(navs).getByLabelText('inner');
fireEvent.click(toc.getByText('Page 2'));
// now both the heading AND the inner nav have Page 2 in it.
// but we only wanna verify that the heading has certain text
// !!! New API !!!
getByText(getAllByRole('heading'), 'Page 2'));
});Instead of limiting a query to a single container, we can now query an array of containers. The end result looks very natural, hopefully!
Suggested implementation:
I'm happy to give this a go! I was thinking of adding this to the queryAllBy* methods:
// query-helpers.js
function createQuery(queryAllBy, container, ...args) => {
if (Array.isArray(container)) {
const elements = container
// get an array of all the query results
.map((el) => queryAllBy(el, ...args))
// flatten the results into a single array
.reduce((array, result) => array.concat(result), []);
// dedupe
return Array.from(new Set(elements));
} else {
return queryAllBy(container, ...args);
}
};
// label-text.js
// I initially thought about adding this to `buildQueries` but then I saw that
// `*LabelText` does not follow the `buildQuery` pattern. So:
queryAllByLabelText = createQuery(queryAllByLabelText);... And then add tests for it. Does that approach seem sound to you? I'd look deeper if I get the 👍 on whether the API change makes sense.
Describe alternatives you've considered:
We could implement the first query as getByRoleAndText(role, text), but it's a bit too specific, especially if we want to add more queries of that kind in the future. If dom-testing-library supports element arrays for the container, then these queries become natural to write without changing the API surface too much.
Teachability, Documentation, Adoption, Migration Strategy:
Migration and adoption should be easy since it's an addition to the query API. Documentation is a bit harder, but I imagine we can start stating that queries now can take an array of elements instead of just one element.