Skip to content

[RFC] fix(fireEvent): Flush effects from discrete events immediately #898

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

Closed
Closed
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
56 changes: 55 additions & 1 deletion src/__tests__/events.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react'
import {render, fireEvent} from '../'
import * as ReactDOM from 'react-dom'
import {render, createEvent, fireEvent} from '../'

const eventTypes = [
{
Expand Down Expand Up @@ -254,3 +255,56 @@ test('blur/focus bubbles in react', () => {
expect(handleFocus).toHaveBeenCalledTimes(1)
expect(handleBubbledFocus).toHaveBeenCalledTimes(1)
})

test.each([
['fireEvent.click', element => fireEvent.click(element)],
['fireEvent()', element => fireEvent(element, createEvent.click(element))],
])(
'discrete events are not wrapped in act when using %s',
(_, dispatchClick) => {
function AddDocumentClickListener({onClick}) {
React.useEffect(() => {
document.addEventListener('click', onClick)
return () => {
document.removeEventListener('click', onClick)
}
}, [onClick])
return null
}
function Component({onDocumentClick}) {
const [open, setOpen] = React.useState(false)

return (
<React.Fragment>
<button onClick={() => setOpen(true)} />
{open &&
ReactDOM.createPortal(
<AddDocumentClickListener onClick={onDocumentClick} />,
document.body,
)}
</React.Fragment>
)
}
const onDocumentClick = jest.fn()
render(<Component onDocumentClick={onDocumentClick} />)

const button = document.querySelector('button')
dispatchClick(button)

// We added a native click listener from an effect.
// There are two possible scenarios:
// 1. If that effect is flushed during the click the native click listener would still receive the event that caused the native listener to be added.
// 2. If that effect is flushed before we return from fireEvent.click the native click listener would not receive the event that caused the native listener to be added.
// React flushes effects scheduled from an update by a "discrete" event immediately if that effect was scheduled from a portaled component.
// but not effects in a batched context (e.g. act(() => {}))
// So if we were in act(() => {}), we would see scenario 2 i.e. `onDocumentClick` would not be called
// If we were not in `act(() => {})`, we would see scenario 1 i.e. `onDocumentClick` would already be called
expect(onDocumentClick).toHaveBeenCalledTimes(1)

// verify we did actually flush the effect before we returned from `fireEvent.click` i.e. the native click listener is mounted.
document.dispatchEvent(
new MouseEvent('click', {bubbles: true, cancelable: true}),
)
expect(onDocumentClick).toHaveBeenCalledTimes(2)
},
)
91 changes: 89 additions & 2 deletions src/fire-event.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,100 @@
import {fireEvent as dtlFireEvent} from '@testing-library/dom'
import act from './act-compat'

// https://github.com/facebook/react/blob/b48b38af68c27fd401fe4b923a8fa0b229693cd4/packages/react-dom/src/events/ReactDOMEventListener.js#L310-L366
const discreteEvents = new Set([
'cancel',
'click',
'close',
'contextmenu',
'copy',
'cut',
'auxclick',
'dblclick',
'dragend',
'dragstart',
'drop',
'focusin',
'focusout',
'input',
'invalid',
'keydown',
'keypress',
'keyup',
'mousedown',
'mouseup',
'paste',
'pause',
'play',
'pointercancel',
'pointerdown',
'pointerup',
'ratechange',
'reset',
'seeked',
'submit',
'touchcancel',
'touchend',
'touchstart',
'volumechange',
'change',
'selectionchange',
'textInput',
'compositionstart',
'compositionend',
'compositionupdate',
'beforeblur',
'afterblur',
'beforeinput',
'blur',
'fullscreenchange',
'focus',
'hashchange',
'popstate',
'select',
'selectstart',
])
function isDiscreteEvent(type) {
return discreteEvents.has(type)
}

function noAct(callback) {
// Don't alter semantics of `callback`.
callback()
// But make sure updates are flushed before returning.
act(() => {})
}

// react-testing-library's version of fireEvent will call
// dom-testing-library's version of fireEvent. The reason
// we make this distinction however is because we have
// a few extra events that work a bit differently
const fireEvent = (...args) => dtlFireEvent(...args)
function fireEvent(element, event, ...args) {
// `act` would simulate how this event would behave if dispatched from a React event listener.
// In almost all cases we want to simulate how this event behaves in response to a user interaction.
// See discussion in https://github.com/facebook/react/pull/21202
const eventWrapper = isDiscreteEvent(event.type) ? noAct : act

let fireEventReturnValue
eventWrapper(() => {
fireEventReturnValue = dtlFireEvent(element, event, ...args)
})
return fireEventReturnValue
}

Object.keys(dtlFireEvent).forEach(key => {
fireEvent[key] = (...args) => dtlFireEvent[key](...args)
fireEvent[key] = (element, ...args) => {
// `act` would simulate how this event would behave if dispatched from a React event listener.
// In almost all cases we want to simulate how this event behaves in response to a user interaction.
// See discussion in https://github.com/facebook/react/pull/21202
const eventWrapper = isDiscreteEvent(key.toLowerCase()) ? noAct : act

let fireEventReturnValue
eventWrapper(() => {
fireEventReturnValue = dtlFireEvent[key](element, ...args)
})
return fireEventReturnValue
}
})

// React event system tracks native mouseOver/mouseOut events for
Expand Down
7 changes: 0 additions & 7 deletions src/pure.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ configureDTL({
})
return result
},
eventWrapper: cb => {
let result
act(() => {
result = cb()
})
return result
},
})

const mountedContainers = new Set()
Expand Down