Add getById and getByClass

[Victorcorcos]

Querying by id and classes is the Hello World of the frontend querying and the library doesn't allow that for now.

What about creating the getById and getByClass on the tests?

Restricting the frontend id querying is not improving, it is quite the opposite, what is really does is decrease the capacity of the library.

Placing a lot of data-testids on the code pollutes it, because it places unnecessary tags on html codes in terms of code functionality. The code becomes polluted with tags only related for testing.

I am in favor of TDDs, but adding things in the code (or worse: on html tags) only for testing is exaggerated. A good TDD will improve the code with more organization! A bad TDD will add things on code only related for testing purposes.

Some people can argue and defend the usage of data-testids on the code for whatever reason, but what I find uncomfortable is to disable for everybody the most basic querying on DOM elements.

[juandiegombr]

You have other ways to query your elements better than class or id without using data-testid.

You can get your elements by text and also give it a try to queries like getByRole or getByLabelText. That will force you to make your app more accessible. It is always something to care of!

[Victorcorcos]

Thank you for the quick response, guys!

@juandiegombr The problem with using getByRole and getByLabelText is that the role and the text can be the same for different html elements. In terms of functionality, sometimes we can display the same text message in two different places of the DOM and it is not a good approach to restrict the functionality because of the testing suite. On the other hand, on a well designed frontend DOM structure, the #id of the elements will be unique, and this will be the advantage of the getById.

@raoufswe The problem with using the approach with querySelector is that it leave behind the features of the getBy... matchers.

I was already aware of these breakthrough approaches to resolve this issue... but I really think it is not the ideal solution because of the issues that I mentioned above.

In addition, using the getById on the test suite will force the elements to have unique ids, which is a good design of html elements. It will intuitively force the code to be more organized. That is a good TDD in my opinion.

The HTML id attribute is used to specify a unique id for an HTML element (the value must be unique within the HTML document).
https://www.w3schools.com/html/html_id.asp

That's why I am in favor of creating a getById and/or getByClass.

[weyert]

Nothing stops you to write you own query helper for getById or getByClass. Personally, I haven't found the need for them.

Can you give an example where you have the same text twice? Most of the time I sorted this in different ways then cluttering up the HTML by adding id-attributes

[Victorcorcos]

@weyert

Can you give an example where you have the same text twice?

Facebook for example we have the same user name in different places of the same page.

As I said, sometimes we can display the same text message in two different places of the DOM and it is not a good approach to restrict the functionality because of the testing suite.

[kentcdodds]

Precisely, if you really need them, you can add custom queries and using querySelector is very easy to do anyway so you can feel free to do that if you want.

We're not going to add more direct support for querying by ID or class name than we already have so I'm going to close this issue.

Just saw your message come in. For something like that you can scope your query using within. That's what the user subconsciously does anyway.

[niltonvasques]

I was searching here also for the solution for this problem.

I'm against to introduce hacks in the app code to write the tests. Because it will pollute the codebase with useless code that are only used on the tests.

This will also increase the bundle pack size in a few bytes, what will affect the user page load in some degree. (I know that we can use another hack to remove the react-testing-library hacks during the the assets precompilation).

Is good that we have ways to write our own finders to bypass this restriction, however since this approach is the standard for this library, this will leads many projects to go with the standard approach and by consequence adding all these problem discussed here.

[weyert]

Facebook for example we have the same user name in different places of the same page.

Personally, I never test the whole page and try to match the same text in such manner in unit tests. The only case where it really occurs is when using Testing Library in Cypress. Which is then easily solved by using within-query.

I don't see how adding id-attribute for texts like in the above example is any stranger than using test-id which is clearly the reason why the id's are being added to ease the testing. Personally, I am not a huge fan of test-id throughout my code so I am using it as last resort really. Luckily, I have been able to avoid using those in most cases.

[Victorcorcos]

I don't see how adding id-attribute for texts like in the above example is any stranger than using test-id

Adding ids to the elements is not only designed for testing things, it will improve the html elements, enabling them for more succinct and readable querying later on. That's why I said that adding getById will intuitively force the code to be more organized. That is a good TDD in my opinion.

