React Testing Library API types prevent usage of SVG elements

[joebobmiles]

• @testing-library/react version: 11.1.1
• Testing Framework and version: jest 25.1.0
• DOM Environment: jsdom

Relevant code or config:

import * as React from "react";
import { render } from "@testing-library/react";
import "@testing-library/jest-dom/extend-expect";

test("Is a circle", () => {
  const { container } = render(
    <circle cx=0 cy=0 />,
    {
      container:
        document.body.appendChild(
          document.createElementNS("http://www.w3.org/2000/svg", "svg")),
    });

  expect(container.firstChild.nodeName).toBe("circle");
});

What you did:

Trying to set SVG as the container element for testing SVG React components.

What happened:

Typescript threw a type error stating that render() requires an HTMLElement.

Reproduction:

I created a repository illustrating my problem.

Problem description:

React allows components to use HTML and SVG tags. SVG tags do not work properly in an HTML context, and consequently must be rendered inside an SVG context. This can be only done by using an SVG tag as the containing element. Since the API's types disallow this, users have to wrap all their SVG based components in SVG tags manually and introducing unnecessary nesting to the render output.

Suggested solution:

Propose the following patch to @testing-library/react/types/index.d.ts:

// TypeScript Version: 3.8

import {OptionsReceived as PrettyFormatOptions} from 'pretty-format'
import {queries, Queries, BoundFunction} from '@testing-library/dom'
import {act as reactAct} from 'react-dom/test-utils'

export * from '@testing-library/dom'

type DOMElement = HTMLElement | SVGElement;

export type RenderResult<Q extends Queries = typeof queries> = {
  container: DOMElement
  baseElement: DOMElement
  debug: (
    baseElement?:
      | DOMElement
      | DocumentFragment
      | Array<DOMElement | DocumentFragment>,
    maxLength?: number,
    options?: PrettyFormatOptions,
  ) => void
  rerender: (ui: React.ReactElement) => void
  unmount: () => boolean
  asFragment: () => DocumentFragment
} & {[P in keyof Q]: BoundFunction<Q[P]>}

export interface RenderOptions<Q extends Queries = typeof queries> {
  container?: DOMElement
  baseElement?: DOMElement
  hydrate?: boolean
  queries?: Q
  wrapper?: React.ComponentType
}

type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>

/**
 * Render into a container which is appended to document.body. It should be used with cleanup.
 */
export function render(
  ui: React.ReactElement,
  options?: Omit<RenderOptions, 'queries'>,
): RenderResult
export function render<Q extends Queries>(
  ui: React.ReactElement,
  options: RenderOptions<Q>,
): RenderResult<Q>

/**
 * Unmounts React trees that were mounted with render.
 */
export function cleanup(): void

/**
 * Simply calls ReactDOMTestUtils.act(cb)
 * If that's not available (older version of react) then it
 * simply calls the given callback immediately
 */
export const act: typeof reactAct extends undefined
  ? (callback: () => void) => void
  : typeof reactAct

I've implemented this patch locally in my project where I discovered the bug and it solves the issue. However, I'm having difficulty committing my patch to react-testing-library due to dtslint commit hook rejecting the types in the React dependency.

[eps1lon]

Element should do just fine. Would you be willing to put up a PR?

[joebobmiles]

Element should do just fine.

Oh, hadn't thought of that.

Would you be willing to put up a PR?

And I would be willing to put up a PR, but as I noted at the end of my issue, I'm having issues with the dtslint commit hook. It keeps rejecting types from the react and eslint packages, which prevents me from committing my patch. I've even tried running the typecheck on master without my patch and it still rejects the types from react and eslint. Any ideas about why it's doing that?

[kentcdodds]

Commit with --no-verify to skip the git hook so you can at least push and we'll see what's up on CI.

[joebobmiles]

I've committed and opened a pull request, #833.

[eps1lon]

Closed in #833 and released:

This PR is included in version 11.2.1

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot

[rickdunkin]

Should there not be a corresponding update to DOM Testing Library for these types? All of the queries take a container of type HTMLElement that's incompatible with this update.

[nickserv]

React Testing Library isn't written in TypeScript, it bundles types for the convenience of users but they aren't checked within the project beyond type tests. As a result, it doesn't matter what the types of DOM Testing Library are, they will continue to work. React Testing Library also replaces those queries with ones bound to rendered React elements, and those are already accurate because there is no element argument for the type check to fail against.

This shouldn't break anything in the other libraries, but I'll still update them for consistency so users can use SVG elements.

[rickdunkin]

I suppose I should have clarified that by incompatible I was specifically referring to the types, not the actual JS implementations. The updated Element type returned from render was incompatible with the container option to the query functions as they expect an HTMLElement still. We can coerce the types, but having DOM Testing Library also accept Element rather than HTMLElement seems the better option.

Appreciate the updates!

[nickserv]

React Testing Library queries have no container option because they are wrapped to pass the rendered element automatically. As a result, React Testing Library users shouldn't have any issues as long as they're not mixing it with DOM Testing Library. Porting this fix will still be useful for DOM Testing Library users though.

[rickdunkin]

I specifically was seeing an issue in a utility function in use in one of my company's repos that utilizes DOM Testing Library functions, but is sometimes supplied a container resulting from React Testing Library's render function.

[nickserv]

In that case, you should be able to pass it an Element in the same way once testing-library/dom-testing-library#834 is merged.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

Retrospectively, should this have been a major version bump? This is a breaking change, and our ^11.1.2 constraint is letting in 11.2.2 which breaks our tests

[eps1lon]

Retrospectively, should this have been a major version bump?

We consider this a bugfix hence the SemVer patch. These can be considered breaking changes in dependent code but that means the dependent code relied on a bug.

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

Did @testing-library/react ever claim to support SVG elements? If not, this seems more like a feature request than a bug.

[nickserv]

I considered it to be a bug fix in the types, as the types should have allowed what was allowed at runtime. Therefore the fix was semver minor, bumping from 11.2.0 to 11.2.1.

If you want different developers or environments to use the same dependency versions, commit your package lock file. If you're getting errors with the DOM Testing Library API, this pull request will fix them. If you're having another issue, please open it on the appropriate repository.