From 756373f78d6a20968f2cc61d639aeadb5a0182be Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 14 Feb 2024 19:25:57 +0100 Subject: [PATCH 1/4] Remove usage of ReactTestUtils from ReactContextValidator --- .../__tests__/ReactContextValidator-test.js | 205 ++++++++++++------ 1 file changed, 140 insertions(+), 65 deletions(-) diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index f8edacb25a306..63e7104c2c8d9 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -18,7 +18,6 @@ let PropTypes; let React; let ReactDOMClient; -let ReactTestUtils; let act; describe('ReactContextValidator', () => { @@ -28,7 +27,6 @@ describe('ReactContextValidator', () => { PropTypes = require('prop-types'); React = require('react'); ReactDOMClient = require('react-dom/client'); - ReactTestUtils = require('react-dom/test-utils'); act = require('internal-test-utils').act; }); @@ -36,7 +34,7 @@ describe('ReactContextValidator', () => { // ensure that this is not required for ES6 classes with Flow. // @gate !disableLegacyContext - it('should filter out context not in contextTypes', () => { + it('should filter out context not in contextTypes', async () => { class Component extends React.Component { render() { return
; @@ -65,9 +63,14 @@ describe('ReactContextValidator', () => { bar: PropTypes.number, }; - const instance = ReactTestUtils.renderIntoDocument( - , - ); + let instance; + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + (instance = current)} />, + ); + }); expect(instance.childRef.current.context).toEqual({foo: 'abc'}); }); @@ -160,7 +163,7 @@ describe('ReactContextValidator', () => { // TODO (bvaughn) Remove this test and the associated behavior in the future. // It has only been added in Fiber to match the (unintentional) behavior in Stack. // @gate !disableLegacyContext || !__DEV__ - it('should warn (but not error) if getChildContext method is missing', () => { + it('should warn (but not error) if getChildContext method is missing', async () => { class ComponentA extends React.Component { static childContextTypes = { foo: PropTypes.string.isRequired, @@ -178,16 +181,31 @@ describe('ReactContextValidator', () => { } } - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: ComponentA.childContextTypes is specified but there is no ' + 'getChildContext() method on the instance. You can either define ' + 'getChildContext() on ComponentA or remove childContextTypes from it.', ); - // Warnings should be deduped by component type - ReactTestUtils.renderIntoDocument(); + let container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: ComponentB.childContextTypes is specified but there is no ' + 'getChildContext() method on the instance. You can either define ' + 'getChildContext() on ComponentB or remove childContextTypes from it.', @@ -197,7 +215,7 @@ describe('ReactContextValidator', () => { // TODO (bvaughn) Remove this test and the associated behavior in the future. // It has only been added in Fiber to match the (unintentional) behavior in Stack. // @gate !disableLegacyContext - it('should pass parent context if getChildContext method is missing', () => { + it('should pass parent context if getChildContext method is missing', async () => { class ParentContextProvider extends React.Component { static childContextTypes = { foo: PropTypes.string, @@ -233,9 +251,13 @@ describe('ReactContextValidator', () => { foo: PropTypes.string.isRequired, }; - expect(() => - ReactTestUtils.renderIntoDocument(), - ).toErrorDev([ + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev([ 'Warning: MiddleMissingContext.childContextTypes is specified but there is no ' + 'getChildContext() method on the instance. You can either define getChildContext() ' + 'on MiddleMissingContext or remove childContextTypes from it.', @@ -366,7 +388,7 @@ describe('ReactContextValidator', () => { }); // @gate !disableLegacyContext || !__DEV__ - it('should warn if both contextType and contextTypes are defined', () => { + it('should warn if both contextType and contextTypes are defined', async () => { const Context = React.createContext(); class ParentContextProvider extends React.Component { @@ -402,38 +424,49 @@ describe('ReactContextValidator', () => { } } - expect(() => - ReactTestUtils.renderIntoDocument( - - - , - ), - ).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); + }); + }).toErrorDev( 'Warning: ComponentA declares both contextTypes and contextType static properties. ' + 'The legacy contextTypes property will be ignored.', ); - // Warnings should be deduped by component type - ReactTestUtils.renderIntoDocument( - - - , - ); - - expect(() => - ReactTestUtils.renderIntoDocument( + let container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( - + , - ), - ).toErrorDev( + ); + }); + + await expect(async () => { + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); + }); + }).toErrorDev( 'Warning: ComponentB declares both contextTypes and contextType static properties. ' + 'The legacy contextTypes property will be ignored.', ); }); // @gate enableRenderableContext || !__DEV__ - it('should warn if an invalid contextType is defined', () => { + it('should warn if an invalid contextType is defined', async () => { const Context = React.createContext(); class ComponentA extends React.Component { static contextType = Context.Consumer; @@ -442,16 +475,23 @@ describe('ReactContextValidator', () => { } } - expect(() => { - ReactTestUtils.renderIntoDocument(); + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); }).toErrorDev( 'Warning: ComponentA defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + 'Did you accidentally pass the Context.Consumer instead?', ); - // Warnings should be deduped by component type - ReactTestUtils.renderIntoDocument(); + let container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); class ComponentB extends React.Component { static contextType = Context.Provider; @@ -459,23 +499,30 @@ describe('ReactContextValidator', () => { return
; } } - // This doesn't warn since Context.Provider === Context now. - ReactTestUtils.renderIntoDocument(); + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); }); - it('should not warn when class contextType is null', () => { + it('should not warn when class contextType is null', async () => { class Foo extends React.Component { static contextType = null; // Handy for conditional declaration render() { return this.context.hello.world; } } - expect(() => { - ReactTestUtils.renderIntoDocument(); - }).toThrow("Cannot read property 'world' of undefined"); + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).rejects.toThrow("Cannot read property 'world' of undefined"); }); - it('should warn when class contextType is undefined', () => { + it('should warn when class contextType is undefined', async () => { class Foo extends React.Component { // This commonly happens with circular deps // https://github.com/facebook/react/issues/13969 @@ -485,10 +532,14 @@ describe('ReactContextValidator', () => { } } - expect(() => { - expect(() => { - ReactTestUtils.renderIntoDocument(); - }).toThrow("Cannot read property 'world' of undefined"); + await expect(async () => { + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).rejects.toThrow("Cannot read property 'world' of undefined"); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + @@ -499,7 +550,7 @@ describe('ReactContextValidator', () => { ); }); - it('should warn when class contextType is an object', () => { + it('should warn when class contextType is an object', async () => { class Foo extends React.Component { // Can happen due to a typo static contextType = { @@ -511,10 +562,14 @@ describe('ReactContextValidator', () => { } } - expect(() => { - expect(() => { - ReactTestUtils.renderIntoDocument(); - }).toThrow("Cannot read property 'hello' of undefined"); + await expect(async () => { + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).rejects.toThrow("Cannot read property 'hello' of undefined"); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + @@ -522,7 +577,7 @@ describe('ReactContextValidator', () => { ); }); - it('should warn when class contextType is a primitive', () => { + it('should warn when class contextType is a primitive', async () => { class Foo extends React.Component { static contextType = 'foo'; render() { @@ -530,10 +585,14 @@ describe('ReactContextValidator', () => { } } - expect(() => { - expect(() => { - ReactTestUtils.renderIntoDocument(); - }).toThrow("Cannot read property 'world' of undefined"); + await expect(async () => { + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).rejects.toThrow("Cannot read property 'world' of undefined"); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + @@ -541,7 +600,7 @@ describe('ReactContextValidator', () => { ); }); - it('should warn if you define contextType on a function component', () => { + it('should warn if you define contextType on a function component', async () => { const Context = React.createContext(); function ComponentA() { @@ -554,14 +613,30 @@ describe('ReactContextValidator', () => { } ComponentB.contextType = Context; - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: ComponentA: Function components do not support contextType.', ); // Warnings should be deduped by component type - ReactTestUtils.renderIntoDocument(); + let container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); - expect(() => ReactTestUtils.renderIntoDocument()).toErrorDev( + await expect(async () => { + container = document.createElement('div'); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(); + }); + }).toErrorDev( 'Warning: ComponentB: Function components do not support contextType.', ); }); From e270e1ed249ead3547fbbbca5bdcaa380b5e6c3c Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 14 Feb 2024 19:32:08 +0100 Subject: [PATCH 2/4] Update error messages --- .../src/__tests__/ReactContextValidator-test.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index 63e7104c2c8d9..11d15b0f038bd 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -519,7 +519,7 @@ describe('ReactContextValidator', () => { await act(() => { root.render(); }); - }).rejects.toThrow("Cannot read property 'world' of undefined"); + }).rejects.toThrow("Cannot read properties of undefined (reading 'world')"); }); it('should warn when class contextType is undefined', async () => { @@ -539,7 +539,9 @@ describe('ReactContextValidator', () => { await act(() => { root.render(); }); - }).rejects.toThrow("Cannot read property 'world' of undefined"); + }).rejects.toThrow( + "Cannot read properties of undefined (reading 'world')", + ); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + @@ -569,7 +571,9 @@ describe('ReactContextValidator', () => { await act(() => { root.render(); }); - }).rejects.toThrow("Cannot read property 'hello' of undefined"); + }).rejects.toThrow( + "Cannot read properties of undefined (reading 'hello')", + ); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + @@ -592,7 +596,9 @@ describe('ReactContextValidator', () => { await act(() => { root.render(); }); - }).rejects.toThrow("Cannot read property 'world' of undefined"); + }).rejects.toThrow( + "Cannot read properties of undefined (reading 'world')", + ); }).toErrorDev( 'Foo defines an invalid contextType. ' + 'contextType should point to the Context object returned by React.createContext(). ' + From dff0b812edb128c984e3fc70c674c7037ebf4c38 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 14 Feb 2024 19:33:00 +0100 Subject: [PATCH 3/4] Restore comments --- packages/react/src/__tests__/ReactContextValidator-test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index 11d15b0f038bd..b5ff3a6b430f1 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -193,6 +193,7 @@ describe('ReactContextValidator', () => { 'getChildContext() on ComponentA or remove childContextTypes from it.', ); + // Warnings should be deduped by component typ let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); await act(() => { @@ -439,6 +440,7 @@ describe('ReactContextValidator', () => { 'The legacy contextTypes property will be ignored.', ); + // Warnings should be deduped by component type let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); await act(() => { From 9baf9d3d6ba3de5e826be9a9ee144d2e4031a6e7 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 15 Feb 2024 16:18:51 +0100 Subject: [PATCH 4/4] Update packages/react/src/__tests__/ReactContextValidator-test.js --- packages/react/src/__tests__/ReactContextValidator-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index b5ff3a6b430f1..5e79635a68dec 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -193,7 +193,7 @@ describe('ReactContextValidator', () => { 'getChildContext() on ComponentA or remove childContextTypes from it.', ); - // Warnings should be deduped by component typ + // Warnings should be deduped by component type let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); await act(() => {