testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.26k stars 467 forks source link

`getByRole` does not support `exact` option #1157

Closed JesusTheHun closed 1 year ago

JesusTheHun commented 2 years ago

Versions

@testing-library/jest-dom v5.16.5 @testing-library/react v13.3.0

The problem

// Given that an option with the name "Option 1"
getByRole("option", { name: "option 1", exact: false }) // should return the element, but doesn't

Reproduction:

https://codesandbox.io/s/react-testing-library-repro-getbyrole-exact-false-5zzq2v

Problem description:

In the documentation we only see the use of regex when using getByRole and the use of exact when using getByText. However the Typescript definition says the exact option is available for getByRole. So it is either a type error or a runtime code error.

Suggested solution:

timdeschryver commented 2 years ago

Hi, AFAIK this is intended. The exact is a configuration for the role name, not for the name config. You can see this behavior in a test.

JesusTheHun commented 2 years ago

Okay. Would you consider a PR to support more than a string for the name option ? Very often I see myself having to add aria-label whereas a screen reader would already read that value correctly just to avoid writing complex test code. Typical issue is text spread amongst html nodes and svg icons.

It would look like the following :

// A tuple : [name: string, config: TextMatch]
// PRO it avoids name collision with the TextMatch config object
// CON it is not very popular
getByRole('option', { name: ['idea 1', { exact: false }], selected: true });

// An object : TextMatch & { text: string }
// PRO devs are more familiar with this format
// CON is exposed to collision
getByRole('option', { name: { text: 'idea 1', exact: false }}, selected: true });

// An object : { text: string, config: TextMatch }
// PRO devs are more familiar with this format & it avoids collisions
// CON 3 nesting level starts to make a lot
getByRole('option', { name: { text: 'idea 1', config: { exact: false }}}, selected: true });
eps1lon commented 1 year ago

Would it make more sense to use a RegExp for the name then? I thought we already support this. If not then I would rather allow a regular expression than options.

timdeschryver commented 1 year ago

A RegExp is already supported. https://testing-playground.com/gist/310d8a2431e7eabe8efc81babf14222b/0299a286fbc844f140e64a31c2670055983b1307

I think what @JesusTheHun is asking, is that MatcherOptions can also be used for the name (which is currently a string, RegExp, or function)

JesusTheHun commented 1 year ago

Yes, MatcherOptions! Especially for byRole because the name is assigned automatically and therefore can contain weird stuff (that it not read by screen readers, like svg icons)

eps1lon commented 1 year ago

But shouldn't a RegExp allow for the same behavior? Adding another API for the same thing would be problematic.

JesusTheHun commented 1 year ago

But shouldn't a RegExp allow for the same behavior? Adding another API for the same thing would be problematic.

Actually it's the same API but for something else. Using a RegExp is a solution for some cases, but not all.

eps1lon commented 1 year ago

Using a RegExp is a solution for some cases, but not all.

What cases could not be solved with a RegExp?

JesusTheHun commented 1 year ago
const MyVG = () => (
    <svg width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
      <path fill-rule="evenodd" clip-rule="evenodd" d="M7.14645 7.14645C7.34171 6.95118 7.65829 6.95118 7.85355 7.14645L11.9978 11.2907L16.1421 7.14645C16.3373 6.95118 16.6539 6.95118 16.8492 7.14645C17.0445 7.34171 17.0445 7.65829 16.8492 7.85355L12.7049 11.9978L16.8492 16.1421C17.0445 16.3373 17.0445 16.6539 16.8492 16.8492C16.6539 17.0445 16.3373 17.0445 16.1421 16.8492L11.9978 12.7049L7.85355 16.8492C7.65829 17.0445 7.34171 17.0445 7.14645 16.8492C6.95118 16.6539 6.95118 16.3373 7.14645 16.1421L11.2907 11.9978L7.14645 7.85355C6.95118 7.65829 6.95118 7.34171 7.14645 7.14645Z" />
    </svg>
  )

  const MyComponent = () => {
    return <select>
      <option><MyVG />A</option> {/* [object Object]A */}
      <option><MyVG />B</option> {/* [object Object]B */}
      <option><MyVG />C</option> {/* [object Object]C */}
    </select>;
  }

  it("should match the option", async () => {
    const { renderBare } = rtlKit();
    renderBare(<MyComponent />);

    expect(screen.getAllByRole('option', { name: /b/i })).toHaveLength(1);
  });
alleksei37 commented 1 year ago

^^ I created a PR to add some clarification regarding the exact option in the docs. I hope this could make it less confusing for other developers.

MatanBobi commented 1 year ago
const MyVG = () => (
    <svg width="24" height="24" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
      <path fill-rule="evenodd" clip-rule="evenodd" d="M7.14645 7.14645C7.34171 6.95118 7.65829 6.95118 7.85355 7.14645L11.9978 11.2907L16.1421 7.14645C16.3373 6.95118 16.6539 6.95118 16.8492 7.14645C17.0445 7.34171 17.0445 7.65829 16.8492 7.85355L12.7049 11.9978L16.8492 16.1421C17.0445 16.3373 17.0445 16.6539 16.8492 16.8492C16.6539 17.0445 16.3373 17.0445 16.1421 16.8492L11.9978 12.7049L7.85355 16.8492C7.65829 17.0445 7.34171 17.0445 7.14645 16.8492C6.95118 16.6539 6.95118 16.3373 7.14645 16.1421L11.2907 11.9978L7.14645 7.85355C6.95118 7.65829 6.95118 7.34171 7.14645 7.14645Z" />
    </svg>
  )

  const MyComponent = () => {
    return <select>
      <option><MyVG />A</option> {/* [object Object]A */}
      <option><MyVG />B</option> {/* [object Object]B */}
      <option><MyVG />C</option> {/* [object Object]C */}
    </select>;
  }

  it("should match the option", async () => {
    const { renderBare } = rtlKit();
    renderBare(<MyComponent />);

    expect(screen.getAllByRole('option', { name: /b/i })).toHaveLength(1);
  });

I'm not sure I'm following. Do you want to match the element in this case or not? Because IMO there's a bug in the code here and this element shouldn't be matched because [object Object] doesn't really mean anything to the user.

JesusTheHun commented 1 year ago

@MatanBobi sorry, it is not very clear. Looking back I think you are right, this code is bugged because option elements don't accept non-text children. I have been away from this lib for a while and will get back to it very soon. I'm going to close this and re-open if I find anything. Thank you for your time.