feat: support TransitionEvent init properties

[VinceMalone]

What:

Add support for the init arguments that can be passed to TransitionEvent — propertyName, elapsedTime, and pseudoElement.

fireEvent.transitionEnd(node, {
  propertyName: 'opacity',
  elapsedTime: 100,
  pseudoElement: '',
});

Why:

This should be supported out-of-the-box, but jsdom doesn't currently support the TransitionEvent constructor https://github.com/jsdom/jsdom/issues/1781

Because window.TransitionEvent is undefined, window.Event ends up being used to create the event.

https://github.com/testing-library/dom-testing-library/blob/accb6cc60628cb6b5bd4d9be2ead41724995e5ae/src/events.js#L50

Which will, of course, ignore the transitionEventInit object.

How:

When the event is being created, look at the event type, if it's TransitionEvent, manually attach the aforementioned properties from eventInit.

Checklist:

• [ ] Documentation added to the docs site
• [x] Tests
• [ ] Typescript definitions updated
• [x] Ready to be merged

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f83aa358a279d8c149fa3d75795c024995aed917:

Sandbox	Source
react-testing-library-examples	Configuration

[eps1lon]

Could you provide a repro that we can test in jsdom and a real browser. We need to verify this doesn't pollute actual browser tests and why this fix can't be applied upstream i.e. to jsdom.

[VinceMalone]

@eps1lon, I'm happy to do that, but can you clarify what exactly you're looking for? 😅 I'm not sure I entirely understand.

[marcosvega91]

Hi @VinceMalone could you please rebase this branch with master?

[codecov[bot]]

Codecov Report

Merging #865 (f83aa35) into master (2830eae) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #865   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          951       957    +6     
  Branches       291       291           
=========================================
+ Hits           951       957    +6     

Impacted Files	Coverage Δ
src/events.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2830eae...f83aa35. Read the comment docs.

[VinceMalone]

@eps1lon I added a check to see if window.TransitionEvent is not a function before monkey patching the fired event — this should prevent any issues with pre-existing working environments.

[eps1lon]

Yeah, I'm not comfortable with monkey patching incomplete environments. This can become super confusing if other libraries don't do that.

Why does a polyfill like https://codesandbox.io/s/jsdom-polyfill-transitionevent-bgfz1 doesn't work for you?

[VinceMalone]

The polyfill works, but this change might still be worth it for the sake of others. There's a silent failure when an event type isn't supported — because window.Event is used as a backup. So in this case, it's not obvious why the transitionEventInit properties aren't working, despite fireEvent.transitionEnd() being available to use and jsdom being listed as a "supported" environment. This is what drove me to dig into the source and eventually create this PR.

I definitely get that monkey patching this isn't really ideal, but is it worth doing (like DataTransfer) until jsdom supports window.TransitionEvent?

[eps1lon]

I definitely get that monkey patching this isn't really ideal, but is it worth doing (like DataTransfer) until jsdom supports window.TransitionEvent?

I consider monkey-patching DataTransfer a mistake as well. Others seem to disagree.

fireEvent.transitionEnd silently working is just as problematic as polyfilling the properties silently. It'll just fail at a different point e.g. when assuming that changing a CSS property triggers a transition. This is the reason JSDOM doesn't polyfill the event but rather waits for a full implementation of the spec.

I don't believe we're actually improving the problem. Rather we're just kicking the problem to someone else by diverging even more. Before we had JSDOM vs browsers. Now we have JSDOM vs browsers vs testing-library in JSDOM. By using testing-library you're tied to a special environment that's more and more diverging from existing environments and I don't think this is the responsible thing to do.

So yeah, ultimately we should not fall back to window.Event. It's a slippery slope we're on so I'm going to repeat the same argument on every PR.

So in this case, it's not obvious why the transitionEventInit properties aren't working, despite fireEvent.transitionEnd()

That's a bit debateable because in the end you're getting an Event not TransitionEvent. I understand that this isn't obvsious because you receive it in a transitionend event handler but some hint does exist.

I I just don't have a good feeling about these monkey-patches since we have to maintain a very specific use case ("support a partial implementation of the Transition spec") which leaves us open to "how much should we fix in testing-library?" debates.

I'd be way more comfortable if we had a package like jsdom-event-polyfill that people would explicitly opt-in and where we could explain potential draw-backs.

[VinceMalone]

@eps1lon those are all really good points; I definitely understand the urge to resist going down this path. It's tough to balance the pros can cons of existing decisions (falling back to window.Event or monkey patching DataTransfer), silent errors, and accommodating a wider pit of success. Until then, I'm happy to get onboard with whatever you decide 👍

[100terres]

This comment is to help others who might stumble on that issue

If anyone is looking for a polyfill or something, here's how we've managed to make it work with jest in our codebase.

https://github.com/react-forked/dnd/pull/323/commits/5ec5a7467882e66bd707bbb4fb7426b70f462ac5#diff-3ab6990a471fa18593a64b7c0de508ff724cd1cd0676d882d475fb40fe624660=

[mikedidomizio]

For Vitest I took the approach of addEventListener to get it working. I add the property on the capture phase of the event so that when onTransitionEnd fires we have the propertyName. This will probably also work with Jest.

I've created a codesandbox to show it working with tests/fireEvent.