New rule: prefer-user-event

[timdeschryver]

When testing-library/dom-testing-library#616 gets merged, userEvent will be part of DOM testing library.

We could create rules for events on userEvent that are preferred over their fireEvent counterparts.

Before we start implementing rules, there are some thing we need to clarify first:

• Kent already mentioned click would be a good candidate, are there any others that could benefit from this rule? I think hover, unhover could also be included in within this rule.

• Should it be one rule for all events, or a rule per event? The latter would allow us to disable/enable specific events... but we could also create a config for it...

[Belco90]

Nice proposal Tim.

• To be honest, click was the only obvious one that came to my mind. I guess we will need to check proper user-event docs or wait for testing library docs to be updated to make sure about which methods we should cover.
• My personal preference would be a single rule to encourage userEvent methods over fireEvent ones. As you mentioned it could be useful to have rule per event method, but we can get that with proper rule config. I think with a single rule is clearer that userEvent wraps fireEvent. Any other potential cons a bout having this in a single rule?

[gndelia]

Should it be one rule for all events, or a rule per event? The latter would allow us to disable/enable specific events... but we could also create a config for it...

I'd go for a single rule to prefer one over the other. Kent mentions some stuff that you might only be allowed to do with the primitive, perhaps we could let the user configure for those methods that are present in both which ones should be allowed to be used with fireEvent?

[Belco90]

We should keep am eye on this issue for having more info/context testing-library/dom-testing-library#616 (comment)

[timdeschryver]

That's a good idea @Belco90

[nickserv]

The merge into dom-testing-library has been cancelled, but it would still be nice to have this rule to inform users about when they should be using the user-event package.

[gndelia]

@nickmccurdy I've read about the recommendation of using user-event instead of fireEvent because it better represents how the browser really behaves - and that fireEvent is perhaps on a lower level.

I like the idea of the rule, but could you point out some docs or relevant information to further understand the difference between these two? I am not sure if there are stuff that can only be achieved with either, or if the rule should just go with a simple "if using click on fireEvent, recommend using userEvent instead".

Thanks

[nickserv]

I like the idea of the rule, but could you point out some docs or relevant information to further understand the difference between these two?

For the most part, it's adding related event calls from browsers to make tests realistic, along with adding some convenience APIs to make calling these events easier. Overall the goal is to make testing like a user both easier and more accurate than calling low level fireEvent methods. See the user-event API documentation for more detailed information.

I am not sure if there are stuff that can only be achieved with either, or if the rule should just go with a simple "if using click on fireEvent, recommend using userEvent instead".

For the most part, we should be recommending any rules in fireEvent that are used in user-event APIs with these higher-level user-event APIs. There are exceptions for unsupported events, which I'd like to allow by default, and having to customize how existing supported events are called at a lower level with fireEvent, which you could do by simply disabling these rules inline.

[nickserv]

@Belco90 I added this to the v4 milestone, do you think it's a good idea or would you rather wait until more events are added to user-event? We have already added a bunch recently though.

[Belco90]

@Belco90 I added this to the v4 milestone, do you think it's a good idea or would you rather wait until more events are added to user-event? We have already added a bunch recently though.

I'd like to include this, but I'm afraid this could delay v4. Do we have clear then about what we want to include in this rule? If so, I'd include it in v4 as optional. If we have everything but this ready for v4 I'd release it and then include this in a potential v4.1. Sounds good?

[nickserv]

Do we have clear then about what we want to include in this rule?

I thought we agreed on adding one new rule that would check all events that should be used with user-event by default, with a config to toggle them individually. That being said, I'm not sure if we'd want to have distinct options for banning the user-event event vs just allowing both (most ESLint rules seem to use string literals for this).

If so, I'd include it in v4 as optional. If we have everything but this ready for v4 I'd release it and then include this in a potential v4.1. Sounds good?

👍

[Belco90]

LGTM

[gndelia]

I can take this one during the week if no one else is working on it.

I will probably come back with questions as I implement it😅

[Belco90]

Assigned to you then. Thanks :)

[gndelia]

That being said, I'm not sure if we'd want to have distinct options for banning the user-event event vs just allowing both (most ESLint rules seem to use string literals for this).

I was thinking that the dev could configure the methods that they want to allow as an array of strings, and eslint would allow using those methods from both apis(userEvent or fireEvent).

What are your thoughts?

[timdeschryver]

I may have misunderstood you, but would this mean the user should add all the events they want to use?
For example ['keydown', 'click', 'type'] ?

If that's the case, I don't think that will work because there are overlapping events, e.g. click.

[gndelia]

My idea was that, by default, I will map some methods from fireEvent to methods in userEvent. For those, the rule will recommend to use the ones in userEvent. For those that don't have an equivalent (yet), the rule will do nothing.

