callstack
Update ByRole queries to only accept roles
Describe the Feature
In short, I want type the role as Role instead of string | Regexp
When using byRole queries, the role is typed as string |Â Regexp. What's the most problematic imo is to not have any autocompletion. It takes more time but mostly it can result in tests failing because of a misspell. This could be solved by using the same trick we use for fireEvent and we could keep the same type. However I don't think any string should be allowed but only valid roles. Also I don't see valid use cases for using regexp with role queries so I don't think it should be allowed. I don't know why it's supported though so I may be missing valid use cases.
What are your thoughts on this @mdjastrzebski @MattAgn ?
Possible Implementations
Related Issues
That seems like a good idea 🌟. We should guide the user towards proper roles.
Some points to consider:
-
RTL seems to accept
string, so we would introduce small discrepancy - I'm not too worried about it. - Such change would be a breaking change, so we need to put it on hold it for v13.
- Perhaps for the current major version we could print a deprecation/warning if user queries with Regex or string that is not a valid role.
- Remember there are two types of roles, classing
AccessibilityRoleand ariaRole. Both are supported but they have different values sometimes, we need to support both.
@pierrezimmermannbam feel free to proceed with point 3 if you have some capacity.
The thing is I'm not sure we could verify if it's a valid role. For that we'd need an array with the roles but RN only exports the type afaik and we can't do runtime validation based on a type. We could however add autocompletion for valid roles before the next major but it would mean some rework. We may need to figure out first when we want to release v13 to see if it's worth it
I agree with the idea of stronger types. @pierrezimmermannbam you mentioned checking roles a runtime, but isn't type checking enough of a safety net?
@MattAgn it is, I was referring to what @mdjastrzebski was suggesting, that we could issue warnings before publishing the next major
@pierrezimmermannbam , @MattAgn: Types vs runtime dichotomy is real, I've browser RN source but they do not seem to export runtime values for allow functions.
My idea about detecting it at runtime was intended for v12 improvement until we are ready to ship v13 with changes types which would be a breaking change. If that is not easily achievable, let's wait with typing change till v13.
I agree with you, and I don't think many users use roles that are neither AccessibilityRole and aria Role. And if they do, I guess it should not be too hard to fix their tests to fit the new typing
@pierrezimmermannbam @MattAgn reviving this old improvement idea. What about doing TS hack on suggesting the proper roles for v12, and then potentially restricting it for v13?
Yes that definitely already be a huge improvement, it's really the autocomplete that is missing rn. This is also fairly easy to do, I could open a pr this week