From da986032cb12384a5f56968da8e703f3dc67a093 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 31 Jan 2024 20:56:02 +0100 Subject: [PATCH 1/2] Convert ReactMultiChildReconcile to createRoot --- .../ReactMultiChildReconcile-test.js | 317 ++++++++++-------- 1 file changed, 185 insertions(+), 132 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js index 0bfb27ade197b..45df4e5dd74a0 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js @@ -10,7 +10,8 @@ 'use strict'; const React = require('react'); -const ReactDOM = require('react-dom'); +const ReactDOMClient = require('react-dom/client'); +const act = require('internal-test-utils').act; const stripEmptyValues = function (obj) { const ret = {}; @@ -221,24 +222,40 @@ function verifyDomOrderingAccurate(outerContainer, statusDisplays) { expect(orderedDomKeys).toEqual(orderedLogicalKeys); } -function testPropsSequenceWithPreparedChildren(sequence, prepareChildren) { +async function testPropsSequenceWithPreparedChildren( + sequence, + prepareChildren, +) { const container = document.createElement('div'); - const parentInstance = ReactDOM.render( - , - container, - ); + const root = ReactDOMClient.createRoot(container); + let parentInstance; + await act(() => { + root.render( + { + if (parentInstance === undefined) { + parentInstance = current; + } + }} + />, + ); + }); let statusDisplays = parentInstance.getStatusDisplays(); let lastInternalStates = getInternalStateByUserName(statusDisplays); verifyStatuses(statusDisplays, sequence[0]); for (let i = 1; i < sequence.length; i++) { - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); verifyStatuses(statusDisplays, sequence[i]); verifyStatesPreserved(lastInternalStates, statusDisplays); @@ -274,13 +291,13 @@ function prepareChildrenModernIterable(childrenArray) { }; } -function testPropsSequence(sequence) { - testPropsSequenceWithPreparedChildren(sequence, prepareChildrenArray); - testPropsSequenceWithPreparedChildren( +async function testPropsSequence(sequence) { + await testPropsSequenceWithPreparedChildren(sequence, prepareChildrenArray); + await testPropsSequenceWithPreparedChildren( sequence, prepareChildrenLegacyIterable, ); - testPropsSequenceWithPreparedChildren( + await testPropsSequenceWithPreparedChildren( sequence, prepareChildrenModernIterable, ); @@ -291,7 +308,7 @@ describe('ReactMultiChildReconcile', () => { jest.resetModules(); }); - it('should reset internal state if removed then readded in an array', () => { + it('should reset internal state if removed then readded in an array', async () => { // Test basics. const props = { usernameToStatus: { @@ -300,32 +317,42 @@ describe('ReactMultiChildReconcile', () => { }; const container = document.createElement('div'); - const parentInstance = ReactDOM.render( - , - container, - ); + const root = ReactDOMClient.createRoot(container); + let parentInstance; + await act(() => { + root.render( + { + if (parentInstance === undefined) { + parentInstance = current; + } + }} + />, + ); + }); let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); - // Now remove the child. - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); - // Now reset the props that cause there to be a child - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeTruthy(); expect(statusDisplays.jcw.getInternalState()).not.toBe( @@ -333,7 +360,7 @@ describe('ReactMultiChildReconcile', () => { ); }); - it('should reset internal state if removed then readded in a legacy iterable', () => { + it('should reset internal state if removed then readded in a legacy iterable', async () => { // Test basics. const props = { usernameToStatus: { @@ -342,32 +369,45 @@ describe('ReactMultiChildReconcile', () => { }; const container = document.createElement('div'); - const parentInstance = ReactDOM.render( - , - container, - ); + const root = ReactDOMClient.createRoot(container); + let parentInstance; + await act(() => { + root.render( + { + if (parentInstance === undefined) { + parentInstance = current; + } + }} + />, + ); + }); + let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); - // Now remove the child. - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); - // Now reset the props that cause there to be a child - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeTruthy(); expect(statusDisplays.jcw.getInternalState()).not.toBe( @@ -375,7 +415,7 @@ describe('ReactMultiChildReconcile', () => { ); }); - it('should reset internal state if removed then readded in a modern iterable', () => { + it('should reset internal state if removed then readded in a modern iterable', async () => { // Test basics. const props = { usernameToStatus: { @@ -384,32 +424,45 @@ describe('ReactMultiChildReconcile', () => { }; const container = document.createElement('div'); - const parentInstance = ReactDOM.render( - , - container, - ); + const root = ReactDOMClient.createRoot(container); + let parentInstance; + await act(() => { + root.render( + { + if (parentInstance === undefined) { + parentInstance = current; + } + }} + />, + ); + }); + let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); - // Now remove the child. - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); - // Now reset the props that cause there to be a child - ReactDOM.render( - , - container, - ); + await act(() => { + root.render( + , + ); + }); + statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeTruthy(); expect(statusDisplays.jcw.getInternalState()).not.toBe( @@ -417,7 +470,7 @@ describe('ReactMultiChildReconcile', () => { ); }); - it('should create unique identity', () => { + it('should create unique identity', async () => { // Test basics. const usernameToStatus = { jcw: 'jcwStatus', @@ -425,10 +478,10 @@ describe('ReactMultiChildReconcile', () => { bob: 'bobStatus', }; - testPropsSequence([{usernameToStatus: usernameToStatus}]); + await testPropsSequence([{usernameToStatus: usernameToStatus}]); }); - it('should preserve order if children order has not changed', () => { + it('should preserve order if children order has not changed', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -443,10 +496,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should transition from zero to one children correctly', () => { + it('should transition from zero to one children correctly', async () => { const PROPS_SEQUENCE = [ {usernameToStatus: {}}, { @@ -455,10 +508,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should transition from one to zero children correctly', () => { + it('should transition from one to zero children correctly', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -467,11 +520,11 @@ describe('ReactMultiChildReconcile', () => { }, {usernameToStatus: {}}, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should transition from one child to null children', () => { - testPropsSequence([ + it('should transition from one child to null children', async () => { + await testPropsSequence([ { usernameToStatus: { first: 'firstStatus', @@ -481,8 +534,8 @@ describe('ReactMultiChildReconcile', () => { ]); }); - it('should transition from null children to one child', () => { - testPropsSequence([ + it('should transition from null children to one child', async () => { + await testPropsSequence([ {}, { usernameToStatus: { @@ -492,8 +545,8 @@ describe('ReactMultiChildReconcile', () => { ]); }); - it('should transition from zero children to null children', () => { - testPropsSequence([ + it('should transition from zero children to null children', async () => { + await testPropsSequence([ { usernameToStatus: {}, }, @@ -501,8 +554,8 @@ describe('ReactMultiChildReconcile', () => { ]); }); - it('should transition from null children to zero children', () => { - testPropsSequence([ + it('should transition from null children to zero children', async () => { + await testPropsSequence([ {}, { usernameToStatus: {}, @@ -514,7 +567,7 @@ describe('ReactMultiChildReconcile', () => { * `FriendsStatusDisplay` renders nulls as empty children (it's a convention * of `FriendsStatusDisplay`, nothing related to React or these test cases. */ - it('should remove nulled out children at the beginning', () => { + it('should remove nulled out children at the beginning', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -529,10 +582,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should remove nulled out children at the end', () => { + it('should remove nulled out children at the end', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -547,10 +600,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should reverse the order of two children', () => { + it('should reverse the order of two children', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -565,10 +618,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should reverse the order of more than two children', () => { + it('should reverse the order of more than two children', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -585,10 +638,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should cycle order correctly', () => { + it('should cycle order correctly', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -632,10 +685,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should cycle order correctly in the other direction', () => { + it('should cycle order correctly in the other direction', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -679,10 +732,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should remove nulled out children and ignore new null children', () => { + it('should remove nulled out children and ignore new null children', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -698,10 +751,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should remove nulled out children and reorder remaining', () => { + it('should remove nulled out children and reorder remaining', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -719,10 +772,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should append children to the end', () => { + it('should append children to the end', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -738,10 +791,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should append multiple children to the end', () => { + it('should append multiple children to the end', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -758,10 +811,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should prepend children to the beginning', () => { + it('should prepend children to the beginning', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -777,10 +830,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should prepend multiple children to the beginning', () => { + it('should prepend multiple children to the beginning', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -797,10 +850,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should not prepend an empty child to the beginning', () => { + it('should not prepend an empty child to the beginning', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -816,10 +869,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should not append an empty child to the end', () => { + it('should not append an empty child to the end', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -835,10 +888,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should not insert empty children in the middle', () => { + it('should not insert empty children in the middle', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -856,10 +909,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should insert one new child in the middle', () => { + it('should insert one new child in the middle', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -875,10 +928,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should insert multiple new truthy children in the middle', () => { + it('should insert multiple new truthy children in the middle', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -896,10 +949,10 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); - it('should insert non-empty children in middle where nulls were', () => { + it('should insert non-empty children in middle where nulls were', async () => { const PROPS_SEQUENCE = [ { usernameToStatus: { @@ -920,6 +973,6 @@ describe('ReactMultiChildReconcile', () => { }, }, ]; - testPropsSequence(PROPS_SEQUENCE); + await testPropsSequence(PROPS_SEQUENCE); }); }); From 4d14dbd305f0584dbdb7a59e283b96289fa5966e Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 4 Feb 2024 11:01:52 +0100 Subject: [PATCH 2/2] Restore comments --- .../src/__tests__/ReactMultiChildReconcile-test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js index 45df4e5dd74a0..3a2a5126882ce 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js @@ -335,6 +335,7 @@ describe('ReactMultiChildReconcile', () => { let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); + // Now remove the child. await act(() => { root.render( , @@ -344,6 +345,7 @@ describe('ReactMultiChildReconcile', () => { statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); + // Now reset the props that cause there to be a child await act(() => { root.render( { let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); + // Now remove the child. await act(() => { root.render( { statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); + // Now reset the props that cause there to be a child await act(() => { root.render( { let statusDisplays = parentInstance.getStatusDisplays(); const startingInternalState = statusDisplays.jcw.getInternalState(); + // Now remove the child. await act(() => { root.render( { statusDisplays = parentInstance.getStatusDisplays(); expect(statusDisplays.jcw).toBeFalsy(); + // Now reset the props that cause there to be a child await act(() => { root.render(