Now, in addition to disabling the rule inline, I was thinking the user could register methods from fireEvent in the configuration as allowedMethods - that way the rule ignores those.

[timdeschryver]

Aaah, got it 👍
That looks good to me @gndelia

[gndelia]

I think the key point of this PR is how we map methods from fireEvent to userEvent

This is my first draft. I'm posting it so I can gather feedback about it while I continue developing the whole rule.

the key is the fireEvent method. the value is an array of possible methods from userEvent that might be the best match.

export const MappingToUserEvent: Record<string, UserEventMethodsType[]> = {
//  blur: ['blur'],
  click: ['click', 'type', 'selectOptions', 'deselectOptions'],
  change: ['upload', 'type', 'clear', 'selectOptions', 'deselectOptions'],
  dblClick: ['dblClick'],
//  focus: ['focus'],
//  focusin: ['focus'],
//  focusout: ['blur'],
  input: ['type', 'upload', 'selectOptions', 'deselectOptions', 'paste'],
  keyDown: ['type', 'tab'],
  keyPress: ['type'],
  keyUp: ['type', 'tab'],
  mouseDown: ['click', 'dblClick', 'selectOptions', 'deselectOptions'],
  mouseEnter: ['hover', 'selectOptions', 'deselectOptions'],
  mouseLeave: ['unhover'],
  mouseMove: ['hover', 'unhover', 'selectOptions', 'deselectOptions'],
  mouseOut: ['unhover'],
  mouseOver: ['hover', 'selectOptions', 'deselectOptions'],
  mouseUp: ['click', 'dblClick', 'selectOptions', 'deselectOptions'],
  paste: ['paste'],
  pointerDown: ['click', 'dblClick', 'selectOptions', 'deselectOptions'],
  pointerEnter: ['hover', 'selectOptions', 'deselectOptions'],
  pointerLeave: ['unhover'],
  pointerMove: ['hover', 'unhover', 'selectOptions', 'deselectOptions'],
  pointerOut: ['unhover'],
  pointerOver: ['hover', 'selectOptions', 'deselectOptions'],
  pointerUp: ['click', 'dblClick', 'selectOptions', 'deselectOptions'],
}

Those events that do not appear as a key is because they are allowed / don't have a matching method

Thoughts?
On a side note, I added the events blur and focus from user-event because there are implementations for both (see blur and focus), but I noticed these are not exported. Is this because they have not been released yet? They are commented, but I guess they won't be implemented.

As of how I came up with this list, I basically checked the implementation of each user-event and took the events from fireEvent to map them. I assume for each fireEvent there are multiples possibilities from userEvent so that's why there's an array.

There are other methods such as submit that have no mapping, but I am not sure if we should assume that the submit takes place after a user interaction - so, in that case, we would suggest for example click.

Let me know your thoughts. Thanks!

[nickserv]

I believe tab relates to focus and blur. I'm not sure if we're adding them as separate functions though.

The content of this looks okay to me, but I feel like we could use a more flexible data structure. There's a bit of repetition here where events has the same or very similar dependencies. Perhaps we could use a Map with some arrays of multiple events as keys, or a more normalized data structure. Worst case if it's not possible to change the structure, we could still make the source easier to understand by extracting common arrays into variables.

[gndelia]

The thing about extracting common arrays into variables is naming - I am not sure how to name them, while being next to the method they override makes it easier to spot where do they belong to. I was hoping the TS compiler to compensate for the lack of normalization, and I think this way it's more readable than combining different atomic sub-arrays

The userEvent methods are typed like this

const UserEventMethods = ['click', 'dblClick', 'type', 'upload', 'clear', 'selectOptions', 'deselectOptions', 'tab', 'hover', 'unhover', 'paste', 'blur', 'focus'] as const
type UserEventMethodsType = typeof UserEventMethods[number]

so even though it's not normalized, TS compiler will warn us whenever a name change from the original source, as well as prevent adding values that don't belong there

By using it as a map (like I've posted) it also makes it easy to access the value once I found the line that has a fireEvent call, as I can just do MappingToUserEvent[fireEventMethod] and access all the suggestions.

I am open to suggestions like the one you mentioned if the rest think it's better.

[gndelia]

Thoughts on the error message?

Given we can map the possible events, my first thought was to construct the error message with the possible values to use.

For instance

Prefer using userEvent.unhover() over fireEvent.pointerLeave()

I think that one looks nice. However, for cases with multiple options, it'd look like this

Prefer using userEvent.type(), userEvent.upload(), userEvent.selectOptions() or userEvent.paste() over fireEvent.input()

Thoughts?

[nickserv]

Good point about the keys. I guess we could still abstract variables to make the values easier to read. Also if TypeScript widens the array type to something like string[] and you don't want that, you can use as const to force the value's type to be used without widening.

[github-actions]

🎉 This issue has been resolved in version 4.0.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