Adding data-testid will pollute the code because the idea behind this is just to add attributes on html elements just for testing purposes. Querying by data-testid later on on the code will just make the code more confusing, because it is not the purpose of the data-testid.

That's why I really believe that: id > data-testid

Also, I used the Facebook as an example just to demonstrate that a well-known, well-designed and consolidated website can contain the same text in different parts of it, imagine the smaller sites then.

[juandiegombr]

I use testing-library in integration tests for whole pages. I usually have a few elements with the same text but there's always forms to get them without querying by class or by Id. Sometimes there is not a one line selector but It doesn't make it more difficult to read, instead it has other benefits as making more explicit and semantic your tests.

As I said in my comment before, If we have accessibility in mind, that two elements with the text of Victor should be in different ARIA landmarks and you can get those areas by role and the get the specific element using within.

[Victorcorcos]

@juandiegombr

Another issue is when we use third party libs to fill the HTML with elements dynamically and we have no control over what and how it will show... in addition, this third party library will eventually have new versions and change the way it display things on the page.

On this case, the only possible way I can find to get the element the third party library renders is by injecting the ugly data-testid, which pollutes a lot the code. To make the tests less likely to have to alter the tests over and over again when the library is upgraded, since we have no control. And I can show you why...

Since we don't have the getById, queryById and etc... we have two choices:

1. Use some getByText or getByRole which can be completely useless if the third party library version is updated and changes the way the elements are shown on the page.
2. Use the data-testid + getByTestId, which makes the code polluted, but will make the tests more consistent without requiring to do maintenance later on. It will decrease the likelihood of doing maintenance just to adapt to the third party library.

On the other hand, most libs that generates automatically html elements on the page already have a property called id which will be used internally by the library to create the elements. For example: https://www.telerik.com/kendo-react-ui/components/upload/api/UploadProps/#toc-id

The new getById on this case is the optimal solution in my opinion, because it will maintain compatibility with new versions of the third party library and it also doesn't pollute our code, since the id property is already being used internally by the library to build components, we just need to provide the id value and everything will work fine.

We will have a better control on the tests without polluting the code.

Each case is different and the getById can be the ideal solution for particular cases like this one.

Removing the getById from the library is just decreasing the capacity of the library to handle different scenarios.

[Victorcorcos]

I took a time to investigate and discovered how to accomplish that

import { configure } from '@testing-library/dom'
configure({ testIdAttribute: 'id' })

That's all I needed.

[r3wt]

you can't use waitFor with container.querySelector() 👎

[Victorcorcos]

@r3wt I upgraded the version of the react-testing-library and looks like it is not possible to use this approach with configure that I discovered anymore.

Could you enable it again guys? @kentcdodds

Thank you very much!

[marcosvega91]

Hi @Victorcorcos the behaviour is not changed, could you please add more details about your problem?
If you can create a case on codesandbox should be fine

Thanks

[gtsop]

With all the respect to the library authors/maintainers/contributors, as a first time user of this specific testing library I really feel like this project tries to enforce me to write code the way the testing library wants rather how I want. I do not object to projects promoting good coding standards, that is desirable.

My markup was like this:

 <li>
      <a className="link" href={link.url}>
          {link.title}
      </a>
    </li>

Following the prompts of this project, I used this line in my tests:

expect(screen.getByText("some text").href).toContain("/some-href")

And due to a spec change, it the component became like this:

 <li>
      <a className="link" href={link.url}>
        {link.icon && (
          <img className="link__icon" src={link.icon} alt="" />
        )}
        <span className="link__text">{link.title}</span>
      </a>
    </li>

Now my test code is broken because I needed to wrap the text in a span element. Had i used a class selector, I wouldn't need to change my tests at all.

