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

RFC: Test selectors #221

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

RFC: Test selectors #221

wants to merge 3 commits into from

Conversation

bvaughn
Copy link
Collaborator

@bvaughn bvaughn commented Jun 6, 2022

This proposal introduces a set of APIs to help with automated testing of React applications.

View formatted RFC

createTestNameSelector('LoginFormButton')
```

## `createHasPseudoClassSelector()`
Copy link

@motiz88 motiz88 Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshedding, but this one is the only selector whose name doesn't make intuitive sense to me. Maybe something like createHasContentsSelector could work?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I meant to propose createContainsSelector.

Copy link
Collaborator Author

@bvaughn bvaughn Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that the naming of this one is a mouthful. The concept (and so the name) is inspired by CSS pseudo-classes. I don't feel strongly about this name being the one we must have though. The important thing is that the name makes it clear that this one works differently than the others: It doesn't select the element(s) at the end of its nested selectors; it just refines the criteria for where it's at in the higher level selection tree. (These words are not a very clear explanation of what it's doing, sorry.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not strongly opposed to the name createContainsSelector exactly, but I do worry it's potentially confusable with createTextSelector (since people might think of elements as containing text).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the semantics close enough that we can make them both overloads of createContainsSelector? i.e. pass a string to Contains -> get the Text behaviour. (Then it's a separate design choice whether to keep the Text selector around separately for convenience/readability.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the closest CSS analogue is :has(), right? So maybe createHasSelector, except that it scans as "has selector" which is awkward. As a compromise from there, createHasElementSelector?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last one of this batch of ideas: createHasMatchSelector

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I jokingly mentioned the lookahead name, but there is a pretty strong similarity. Of the names we've discussed, I think maybe that one gives the strongest indicator of what it does / how it works relative to some other established concept.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2 cents on "lookahead", while we're here, is that it may be an established term, but still seems niche / context-specific. It's also more imperative than declarative, if that makes sense: it tells the selector matcher what to do instead of what element to look for.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, "Lookahead" would be an improvement over "HasPseudoClass" for me as a reader but I'd still worry about it being too opaque to a general audience.

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

Successfully merging this pull request may close these issues.

None yet

4 participants