Repository navigation
Allow providing custom queries #971
Description
Describe the Feature
Allow to add custom queries in render call. Similar to RTL API.
Possible Implementations
Option 1 (RTL-like):
- Provide a new
queriesoption torender&withinfunctions as in RTL,. - Since RTL API requires you to import original
queriesif you want to retain them (and you generally want this), so we probably need to provider such export. - Correctly managing TS types might be the most difficult part of the task since we would want
renderandwithinoutput to be typed with the actual queries passed - Implementation might consider changing way we internally export the queries from
bindXxxQueriesto named particular queries, e.g.bindGetByText: (instance) => (text, options) => ReactTestInstanceor event introducing "unbound" queries (getByText: (instance, text, options) => ReactTestInstance) as in Dom Testing Library if that simplifies the exposed API / type management - Following RTL we should also export
makeQueriesinternal utils, so that users need only to preparequeryAllvariant of the query.
Option 2 (roll our own API):
- We could probably simplify the API surface by accepting list of additional queries only in
queryAllByverb and then invokingmakeQueryinternally. - We could automatically forward all built-in queries and only append new queries from the user. That we we would keep the strong typing for built-in queries with potentially more lax typing of user provided queries (more usage
anyfor query predicate / options). - This options seems to have much better trade off between lower increasing code complexity & niche popularity of the feature. However it is not API compatible with RTL.
Option 3 (expose generic query function accepting user predicate):
- We could expose a full set of queries (
get, etc) forByPredicatequery accepting a function(instance: ReactTestInstance) => boolean. - User then could define his own predicates and feed them to our
getByPredicate, etc queries. That would give user total flexibility in types of built queries: number of accepted params, typing, logic, etc - These custom queries would do thing like filtering our composite components for the user, so he can focus on query logic.
- Pros: least increase of code & API surface complexity, proper typing for all queries
- Cons: not compatible with RTL, but seems so good that we can suggest RTL to add it ;-)
- Quick and dirty POC implementation of this option: feat: predicate query #973
Related Issues
- Initial implementation attempt: Adding support for extending with custom queries #573 but the code is stale. Should be easy to re-apply. Some implementation and tests can be salved from that PR
Activity
mdjastrzebski commentedon May 4, 2022
@thymikee @AugustinLF @pvinis: what do you think?
V1shvesh commentedon Jun 21, 2022
@mdjastrzebski I'd love to contribute to this issue, is this up for grabs?
thymikee commentedon Jun 21, 2022
@V1shvesh sure :)
mdjastrzebski commentedon Jun 22, 2022
@thymikee @V1shvesh wait guys! In the issue description I've presented 3 options how we can get the problem solved. Let's have some discussion before jumping to code.
V1shvesh commentedon Jun 22, 2022
@mdjastrzebski I went through that as well. Would love to connect and get more context around this!
mdjastrzebski commentedon Jun 23, 2022
@V1shvesh In general we try to be as compatible in API terms as possible with RTL for easily the life of developers using RNTL. Also RTL is the original Testing Library, and they have put a lot of time and work, and re-work, in polishing their APIs.
That being said, their custom query implementation has number of problems:
getBy,getAllBy,queryBy,queryAllBy,findBy,findAllBy) instead of only makingqueryAllByand deriving other queries for it. RTL provides a helper for that, but still it's up to the user to do it.RenderResultis hence generically typed based on passed queries. Which complicates the typing for various usages.It seems that these problems come from importing their queries in the format of DOM Testing Library queries. Since we do not have our version of "DOM Testing Library", I think we should seriously evaluate pros and cons of alternative approaches to providing custom queries.
I've highlighted the few possible options in the issue description. All could provide the custom queries feature to our users, but it's a matter of deciding which one offers the best trade-offs between various aspects: ease of use, compatibility with RTL, etc.
V1shvesh commentedon Jun 28, 2022
Hey @mdjastrzebski,
I went through your POC draft #973, accepting the predicate logic from user really makes sense to me as a functional implementation for this API.
Can you shed more light on the ramifications of breaking compatibility with RTL? I'm really hopeful for having the same API in RTL as well!
V1shvesh commentedon Jul 1, 2022
@mdjastrzebski @thymikee If we can agree on the 3rd approach, shall I begin working on it?
mdjastrzebski commentedon Aug 19, 2022
@thymikee @AugustinLF @MattAgn @pierrezimmermannbam Could you give your feedback on the proposal (initial post)?
MattAgn commentedon Aug 22, 2022
@mdjastrzebski I love the 3rd option! As you say, considering the low amount of people using this feature, I think RNTL can have its own API
And as @V1shvesh said, the 3rd option also seems like the easiest to use and understand from a user perspective
V1shvesh commentedon Aug 22, 2022
@mdjastrzebski @MattAgn awesome then, I'll start working on this 👍🏻
pierrezimmermannbam commentedon Aug 22, 2022
I haven't used this feature ever but I don't really see the benefits of not retaining the original queries so I think it would be better if they were kept by default, but I may be missing a particular use case ? (maybe we should propose testing-library/react to change their api). I also think the 3rd option is a very good one at least for a beginning considering the differences in the internal implementation of queries, and maybe work progressively towards a common api for both librairies
AugustinLF commentedon Aug 25, 2022
To me the poor typing support of the first solution is a deal-breaker. I don't have a strong use case for custom queries, but since this seems to be a user requested feature, I do think the third one is the one that makes the most sense, make users life easier, while still making the API maintainable.
pierrezimmermannbam commentedon Nov 10, 2022
@V1shvesh have you been able to make some progress on this ?
mdjastrzebski commentedon Nov 10, 2022
@pierrezimmermannbam I went quite far with the protype implementation of options 3 in #973. You can start by reviewing that PR.
mdjastrzebski commentedon Dec 12, 2022
@thymikee @AugustinLF @MattAgn @pierrezimmermannbam feel free to express whether our users would actually benefit from this feature or it might be just unused piece of coda that we would have to maintain.
I treat this feature as an escape hatch for more advanced users who need more flexibility in querying items. We already have one escape hatch in form of
*ByTestIDqueries. The fact that RTL does have custom queries (although in a bit different shape but functionally equivalent) does not mean that we need to add this as well if there is no valid use case for it. I have my own doubts here, so looking forward for your take.pierrezimmermannbam commentedon Dec 13, 2022
Not sure as well that this is valuable, I don't really see use cases for this feature. I think I'd rather use regular queries and build a custom jest matcher if needed and not available in testing-library/jest-native. Custom matchers have the advantage of being available globally whereas this would be used in a single place if we don't have the possibility to extend the screen object (which would be way more complicated to implement)
mdjastrzebski commentedon Dec 29, 2022
Closing as we struggle to come up with any reasonable use case. We can come back to this idea if our users request it and have a reasonable use case for it.