You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Just note dispatchEvent will only work if you enable bubbling and node is in the document
I think adding the container to the document should be fine normally, especially in Jest where the test files run in isolated environments. But I'm a little bit worried that we'd fill up the document with mounted components that never get unmounted. We could encourage people to have a beforeEach that cleans things up, but that'd be annoying and reduce the simplicity. We could simplify it with a import 'react-testing-library/autoclean' or something, but still that's a reduction in simplicity. So I'm not certain how to solve this problem...
It would be cool if we could have a utility that does actual events and also encourage people to just use .click() and .focus() methods native to DOM nodes. So I'd definitely like to explore this.
Alrighty, it looks like the best solution would be to implement this: react/react#2043
The latest effort behind it was closed because issues were raised which were never addressed. However, the changes appear fairly minimal and should give anyone who wants to give it a try a head start! Review the discussion in there and open a new PR that addresses those issues.
After further review (and tweets), it appears that this will be a fairly significant effort and perhaps even require a breaking change in React. So we'll have to wait until React 17. Which is unfortunately, but I think it'd be great if we could make this happen in time to get on the React 17 train (whenever that leaves the station...).
It would be cool if we could have a utility that does actual events and also encourage people to just use .click() and .focus() methods native to DOM nodes.
The native click and focus methods do not trigger the DOM side-effects (events) that React and the tested app rely on. Using them contradicts to the core principle of react-testing-library to use the components the same way the users would use them.
Ideally, there has to be a way to simulate real user input followed by its side-effects such as DOM state changes (inputs value/checked/selected, scroll position including overflow, urlbar location, fullscreen API unblock, video play API unblock), DOM events that tell the app about some of these changes, CSS hidden state changes (hover, transition state, animation state) from within the JS platform, but this contradicts to the JS/browser security policy of app sandboxing, thus renders the test automation tools that work from within the JS/browser platform (hi Cypress) helpless about the real user input simulation, leaving them with the hacks like dispatchEvent.
Unfortunately I don't see a solution from within the browser/JS sandbox. The limited options I see now are:
the app testing framework implements functions that emulate the browser behavior as close as possible (all the events, bubbling, internal state changes etc), but that still won't match the actual browser behavior with some events, hover, focus, animation, history, media, and so on as no-one can tamper with the browser internal state (please don't tell me not to test it, Cypress);
the app implementation framework like React limits what's possible to do on the browser platform, especially with positioning, scrolling, focus management, and all things listed above for emulation (this tears the creativity of all the CSS and DOM hackers down to zero which may not be practical in real world apps).
@kentcdodds Just a dumb question here.. I'm just trying to understand the reason behind this idea. Is there any downside of using TestUtils.Simulate? To me it just actually takes care of Synthetic events which is what React uses internally. Why do we move away from TestUtils.Simulate and use dispatchEvent?
@kentcdodds Yup, saw the comment from Dan as well.. But was just wondering, why we need to do so.. Is there any benefit for dispatchEvent or not, I'm not sure. May be we need to ask him in the same twitter thread.
I think we should add something like this to dom-testing-library instead. I was thinking about it for the initial release but didn't have time and didn't want to rush it.
Thanks to @jomaxx, this was resolved in #48 as well as we can hope for now. It really would be great to get react updated to either remove event delegation entirely or attach event handlers to the root of the react tree rather than the document. For now the solution we have is great though. Thanks!
Our tests are already passing on React 17's RC. Is there anything else we'd need to change to ensure stability in future React 17 releases? Based on my understanding of the blog post I couldn't find any major issues. Is it just extra code for React 16 compatibility?
@nickmccurdy are we sure that's the case? What I remember is that we're running our tests on latest, next and experimental. I think that next only holds the next minor version. @gaearon is there a way to know if experimental tag is based on React 17? Since the version number is based on a hash, I'm not aware of a way to know that..
I may have gotten this wrong, so please @nickmccurdy correct me :)
We added experimental because next became out of date.
The only change we did for React 17 was dispatching focusin as well when calling fireEvent.focus. That is in line with how we handle mouseEnter/mouseLeave though not very "vanilla". Ideally people would just use .focus but that only works with React 17 in jsdom@^16.3.0 so we went with the practical solution.
Do you also dispatch focusout for the "left" element?
No. We also don't dispatch mouseout on fireEvent.mouseEnter. fireEvent is only meant as a vanilla event dispatch not for simulating user events. We have a separate package for that.
.focus really is the best simulation of focus since it also takes care of dispatching blur related events, changing document.activeElement and checking if the element is even focusable.
Edit: .focus also makes sure that no focus event is dispatched if the target is already focused.
This is why we suggest people use @testing-library/user-event which may eventually be built-in.
I'm excited about the events being attached to the root. Because they were attached to the document we had to render everything into document.body for events to work properly (which is where the limitation came from). This meant we had to get creative with cleanup.
However, I'm actually pretty happy with what we have now and I don't see a reason to change. I think it still makes the most sense to actually render everything in document.body and have automatic cleanup like we do. I feel like this will result in the fewest surprises (especially when testing components that use portals) and it allows people to use screen.* which is all the queries scoped to document.body.
So while I think it's great that events are now attached to the root, I don't think that will change anything in React Testing Library. We ended up with a better solution anyway :)
Reacted by dan, Matan Borenkraout and Nicky McCurdy
Activity
kentcdodds commentedon Apr 2, 2018
Alrighty, it looks like the best solution would be to implement this: react/react#2043
The latest effort behind it was closed because issues were raised which were never addressed. However, the changes appear fairly minimal and should give anyone who wants to give it a try a head start! Review the discussion in there and open a new PR that addresses those issues.
Anyone want to do that?
kentcdodds commentedon Apr 2, 2018
After further review (and tweets), it appears that this will be a fairly significant effort and perhaps even require a breaking change in React. So we'll have to wait until React 17. Which is unfortunately, but I think it'd be great if we could make this happen in time to get on the React 17 train (whenever that leaves the station...).
sompylasar commentedon Apr 2, 2018
The native click and focus methods do not trigger the DOM side-effects (events) that React and the tested app rely on. Using them contradicts to the core principle of react-testing-library to use the components the same way the users would use them.
Ideally, there has to be a way to simulate real user input followed by its side-effects such as DOM state changes (inputs value/checked/selected, scroll position including overflow, urlbar location, fullscreen API unblock, video play API unblock), DOM events that tell the app about some of these changes, CSS hidden state changes (hover, transition state, animation state) from within the JS platform, but this contradicts to the JS/browser security policy of app sandboxing, thus renders the test automation tools that work from within the JS/browser platform (hi Cypress) helpless about the real user input simulation, leaving them with the hacks like dispatchEvent.
kentcdodds commentedon Apr 2, 2018
Hi @sompylasar,
Thanks for a good overview of the problem. What solutions do you have for us that make attractive trade-offs?
sompylasar commentedon Apr 3, 2018
Hi @kentcdodds
Unfortunately I don't see a solution from within the browser/JS sandbox. The limited options I see now are:
kentcdodds commentedon Apr 3, 2018
Thanks @sompylasar, I'm afraid that neither of those solutions is reasonable for this project so we'll have to try for the next best thing.
antsmartian commentedon Apr 3, 2018
@kentcdodds Just a dumb question here.. I'm just trying to understand the reason behind this idea. Is there any downside of using
TestUtils.Simulate? To me it just actually takes care ofSyntheticevents which is what React uses internally. Why do we move away fromTestUtils.Simulateand usedispatchEvent?kentcdodds commentedon Apr 3, 2018
Because Dan said so 🤷♂️ as noted in the first comment.
antsmartian commentedon Apr 3, 2018
@kentcdodds Yup, saw the comment from Dan as well.. But was just wondering, why we need to do so.. Is there any benefit for
dispatchEventor not, I'm not sure. May be we need to ask him in the same twitter thread.kentcdodds commentedon Apr 6, 2018
I think we should add something like this to dom-testing-library instead. I was thinking about it for the initial release but didn't have time and didn't want to rush it.
kentcdodds commentedon Apr 9, 2018
Really awesome progress here: testing-library/dom-testing-library#13 Thanks @jomaxx!!
I'll go ahead and leave this open until that gets released and we update react-testing-library to re-export it.
kentcdodds commentedon Apr 10, 2018
Thanks to @jomaxx, this was resolved in #48 as well as we can hope for now. It really would be great to get react updated to either remove event delegation entirely or attach event handlers to the root of the react tree rather than the document. For now the solution we have is great though. Thanks!
feat(queryByValue): add get/query by value (testing-library#35)
Merge pull request testing-library#35 from Gpx/fix-build
gaearon commentedon Aug 11, 2020
We're attaching to roots in 17.
https://reactjs.org/blog/2020/08/10/react-v17-rc.html
nickserv commentedon Aug 11, 2020
Our tests are already passing on React 17's RC. Is there anything else we'd need to change to ensure stability in future React 17 releases? Based on my understanding of the blog post I couldn't find any major issues. Is it just extra code for React 16 compatibility?
gaearon commentedon Aug 11, 2020
If tests are passing you're probably all good. :-)
I commented on this thread because binding to document was brought up as a limitation.
MatanBobi commentedon Aug 11, 2020
@nickmccurdy are we sure that's the case? What I remember is that we're running our tests on
latest,nextandexperimental. I think thatnextonly holds the next minor version.@gaearon is there a way to know if
experimentaltag is based on React 17? Since the version number is based on a hash, I'm not aware of a way to know that..I may have gotten this wrong, so please @nickmccurdy correct me :)
eps1lon commentedon Aug 11, 2020
@MatanBobi
nextpoints to 17.0.0-rc.0: https://www.npmjs.com/package/react?activeTab=versionsWe added
experimentalbecausenextbecame out of date.The only change we did for React 17 was dispatching focusin as well when calling
fireEvent.focus. That is in line with how we handle mouseEnter/mouseLeave though not very "vanilla". Ideally people would just use.focusbut that only works with React 17 injsdom@^16.3.0so we went with the practical solution.gaearon commentedon Aug 11, 2020
Do you also dispatch focusout for the "left" element?
eps1lon commentedon Aug 11, 2020
No. We also don't dispatch mouseout on
fireEvent.mouseEnter.fireEventis only meant as a vanilla event dispatch not for simulating user events. We have a separate package for that..focusreally is the best simulation of focus since it also takes care of dispatching blur related events, changingdocument.activeElementand checking if the element is even focusable.Edit:
.focusalso makes sure that no focus event is dispatched if the target is already focused.kentcdodds commentedon Aug 11, 2020
This is why we suggest people use
@testing-library/user-eventwhich may eventually be built-in.I'm excited about the events being attached to the root. Because they were attached to the
documentwe had to render everything intodocument.bodyfor events to work properly (which is where the limitation came from). This meant we had to get creative with cleanup.However, I'm actually pretty happy with what we have now and I don't see a reason to change. I think it still makes the most sense to actually render everything in
document.bodyand have automatic cleanup like we do. I feel like this will result in the fewest surprises (especially when testing components that use portals) and it allows people to usescreen.*which is all the queries scoped todocument.body.So while I think it's great that events are now attached to the root, I don't think that will change anything in React Testing Library. We ended up with a better solution anyway :)