From 8d30643a6e20977345af2a6e0bc1f56eb9b843db Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Mon, 22 Jan 2024 11:27:45 -0500 Subject: [PATCH 1/9] Convert ReactDOMEventListener to createRoot --- .../__tests__/ReactDOMEventListener-test.js | 106 +++++++++++------- 1 file changed, 63 insertions(+), 43 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index d884b92d7fd49..1cec7bd9fed83 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -11,18 +11,21 @@ describe('ReactDOMEventListener', () => { let React; - let ReactDOM; + let ReactDOMX; let ReactDOMClient; let ReactDOMServer; let act; + let waitForPaint; beforeEach(() => { jest.resetModules(); React = require('react'); - ReactDOM = require('react-dom'); + ReactDOMX = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; + const InternalTestUtils = require('internal-test-utils'); + waitForPaint = InternalTestUtils.waitForPaint; }); describe('Propagation', () => { @@ -111,10 +114,10 @@ describe('ReactDOMEventListener', () => { this.setState({clicked: true}); }; componentDidMount() { - expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); + expect(ReactDOMX.findDOMNode(this)).toBe(container.firstChild); } componentDidUpdate() { - expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); + expect(ReactDOMX.findDOMNode(this)).toBe(container.firstChild); } render() { if (this.state.clicked) { @@ -143,29 +146,42 @@ describe('ReactDOMEventListener', () => { } }); - it('should batch between handlers from different roots', () => { + it('should batch between handlers from different roots', async () => { const mock = jest.fn(); const childContainer = document.createElement('div'); - const handleChildMouseOut = () => { - ReactDOM.render(
1
, childContainer); - mock(childNode.textContent); - }; - const parentContainer = document.createElement('div'); - const handleParentMouseOut = () => { - ReactDOM.render(
2
, childContainer); - mock(childNode.textContent); - }; + const childRoot = ReactDOMClient.createRoot(childContainer); + const parentRoot = ReactDOMClient.createRoot(parentContainer); + let childSetState; + + function Parent() { + // eslint-disable-next-line no-unused-vars + const [state, _] = React.useState('Parent'); + const handleMouseOut = () => { + childSetState(2); + mock(childContainer.firstChild.textContent); + }; + return
{state}
; + } - const childNode = ReactDOM.render( -
Child
, - childContainer, - ); - const parentNode = ReactDOM.render( -
Parent
, - parentContainer, - ); + function Child() { + const [state, setState] = React.useState('Child'); + childSetState = setState; + const handleMouseOut = () => { + setState(1); + mock(childContainer.firstChild.textContent); + }; + return
{state}
; + } + + await act(() => { + childRoot.render(); + parentRoot.render(); + }); + + const childNode = childContainer.firstChild; + const parentNode = parentContainer.firstChild; parentNode.appendChild(childContainer); document.body.appendChild(parentContainer); @@ -176,23 +192,20 @@ describe('ReactDOMEventListener', () => { // Child and parent should both call from event handlers. expect(mock).toHaveBeenCalledTimes(2); - // The first call schedules a render of '1' into the 'Child'. - // However, we're batching so it isn't flushed yet. - expect(mock.mock.calls[0][0]).toBe('Child'); - // As we have two roots, it means we have two event listeners. - // This also means we enter the event batching phase twice, - // flushing the child to be 1. - - // We don't have any good way of knowing if another event will - // occur because another event handler might invoke - // stopPropagation() along the way. After discussions internally - // with Sebastian, it seems that for now over-flushing should - // be fine, especially as the new event system is a breaking - // change anyway. We can maybe revisit this later as part of - // the work to refine this in the scheduler (maybe by leveraging - // isInputPending?). - expect(mock.mock.calls[1][0]).toBe('1'); - // By the time we leave the handler, the second update is flushed. + + // However, we're batching, so they aren't flushed yet. + expect(mock).toHaveBeenNthCalledWith(1, 'Child'); + expect(mock).toHaveBeenNthCalledWith(2, 'Child'); + expect(parentNode.textContent).toBe('ParentChild'); + expect(childNode.textContent).toBe('Child'); + mock.mockClear(); + + // Flush the batched updates. + await waitForPaint([]); + + // The batched updates are applied. + expect(mock).not.toBeCalled(); + expect(parentNode.textContent).toBe('Parent2'); expect(childNode.textContent).toBe('2'); } finally { document.body.removeChild(parentContainer); @@ -204,13 +217,18 @@ describe('ReactDOMEventListener', () => { const mouseOut = jest.fn(); const onMouseOut = event => mouseOut(event.target); - const innerRef = React.createRef(); class Wrapper extends React.Component { + innerRef = React.createRef(); + getInner = () => { + return this.innerRef.current; + }; + render() { + const inner =
Inner
; return (
-
Inner
+ {inner}
); @@ -223,16 +241,18 @@ describe('ReactDOMEventListener', () => { root.render(); }); + const instance = container.firstChild.firstChild; + document.body.appendChild(container); try { const nativeEvent = document.createEvent('Event'); nativeEvent.initEvent('mouseout', true, true); await act(() => { - innerRef.current.dispatchEvent(nativeEvent); + instance.dispatchEvent(nativeEvent); }); - expect(mouseOut).toBeCalledWith(innerRef.current); + expect(mouseOut).toBeCalledWith(instance); } finally { document.body.removeChild(container); } From 1fcc459f3615cad10bc71a1e8e0ef208ae01172a Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 23 Jan 2024 23:13:04 -0500 Subject: [PATCH 2/9] Fix innerRef and ReactDOMX --- .../__tests__/ReactDOMEventListener-test.js | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 1cec7bd9fed83..4335864cec981 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -11,7 +11,7 @@ describe('ReactDOMEventListener', () => { let React; - let ReactDOMX; + let ReactDOM; let ReactDOMClient; let ReactDOMServer; let act; @@ -20,7 +20,7 @@ describe('ReactDOMEventListener', () => { beforeEach(() => { jest.resetModules(); React = require('react'); - ReactDOMX = require('react-dom'); + ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; @@ -114,10 +114,10 @@ describe('ReactDOMEventListener', () => { this.setState({clicked: true}); }; componentDidMount() { - expect(ReactDOMX.findDOMNode(this)).toBe(container.firstChild); + expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); } componentDidUpdate() { - expect(ReactDOMX.findDOMNode(this)).toBe(container.firstChild); + expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); } render() { if (this.state.clicked) { @@ -217,18 +217,13 @@ describe('ReactDOMEventListener', () => { const mouseOut = jest.fn(); const onMouseOut = event => mouseOut(event.target); + const innerRef = React.createRef(); class Wrapper extends React.Component { - innerRef = React.createRef(); - getInner = () => { - return this.innerRef.current; - }; - render() { - const inner =
Inner
; return (
- {inner} +
Inner
); @@ -241,18 +236,16 @@ describe('ReactDOMEventListener', () => { root.render(); }); - const instance = container.firstChild.firstChild; - document.body.appendChild(container); try { const nativeEvent = document.createEvent('Event'); nativeEvent.initEvent('mouseout', true, true); await act(() => { - instance.dispatchEvent(nativeEvent); + innerRef.current.dispatchEvent(nativeEvent); }); - expect(mouseOut).toBeCalledWith(instance); + expect(mouseOut).toBeCalledWith(innerRef.current); } finally { document.body.removeChild(container); } From ca7c9f6865596a9ae3aed4fe9c298c9c43ba39f6 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Wed, 24 Jan 2024 21:32:03 -0500 Subject: [PATCH 3/9] Revert batching test back, will follow up --- .../__tests__/ReactDOMEventListener-test.js | 82 ++++++++----------- 1 file changed, 36 insertions(+), 46 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 4335864cec981..345da264473c8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -146,42 +146,29 @@ describe('ReactDOMEventListener', () => { } }); - it('should batch between handlers from different roots', async () => { + it('should batch between handlers from different roots', () => { const mock = jest.fn(); const childContainer = document.createElement('div'); - const parentContainer = document.createElement('div'); - const childRoot = ReactDOMClient.createRoot(childContainer); - const parentRoot = ReactDOMClient.createRoot(parentContainer); - let childSetState; - - function Parent() { - // eslint-disable-next-line no-unused-vars - const [state, _] = React.useState('Parent'); - const handleMouseOut = () => { - childSetState(2); - mock(childContainer.firstChild.textContent); - }; - return
{state}
; - } - - function Child() { - const [state, setState] = React.useState('Child'); - childSetState = setState; - const handleMouseOut = () => { - setState(1); - mock(childContainer.firstChild.textContent); - }; - return
{state}
; - } - - await act(() => { - childRoot.render(); - parentRoot.render(); - }); + const handleChildMouseOut = () => { + ReactDOM.render(
1
, childContainer); + mock(childNode.textContent); + }; - const childNode = childContainer.firstChild; - const parentNode = parentContainer.firstChild; + const parentContainer = document.createElement('div'); + const handleParentMouseOut = () => { + ReactDOM.render(
2
, childContainer); + mock(childNode.textContent); + }; + + const childNode = ReactDOM.render( +
Child
, + childContainer, + ); + const parentNode = ReactDOM.render( +
Parent
, + parentContainer, + ); parentNode.appendChild(childContainer); document.body.appendChild(parentContainer); @@ -192,20 +179,23 @@ describe('ReactDOMEventListener', () => { // Child and parent should both call from event handlers. expect(mock).toHaveBeenCalledTimes(2); - - // However, we're batching, so they aren't flushed yet. - expect(mock).toHaveBeenNthCalledWith(1, 'Child'); - expect(mock).toHaveBeenNthCalledWith(2, 'Child'); - expect(parentNode.textContent).toBe('ParentChild'); - expect(childNode.textContent).toBe('Child'); - mock.mockClear(); - - // Flush the batched updates. - await waitForPaint([]); - - // The batched updates are applied. - expect(mock).not.toBeCalled(); - expect(parentNode.textContent).toBe('Parent2'); + // The first call schedules a render of '1' into the 'Child'. + // However, we're batching so it isn't flushed yet. + expect(mock.mock.calls[0][0]).toBe('Child'); + // As we have two roots, it means we have two event listeners. + // This also means we enter the event batching phase twice, + // flushing the child to be 1. + + // We don't have any good way of knowing if another event will + // occur because another event handler might invoke + // stopPropagation() along the way. After discussions internally + // with Sebastian, it seems that for now over-flushing should + // be fine, especially as the new event system is a breaking + // change anyway. We can maybe revisit this later as part of + // the work to refine this in the scheduler (maybe by leveraging + // isInputPending?). + expect(mock.mock.calls[1][0]).toBe('1'); + // By the time we leave the handler, the second update is flushed. expect(childNode.textContent).toBe('2'); } finally { document.body.removeChild(parentContainer); From d2eae5b410a29600e6a7f37760b12f27cd5c2ba8 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Wed, 24 Jan 2024 22:33:26 -0500 Subject: [PATCH 4/9] Fix lint --- packages/react-dom/src/__tests__/ReactDOMEventListener-test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 345da264473c8..d884b92d7fd49 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -15,7 +15,6 @@ describe('ReactDOMEventListener', () => { let ReactDOMClient; let ReactDOMServer; let act; - let waitForPaint; beforeEach(() => { jest.resetModules(); @@ -24,8 +23,6 @@ describe('ReactDOMEventListener', () => { ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; - const InternalTestUtils = require('internal-test-utils'); - waitForPaint = InternalTestUtils.waitForPaint; }); describe('Propagation', () => { From 1e5f3cec737428dac67b6e46f055cf78a9eda7c7 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Wed, 24 Jan 2024 22:32:24 -0500 Subject: [PATCH 5/9] Add waitForEvents to test ReactDOMEventListener --- .../ReactInternalTestUtils.js | 37 ++++ .../ReactInternalTestUtilsDOM-test.js | 207 ++++++++++++++++++ .../__tests__/ReactDOMEventListener-test.js | 128 +++++++++-- 3 files changed, 349 insertions(+), 23 deletions(-) create mode 100644 packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js diff --git a/packages/internal-test-utils/ReactInternalTestUtils.js b/packages/internal-test-utils/ReactInternalTestUtils.js index 8e68e83344241..f50f3a3e85fe9 100644 --- a/packages/internal-test-utils/ReactInternalTestUtils.js +++ b/packages/internal-test-utils/ReactInternalTestUtils.js @@ -264,3 +264,40 @@ ${diff(expectedLog, actualLog)} Error.captureStackTrace(error, assertLog); throw error; } + +// Simulates dispatching events, waiting for microtasks in between. +// This matches the browser behavior, which will flush microtasks +// between each event handler. +export async function waitForEvents( + node: Node, + eventType: string, +): Promise { + assertYieldsWereCleared(waitForDiscrete); + + // Bubbling phase + // Walk the path from the element to the document and dispatch + // the event on each node, flushing microtasks in between. + for (let current = node; current; current = current.parentNode) { + const customEvent = new Event(eventType, { + bubbles: false, + cancelable: true, + }); + + Object.defineProperty(customEvent, 'eventPhase', { + // Avoid going through the capture/bubbling phases, + // since we're doing it manually. + value: Event.AT_TARGET, + }); + + Object.defineProperty(customEvent, 'target', { + // Override the target to the node on which we dispatched the event. + value: node, + }); + + // Dispatch the event on the target + current.dispatchEvent(customEvent); + + // Flush microtasks + await waitForMicrotasks(); + } +} diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js new file mode 100644 index 0000000000000..e94fe44aec00a --- /dev/null +++ b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js @@ -0,0 +1,207 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let act; +let Scheduler; +let ReactDOMClient; +let waitForEvents; +let assertLog; + +describe('ReactInternalTestUtilsDOM', () => { + beforeEach(() => { + jest.resetModules(); + act = require('internal-test-utils').act; + waitForEvents = require('internal-test-utils').waitForEvents; + Scheduler = require('scheduler/unstable_mock'); + ReactDOMClient = require('react-dom/client'); + React = require('react'); + assertLog = require('internal-test-utils').assertLog; + }); + describe('waitForEvents', () => { + test('should fire capture events (discrete)', async () => { + let childRef; + function Component() { + const [state, setState] = React.useState(0); + Scheduler.log(`Render ${state}`); + return ( +
{ + setState(1); + Scheduler.log('onClickCapture parent'); + }}> +
+ ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render 0']); + + await act(async () => { + await waitForEvents(childRef, 'click'); + }); + + // Capture runs on every event we dispatch, + // which means we get two for the parent, and one for the child. + assertLog([ + 'onClickCapture parent', + 'onClickCapture child', + 'Render 2', + 'onClickCapture parent', + 'onClickCapture child', + 'Render 2', + 'onClickCapture parent', + 'onClickCapture child', + 'Render 2', + ]); + + document.body.removeChild(container); + }); + + test('should fire target events (discrete)', async () => { + let childRef; + function Component() { + const [state, setState] = React.useState(0); + Scheduler.log(`Render ${state}`); + return ( +
{ + setState(1); + Scheduler.log('onClick parent'); + }}> +
+ ); + } + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render 0']); + + await act(async () => { + await waitForEvents(childRef, 'click'); + }); + + assertLog(['onClick child', 'onClick parent', 'Render 1']); + }); + + test('should fire capture events (continuous)', async () => { + let childRef; + function Component() { + const [state, setState] = React.useState(0); + Scheduler.log(`Render ${state}`); + return ( +
{ + setState(1); + Scheduler.log('onMouseOutCapture parent'); + }}> +
+ ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render 0']); + + await act(async () => { + await waitForEvents(childRef, 'mouseout'); + }); + + // Capture runs on every event we dispatch, + // which means we get two for the parent, and one for the child. + // Since this is continuous, we batch. + assertLog([ + 'onMouseOutCapture parent', + 'onMouseOutCapture child', + 'onMouseOutCapture parent', + 'onMouseOutCapture child', + 'onMouseOutCapture parent', + 'onMouseOutCapture child', + 'Render 2', + ]); + + document.body.removeChild(container); + }); + + test('should fire target events (continuous)', async () => { + let childRef; + function Component() { + const [state, setState] = React.useState(0); + Scheduler.log(`Render ${state}`); + return ( +
{ + setState(1); + Scheduler.log('onMouseOut parent'); + }}> +
+ ); + } + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render 0']); + + await act(async () => { + await waitForEvents(childRef, 'mouseout'); + }); + + assertLog(['onMouseOut child', 'onMouseOut parent', 'Render 1']); + }); + }); +}); diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index d884b92d7fd49..6358307b68f80 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -15,6 +15,7 @@ describe('ReactDOMEventListener', () => { let ReactDOMClient; let ReactDOMServer; let act; + let waitForEvents; beforeEach(() => { jest.resetModules(); @@ -23,6 +24,7 @@ describe('ReactDOMEventListener', () => { ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; + waitForEvents = require('internal-test-utils').waitForEvents; }); describe('Propagation', () => { @@ -143,36 +145,51 @@ describe('ReactDOMEventListener', () => { } }); - it('should batch between handlers from different roots', () => { + it('should batch between handlers from different roots (discrete)', async () => { const mock = jest.fn(); const childContainer = document.createElement('div'); - const handleChildMouseOut = () => { - ReactDOM.render(
1
, childContainer); - mock(childNode.textContent); - }; + const parentContainer = document.createElement('main'); + + const childRoot = ReactDOMClient.createRoot(childContainer); + const parentRoot = ReactDOMClient.createRoot(parentContainer); + let childSetState; + + function Parent() { + // eslint-disable-next-line no-unused-vars + const [state, _] = React.useState('Parent'); + const handleMouseOut = () => { + childSetState(2); + mock(childContainer.firstChild.textContent); + }; + return
{state}
; + } + + function Child() { + const [state, setState] = React.useState('Child'); + childSetState = setState; + const handleMouseOut = () => { + setState(1); + mock(childContainer.firstChild.textContent); + }; + return {state}; + } + + await act(() => { + childRoot.render(); + parentRoot.render(); + }); + + const childNode = childContainer.firstChild; + const parentNode = parentContainer.firstChild; - const parentContainer = document.createElement('div'); - const handleParentMouseOut = () => { - ReactDOM.render(
2
, childContainer); - mock(childNode.textContent); - }; - - const childNode = ReactDOM.render( -
Child
, - childContainer, - ); - const parentNode = ReactDOM.render( -
Parent
, - parentContainer, - ); parentNode.appendChild(childContainer); document.body.appendChild(parentContainer); try { - const nativeEvent = document.createEvent('Event'); - nativeEvent.initEvent('mouseout', true, true); - childNode.dispatchEvent(nativeEvent); + await act(async () => { + await waitForEvents(childNode, 'click'); + }); // Child and parent should both call from event handlers. expect(mock).toHaveBeenCalledTimes(2); @@ -191,8 +208,73 @@ describe('ReactDOMEventListener', () => { // change anyway. We can maybe revisit this later as part of // the work to refine this in the scheduler (maybe by leveraging // isInputPending?). + // + // Since this is a discrete event, the previous update is already done. expect(mock.mock.calls[1][0]).toBe('1'); - // By the time we leave the handler, the second update is flushed. + // And by the time we leave the handler, the second update is flushed. + expect(childNode.textContent).toBe('2'); + } finally { + document.body.removeChild(parentContainer); + } + }); + + it('should batch between handlers from different roots (continuous)', async () => { + const mock = jest.fn(); + + const childContainer = document.createElement('div'); + const parentContainer = document.createElement('main'); + + const childRoot = ReactDOMClient.createRoot(childContainer); + const parentRoot = ReactDOMClient.createRoot(parentContainer); + let childSetState; + + function Parent() { + // eslint-disable-next-line no-unused-vars + const [state, _] = React.useState('Parent'); + const handleMouseOut = () => { + childSetState(2); + mock(childContainer.firstChild.textContent); + }; + return
{state}
; + } + + function Child() { + const [state, setState] = React.useState('Child'); + childSetState = setState; + const handleMouseOut = () => { + setState(1); + mock(childContainer.firstChild.textContent); + }; + return {state}; + } + + await act(() => { + childRoot.render(); + parentRoot.render(); + }); + + const childNode = childContainer.firstChild; + const parentNode = parentContainer.firstChild; + + parentNode.appendChild(childContainer); + document.body.appendChild(parentContainer); + + try { + await act(async () => { + await waitForEvents(childNode, 'mouseout'); + }); + + // Child and parent should both call from event handlers. + expect(mock).toHaveBeenCalledTimes(2); + // The first call schedules a render of '1' into the 'Child'. + // However, we're batching, so it isn't flushed yet. + expect(mock.mock.calls[0][0]).toBe('Child'); + // As we have two roots, it means we have two event listeners. + // This also means we enter the event batching phase twice. + // But since this is a continuous event, we still haven't flushed. + expect(mock.mock.calls[1][0]).toBe('Child'); + + // The batched update is applied after the events. expect(childNode.textContent).toBe('2'); } finally { document.body.removeChild(parentContainer); From d0f21658c57228aafd1a2df322053bfb1ed4f0c2 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Thu, 25 Jan 2024 12:18:34 -0500 Subject: [PATCH 6/9] Legacy FB support --- .../src/__tests__/ReactDOMEventListener-test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 6358307b68f80..d4e2362955a23 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -9,6 +9,8 @@ 'use strict'; +import {enableLegacyFBSupport} from 'shared/forks/ReactFeatureFlags.www-dynamic'; + describe('ReactDOMEventListener', () => { let React; let ReactDOM; @@ -210,7 +212,14 @@ describe('ReactDOMEventListener', () => { // isInputPending?). // // Since this is a discrete event, the previous update is already done. - expect(mock.mock.calls[1][0]).toBe('1'); + if (gate(flags => flags.enableLegacyFBSupport)) { + // Legacy FB support mode attaches to the document, which is a single event + // dispatch for both roots, so this is batched. + expect(mock.mock.calls[1][0]).toBe('Child'); + } else { + expect(mock.mock.calls[1][0]).toBe('1'); + } + // And by the time we leave the handler, the second update is flushed. expect(childNode.textContent).toBe('2'); } finally { From e6e144ce06e8a95d0e462aa9b6273b29db8251d9 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Thu, 25 Jan 2024 12:20:35 -0500 Subject: [PATCH 7/9] Rename to dispatchAndWaitForDiscrete. --- .../internal-test-utils/ReactInternalTestUtils.js | 2 +- .../__tests__/ReactInternalTestUtilsDOM-test.js | 15 ++++++++------- .../src/__tests__/ReactDOMEventListener-test.js | 11 ++++++----- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/internal-test-utils/ReactInternalTestUtils.js b/packages/internal-test-utils/ReactInternalTestUtils.js index f50f3a3e85fe9..f2e9ce0030aa5 100644 --- a/packages/internal-test-utils/ReactInternalTestUtils.js +++ b/packages/internal-test-utils/ReactInternalTestUtils.js @@ -268,7 +268,7 @@ ${diff(expectedLog, actualLog)} // Simulates dispatching events, waiting for microtasks in between. // This matches the browser behavior, which will flush microtasks // between each event handler. -export async function waitForEvents( +export async function dispatchAndWaitForDiscrete( node: Node, eventType: string, ): Promise { diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js index e94fe44aec00a..68ee7c0d1e7c3 100644 --- a/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js +++ b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js @@ -13,20 +13,21 @@ let React; let act; let Scheduler; let ReactDOMClient; -let waitForEvents; +let dispatchAndWaitForDiscrete; let assertLog; describe('ReactInternalTestUtilsDOM', () => { beforeEach(() => { jest.resetModules(); act = require('internal-test-utils').act; - waitForEvents = require('internal-test-utils').waitForEvents; + dispatchAndWaitForDiscrete = + require('internal-test-utils').dispatchAndWaitForDiscrete; Scheduler = require('scheduler/unstable_mock'); ReactDOMClient = require('react-dom/client'); React = require('react'); assertLog = require('internal-test-utils').assertLog; }); - describe('waitForEvents', () => { + describe('dispatchAndWaitForDiscrete', () => { test('should fire capture events (discrete)', async () => { let childRef; function Component() { @@ -59,7 +60,7 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await waitForEvents(childRef, 'click'); + await dispatchAndWaitForDiscrete(childRef, 'click'); }); // Capture runs on every event we dispatch, @@ -110,7 +111,7 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await waitForEvents(childRef, 'click'); + await dispatchAndWaitForDiscrete(childRef, 'click'); }); assertLog(['onClick child', 'onClick parent', 'Render 1']); @@ -148,7 +149,7 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await waitForEvents(childRef, 'mouseout'); + await dispatchAndWaitForDiscrete(childRef, 'mouseout'); }); // Capture runs on every event we dispatch, @@ -198,7 +199,7 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await waitForEvents(childRef, 'mouseout'); + await dispatchAndWaitForDiscrete(childRef, 'mouseout'); }); assertLog(['onMouseOut child', 'onMouseOut parent', 'Render 1']); diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index d4e2362955a23..54803405f12d1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -9,7 +9,7 @@ 'use strict'; -import {enableLegacyFBSupport} from 'shared/forks/ReactFeatureFlags.www-dynamic'; +import {dispatchAndWaitForDiscrete} from 'internal-test-utils'; describe('ReactDOMEventListener', () => { let React; @@ -17,7 +17,7 @@ describe('ReactDOMEventListener', () => { let ReactDOMClient; let ReactDOMServer; let act; - let waitForEvents; + let dispatchAndWaitForDiscrete; beforeEach(() => { jest.resetModules(); @@ -26,7 +26,8 @@ describe('ReactDOMEventListener', () => { ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; - waitForEvents = require('internal-test-utils').waitForEvents; + dispatchAndWaitForDiscrete = + require('internal-test-utils').dispatchAndWaitForDiscrete; }); describe('Propagation', () => { @@ -190,7 +191,7 @@ describe('ReactDOMEventListener', () => { try { await act(async () => { - await waitForEvents(childNode, 'click'); + await dispatchAndWaitForDiscrete(childNode, 'click'); }); // Child and parent should both call from event handlers. @@ -270,7 +271,7 @@ describe('ReactDOMEventListener', () => { try { await act(async () => { - await waitForEvents(childNode, 'mouseout'); + await dispatchAndWaitForDiscrete(childNode, 'mouseout'); }); // Child and parent should both call from event handlers. From 6b73d5d39731caa233fc4c935f6b2d75f5181d81 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Mon, 5 Feb 2024 23:05:05 -0500 Subject: [PATCH 8/9] Fork dispatchEvent, fix and add tests --- .../ReactInternalTestUtils.js | 57 +-- .../ReactInternalTestUtilsDOM-test.js | 438 ++++++++++++++++-- .../simulateBrowserEventDispatch.js | 390 ++++++++++++++++ .../__tests__/ReactDOMEventListener-test.js | 28 +- 4 files changed, 827 insertions(+), 86 deletions(-) create mode 100644 packages/internal-test-utils/simulateBrowserEventDispatch.js diff --git a/packages/internal-test-utils/ReactInternalTestUtils.js b/packages/internal-test-utils/ReactInternalTestUtils.js index f2e9ce0030aa5..df77f23448b13 100644 --- a/packages/internal-test-utils/ReactInternalTestUtils.js +++ b/packages/internal-test-utils/ReactInternalTestUtils.js @@ -9,6 +9,7 @@ import * as SchedulerMock from 'scheduler/unstable_mock'; import {diff} from 'jest-diff'; import {equals} from '@jest/expect-utils'; import enqueueTask from './enqueueTask'; +import simulateBrowserEventDispatch from './simulateBrowserEventDispatch'; export {act} from './internalAct'; @@ -267,37 +268,37 @@ ${diff(expectedLog, actualLog)} // Simulates dispatching events, waiting for microtasks in between. // This matches the browser behavior, which will flush microtasks -// between each event handler. -export async function dispatchAndWaitForDiscrete( +// between each event handler. This will allow discrete events to +// flush between events across different event handlers. +export async function simulateEventDispatch( node: Node, eventType: string, ): Promise { - assertYieldsWereCleared(waitForDiscrete); - - // Bubbling phase - // Walk the path from the element to the document and dispatch - // the event on each node, flushing microtasks in between. + // Ensure the node is in the document. for (let current = node; current; current = current.parentNode) { - const customEvent = new Event(eventType, { - bubbles: false, - cancelable: true, - }); - - Object.defineProperty(customEvent, 'eventPhase', { - // Avoid going through the capture/bubbling phases, - // since we're doing it manually. - value: Event.AT_TARGET, - }); - - Object.defineProperty(customEvent, 'target', { - // Override the target to the node on which we dispatched the event. - value: node, - }); - - // Dispatch the event on the target - current.dispatchEvent(customEvent); - - // Flush microtasks - await waitForMicrotasks(); + if (current === document) { + break; + } else if (current.parentNode == null) { + return; + } + } + + const customEvent = new Event(eventType, { + bubbles: true, + }); + + Object.defineProperty(customEvent, 'target', { + // Override the target to the node on which we dispatched the event. + value: node, + }); + + const impl = Object.getOwnPropertySymbols(node)[0]; + const oldDispatch = node[impl].dispatchEvent; + try { + node[impl].dispatchEvent = simulateBrowserEventDispatch; + + await node.dispatchEvent(customEvent); + } finally { + node[impl].dispatchEvent = oldDispatch; } } diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js index 68ee7c0d1e7c3..6fe3aede714c6 100644 --- a/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js +++ b/packages/internal-test-utils/__tests__/ReactInternalTestUtilsDOM-test.js @@ -13,22 +13,23 @@ let React; let act; let Scheduler; let ReactDOMClient; -let dispatchAndWaitForDiscrete; +let simulateEventDispatch; let assertLog; describe('ReactInternalTestUtilsDOM', () => { beforeEach(() => { jest.resetModules(); act = require('internal-test-utils').act; - dispatchAndWaitForDiscrete = - require('internal-test-utils').dispatchAndWaitForDiscrete; + simulateEventDispatch = + require('internal-test-utils').simulateEventDispatch; Scheduler = require('scheduler/unstable_mock'); ReactDOMClient = require('react-dom/client'); React = require('react'); assertLog = require('internal-test-utils').assertLog; }); - describe('dispatchAndWaitForDiscrete', () => { - test('should fire capture events (discrete)', async () => { + + describe('simulateEventDispatch', () => { + it('should batch discrete capture events', async () => { let childRef; function Component() { const [state, setState] = React.useState(0); @@ -36,12 +37,18 @@ describe('ReactInternalTestUtilsDOM', () => { return (
{ + queueMicrotask(() => { + Scheduler.log('Parent microtask'); + }); setState(1); Scheduler.log('onClickCapture parent'); }}>
@@ -103,6 +112,7 @@ describe('ReactInternalTestUtilsDOM', () => { } const container = document.createElement('div'); + document.body.appendChild(container); const root = ReactDOMClient.createRoot(container); await act(() => { root.render(); @@ -111,28 +121,40 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await dispatchAndWaitForDiscrete(childRef, 'click'); + await simulateEventDispatch(childRef, 'mouseout'); }); - assertLog(['onClick child', 'onClick parent', 'Render 1']); + assertLog([ + 'onMouseOutCapture parent', + 'onMouseOutCapture child', + 'Parent microtask', + 'Child microtask', + 'Render 2', + ]); }); - test('should fire capture events (continuous)', async () => { + it('should batch bubbling discrete events', async () => { let childRef; function Component() { const [state, setState] = React.useState(0); Scheduler.log(`Render ${state}`); return (
{ + onClick={() => { + queueMicrotask(() => { + Scheduler.log('Parent microtask'); + }); setState(1); - Scheduler.log('onMouseOutCapture parent'); + Scheduler.log('onClick parent'); }}>
@@ -149,26 +171,19 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await dispatchAndWaitForDiscrete(childRef, 'mouseout'); + await simulateEventDispatch(childRef, 'click'); }); - // Capture runs on every event we dispatch, - // which means we get two for the parent, and one for the child. - // Since this is continuous, we batch. assertLog([ - 'onMouseOutCapture parent', - 'onMouseOutCapture child', - 'onMouseOutCapture parent', - 'onMouseOutCapture child', - 'onMouseOutCapture parent', - 'onMouseOutCapture child', - 'Render 2', + 'onClick child', + 'onClick parent', + 'Child microtask', + 'Render 1', + 'Parent microtask', ]); - - document.body.removeChild(container); }); - test('should fire target events (continuous)', async () => { + it('should batch bubbling continuous events', async () => { let childRef; function Component() { const [state, setState] = React.useState(0); @@ -176,12 +191,18 @@ describe('ReactInternalTestUtilsDOM', () => { return (
{ + queueMicrotask(() => { + Scheduler.log('Parent microtask'); + }); setState(1); Scheduler.log('onMouseOut parent'); }}>
+ ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); const root = ReactDOMClient.createRoot(container); await act(() => { root.render(); @@ -199,10 +283,284 @@ describe('ReactInternalTestUtilsDOM', () => { assertLog(['Render 0']); await act(async () => { - await dispatchAndWaitForDiscrete(childRef, 'mouseout'); + await simulateEventDispatch(childRef.current, 'click'); + }); + + assertLog([ + 'Click child', + 'Child microtask', + 'Render 1', + 'Click parent', + 'Parent microtask', + 'Render 2', + ]); + }); + + it('should batch continuous events between handlers', async () => { + let childRef = React.createRef(); + function Component() { + const [state, setState] = React.useState(0); + const parentRef = React.useRef(); + React.useEffect(() => { + function handleChildEvent() { + queueMicrotask(() => { + Scheduler.log('Child microtask'); + }); + setState(1); + Scheduler.log(`Mouseout child`); + } + function handleParentEvent() { + queueMicrotask(() => { + Scheduler.log('Parent microtask'); + }); + setState(2); + Scheduler.log(`Mouseout parent`); + } + parentRef.current.addEventListener('mouseout', handleParentEvent); + + childRef.current.addEventListener('mouseout', handleChildEvent); + + return () => { + parentRef.current.removeEventListener( + 'mouseout', + handleParentEvent + ); + + childRef.current.removeEventListener('mouseout', handleChildEvent); + }; + }); + + Scheduler.log(`Render ${state}`); + return ( +
+
+ ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render 0']); + + await act(async () => { + await simulateEventDispatch(childRef.current, 'mouseout'); + }); + + assertLog([ + 'Mouseout child', + 'Child microtask', + 'Mouseout parent', + 'Parent microtask', + 'Render 2', + ]); + }); + + it('should flush discrete events between handlers from different roots', async () => { + const childContainer = document.createElement('div'); + const parentContainer = document.createElement('main'); + + const childRoot = ReactDOMClient.createRoot(childContainer); + const parentRoot = ReactDOMClient.createRoot(parentContainer); + let childSetState; + + function Parent() { + // eslint-disable-next-line no-unused-vars + const [state, _] = React.useState('Parent'); + const handleClick = () => { + Promise.resolve().then(() => Scheduler.log('Flush Parent microtask')); + childSetState(2); + Scheduler.log('Parent click'); + }; + return
{state}
; + } + + function Child() { + const [state, setState] = React.useState('Child'); + childSetState = setState; + const handleClick = () => { + Promise.resolve().then(() => Scheduler.log('Flush Child microtask')); + setState(1); + Scheduler.log('Child click'); + }; + Scheduler.log('Render ' + state); + return {state}; + } + + await act(() => { + childRoot.render(); + parentRoot.render(); + }); + + const childNode = childContainer.firstChild; + const parentNode = parentContainer.firstChild; + + parentNode.appendChild(childContainer); + document.body.appendChild(parentContainer); + + assertLog(['Render Child']); + try { + await act(async () => { + await simulateEventDispatch(childNode, 'click'); + }); + + // Since discrete events flush in a microtasks, they flush before + // the handler for the other root is called, after the microtask + // scheduled in the event fires. + assertLog([ + 'Child click', + 'Flush Child microtask', + 'Render 1', + 'Parent click', + 'Flush Parent microtask', + 'Render 2', + ]); + } finally { + document.body.removeChild(parentContainer); + } + }); + + it('should batch continuous events between handlers from different roots', async () => { + const childContainer = document.createElement('div'); + const parentContainer = document.createElement('main'); + + const childRoot = ReactDOMClient.createRoot(childContainer); + const parentRoot = ReactDOMClient.createRoot(parentContainer); + let childSetState; + + function Parent() { + // eslint-disable-next-line no-unused-vars + const [state, _] = React.useState('Parent'); + const handleMouseOut = () => { + Promise.resolve().then(() => Scheduler.log('Flush Parent microtask')); + childSetState(2); + Scheduler.log('Parent mouseout'); + }; + return
{state}
; + } + + function Child() { + const [state, setState] = React.useState('Child'); + childSetState = setState; + const handleMouseOut = () => { + Promise.resolve().then(() => Scheduler.log('Flush Child microtask')); + setState(1); + Scheduler.log('Child mouseout'); + }; + Scheduler.log('Render ' + state); + return {state}; + } + + await act(() => { + childRoot.render(); + parentRoot.render(); + }); + + const childNode = childContainer.firstChild; + const parentNode = parentContainer.firstChild; + + parentNode.appendChild(childContainer); + document.body.appendChild(parentContainer); + + assertLog(['Render Child']); + try { + await act(async () => { + await simulateEventDispatch(childNode, 'mouseout'); + }); + + // Since continuous events flush in a macrotask, they are batched after + // with the handler for the other root, but the microtasks scheduled + // in the event handlers still fire in between. + assertLog([ + 'Child mouseout', + 'Flush Child microtask', + 'Parent mouseout', + 'Flush Parent microtask', + 'Render 2', + ]); + } finally { + document.body.removeChild(parentContainer); + } + }); + + it('should fire on nodes removed while dispatching', async () => { + let childRef; + function Component() { + const parentRef = React.useRef(); + const middleRef = React.useRef(); + Scheduler.log(`Render`); + return ( +
{ + Scheduler.log('onMouseOut parent'); + }}> +
+
+
+ ); + } + + const container = document.createElement('div'); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + + assertLog(['Render']); + + await act(async () => { + await simulateEventDispatch(childRef, 'click'); + }); + + assertLog(['onMouseOut child', 'onMouseOut parent']); + }); + + it('should not fire if node is not in the document', async () => { + let childRef; + function Component() { + Scheduler.log(`Render`); + return ( +
{ + Scheduler.log('onMouseOut parent'); + }}> +
+ ); + } + + // Do not attach root to document. + const root = ReactDOMClient.createRoot(document.createElement('div')); + await act(() => { + root.render(); + }); + + assertLog(['Render']); + + await act(async () => { + await simulateEventDispatch(childRef, 'mouseout'); }); - assertLog(['onMouseOut child', 'onMouseOut parent', 'Render 1']); + // No events flushed, root not in document. + assertLog([]); }); }); }); diff --git a/packages/internal-test-utils/simulateBrowserEventDispatch.js b/packages/internal-test-utils/simulateBrowserEventDispatch.js new file mode 100644 index 0000000000000..daa6d97dafe88 --- /dev/null +++ b/packages/internal-test-utils/simulateBrowserEventDispatch.js @@ -0,0 +1,390 @@ +const idlUtils = require('jsdom/lib/jsdom/living/generated/utils'); +const DOMException = require('domexception/webidl2js-wrapper'); +const {nodeRoot} = require('jsdom/lib/jsdom/living/helpers/node'); +const { + isNode, + isShadowRoot, + isSlotable, + getEventTargetParent, + isShadowInclusiveAncestor, + retarget, +} = require('jsdom/lib/jsdom/living/helpers/shadow-dom'); + +const {waitForMicrotasks} = require('./ReactInternalTestUtils'); +const EVENT_PHASE = { + NONE: 0, + CAPTURING_PHASE: 1, + AT_TARGET: 2, + BUBBLING_PHASE: 3, +}; + +// Hack to get Symbol(wrapper) for target nodes. +let wrapperSymbol; +function wrapperForImpl(impl) { + if (impl == null) { + return null; + } + + return impl[wrapperSymbol]; +} + +// This is a forked implementation of the jsdom dispatchEvent. The goal of +// this fork is to match the actual browser behavior of user events more closely. +// Real browser events yield to microtasks in-between event handlers, which is +// different from programmatically calling dispatchEvent (which does not yield). +// JSDOM correctly implements programmatic dispatchEvent, but sometimes we need +// to test the behavior of real user interactions, so we simulate it. +// +// It's async because we need to wait for microtasks between event handlers. +// +// Taken from: +// https://github.com/jsdom/jsdom/blob/2f8a7302a43fff92f244d5f3426367a8eb2b8896/lib/jsdom/living/events/EventTarget-impl.js#L88 +async function simulateEventDispatch(eventImpl) { + if (eventImpl._dispatchFlag || !eventImpl._initializedFlag) { + throw DOMException.create(this._globalObject, [ + 'Tried to dispatch an uninitialized event', + 'InvalidStateError', + ]); + } + if (eventImpl.eventPhase !== EVENT_PHASE.NONE) { + throw DOMException.create(this._globalObject, [ + 'Tried to dispatch a dispatching event', + 'InvalidStateError', + ]); + } + + eventImpl.isTrusted = false; + + await _dispatch.call(this, eventImpl); +} + +async function _dispatch(eventImpl, legacyTargetOverrideFlag) { + // Hack: save the wrapper Symbol. + wrapperSymbol = Object.getOwnPropertySymbols(eventImpl)[0]; + + let targetImpl = this; + let clearTargets = false; + let activationTarget = null; + + eventImpl._dispatchFlag = true; + + const targetOverride = legacyTargetOverrideFlag + ? idlUtils.implForWrapper(targetImpl._globalObject._document) + : targetImpl; + let relatedTarget = retarget(eventImpl.relatedTarget, targetImpl); + + if (targetImpl !== relatedTarget || targetImpl === eventImpl.relatedTarget) { + const touchTargets = []; + + appendToEventPath( + eventImpl, + targetImpl, + targetOverride, + relatedTarget, + touchTargets, + false, + ); + + const isActivationEvent = false; // TODO Not ported in fork. + + if (isActivationEvent && targetImpl._hasActivationBehavior) { + activationTarget = targetImpl; + } + + let slotInClosedTree = false; + let slotable = + isSlotable(targetImpl) && targetImpl._assignedSlot ? targetImpl : null; + let parent = getEventTargetParent(targetImpl, eventImpl); + + // Populate event path + // https://dom.spec.whatwg.org/#event-path + while (parent !== null) { + if (slotable !== null) { + if (parent.localName !== 'slot') { + throw new Error(`JSDOM Internal Error: Expected parent to be a Slot`); + } + + slotable = null; + + const parentRoot = nodeRoot(parent); + if (isShadowRoot(parentRoot) && parentRoot.mode === 'closed') { + slotInClosedTree = true; + } + } + + if (isSlotable(parent) && parent._assignedSlot) { + slotable = parent; + } + + relatedTarget = retarget(eventImpl.relatedTarget, parent); + + if ( + (isNode(parent) && + isShadowInclusiveAncestor(nodeRoot(targetImpl), parent)) || + wrapperForImpl(parent).constructor.name === 'Window' + ) { + if ( + isActivationEvent && + eventImpl.bubbles && + activationTarget === null && + parent._hasActivationBehavior + ) { + activationTarget = parent; + } + + appendToEventPath( + eventImpl, + parent, + null, + relatedTarget, + touchTargets, + slotInClosedTree, + ); + } else if (parent === relatedTarget) { + parent = null; + } else { + targetImpl = parent; + + if ( + isActivationEvent && + activationTarget === null && + targetImpl._hasActivationBehavior + ) { + activationTarget = targetImpl; + } + + appendToEventPath( + eventImpl, + parent, + targetImpl, + relatedTarget, + touchTargets, + slotInClosedTree, + ); + } + + if (parent !== null) { + parent = getEventTargetParent(parent, eventImpl); + } + + slotInClosedTree = false; + } + + let clearTargetsStructIndex = -1; + for ( + let i = eventImpl._path.length - 1; + i >= 0 && clearTargetsStructIndex === -1; + i-- + ) { + if (eventImpl._path[i].target !== null) { + clearTargetsStructIndex = i; + } + } + const clearTargetsStruct = eventImpl._path[clearTargetsStructIndex]; + + clearTargets = + (isNode(clearTargetsStruct.target) && + isShadowRoot(nodeRoot(clearTargetsStruct.target))) || + (isNode(clearTargetsStruct.relatedTarget) && + isShadowRoot(nodeRoot(clearTargetsStruct.relatedTarget))); + + if ( + activationTarget !== null && + activationTarget._legacyPreActivationBehavior + ) { + activationTarget._legacyPreActivationBehavior(); + } + + for (let i = eventImpl._path.length - 1; i >= 0; --i) { + const struct = eventImpl._path[i]; + + if (struct.target !== null) { + eventImpl.eventPhase = EVENT_PHASE.AT_TARGET; + } else { + eventImpl.eventPhase = EVENT_PHASE.CAPTURING_PHASE; + } + + await invokeEventListeners(struct, eventImpl, 'capturing'); + } + + for (let i = 0; i < eventImpl._path.length; i++) { + const struct = eventImpl._path[i]; + + if (struct.target !== null) { + eventImpl.eventPhase = EVENT_PHASE.AT_TARGET; + } else { + if (!eventImpl.bubbles) { + continue; + } + + eventImpl.eventPhase = EVENT_PHASE.BUBBLING_PHASE; + } + + await invokeEventListeners(struct, eventImpl, 'bubbling'); + } + } + + eventImpl.eventPhase = EVENT_PHASE.NONE; + + eventImpl.currentTarget = null; + eventImpl._path = []; + eventImpl._dispatchFlag = false; + eventImpl._stopPropagationFlag = false; + eventImpl._stopImmediatePropagationFlag = false; + + if (clearTargets) { + eventImpl.target = null; + eventImpl.relatedTarget = null; + } + + if (activationTarget !== null) { + if (!eventImpl._canceledFlag) { + activationTarget._activationBehavior(eventImpl); + } else if (activationTarget._legacyCanceledActivationBehavior) { + activationTarget._legacyCanceledActivationBehavior(); + } + } + + return !eventImpl._canceledFlag; +} + +async function invokeEventListeners(struct, eventImpl, phase) { + const structIndex = eventImpl._path.indexOf(struct); + for (let i = structIndex; i >= 0; i--) { + const t = eventImpl._path[i]; + if (t.target) { + eventImpl.target = t.target; + break; + } + } + + eventImpl.relatedTarget = wrapperForImpl(struct.relatedTarget); + + if (eventImpl._stopPropagationFlag) { + return; + } + + eventImpl.currentTarget = wrapperForImpl(struct.item); + + const listeners = struct.item._eventListeners; + await innerInvokeEventListeners( + eventImpl, + listeners, + phase, + struct.itemInShadowTree, + ); +} + +async function innerInvokeEventListeners( + eventImpl, + listeners, + phase, + itemInShadowTree, +) { + let found = false; + + const {type, target} = eventImpl; + const wrapper = wrapperForImpl(target); + + if (!listeners || !listeners[type]) { + return found; + } + + // Copy event listeners before iterating since the list can be modified during the iteration. + const handlers = listeners[type].slice(); + + for (let i = 0; i < handlers.length; i++) { + const listener = handlers[i]; + const {capture, once, passive} = listener.options; + + // Check if the event listener has been removed since the listeners has been cloned. + if (!listeners[type].includes(listener)) { + continue; + } + + found = true; + + if ( + (phase === 'capturing' && !capture) || + (phase === 'bubbling' && capture) + ) { + continue; + } + + if (once) { + listeners[type].splice(listeners[type].indexOf(listener), 1); + } + + let window = null; + if (wrapper && wrapper._document) { + // Triggered by Window + window = wrapper; + } else if (target._ownerDocument) { + // Triggered by most webidl2js'ed instances + window = target._ownerDocument._defaultView; + } else if (wrapper._ownerDocument) { + // Currently triggered by some non-webidl2js things + window = wrapper._ownerDocument._defaultView; + } + + let currentEvent; + if (window) { + currentEvent = window._currentEvent; + if (!itemInShadowTree) { + window._currentEvent = eventImpl; + } + } + + if (passive) { + eventImpl._inPassiveListenerFlag = true; + } + + try { + listener.callback.call(eventImpl.currentTarget, eventImpl); + } catch (e) { + if (window) { + reportException(window, e); + } + // Errors in window-less documents just get swallowed... can you think of anything better? + } + + eventImpl._inPassiveListenerFlag = false; + + if (window) { + window._currentEvent = currentEvent; + } + + if (eventImpl._stopImmediatePropagationFlag) { + return found; + } + + // IMPORTANT: Flush microtasks + await waitForMicrotasks(); + } + + return found; +} + +function appendToEventPath( + eventImpl, + target, + targetOverride, + relatedTarget, + touchTargets, + slotInClosedTree, +) { + const itemInShadowTree = isNode(target) && isShadowRoot(nodeRoot(target)); + const rootOfClosedTree = isShadowRoot(target) && target.mode === 'closed'; + + eventImpl._path.push({ + item: target, + itemInShadowTree, + target: targetOverride, + relatedTarget, + touchTargets, + rootOfClosedTree, + slotInClosedTree, + }); +} + +export default simulateEventDispatch; diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 54803405f12d1..f059d7bd2f560 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -9,15 +9,13 @@ 'use strict'; -import {dispatchAndWaitForDiscrete} from 'internal-test-utils'; - describe('ReactDOMEventListener', () => { let React; let ReactDOM; let ReactDOMClient; let ReactDOMServer; let act; - let dispatchAndWaitForDiscrete; + let simulateEventDispatch; beforeEach(() => { jest.resetModules(); @@ -26,8 +24,8 @@ describe('ReactDOMEventListener', () => { ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); act = require('internal-test-utils').act; - dispatchAndWaitForDiscrete = - require('internal-test-utils').dispatchAndWaitForDiscrete; + simulateEventDispatch = + require('internal-test-utils').simulateEventDispatch; }); describe('Propagation', () => { @@ -161,21 +159,21 @@ describe('ReactDOMEventListener', () => { function Parent() { // eslint-disable-next-line no-unused-vars const [state, _] = React.useState('Parent'); - const handleMouseOut = () => { + const handleClick = () => { childSetState(2); mock(childContainer.firstChild.textContent); }; - return
{state}
; + return
{state}
; } function Child() { const [state, setState] = React.useState('Child'); childSetState = setState; - const handleMouseOut = () => { + const handleClick = () => { setState(1); mock(childContainer.firstChild.textContent); }; - return {state}; + return {state}; } await act(() => { @@ -191,7 +189,7 @@ describe('ReactDOMEventListener', () => { try { await act(async () => { - await dispatchAndWaitForDiscrete(childNode, 'click'); + await simulateEventDispatch(childNode, 'click'); }); // Child and parent should both call from event handlers. @@ -213,13 +211,7 @@ describe('ReactDOMEventListener', () => { // isInputPending?). // // Since this is a discrete event, the previous update is already done. - if (gate(flags => flags.enableLegacyFBSupport)) { - // Legacy FB support mode attaches to the document, which is a single event - // dispatch for both roots, so this is batched. - expect(mock.mock.calls[1][0]).toBe('Child'); - } else { - expect(mock.mock.calls[1][0]).toBe('1'); - } + expect(mock.mock.calls[1][0]).toBe('1'); // And by the time we leave the handler, the second update is flushed. expect(childNode.textContent).toBe('2'); @@ -271,7 +263,7 @@ describe('ReactDOMEventListener', () => { try { await act(async () => { - await dispatchAndWaitForDiscrete(childNode, 'mouseout'); + await simulateEventDispatch(childNode, 'mouseout'); }); // Child and parent should both call from event handlers. From b9ed28268c13e6714bfb1038fee3b7df2da020c2 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Mon, 5 Feb 2024 23:12:18 -0500 Subject: [PATCH 9/9] Fix lint --- packages/internal-test-utils/simulateBrowserEventDispatch.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/internal-test-utils/simulateBrowserEventDispatch.js b/packages/internal-test-utils/simulateBrowserEventDispatch.js index daa6d97dafe88..4dcf916a1d12c 100644 --- a/packages/internal-test-utils/simulateBrowserEventDispatch.js +++ b/packages/internal-test-utils/simulateBrowserEventDispatch.js @@ -1,6 +1,6 @@ -const idlUtils = require('jsdom/lib/jsdom/living/generated/utils'); const DOMException = require('domexception/webidl2js-wrapper'); const {nodeRoot} = require('jsdom/lib/jsdom/living/helpers/node'); +const reportException = require('jsdom/lib/jsdom/living/helpers/runtime-script-errors'); const { isNode, isShadowRoot, @@ -11,6 +11,7 @@ const { } = require('jsdom/lib/jsdom/living/helpers/shadow-dom'); const {waitForMicrotasks} = require('./ReactInternalTestUtils'); + const EVENT_PHASE = { NONE: 0, CAPTURING_PHASE: 1, @@ -69,7 +70,7 @@ async function _dispatch(eventImpl, legacyTargetOverrideFlag) { eventImpl._dispatchFlag = true; const targetOverride = legacyTargetOverrideFlag - ? idlUtils.implForWrapper(targetImpl._globalObject._document) + ? wrapperForImpl(targetImpl._globalObject._document) : targetImpl; let relatedTarget = retarget(eventImpl.relatedTarget, targetImpl);