tailwindlabs / headlessui

Completely unstyled, fully accessible UI components, designed to integrate beautifully with Tailwind CSS.
https://headlessui.com
MIT License
25.81k stars 1.07k forks source link

Prevent crash in environments where `Element.prototype.getAnimations` is not available #3473

Closed RobinMalfait closed 2 weeks ago

RobinMalfait commented 2 weeks ago

Recently we made improvements to the Transition component and internal useTransition hook. We now use the Element.prototype.getAnimations API to know whether or not all transitions are done.

This API has been available in browsers since 2020, however jsdom doesn't have support for this. This results in a lot of failing tests where users rely on jsdom (e.g. inside of Jest or Vitest).

In a perfect world, jsdom is not used because it's not a real browser and there is a lot you need to workaround to even mimic a real browser.

I understand that just switching to real browser tests (using Playwright for example) is not an easy task that can be done easily.

Even our tests still rely on jsdom…

So to make the development experience better, we polyfill the Element.prototype.getAnimations API only in tests (process.env.NODE_ENV === 'test') and show a warning in the console on how to proceed.

The polyfill we ship simply returns an empty array for node.getAnimations(). This means that it will be enough for most tests to pass. The exception is if you are actually relying on transition-duration and transition-delay CSS properties.

The warning you will get looks like this:

Headless UI has polyfilled `Element.prototype.getAnimations` for your tests.
Please install a proper polyfill e.g. `jsdom-testing-mocks`, to silence these warnings.

Example usage:
```js
import { mockAnimationsApi } from 'jsdom-testing-mocks'
mockAnimationsApi()


Fixes: #3470
Fixes: #3469
Fixes: #3468
vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 11, 2024 3:18pm
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 11, 2024 3:18pm
plavski commented 2 weeks ago

Just a note to say that happy-dom, another very popular framework for tests, also does not support getAnimations: https://github.com/capricorn86/happy-dom/blob/afc3692e8dde4d3769706fb36f7818b6d41197bc/packages/happy-dom/src/nodes/shadow-root/ShadowRoot.ts#L185

So any messages should not be jsdom specific imo

RobinMalfait commented 2 weeks ago

@plavski the jsdom-testing-mocks has jsdom in the name but isn't specific to jsdom. It has general purpose mocks. I don't think it's useful to try and detect in which environment you are. jsdom is typically the default, and since the mock package is not specific to jsdom (regardless the name) it should be fine to use 👍

RobinMalfait commented 2 weeks ago

Updated the wording slightly

plavski commented 2 weeks ago

@plavski the jsdom-testing-mocks has jsdom in the name but isn't specific to jsdom. It has general purpose mocks. I don't think it's useful to try and detect in which environment you are. jsdom is typically the default, and since the mock package is not specific to jsdom (regardless the name) it should be fine to use 👍

Cool. It was only the wording feeling overly specific that was the issue for me, and your change has cleared that up. Thanks!

corneliusroemer commented 2 weeks ago

Just tested this and it all works as expected. Without explicit polyfill there's a warning. With polyfill as suggested, no warning, see https://github.com/loculus-project/loculus/pull/2766/files for implementation details.