Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poll: New findByText() helper? #85

Open
cibernox opened this issue Jul 4, 2017 · 7 comments
Open

Poll: New findByText() helper? #85

cibernox opened this issue Jul 4, 2017 · 7 comments

Comments

@cibernox
Copy link
Owner

cibernox commented Jul 4, 2017

After migrating a few codebases to async/await and ember-native-dom-helpers, the single most annoying pattern to transform is selectors that use the :contains("text") pseudoselector from jQuery.

I proposed supporting a subset of sizzle pseudoselectors in #32, but I kind of agree that we shouldn't, because even those selectors are going to be deprecated in jQuery itself.

The question is wether or not we want to add some sort of utility to make finding elements with a certain text easier. A tentative method and signature could be findByText('.selector', 'text', context?).

Now vs this helper:

findAll('h1').find((el) => el.textContent.includes('Tomster')) vs findByText('h1', 'Tomster')

The pros are obviously, ergonomics.
The const are one more helper to learn and arguably by making it more convenient we are encouraging a testing pattern that many people see as dubious or harmful (test selectors could be a better)

@RuslanZavacky
Copy link

RuslanZavacky commented Jul 4, 2017

It's like full-text search, maybe something like match('h1', 'Tomster'), matchAll('h1', 'Tomster') ? But generally, I like the idea, to simplify this action, as its quite common :)

@cibernox
Copy link
Owner Author

cibernox commented Jul 4, 2017

In case we implement this I think I'd rather have findByText/findAllByText for parallelism with find/findAll.
match makes me think in regular expressions, but perhaps it's just me.

The most important question is wether we should add this helper or by doing so we would be encouraging a bad practice.

@Turbo87
Copy link
Contributor

Turbo87 commented Jul 4, 2017

IMHO a helper like that can make sense when you haven't adopted something like test selectors yet, but as you mentioned having it available directly in this addon would suggest bad patterns to our users.

I think it would make sense to mention in the documentation how it would be possible to implement such a helper function yourself, even including the code snippet, but not actually add the helper function itself to the project. That way we can add a disclaimer to the documentation that this is considered an anti-pattern and link to better patterns.

@RuslanZavacky
Copy link

@Turbo87 I kinda agree with you on this one, and I've checked, that I am almost not using these selectors. But the places where they are used, are not related to tests.

On another hand, I'd say we shouldn't be limiting people and not giving them some handy, simple functions. In the end, everyone then will re-implement what @cibernox wrote in their own codebase, which makes no big sense.

Don't think of this library as only used for tests, it has much bigger potential to be used in the main code base, where test selectors can't be added for example.

@cibernox
Copy link
Owner Author

cibernox commented Jul 4, 2017

@RuslanZavacky This library is only intended to be used in tests. Actually I believe you cannot even import any of this functions from outside tests.

@RuslanZavacky
Copy link

@cibernox indeed, sorry. I've specifically re-implemented focus/blur as functions on the app level, to be consistent between tests & app itself :)

@tomwayson
Copy link

I found this issue b/c I was wondering if anyone had implemented selectors similar to those in react-testing-library. I've bought into the guiding principles behind Kent C. Dodd's Testing Library and would love it there were ember test helpers that would help me prioritize querying elements by role and accessible text.

I'm wondering if anyone else would be interested in an addon like ember-testing-library that wraps those @testing-library/dom fns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants