[FEAT] `xByTagName` API

crutchcorn commented 2 years ago

Describe the feature you'd like:

I would love for there to be a query for an element's tag name. In particular, it would help users (like me) who're looking to test for the existence of particular <head> tags (such as script or meta) in my SSG framework's codebase.

This could also be useful for testing Next.JS's <head> tags in production applications, using a similar mock to the one I've created for my blog

Suggested implementation:

I've created an implementation using buildQueries and other dom-testing-library methods of building query APIs here:

https://github.com/unicorn-utterances/magic-maker/blob/d7c8b930c9dda26c6acd0609ddd93798f686a09c/src/utils/test-utils/queries.ts

I'd be happy to make a PR internally to add this in!

Describe alternatives you've considered:

Alternatively, we could use document.querySelector, but this loses out on many of the benefits this library provides (such as testing framework integration, timing management, etc)

There's also the option for those who want it (such as myself) to simply add the implementation I provided into their codebase, but it requires utilizing non-exported internal methods of dom-testing-library.

Teachability, Documentation, Adoption, Migration Strategy:

Here's an example of what the docs could look like:

getByTagName, queryByTagName, getAllByTagName, queryAllByTagName, findByTagName, findAllByTagName

API

getByTagName(
  // If you're using `screen`, then skip the container argument:
  container: HTMLElement,
  tagName: string
  ): HTMLElement

Returns the element that has the matching tag name.

import {screen} from '@testing-library/dom'

const metaElement = screen.getByTagName('meta')
const scriptElement = screen.getByTagName('script')

MatanBobi commented 2 years ago

Hi @crutchcorn, thanks for the detailed request! My main concern is that I think this feature can easily be misused for "implementation details" testing.. Wdyt about that? @eps1lon I'd be happy to hear your thoughts on this one too

crutchcorn commented 2 years ago

While I generally agree that there's the potential for "implementation details" testing, I think that the real-world use-cases here introduce a reasonable explanation for when/when not to use them. Otherwise, when testing for head elements, it's a pretty mediocre experience.

I think that this concern can be handled with a bit of prescriptivist API documentation when the feature is released.

This API should be used sparingly in order to avoid testing for implementation details. One primary use-case where this API is useful is testing against a page's head children, such as meta tags.

One use-case where this API is not appropriate is in usage for testing against an element's role. For those use-cases, you should use the ByRole query.

eps1lon commented 2 years ago

I think that this concern can be handled with a bit of prescriptivist API documentation when the feature is released.

While that sounds simple, it's not in practice. Every instruction we add is another instruction people have to apply. It's another instruction that might be misinterpreted etc.

What we want to offer is an API that solves 99% of the use cases with a minimal API surface. And for the long tail we should have escape hatches ready. One of that is buildQueries:

import {queryHelpers, buildQueries} from '@testing-library/react'

const queryAllByTagName = (container, tagName) => container.getAllByTagName(tagName)

const getMultipleError = (container, tagName) =>
  `Found multiple elements with the tag name: ${tagName}`
const getMissingError = (container, tagName) =>
  `Unable to find an element with the tag name: ${tagName}`

const [
  queryByTagName,
  getAllByTagName,
  getByTagName,
  findAllByTagName,
  findByTagName,
] = buildQueries(queryAllByTagName, getMultipleError, getMissingError)

export {
  queryByTagName,
  getAllByTagName,
  getByTagName,
  findAllByTagName,
  findByTagName,
}

If you find that query helpful in your codebase then you should use it. That is a way easier decision than trying to document how to use that query properly in any codebase.

eps1lon commented 2 years ago

Closing since this API doesn't seem like it'll solve a lot of use cases. buildQueries seems more appropriate for niche use cases though I would recommend going with document.queryAllByTagName for things not covered by ByRole

testing-library / dom-testing-library