feat: predicate query #973
Conversation
There was a problem hiding this comment.
Well we may not have the same definition for quick and dirty, looks rather clean to me ! What I think is missing for now is some documentation, an update of flow types and maybe an additional test case for within but otherwise the feature looks complete to me and could be released and iterated on later if needed
|
@pierrezimmermannbam yeah agree, it's pretty advanced POC 😂 I will try to revisit it soon, and add missing parts. |
mdjastrzebski
force-pushed
the
feat/custom-predicate-query
branch
from
5a41f40 to
3da4c4a
Compare
mdjastrzebski
changed the title
Codecov ReportBase: 94.94% // Head: 95.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #973 +/- ##
==========================================
+ Coverage 94.94% 95.06% +0.11%
==========================================
Files 45 46 +1
Lines 3088 3159 +71
Branches 457 462 +5
==========================================
+ Hits 2932 3003 +71
Misses 156 156
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@pierrezimmermannbam I've rebased and updated the PR with missing PR. Looking forward for your review. |
|
|
||
| Returns a host element matching a custom `predicate` function. | ||
|
|
||
| This query type is an escape hatch and should be used with care. In most cases you using the standard queries like `getByRole` or `getByText` will lead to test more-resembling user perspective. Use this query with care in rare cases where more flexibility is needed. |
There was a problem hiding this comment.
could we provide an example of a real scenario where we might need it and how to use it in that case? (like it's done for ByAccessibilityValue above)
There was a problem hiding this comment.
Good idea to add some examples there. One thought that I have atm is that these examples will be somewhat hacky, as if they were really good ideas we would make regular queries from them ;-)
There was a problem hiding this comment.
i see what you mean, but then RNTL only provides queries for common case scenarios. We could find a legit scenario that's very uncommon maybe? Like find all underline text? (so hard to find a rare but legit scenario haha ^^')
There was a problem hiding this comment.
Yeah, we can build examples to showcase how you can recreate some existing queries like *ByTestId.
I went back to the source, DTL docs, to check what examples they give, but they are basically implementing *ByTestId queries using some queryByAttribute function. All that seems like legacy stuff that has no good examples.
|
@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 *ByTestID queries. 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. |
|
I haven't had the need for it since we went for feature parity with RTL on accessibility related matchers. |
|
Same for me or other projects at my company, we don't remember having encountered such a scenario |
|
Closing as not really needed. Having this API would allow extra flexibility for users, but we haven't really found any good example for that. I can such case appears in the future we can came back to this PR. |
Implementation of custom queries based on option 3 from #971. This is different from custom query impl. in RTL, but because of the reasons explained in #971 I believe that their implementation has significant flaws, and we can provide a better API for such feature.
Summary
*ByPredicate(predicate: (instance: ReactTestInstance) => boolean)Test plan
Looking forward for your comments.