Skip to content

RFC: Test selectors - #221

Open
bvaughn wants to merge 3 commits into
mainfrom
test-selectors
Open

RFC: Test selectors#221
bvaughn wants to merge 3 commits into
mainfrom
test-selectors

Conversation

@bvaughn

@bvaughn bvaughn commented Jun 6, 2022

Copy link
Copy Markdown
Contributor

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

View formatted RFC

Comment thread text/0000-test-selectors.md
createTestNameSelector('LoginFormButton')
```

## `createHasPseudoClassSelector()`

@motiz88 motiz88 Jun 7, 2022

Copy link
Copy Markdown

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
Copy Markdown

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.

@bvaughn bvaughn Jun 7, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor 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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
Contributor 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
Copy Markdown

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
Copy Markdown

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.

4 participants