From 5b3cf780289cbc29c9a97861bf0e71088732eeee Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 14 Feb 2024 19:49:53 +0100 Subject: [PATCH 1/2] Remove usage of ReactTestUtils from ReactFunctionComponent --- .../__tests__/ReactFunctionComponent-test.js | 201 ++++++++++++------ 1 file changed, 137 insertions(+), 64 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js index 2d04c4af90937..e1692ddcd1904 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -12,7 +12,6 @@ let PropTypes; let React; let ReactDOMClient; -let ReactTestUtils; let act; function FunctionComponent(props) { @@ -26,7 +25,6 @@ describe('ReactFunctionComponent', () => { React = require('react'); ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; - ReactTestUtils = require('react-dom/test-utils'); }); it('should render stateless component', async () => { @@ -161,25 +159,33 @@ describe('ReactFunctionComponent', () => { ); }); - it('should not throw when stateless component returns undefined', () => { + it('should not throw when stateless component returns undefined', async () => { function NotAComponent() {} - expect(function () { - ReactTestUtils.renderIntoDocument( -
- -
, - ); - }).not.toThrowError(); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect( + act(() => { + root.render( +
+ +
, + ); + }), + ).resolves.not.toThrowError(); }); - it('should throw on string refs in pure functions', () => { + it('should throw on string refs in pure functions', async () => { function Child() { return
; } - expect(function () { - ReactTestUtils.renderIntoDocument(); - }).toThrowError( + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect( + act(() => { + root.render(); + }), + ).rejects.toThrowError( __DEV__ ? 'Function components cannot have string refs. We recommend using useRef() instead.' : // It happens because we don't save _owner in production for @@ -193,7 +199,7 @@ describe('ReactFunctionComponent', () => { ); }); - it('should warn when given a string ref', () => { + it('should warn when given a string ref', async () => { function Indirection(props) { return
{props.children}
; } @@ -208,9 +214,13 @@ describe('ReactFunctionComponent', () => { } } - expect(() => - ReactTestUtils.renderIntoDocument(), - ).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: Function components cannot be given refs. ' + 'Attempts to access this ref will fail. ' + 'Did you mean to use React.forwardRef()?\n\n' + @@ -222,11 +232,14 @@ describe('ReactFunctionComponent', () => { ' in ParentUsingStringRef (at **)', ); - // No additional warnings should be logged - ReactTestUtils.renderIntoDocument(); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); }); - it('should warn when given a function ref', () => { + it('should warn when given a function ref', async () => { function Indirection(props) { return
{props.children}
; } @@ -246,9 +259,13 @@ describe('ReactFunctionComponent', () => { } } - expect(() => - ReactTestUtils.renderIntoDocument(), - ).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: Function components cannot be given refs. ' + 'Attempts to access this ref will fail. ' + 'Did you mean to use React.forwardRef()?\n\n' + @@ -260,11 +277,14 @@ describe('ReactFunctionComponent', () => { ' in ParentUsingFunctionRef (at **)', ); - // No additional warnings should be logged - ReactTestUtils.renderIntoDocument(); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); }); - it('deduplicates ref warnings based on element or owner', () => { + it('deduplicates ref warnings based on element or owner', async () => { // When owner uses JSX, we can use exact line location to dedupe warnings class AnonymousParentUsingJSX extends React.Component { render() { @@ -274,15 +294,24 @@ describe('ReactFunctionComponent', () => { let instance1; - expect(() => { - instance1 = ReactTestUtils.renderIntoDocument( - , - ); + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + (instance1 = current)} />, + ); + }); }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (offending element is on the same line): instance1.forceUpdate(); - // Should also be deduped (offending element is on the same line): - ReactTestUtils.renderIntoDocument(); + let container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); // When owner doesn't use JSX, and is anonymous, we warn once per internal instance. class AnonymousParentNotUsingJSX extends React.Component { @@ -295,15 +324,22 @@ describe('ReactFunctionComponent', () => { } let instance2; - expect(() => { - instance2 = ReactTestUtils.renderIntoDocument( - , - ); + await expect(async () => { + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + (instance2 = current)} />, + ); + }); }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (same internal instance, no additional warnings) instance2.forceUpdate(); - // Could not be differentiated (since owner is anonymous and no source location) - ReactTestUtils.renderIntoDocument(); + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); // When owner doesn't use JSX, but is named, we warn once per owner name class NamedParentNotUsingJSX extends React.Component { @@ -315,19 +351,28 @@ describe('ReactFunctionComponent', () => { } } let instance3; - expect(() => { - instance3 = ReactTestUtils.renderIntoDocument(); + await expect(async () => { + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + (instance3 = current)} />, + ); + }); }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (same owner name, no additional warnings): instance3.forceUpdate(); - // Should also be deduped (same owner name, no additional warnings): - ReactTestUtils.renderIntoDocument(); + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); }); // This guards against a regression caused by clearing the current debug fiber. // https://github.com/facebook/react/issues/10831 // @gate !disableLegacyContext || !__DEV__ - it('should warn when giving a function ref with context', () => { + it('should warn when giving a function ref with context', async () => { function Child() { return null; } @@ -349,7 +394,13 @@ describe('ReactFunctionComponent', () => { } } - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: Function components cannot be given refs. ' + 'Attempts to access this ref will fail. ' + 'Did you mean to use React.forwardRef()?\n\n' + @@ -360,21 +411,18 @@ describe('ReactFunctionComponent', () => { ); }); - it('should provide a null ref', () => { - function Child() { - return
; - } - - const comp = ReactTestUtils.renderIntoDocument(); - expect(comp).toBe(null); - }); - - it('should use correct name in key warning', () => { + it('should use correct name in key warning', async () => { function Child() { return
{[]}
; } - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Each child in a list should have a unique "key" prop.\n\n' + 'Check the render method of `Child`.', ); @@ -382,14 +430,21 @@ describe('ReactFunctionComponent', () => { // TODO: change this test after we deprecate default props support // for function components - it('should support default props and prop types', () => { + it('should support default props and prop types', async () => { function Child(props) { return
{props.test}
; } Child.defaultProps = {test: 2}; Child.propTypes = {test: PropTypes.string}; - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev([ + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + }).toErrorDev([ 'Warning: Child: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.', 'Warning: Failed prop type: Invalid prop `test` of type `number` ' + 'supplied to `Child`, expected `string`.\n' + @@ -427,7 +482,7 @@ describe('ReactFunctionComponent', () => { expect(el.textContent).toBe('en'); }); - it('should work with arrow functions', () => { + it('should work with arrow functions', async () => { let Child = function () { return
; }; @@ -435,20 +490,38 @@ describe('ReactFunctionComponent', () => { // arrow function. Child = Child.bind(this); - expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).not.toThrow(); }); - it('should allow simple functions to return null', () => { + it('should allow simple functions to return null', async () => { const Child = function () { return null; }; - expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).not.toThrow(); }); - it('should allow simple functions to return false', () => { + it('should allow simple functions to return false', async () => { function Child() { return false; } - expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect( + act(() => { + root.render(); + }), + ).resolves.not.toThrow(); }); }); From 7ebf75d7c418cf0721de0fe07efd4c9ea257285d Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 14 Feb 2024 19:52:53 +0100 Subject: [PATCH 2/2] Test that we don't call ref --- .../__tests__/ReactFunctionComponent-test.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js index e1692ddcd1904..08406bdd72d04 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -232,6 +232,7 @@ describe('ReactFunctionComponent', () => { ' in ParentUsingStringRef (at **)', ); + // No additional warnings should be logged const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); await act(() => { @@ -239,21 +240,17 @@ describe('ReactFunctionComponent', () => { }); }); - it('should warn when given a function ref', async () => { + it('should warn when given a function ref and ignore them', async () => { function Indirection(props) { return
{props.children}
; } + const ref = jest.fn(); class ParentUsingFunctionRef extends React.Component { render() { return ( - { - expect(arg).toBe(null); - }} - /> + ); } @@ -276,7 +273,9 @@ describe('ReactFunctionComponent', () => { ' in Indirection (at **)\n' + ' in ParentUsingFunctionRef (at **)', ); + expect(ref).not.toHaveBeenCalled(); + // No additional warnings should be logged const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); await act(() => { @@ -306,9 +305,9 @@ describe('ReactFunctionComponent', () => { }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (offending element is on the same line): instance1.forceUpdate(); + // Should also be deduped (offending element is on the same line): let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); - await act(() => { root.render(); }); @@ -335,6 +334,7 @@ describe('ReactFunctionComponent', () => { }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (same internal instance, no additional warnings) instance2.forceUpdate(); + // Could not be differentiated (since owner is anonymous and no source location) container = document.createElement('div'); root = ReactDOMClient.createRoot(container); await act(() => { @@ -362,6 +362,7 @@ describe('ReactFunctionComponent', () => { }).toErrorDev('Warning: Function components cannot be given refs.'); // Should be deduped (same owner name, no additional warnings): instance3.forceUpdate(); + // Should also be deduped (same owner name, no additional warnings): container = document.createElement('div'); root = ReactDOMClient.createRoot(container); await act(() => {