From faf5dbc2b5c483125e49f911fa80a288965903ed Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 25 Jan 2024 23:31:57 -0800 Subject: [PATCH 1/4] Convert ReactIdentity-test.js to createRoot --- .../src/__tests__/ReactIdentity-test.js | 111 +++++++++++------- 1 file changed, 66 insertions(+), 45 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactIdentity-test.js b/packages/react-dom/src/__tests__/ReactIdentity-test.js index 47814fc39d18e..0bd04b8c20ff0 100644 --- a/packages/react-dom/src/__tests__/ReactIdentity-test.js +++ b/packages/react-dom/src/__tests__/ReactIdentity-test.js @@ -10,18 +10,20 @@ 'use strict'; let React; -let ReactDOM; +let ReactDOMClient; let ReactTestUtils; +let act; describe('ReactIdentity', () => { beforeEach(() => { jest.resetModules(); React = require('react'); - ReactDOM = require('react-dom'); + ReactDOMClient = require('react-dom/client'); ReactTestUtils = require('react-dom/test-utils'); + act = require('internal-test-utils').act; }); - it('should allow key property to express identity', () => { + it('should allow key property to express identity', async () => { let node; const Component = props => (
(node = c)}> @@ -31,15 +33,20 @@ describe('ReactIdentity', () => { ); const container = document.createElement('div'); - ReactDOM.render(, container); + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); const origChildren = Array.from(node.childNodes); - ReactDOM.render(, container); + await act(async () => { + root.render(); + }); const newChildren = Array.from(node.childNodes); expect(origChildren[0]).toBe(newChildren[1]); expect(origChildren[1]).toBe(newChildren[0]); }); - it('should use composite identity', () => { + it('should use composite identity', async () => { class Wrapper extends React.Component { render() { return {this.props.children}; @@ -47,25 +54,27 @@ describe('ReactIdentity', () => { } const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); let node1; let node2; - ReactDOM.render( - - (node1 = c)} /> - , - container, - ); - ReactDOM.render( - - (node2 = c)} /> - , - container, - ); - + await act(async () => { + root.render( + + (node1 = c)} /> + , + ); + }); + await act(async () => { + root.render( + + (node2 = c)} /> + , + ); + }); expect(node1).not.toBe(node2); }); - function renderAComponentWithKeyIntoContainer(key, container) { + async function renderAComponentWithKeyIntoContainer(key, root) { class Wrapper extends React.Component { spanRef = React.createRef(); render() { @@ -76,36 +85,41 @@ describe('ReactIdentity', () => { ); } } - - const instance = ReactDOM.render(, container); - const span = instance.spanRef.current; + const wrapperRef = React.createRef(); + await act(async () => { + root.render(); + }); + const span = wrapperRef.current.spanRef.current; expect(span).not.toBe(null); } - it('should allow any character as a key, in a detached parent', () => { + it('should allow any character as a key, in a detached parent', async () => { const detachedContainer = document.createElement('div'); - renderAComponentWithKeyIntoContainer("<'WEIRD/&\\key'>", detachedContainer); + const root = ReactDOMClient.createRoot(detachedContainer); + await renderAComponentWithKeyIntoContainer("<'WEIRD/&\\key'>", root); }); - it('should allow any character as a key, in an attached parent', () => { + it('should allow any character as a key, in an attached parent', async () => { // This test exists to protect against implementation details that // incorrectly query escaped IDs using DOM tools like getElementById. const attachedContainer = document.createElement('div'); + const root = ReactDOMClient.createRoot(attachedContainer); document.body.appendChild(attachedContainer); - renderAComponentWithKeyIntoContainer("<'WEIRD/&\\key'>", attachedContainer); + await renderAComponentWithKeyIntoContainer("<'WEIRD/&\\key'>", root); document.body.removeChild(attachedContainer); }); - it('should not allow scripts in keys to execute', () => { + it('should not allow scripts in keys to execute', async () => { const h4x0rKey = '">
; const instance1 = ; - let wrapped = ; - - wrapped = ReactDOM.render(wrapped, document.createElement('div')); - const div = ReactDOM.findDOMNode(wrapped); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + let divRef = React.createRef(); + await act(async () => { + root.render( + , + ); + }); + // this doesn't work + // divRef.current doesn't have `childNodes` attribute + const div = divRef.current; const beforeA = div.childNodes[0]; const beforeB = div.childNodes[1]; - wrapped.swap(); + div.swap(); const afterA = div.childNodes[1]; const afterB = div.childNodes[0]; @@ -264,7 +285,7 @@ describe('ReactIdentity', () => { }).not.toThrow(); }); - it('should throw if key is a Temporal-like object', () => { + it('should throw if key is a Temporal-like object', async () => { class TemporalLike { valueOf() { // Throwing here is the behavior of ECMAScript "Temporal" date/time API. @@ -277,15 +298,15 @@ describe('ReactIdentity', () => { } const el = document.createElement('div'); - const test = () => - ReactDOM.render( -
- -
, - el, - ); - expect(() => - expect(test).toThrowError(new TypeError('prod message')), + const root = ReactDOMClient.createRoot(el); + await expect(() => + expect(() => { + root.render( +
+ +
, + ); + }).toThrowError(new TypeError('prod message')), ).toErrorDev( 'The provided key is an unsupported type TemporalLike.' + ' This value must be coerced to a string before using it here.', From 9838024594a12948734f5b684ae860cc92cd6935 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 25 Jan 2024 23:42:02 -0800 Subject: [PATCH 2/4] Fix test that requires findDOMNode --- .../src/__tests__/ReactIdentity-test.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactIdentity-test.js b/packages/react-dom/src/__tests__/ReactIdentity-test.js index 0bd04b8c20ff0..746870e79e939 100644 --- a/packages/react-dom/src/__tests__/ReactIdentity-test.js +++ b/packages/react-dom/src/__tests__/ReactIdentity-test.js @@ -10,6 +10,7 @@ 'use strict'; let React; +let ReactDOM; let ReactDOMClient; let ReactTestUtils; let act; @@ -18,6 +19,7 @@ describe('ReactIdentity', () => { beforeEach(() => { jest.resetModules(); React = require('react'); + ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactTestUtils = require('react-dom/test-utils'); act = require('internal-test-utils').act; @@ -252,19 +254,22 @@ describe('ReactIdentity', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - let divRef = React.createRef(); + let wrappedRef = React.createRef(); await act(async () => { root.render( - , + , ); }); - // this doesn't work - // divRef.current doesn't have `childNodes` attribute - const div = divRef.current; + const wrapped = wrappedRef.current; + // There isn't an alternative to findDOMNode for this: + // https://react.dev/reference/react-dom/findDOMNode#alternatives + const div = ReactDOM.findDOMNode(wrapped); const beforeA = div.childNodes[0]; const beforeB = div.childNodes[1]; - div.swap(); + await act(async () => { + wrapped.swap(); + }); const afterA = div.childNodes[1]; const afterB = div.childNodes[0]; From 3720e9334544d7d282ea5cf231beb2a283e974d7 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 25 Jan 2024 23:54:18 -0800 Subject: [PATCH 3/4] Fix lint error --- packages/react-dom/src/__tests__/ReactIdentity-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactIdentity-test.js b/packages/react-dom/src/__tests__/ReactIdentity-test.js index 746870e79e939..9a5780ec69247 100644 --- a/packages/react-dom/src/__tests__/ReactIdentity-test.js +++ b/packages/react-dom/src/__tests__/ReactIdentity-test.js @@ -254,7 +254,7 @@ describe('ReactIdentity', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - let wrappedRef = React.createRef(); + const wrappedRef = React.createRef(); await act(async () => { root.render( , From beb9db6af06eba1c7bd4a3e20fbe768f7b5fa247 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Fri, 26 Jan 2024 09:13:39 -0800 Subject: [PATCH 4/4] remove findDOMNode --- .../src/__tests__/ReactIdentity-test.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactIdentity-test.js b/packages/react-dom/src/__tests__/ReactIdentity-test.js index 9a5780ec69247..5c25daf7d8f30 100644 --- a/packages/react-dom/src/__tests__/ReactIdentity-test.js +++ b/packages/react-dom/src/__tests__/ReactIdentity-test.js @@ -10,7 +10,6 @@ 'use strict'; let React; -let ReactDOM; let ReactDOMClient; let ReactTestUtils; let act; @@ -19,7 +18,6 @@ describe('ReactIdentity', () => { beforeEach(() => { jest.resetModules(); React = require('react'); - ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactTestUtils = require('react-dom/test-utils'); act = require('internal-test-utils').act; @@ -260,18 +258,15 @@ describe('ReactIdentity', () => { , ); }); - const wrapped = wrappedRef.current; - // There isn't an alternative to findDOMNode for this: - // https://react.dev/reference/react-dom/findDOMNode#alternatives - const div = ReactDOM.findDOMNode(wrapped); + const div = container.firstChild; - const beforeA = div.childNodes[0]; - const beforeB = div.childNodes[1]; + const beforeA = div.firstChild; + const beforeB = div.lastChild; await act(async () => { - wrapped.swap(); + wrappedRef.current.swap(); }); - const afterA = div.childNodes[1]; - const afterB = div.childNodes[0]; + const afterA = div.lastChild; + const afterB = div.firstChild; expect(beforeA).toBe(afterA); expect(beforeB).toBe(afterB);