Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions jest/polyfills.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import ResizeObserverPolyfill from 'resize-observer-polyfill'
import { mockAnimationsApi, mockResizeObserver } from 'jsdom-testing-mocks'

if (typeof ResizeObserver === 'undefined') {
global.ResizeObserver = ResizeObserverPolyfill
}
mockAnimationsApi() // `Element.prototype.getAnimations` and `CSSTransition` polyfill
mockResizeObserver() // `ResizeObserver` polyfill

// JSDOM Doesn't implement innerText yet: https://github.com/jsdom/jsdom/issues/1245
// So this is a hacky way of implementing it using `textContent`.
Expand Down
29 changes: 29 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Prevent crash in environments where `Element.prototype.getAnimations` is not available ([#3473](https://github.com/tailwindlabs/headlessui/pull/3473))

## [2.1.6] - 2024-09-09

Expand Down
45 changes: 0 additions & 45 deletions packages/@headlessui-react/jest.setup.js
Original file line number Diff line number Diff line change
@@ -1,46 +1 @@
globalThis.IS_REACT_ACT_ENVIRONMENT = true

// These are not 1:1 perfect polyfills, but they implement the parts we need for
// testing. The implementation of the `getAnimations` uses the `setTimeout`
// approach we used in the past.
//
// This is only necessary because JSDOM does not implement `getAnimations` or
// `CSSTransition` yet. This is a temporary solution until JSDOM implements
// these features. Or, until we use proper browser tests using Puppeteer or
// Playwright.
{
if (typeof CSSTransition === 'undefined') {
globalThis.CSSTransition = class CSSTransition {
constructor(duration) {
this.duration = duration
}

finished = new Promise((resolve) => {
setTimeout(resolve, this.duration)
})
}
}

if (typeof Element.prototype.getAnimations !== 'function') {
Element.prototype.getAnimations = function () {
let { transitionDuration, transitionDelay } = getComputedStyle(this)

let [durationMs, delayMs] = [transitionDuration, transitionDelay].map((value) => {
let [resolvedValue = 0] = value
.split(',')
// Remove falsy we can't work with
.filter(Boolean)
// Values are returned as `0.3s` or `75ms`
.map((v) => (v.includes('ms') ? parseFloat(v) : parseFloat(v) * 1000))
.sort((a, z) => z - a)

return resolvedValue
})

let totalDuration = durationMs + delayMs
if (totalDuration === 0) return []

return [new CSSTransition(totalDuration)]
}
}
}
1 change: 1 addition & 0 deletions packages/@headlessui-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"@testing-library/react": "^15.0.7",
"@types/react": "^18.3.3",
"@types/react-dom": "^18.3.0",
"jsdom-testing-mocks": "^1.13.1",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"snapshot-diff": "^0.10.0"
Expand Down
31 changes: 30 additions & 1 deletion packages/@headlessui-react/src/hooks/use-transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,34 @@ import { useDisposables } from './use-disposables'
import { useFlags } from './use-flags'
import { useIsoMorphicEffect } from './use-iso-morphic-effect'

if (
typeof process !== 'undefined' &&
typeof globalThis !== 'undefined' &&
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also check for typeof process.env !== 'undefined' or make the process.env[] lookup an optional chain, just to prevent a crash in some very weird setups:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah not a bad idea honestly

// Strange string concatenation is on purpose to prevent `esbuild` from
// replacing `process.env.NODE_ENV` with `production` in the build output,
// eliminating this whole branch.
process?.env?.['NODE' + '_' + 'ENV'] === 'test'
) {
if (typeof Element.prototype.getAnimations === 'undefined') {
Element.prototype.getAnimations = function getAnimationsPolyfill() {
console.warn(
[
'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()',
'```',
].join('\n')
)

return []
}
}
}

/**
* ```
* ┌──────┐ │ ┌──────────────┐
Expand Down Expand Up @@ -233,7 +261,8 @@ function waitForTransition(node: HTMLElement | null, done: () => void) {
cancelled = true
})

let transitions = node.getAnimations().filter((animation) => animation instanceof CSSTransition)
let transitions =
node.getAnimations?.().filter((animation) => animation instanceof CSSTransition) ?? []
// If there are no transitions, we can stop early.
if (transitions.length === 0) {
done()
Expand Down