User-event: fireEvent.focus should be discouraged

Created on 21 Jun 2019  路  14Comments  路  Source: testing-library/user-event

Describe the feature you'd like:

Discourage fireEvent.focus(element)

Suggested implementation:

Prefer element.focus()

Describe alternatives you've considered:

Don't use existing API or call element.focus() internally.

Teachability, Documentation, Adoption, Migration Strategy:

fireEvent.focus(element) will not move the focus to the element i.e. document.activeElement !== element. element.focus() will result in document.activeElement === element and dispatch the focus event. fireEvent.focus will also work on non-interactive elements whereas element.focus does not.

Showcase of fireEvent vs imperative focus

Most helpful comment

That's a fair point. Maybe @eps1lon's original suggestion is the best we can do. Recommend that people use .focus() on the dom node rather than fireEvent.focus.

All 14 comments

DTL tries not to expose DOM nodes for direct interaction, because users can only interact via events. It sounds like the browser interaction simulation in the user-event package could make use of this, however.

Also we should probably start more heavily encouraging user-event for user interactions.

because users can only interact via events

They don't that's my point. The event system is just another implementation detail. If you're ok with using this one then we can close. I know I won't manually dispatch focus events because of the 2 drawbacks outlined. What I didn't mention is that the change of document.activeElement is important for simulating user interaction. If I fire a focus event and later a keydown on document.activeElement then I won't accurately simulate what's happening in a browser.

DTL tries not to expose DOM nodes for direct interaction

It only exposes DOM nodes as far as I can tell. Every query method I used so far returned an instance of HTMLElement with which I later interact by manually dispatching events.

But I guess I see a pattern here combined with testing-library/dom-testing-library#292. I'd hope you wouldn't expose this API in @testing-library/react because while an improvement over TestUtils.simulate it's still too much implementation detail IMO.

Feel free to close.

Thanks for clarifying. It sounds like events don't really represent what the browser does in many interactions. We should lean more on user-event and possibly re-export it from wrappers like RTL instead of fireEvent.

What is the current state of focus management in JSDOM? Does it do what describe?

What is the current state of focus management in JSDOM? Does it do what describe?

Works as far as I can tell and I couldn't spot any difference between JSDOM and browser environments for the same test. But I didn't check for blur events or focusin/out (I think the last two aren't available in react)

I guess manually dispatching events is still something we have to live with. Anything related to events should probably be done in an actual e2e test. For focus it's pretty simple to simulate a browser but something like a keydown comes with 1-2 pages of specification.

We should lean more on user-event and possibly re-export it from wrappers like RTL instead of fireEvent.

I'm liking the idea of encouraging user-event more. I'm conflicted on whether it's a good idea to package it and re-export though. I see pros and cons... I don't want to have a confusing situation for people. We definitely need to continue to expose fireEvent I think...

We definitely need to continue to expose fireEvent I think...

Having it used more I'm leaning towards this as well. There's still some benefit to mocking user behavior by dispatching events manually.

I think if you want to test actual user events you probably just want to jump straight into e2e testing in real browsers. Something like a mouse click will give you way more confidence if it happened in the browsers instead of a mocked sequence of DOM events.

But there's definitely some low hanging fruit like element.focus() instead of fireEvent.focus(element) or fireEvent.keyDown(document.activeElement) instead of fireEvent.keyDown(element).

Maybe add a guide to the docs that makes the case for and against fireEvent and to chose between fireEvent, user-event or cy.click().

I think a guide like that would be great 馃憤

Could it be as _simple_ as "Given an event, if available in user-event, use that. Otherwise, go with fireEvent" and then explain why events from fireEvent are still exposed anyway?

Yes, though including an explanation of why user-event is preferable and what it's actually doing would probably be good as well.

@kentcdodds I wouldn't re-export user-event for now. I think it's still not mature enough. We also have to figure out how to make it work well with other libraries like Vue.
I agree with having a section in the docs that explains when to use user-event and when to use fireEvent and what the differences are.

As for the focus issue, user-event doesn't expose a focus event. But if you click on an element, it will call element.focus on it and blur any previously focused element.

I wonder if we could add a focus to user-event for this a particular issue?

How would it look like? user-event tries to mimic what a user does. AFAIK users can't give focus to an element. They have to click on it or use tab

That's a fair point. Maybe @eps1lon's original suggestion is the best we can do. Recommend that people use .focus() on the dom node rather than fireEvent.focus.

We've added focus to testing-library/dom-testing-library#616

Was this page helpful?
0 / 5 - 0 ratings

Related issues

marquesm91 picture marquesm91  路  6Comments

piyushbajaj picture piyushbajaj  路  3Comments

beckerei picture beckerei  路  6Comments

kentcdodds picture kentcdodds  路  5Comments

MattyBalaam picture MattyBalaam  路  4Comments