According to this project I should go back to my markup and add a bunch of data-testids (which I am not inherently against) and fix all my test cases to use getByTestId. Not good times. How will I avoid this in the future? I will put a data-testid on everything, increase the amount of markup I write on my component and get rid of the getByRole/getByText selectors in my code (so what's the purpose of these selecotrs anyway then?)

Is there a better way to structure my code to avoid this pain? Maybe there is. I can't think of it, sorry, maybe I am not as good at it as others. And I assume I can't rely on the help of volunteers on this forum to help me along the way as I build my app. So i have three options:

1. Write custom code to accomplish this simple task and maintain it myself, essentially hacking around the library and having code that may not work in the future, since these aren't supported solutions (as one person demonstrated before)
2. Accept a coding style in a context that doesn't make sense according to my judgment just to be able to test my code easily
3. Replace my testing library.

You see, I don't really wish to do any of those, all of these three options have a negative impact on my productivity. What I will probably end up doing is suck it up, deliver my current project and never touch this library again, which is sad because I think it has some serious benefits over some others.

Edit: And all of that just because it doesn't fit with the philosophy of the project. Because nowhere did I see that this is not implemented due to code complexities or lack of resources.

[r3wt]

Just fyi to the maintainers, testing table elements is horror in this library. could all be remedied by providing css/id/class selectors

[coler-j]

How do you test a component that does not have text without being able to use class name etc? I.e. something that renders like this?

 <button
    aria-label="close dialog"
    class="wv-close--large"
    />

[brendan-donegan]

@coler, for that case you use `getByRole('button', {name: 'close dialog'});`, this will use the aria label to find it

…

On Mon, 16 Nov 2020, 9:42 pm Coler, ***@***.***> wrote: How do you test a component that does not have text without being able to use class name etc? I.e. something that renders like this? <button aria-label="close dialog" class="wv-close--large" /> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#683 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASLMLXZAVYLVP6BALOICGLSQGMD5ANCNFSM4NOR3EHQ> .

[r3wt]

@coler-j use getByLabelText('close dialog'), which supports both label and aria-label matching. we are using this alot in our apps at work, and it works well with material-design (unlike inputs rofl)

[coler-j]

@brendan-donegan @r3wt thanks guys, I found that a bit after commenting.

Unfortunately stuck on v 9.5 where it doesn't seem to work. Working on getting my org updated.

Thanks!

[tiagokall]

You have other ways to query your elements better than class or id without using data-testid.

You can get your elements by text and also give it a try to queries like getByRole or getByLabelText. That will force you to make your app more accessible. It is always something to care of!

This library shouldn`t force anything but provide as many helpers as possible, I'm also here stuck figuring out if I add the useless test-ids since there's no better option.

[jbmilgrom]

The library gives access to normal DOM selectors so can do this

    const { container } = render(<Skeleton height={10} paragraph={4} />);
    expect(container.getElementsByClassName('skeleton-paragraph-row').length).toBe(4);

[BlackGlory]

I want to express my opinion here sincerely. The text below may offend others, sorry.

getByText and getByLabelText are jokes, in order to fix these jokes, there is queryByTestId: a bigger joke.

For content such as text and aria-label that are easy to change, it is quite inappropriate to use them as selectors (and you guys even use regular expressions to increase the complexity of these selectors). One modification may cause a large number of selectors to fail, and then cause the test cases to fail.

This is really not TDD.

[gtsop]

The library gives access to normal DOM selectors so can do this

    const { container } = render(<Skeleton height={10} paragraph={4} />);
    expect(container.getElementsByClassName('skeleton-paragraph-row').length).toBe(4);

True, but the project prompts to use screen to query elements and these dom selectors aren't defined on it:

[gtsop]

@BlackGlory I have the same problem as you (as posted earlier in this ticket), but turning on the heat against the developers of an open source library while asking them to support a feature... surely isn't beneficial.

[r3wt]

@gtsop why would it not be beneficial? These situations are what lead to forks in open source. Either the maintainers succumb to pressure from the community and provide this common sense feature, or the community will have had enough and fork the library. That?s just how it goes in open source. Everyone here while frustrated has remained respectful, even though the library maintainers design choices are effecting us in negative ways.On Tue, Jan 19, 2021 at 5:23 AM gtsop <notifications@github.com> wrote: @BlackGlory I have the same problem as you (as posted earlier in this ticket), but turning on the heat against the developers of an open source library while asking them to support a feature... surely isn't beneficial. ?You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.

[gtsop]

@gtsop why would it not be beneficial? These situations are what lead to forks in open source. Either the maintainers succumb to pressure from the community and provide this common sense feature, or the community will have had enough and fork the library. That?s just how it goes in open source. Everyone here while frustrated has remained respectful, even though the library maintainers design choices are effecting us in negative ways.On Tue, Jan 19, 2021 at 5:23 AM gtsop notifications@github.com wrote: @BlackGlory I have the same problem as you (as posted earlier in this ticket), but turning on the heat against the developers of an open source library while asking them to support a feature... surely isn't beneficial. ?You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.

I don't think we disagree here. The only thing you potentially missed is BlackGlory's characterization of getByText, getByName and getByTestId as jokes, with a clear disclaimer that his opinion might offend others. It didn't seem that much respectful and that's my only point here, the criticism is welcome and beneficial as you expressed.

[hpachy]

Hello,
I crossed the same problem as some of you I tried the @Victorcorcos 's advice which helped me alot I did a function like the following code :

import { configure } from "@testing-library/dom";
export default function testAttribut(toDo, tag) {
  configure({ testIdAttribute: tag });
  return toDo();
}

const nav = testAttribut( () => getByText("totoDiv"), 'id')

But after few more research I discovered the amazing function which is

querySelector

from container, it's pretty useful (work like document.querySelector) ex:

// const { contain } = render(<Component {...props}/>)

const iframe = container.querySelector("iframe");
const divId = container.querySelector("div#myId");
const divClass = container.querySelector("div#myClass");
const divIdInDivClass = container.querySelector("div#myClass > #otherId");

Some of you probably noticed the function but no mention as potential solution

Hopefully some of you will find it useful, have a good day, regard!

[redjpark]

most useful!

[r3wt]

Hello,
I crossed the same problem as some of you I tried the @Victorcorcos 's advice which helped me alot I did a function like the following code :

import { configure } from "@testing-library/dom";
export default function testAttribut(toDo, tag) {
  configure({ testIdAttribute: tag });
  return toDo();
}

const nav = testAttribut( () => getByText("totoDiv"), 'id')

But after few more research I discovered the amazing function which is

querySelector

from container, it's pretty useful (work like document.querySelector) ex:

// const { contain } = render(<Component {...props}/>)

const iframe = container.querySelector("iframe");
const divId = container.querySelector("div#myId");
const divClass = container.querySelector("div#myClass");
const divIdInDivClass = container.querySelector("div#myClass > #otherId");

Some of you probably noticed the function but no mention as potential solution

Hopefully some of you will find it useful, have a good day, regard!

You must have missed my post above, container.querySelector doesn't work with waitFor, so while it will work for getting the element, there is no way to ensure the element is there as normal using await waitFor( some query )

[hpachy]

You must have missed my post above, container.querySelector doesn't work with waitFor, so while it will work for getting the element, there is no way to ensure the element is there as normal using await waitFor( some query )

I didn't cross the same problem as you so I can't really help you but I use to use fireEvent now that trigger some of my side effect. Before when I was using the enzyme library I did stuff like await act(async () => { stuff}) .... and now I do not need to use await any more. FireEvent doing it for us more simple ;) hopefully it's will help you

[lucasgcb]

I'm legitimately still trying to understand how the unique ids in markup are implementation code, thus bad to test with, while data-testids, which are exclusively for my source code that the library recommends I get an extra dependency to remove afterwards, are somehow not implementation details.

What gives? Why is there a FAQ telling me to deal with it, instead of simply explaining why it's better to introduce these changes to my codebase? It's legitimately concerning because if there's a good reason for it, which it certainly does, then it's lost in pretty much everyone if you slightly glance over this issue page.

[adamchenwei]

Instead of disallowing it, getById should be added with the library but with a warning indicate that its an "anti-pattern".
Why? Because not everyone uses this library start from scratch and in my case, an app has baggage using massive amount of ids as QA dom pickers and app render identifier. a mess. Sametime, we also got deadlines, so a compromise that always gives a warning will allow devs constantly reminded to get it removed when we can, instead having to spend time on finding a hacky alternative to meet deadline. By allowing such slack, devs now have more time to address issues in piecemeal.
Its a more softer and more progressive approach, what u think? @kentcdodds

[kentcdodds]

You can do this:

document.getElementById('id')
document.querySelector('.class-name')

We're not adding an API to abstract that away. Just do that.