From 9e124aff56470d0090a841ab0471028d474e903b Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 6 Feb 2024 00:09:58 -0500 Subject: [PATCH 1/4] Convert ReactBrowserEventEmitter to createRoot --- .../ReactBrowserEventEmitter-test.js | 216 ++++++++++++------ 1 file changed, 141 insertions(+), 75 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index a226c3736bc26..53819c51d6037 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -10,8 +10,10 @@ 'use strict'; let React; -let ReactDOM; +let ReactDOMX; +let ReactDOMClient; let ReactTestUtils; +let act; let idCallOrder; const recordID = function (id) { @@ -42,14 +44,15 @@ let container; // This test is written in a bizarre way because it was previously using internals. // It should probably be rewritten but we're keeping it for some extra coverage. describe('ReactBrowserEventEmitter', () => { - beforeEach(() => { + beforeEach(async () => { jest.resetModules(); LISTENER.mockClear(); React = require('react'); - ReactDOM = require('react-dom'); + ReactDOMX = require('react-dom'); + ReactDOMClient = require('react-dom/client'); ReactTestUtils = require('react-dom/test-utils'); - + act = require('internal-test-utils').act; container = document.createElement('div'); document.body.appendChild(container); @@ -68,21 +71,28 @@ describe('ReactBrowserEventEmitter', () => { } } - function renderTree() { - ReactDOM.render( -
(GRANDPARENT = c)} {...GRANDPARENT_PROPS}> -
(PARENT = c)} {...PARENT_PROPS}> - -
-
, - container, - ); + const root = ReactDOMClient.createRoot(container); + + async function renderTree() { + await act(() => { + root.render( +
(GRANDPARENT = c)} {...GRANDPARENT_PROPS}> +
(PARENT = c)} {...PARENT_PROPS}> + +
+
, + ); + }); } - renderTree(); + await renderTree(); - putListener = function (node, eventName, listener) { + putListener = async function (node, eventName, listener) { switch (node) { case CHILD: CHILD_PROPS[eventName] = listener; @@ -98,9 +108,9 @@ describe('ReactBrowserEventEmitter', () => { break; } // Rerender with new event listeners - renderTree(); + await renderTree(); }; - deleteAllListeners = function (node) { + deleteAllListeners = async function (node) { switch (node) { case CHILD: CHILD_PROPS = {}; @@ -115,7 +125,7 @@ describe('ReactBrowserEventEmitter', () => { BUTTON_PROPS = {}; break; } - renderTree(); + await renderTree(); }; idCallOrder = []; @@ -126,120 +136,170 @@ describe('ReactBrowserEventEmitter', () => { container = null; }); - it('should bubble simply', () => { - putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); - putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - CHILD.click(); + it('should bubble simply', async () => { + await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); + await putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); expect(idCallOrder[2]).toBe(GRANDPARENT); }); - it('should bubble to the right handler after an update', () => { - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, 'GRANDPARENT')); - putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, 'PARENT')); - putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, 'CHILD')); - CHILD.click(); + it('should bubble to the right handler after an update', async () => { + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, 'GRANDPARENT'), + ); + await putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, 'PARENT')); + await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, 'CHILD')); + await act(() => { + CHILD.click(); + }); expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'GRANDPARENT']); idCallOrder = []; // Update just the grand parent without updating the child. - putListener( + await putListener( GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, 'UPDATED_GRANDPARENT'), ); - CHILD.click(); + await act(() => { + CHILD.click(); + }); expect(idCallOrder).toEqual(['CHILD', 'PARENT', 'UPDATED_GRANDPARENT']); }); - it('should continue bubbling if an error is thrown', () => { - putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); - putListener(PARENT, ON_CLICK_KEY, function () { + it('should continue bubbling if an error is thrown', async () => { + await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); + await putListener(PARENT, ON_CLICK_KEY, function () { recordID(PARENT); throw new Error('Handler interrupted'); }); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - expect(function () { - ReactTestUtils.Simulate.click(CHILD); - }).toThrow(); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await expect( + act(() => { + ReactTestUtils.Simulate.click(CHILD); + }), + ).rejects.toThrow(); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); expect(idCallOrder[2]).toBe(GRANDPARENT); }); - it('should set currentTarget', () => { - putListener(CHILD, ON_CLICK_KEY, function (event) { + it('should set currentTarget', async () => { + await putListener(CHILD, ON_CLICK_KEY, function (event) { recordID(CHILD); expect(event.currentTarget).toBe(CHILD); }); - putListener(PARENT, ON_CLICK_KEY, function (event) { + await putListener(PARENT, ON_CLICK_KEY, function (event) { recordID(PARENT); expect(event.currentTarget).toBe(PARENT); }); - putListener(GRANDPARENT, ON_CLICK_KEY, function (event) { + await putListener(GRANDPARENT, ON_CLICK_KEY, function (event) { recordID(GRANDPARENT); expect(event.currentTarget).toBe(GRANDPARENT); }); - CHILD.click(); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); expect(idCallOrder[2]).toBe(GRANDPARENT); }); - it('should support stopPropagation()', () => { - putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); - putListener( + it('should support stopPropagation()', async () => { + await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); + await putListener( PARENT, ON_CLICK_KEY, recordIDAndStopPropagation.bind(null, PARENT), ); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - CHILD.click(); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(2); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); }); - it('should support overriding .isPropagationStopped()', () => { + it('should support overriding .isPropagationStopped()', async () => { // Ew. See D4504876. - putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); - putListener(PARENT, ON_CLICK_KEY, function (e) { + await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); + await putListener(PARENT, ON_CLICK_KEY, function (e) { recordID(PARENT, e); // This stops React bubbling but avoids touching the native event e.isPropagationStopped = () => true; }); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - CHILD.click(); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(2); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); }); - it('should stop after first dispatch if stopPropagation', () => { - putListener( + it('should stop after first dispatch if stopPropagation', async () => { + await putListener( CHILD, ON_CLICK_KEY, recordIDAndStopPropagation.bind(null, CHILD), ); - putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - CHILD.click(); + await putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(1); expect(idCallOrder[0]).toBe(CHILD); }); - it('should not stopPropagation if false is returned', () => { - putListener(CHILD, ON_CLICK_KEY, recordIDAndReturnFalse.bind(null, CHILD)); - putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); - putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT)); - CHILD.click(); + it('should not stopPropagation if false is returned', async () => { + await putListener( + CHILD, + ON_CLICK_KEY, + recordIDAndReturnFalse.bind(null, CHILD), + ); + await putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); + await putListener( + GRANDPARENT, + ON_CLICK_KEY, + recordID.bind(null, GRANDPARENT), + ); + await act(() => { + CHILD.click(); + }); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); @@ -255,30 +315,36 @@ describe('ReactBrowserEventEmitter', () => { * these new listeners. */ - it('should invoke handlers that were removed while bubbling', () => { + it('should invoke handlers that were removed while bubbling', async () => { const handleParentClick = jest.fn(); const handleChildClick = function (event) { deleteAllListeners(PARENT); }; - putListener(CHILD, ON_CLICK_KEY, handleChildClick); - putListener(PARENT, ON_CLICK_KEY, handleParentClick); - CHILD.click(); + await putListener(CHILD, ON_CLICK_KEY, handleChildClick); + await putListener(PARENT, ON_CLICK_KEY, handleParentClick); + await act(() => { + CHILD.click(); + }); expect(handleParentClick).toHaveBeenCalledTimes(1); }); - it('should not invoke newly inserted handlers while bubbling', () => { + it('should not invoke newly inserted handlers while bubbling', async () => { const handleParentClick = jest.fn(); - const handleChildClick = function (event) { - putListener(PARENT, ON_CLICK_KEY, handleParentClick); + const handleChildClick = async function (event) { + await putListener(PARENT, ON_CLICK_KEY, handleParentClick); }; - putListener(CHILD, ON_CLICK_KEY, handleChildClick); - CHILD.click(); + await putListener(CHILD, ON_CLICK_KEY, handleChildClick); + await act(() => { + CHILD.click(); + }); expect(handleParentClick).toHaveBeenCalledTimes(0); }); - it('should have mouse enter simulated by test utils', () => { - putListener(CHILD, ON_MOUSE_ENTER_KEY, recordID.bind(null, CHILD)); - ReactTestUtils.Simulate.mouseEnter(CHILD); + it('should have mouse enter simulated by test utils', async () => { + await putListener(CHILD, ON_MOUSE_ENTER_KEY, recordID.bind(null, CHILD)); + await act(() => { + ReactTestUtils.Simulate.mouseEnter(CHILD); + }); expect(idCallOrder.length).toBe(1); expect(idCallOrder[0]).toBe(CHILD); }); From d5fef1c601e7e8983e5745dcfaa6f5b4510bd655 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 6 Feb 2024 00:24:35 -0500 Subject: [PATCH 2/4] Fix lint --- .../react-dom/src/__tests__/ReactBrowserEventEmitter-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index 53819c51d6037..ddbc4274c070e 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -10,7 +10,6 @@ 'use strict'; let React; -let ReactDOMX; let ReactDOMClient; let ReactTestUtils; let act; @@ -49,7 +48,6 @@ describe('ReactBrowserEventEmitter', () => { LISTENER.mockClear(); React = require('react'); - ReactDOMX = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactTestUtils = require('react-dom/test-utils'); act = require('internal-test-utils').act; From 1c121d0ec4f475a31392e1b8fa0f70bb24ae5912 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 6 Feb 2024 01:16:26 -0500 Subject: [PATCH 3/4] plz work --- .../src/__tests__/ReactBrowserEventEmitter-test.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index ddbc4274c070e..c4a1d4e4d9f4e 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -43,7 +43,7 @@ let container; // This test is written in a bizarre way because it was previously using internals. // It should probably be rewritten but we're keeping it for some extra coverage. describe('ReactBrowserEventEmitter', () => { - beforeEach(async () => { + beforeEach(() => { jest.resetModules(); LISTENER.mockClear(); @@ -88,8 +88,6 @@ describe('ReactBrowserEventEmitter', () => { }); } - await renderTree(); - putListener = async function (node, eventName, listener) { switch (node) { case CHILD: @@ -108,6 +106,7 @@ describe('ReactBrowserEventEmitter', () => { // Rerender with new event listeners await renderTree(); }; + deleteAllListeners = async function (node) { switch (node) { case CHILD: @@ -127,6 +126,8 @@ describe('ReactBrowserEventEmitter', () => { }; idCallOrder = []; + + return renderTree(); }); afterEach(() => { @@ -315,8 +316,8 @@ describe('ReactBrowserEventEmitter', () => { it('should invoke handlers that were removed while bubbling', async () => { const handleParentClick = jest.fn(); - const handleChildClick = function (event) { - deleteAllListeners(PARENT); + const handleChildClick = async function (event) { + await deleteAllListeners(PARENT); }; await putListener(CHILD, ON_CLICK_KEY, handleChildClick); await putListener(PARENT, ON_CLICK_KEY, handleParentClick); From d3a81b38ff7f2c32f323aa9c2d6fa39de5e90afc Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 6 Feb 2024 10:08:39 -0500 Subject: [PATCH 4/4] Try this --- .../__tests__/ReactBrowserEventEmitter-test.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index c4a1d4e4d9f4e..1dd665ce01145 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -35,6 +35,7 @@ let PARENT; let CHILD; let BUTTON; +let renderTree; let putListener; let deleteAllListeners; @@ -71,7 +72,7 @@ describe('ReactBrowserEventEmitter', () => { const root = ReactDOMClient.createRoot(container); - async function renderTree() { + renderTree = async function () { await act(() => { root.render(
(GRANDPARENT = c)} {...GRANDPARENT_PROPS}> @@ -86,7 +87,7 @@ describe('ReactBrowserEventEmitter', () => {
, ); }); - } + }; putListener = async function (node, eventName, listener) { switch (node) { @@ -126,8 +127,6 @@ describe('ReactBrowserEventEmitter', () => { }; idCallOrder = []; - - return renderTree(); }); afterEach(() => { @@ -136,6 +135,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should bubble simply', async () => { + await renderTree(); await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); await putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT)); await putListener( @@ -153,6 +153,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should bubble to the right handler after an update', async () => { + await renderTree(); await putListener( GRANDPARENT, ON_CLICK_KEY, @@ -181,6 +182,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should continue bubbling if an error is thrown', async () => { + await renderTree(); await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); await putListener(PARENT, ON_CLICK_KEY, function () { recordID(PARENT); @@ -203,6 +205,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should set currentTarget', async () => { + await renderTree(); await putListener(CHILD, ON_CLICK_KEY, function (event) { recordID(CHILD); expect(event.currentTarget).toBe(CHILD); @@ -225,6 +228,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should support stopPropagation()', async () => { + await renderTree(); await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); await putListener( PARENT, @@ -245,6 +249,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should support overriding .isPropagationStopped()', async () => { + await renderTree(); // Ew. See D4504876. await putListener(CHILD, ON_CLICK_KEY, recordID.bind(null, CHILD)); await putListener(PARENT, ON_CLICK_KEY, function (e) { @@ -266,6 +271,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should stop after first dispatch if stopPropagation', async () => { + await renderTree(); await putListener( CHILD, ON_CLICK_KEY, @@ -285,6 +291,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should not stopPropagation if false is returned', async () => { + await renderTree(); await putListener( CHILD, ON_CLICK_KEY, @@ -315,6 +322,7 @@ describe('ReactBrowserEventEmitter', () => { */ it('should invoke handlers that were removed while bubbling', async () => { + await renderTree(); const handleParentClick = jest.fn(); const handleChildClick = async function (event) { await deleteAllListeners(PARENT); @@ -328,6 +336,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should not invoke newly inserted handlers while bubbling', async () => { + await renderTree(); const handleParentClick = jest.fn(); const handleChildClick = async function (event) { await putListener(PARENT, ON_CLICK_KEY, handleParentClick); @@ -340,6 +349,7 @@ describe('ReactBrowserEventEmitter', () => { }); it('should have mouse enter simulated by test utils', async () => { + await renderTree(); await putListener(CHILD, ON_MOUSE_ENTER_KEY, recordID.bind(null, CHILD)); await act(() => { ReactTestUtils.Simulate.mouseEnter(CHILD);