callstack
RFC: UserEvent to throw errors in case element is disabled or not focusable
Describe the Feature
User Event should throw errors in case of given element is disabled in any way that prevents the interaction to run in order to improve DX.
OG User Event does throw errors in similar cases for clear(), and click()/pointer() interactions.
Note: This would be a breaking change, but according to User Event beta status would only trigger a minor change in the version.
Note 2: In the past we fireEvent was already throwing errors in the past when no enable event handler was found but that has been removed. Here are refs to the issue: https://github.com/callstack/react-native-testing-library/issues/469, and PR https://github.com/callstack/react-native-testing-library/pull/470
Possible Implementations
-
type()&clear()should throw helpful error when element is noteditableor not focusable due topointerEventprop on itself or any ancestor -
press()should throw helpful error when element is not pressable due topointerEventsprop on itself on ancestor.
These should not throw in case no event handler is found, as user interaction can invoke multiple events handler, hence any particular event handler is optional.
Related Issues
CC: @pierrezimmermannbam @MattAgn @AugustinLF @thymikee
Sounds great to me! I'm not sure i understand why we removed the error throwing in fireEvent, is it to test when touchable opacitys are disabled? if that's the case, we could promote more the helper toBeDisabled() instead
If I recall correctly the reasoning behind removing the error in fireEvent was that it was a test specific behaviour and we try to match production as much as possible. I'm not sure why we would want to change this for userEvent. If the OG userEvent does throw then it's definitely an argument in favour of throwing though.
Personally I like the fact that fireEvent doesn't throw because it allows me to test that my button is disabled by trying to click on it and verifying nothing happened. I'd do the same things for inputs. I'd argue it's the best way to test that it's disabled because what you really want is that user cannot interact with it so how best to test it than to try to interact with it?
For press I don't really know how we could throw errors because we go up in the tree if the element is disabled so we could throw eventually if all elements are disabled but I'm not sure it would be very valuable
@mdjastrzebski I would be curious to have your thoughts on how throwing would improve DX. Thinking of it it could be useful for debugging tests but because I want to be able to fire events on disabled elements I would not want this feature always. Maybe it could be an optional behaviour that could be configured through configure API? It would allow more flexibility and like you could enable throw while debugging test. The downside is that it makes the API more complicated and I don't know if many users would use it
Thinking more about it jest native marchers are a very good alternative to test disabled elements. The only downside is you need direct access to the TextInput but you also need it for type and clear anyway.
So now I think it's a good idea to throw. I'm just wondering now if the error message should recommend jest native marchers as a way to test that an element is disabled
I've done some experiments in RTL's User Event on how do they handle disabled elements:
- click() - no error 🙈
- type() - no error 🙈
- clear() - throws error if not enable or not focusable
BTW: you can tinker yourself in #1447 RTL experiments app.
If we wanted to refer to OG User Event for consistent, well-thought approach, then it's not there 🤷♂️ While there might be some logic with click not throwing error, there is no reason why type does not throw and clear does throw on disabled element.
Few thoughts that I have about throwing/no throwing errors.
- We should throw errors if user does invalid action, e.g. calling
type()on something other thanTextInput, or invoking any operation on composite component, as UE does not support it, and could behave incorrectly. - Invoking action on disabled element is kind of grey area, one might argue that this is valid, as users wants to check that disabled button does not cause event handler to fire, but another one may argue that this is invalid action as you shouldn't
typein disabled text input. There is some merit in both sides of argument. - From TTD perspective not throwing errors seems a slightly better option, but from diagnosing test issues throwing seems better.
I think I would throw when user uses type on non editable TextInput but never throw with press because you can have an enabled pressable around, although one may argue this is a strange pattern
I am also leaning to the direction that we should throw on invalid actions: e.g. typing or clearing disabled TextInput. OG UserEvent seem so be confusing in that regard, and warning users (by throwing error) would improve the DX as they would not have to figure out why their interaction is not working.