From b380c24852b43856028167d5355926483f51d9fc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 7 Mar 2023 10:14:41 -0500 Subject: [PATCH] Convert class equivlance tests to flushSync (#26333) There's an old collection of test suites that test class component behavior across ES6 (regular JavaScript classes), CoffeeScript classes, and TypeScript classes. They work by running the same tests in all environments and comparing the results. Rather than use `act` or `waitFor` in these, I've changed them to use `flushSync` instead so that they can flush synchronously. The reason is that CoffeeScript doesn't have async/await, so we'd have to write those tests differently than how they are written in the corresponding modules. Since none of these tests cover any concurrent behavior, I believe it's fine in this case to do everything synchronously; they don't use any concurrent features, anyway, so effectively it's just skipping a microtask. --- .../ReactCoffeeScriptClass-test.coffee | 22 +++++++++---------- .../react/src/__tests__/ReactES6Class-test.js | 22 +++++++++---------- .../__tests__/ReactTypeScriptClass-test.ts | 22 +++++++++---------- .../__tests__/testDefinitions/ReactDOM.d.ts | 1 + 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index 5bd9b24d52277..928ddb8959a5d 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -22,7 +22,6 @@ describe 'ReactCoffeeScriptClass', -> React = require 'react' ReactDOM = require 'react-dom' ReactDOMClient = require 'react-dom/client' - act = require('jest-react').act PropTypes = require 'prop-types' container = document.createElement 'div' root = ReactDOMClient.createRoot container @@ -36,7 +35,7 @@ describe 'ReactCoffeeScriptClass', -> return React.createElement('div', className: this.props.name) test = (element, expectedTag, expectedClassName) -> - act -> + ReactDOM.flushSync -> root.render(element) expect(container.firstChild).not.toBeNull() expect(container.firstChild.tagName).toBe(expectedTag) @@ -50,7 +49,7 @@ describe 'ReactCoffeeScriptClass', -> class Foo extends React.Component expect(-> expect(-> - act -> + ReactDOM.flushSync -> root.render React.createElement(Foo) ).toThrow() ).toErrorDev([ @@ -103,7 +102,8 @@ describe 'ReactCoffeeScriptClass', -> ref = React.createRef() test React.createElement(Foo, initialValue: 'foo', ref: ref), 'DIV', 'foo' - ref.current.changeState() + ReactDOM.flushSync -> + ref.current.changeState() test React.createElement(Foo), 'SPAN', 'bar' it 'sets initial state with value returned by static getDerivedStateFromProps', -> @@ -129,7 +129,7 @@ describe 'ReactCoffeeScriptClass', -> getDerivedStateFromProps: -> {} expect(-> - act -> + ReactDOM.flushSync -> root.render React.createElement(Foo, foo: 'foo') return ).toErrorDev 'Foo: getDerivedStateFromProps() is defined as an instance method and will be ignored. Instead, declare it as a static method.' @@ -141,7 +141,7 @@ describe 'ReactCoffeeScriptClass', -> getDerivedStateFromError: -> {} expect(-> - act -> + ReactDOM.flushSync -> root.render React.createElement(Foo, foo: 'foo') return ).toErrorDev 'Foo: getDerivedStateFromError() is defined as an instance method and will be ignored. Instead, declare it as a static method.' @@ -153,7 +153,7 @@ describe 'ReactCoffeeScriptClass', -> Foo.getSnapshotBeforeUpdate = () -> {} expect(-> - act -> + ReactDOM.flushSync -> root.render React.createElement(Foo, foo: 'foo') return ).toErrorDev 'Foo: getSnapshotBeforeUpdate() is defined as a static method and will be ignored. Instead, declare it as an instance method.' @@ -170,7 +170,7 @@ describe 'ReactCoffeeScriptClass', -> bar: 'bar' } expect(-> - act -> + ReactDOM.flushSync -> root.render React.createElement(Foo, foo: 'foo') return ).toErrorDev ( @@ -303,7 +303,7 @@ describe 'ReactCoffeeScriptClass', -> ) test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo' - act -> + ReactDOM.flushSync -> attachedListener() expect(renderedName).toBe 'bar' @@ -340,7 +340,7 @@ describe 'ReactCoffeeScriptClass', -> ) test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo' - act -> + ReactDOM.flushSync -> attachedListener() expect(renderedName).toBe 'bar' @@ -391,7 +391,7 @@ describe 'ReactCoffeeScriptClass', -> 'did-update', { value: 'foo' }, {} ] lifeCycles = [] # reset - act -> + ReactDOM.flushSync -> root.unmount() expect(lifeCycles).toEqual ['will-unmount'] diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index 62f020849489f..70944b398dcbd 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -13,7 +13,6 @@ let PropTypes; let React; let ReactDOM; let ReactDOMClient; -let act; describe('ReactES6Class', () => { let container; @@ -31,7 +30,6 @@ describe('ReactES6Class', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); - act = require('jest-react').act; container = document.createElement('div'); root = ReactDOMClient.createRoot(container); attachedListener = null; @@ -49,7 +47,7 @@ describe('ReactES6Class', () => { }); function test(element, expectedTag, expectedClassName) { - act(() => root.render(element)); + ReactDOM.flushSync(() => root.render(element)); expect(container.firstChild).not.toBeNull(); expect(container.firstChild.tagName).toBe(expectedTag); expect(container.firstChild.className).toBe(expectedClassName); @@ -63,7 +61,7 @@ describe('ReactES6Class', () => { it('throws if no render function is defined', () => { class Foo extends React.Component {} expect(() => { - expect(() => act(() => root.render())).toThrow(); + expect(() => ReactDOM.flushSync(() => root.render())).toThrow(); }).toErrorDev([ // A failed component renders four times in DEV in concurrent mode 'Warning: Foo(...): No `render` method found on the returned component ' + @@ -118,7 +116,7 @@ describe('ReactES6Class', () => { } const ref = React.createRef(); test(, 'DIV', 'foo'); - act(() => ref.current.changeState()); + ReactDOM.flushSync(() => ref.current.changeState()); test(, 'SPAN', 'bar'); }); @@ -148,7 +146,7 @@ describe('ReactES6Class', () => { } } expect(() => { - act(() => root.render()); + ReactDOM.flushSync(() => root.render()); }).toErrorDev( 'Foo: getDerivedStateFromProps() is defined as an instance method ' + 'and will be ignored. Instead, declare it as a static method.', @@ -165,7 +163,7 @@ describe('ReactES6Class', () => { } } expect(() => { - act(() => root.render()); + ReactDOM.flushSync(() => root.render()); }).toErrorDev( 'Foo: getDerivedStateFromError() is defined as an instance method ' + 'and will be ignored. Instead, declare it as a static method.', @@ -180,7 +178,7 @@ describe('ReactES6Class', () => { } } expect(() => { - act(() => root.render()); + ReactDOM.flushSync(() => root.render()); }).toErrorDev( 'Foo: getSnapshotBeforeUpdate() is defined as a static method ' + 'and will be ignored. Instead, declare it as an instance method.', @@ -200,7 +198,7 @@ describe('ReactES6Class', () => { } } expect(() => { - act(() => root.render()); + ReactDOM.flushSync(() => root.render()); }).toErrorDev( '`Foo` uses `getDerivedStateFromProps` but its initial state is ' + 'undefined. This is not recommended. Instead, define the initial state by ' + @@ -347,7 +345,7 @@ describe('ReactES6Class', () => { } test(, 'DIV', 'foo'); - act(() => attachedListener()); + ReactDOM.flushSync(() => attachedListener()); expect(renderedName).toBe('bar'); }); @@ -388,7 +386,7 @@ describe('ReactES6Class', () => { } } test(, 'DIV', 'foo'); - act(() => attachedListener()); + ReactDOM.flushSync(() => attachedListener()); expect(renderedName).toBe('bar'); }); @@ -437,7 +435,7 @@ describe('ReactES6Class', () => { 'did-update', freeze({value: 'foo'}), {}, ]); lifeCycles = []; // reset - act(() => root.unmount()); + ReactDOM.flushSync(() => root.unmount()); expect(lifeCycles).toEqual(['will-unmount']); }); diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index dfea3cea47b3f..efaac47524efb 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -16,7 +16,6 @@ import ReactDOM = require('react-dom'); import ReactDOMClient = require('react-dom/client'); import ReactDOMTestUtils = require('react-dom/test-utils'); import PropTypes = require('prop-types'); -import internalAct = require('jest-react'); // Before Each @@ -24,7 +23,6 @@ let container; let root; let attachedListener = null; let renderedName = null; -let act = internalAct.act; class Inner extends React.Component { getName() { @@ -38,7 +36,7 @@ class Inner extends React.Component { } function test(element, expectedTag, expectedClassName) { - act(() => root.render(element)); + ReactDOM.flushSync(() => root.render(element)); expect(container.firstChild).not.toBeNull(); expect(container.firstChild.tagName).toBe(expectedTag); expect(container.firstChild.className).toBe(expectedClassName); @@ -331,7 +329,7 @@ describe('ReactTypeScriptClass', function() { it('throws if no render function is defined', function() { expect(() => { expect(() => - act(() => root.render(React.createElement(Empty))) + ReactDOM.flushSync(() => root.render(React.createElement(Empty))) ).toThrow(); }).toErrorDev([ // A failed component renders four times in DEV in concurrent mode @@ -366,7 +364,7 @@ describe('ReactTypeScriptClass', function() { 'DIV', 'foo' ); - act(() => ref.current.changeState()); + ReactDOM.flushSync(() => ref.current.changeState()); test(React.createElement(StateBasedOnProps), 'SPAN', 'bar'); }); @@ -401,7 +399,7 @@ describe('ReactTypeScriptClass', function() { } } expect(function() { - act(() => + ReactDOM.flushSync(() => root.render(React.createElement(Foo, {foo: 'foo'})) ); }).toErrorDev( @@ -420,7 +418,7 @@ describe('ReactTypeScriptClass', function() { } } expect(function() { - act(() => + ReactDOM.flushSync(() => root.render(React.createElement(Foo, {foo: 'foo'})) ); }).toErrorDev( @@ -437,7 +435,7 @@ describe('ReactTypeScriptClass', function() { } } expect(function() { - act(() => + ReactDOM.flushSync(() => root.render(React.createElement(Foo, {foo: 'foo'})) ); }).toErrorDev( @@ -461,7 +459,7 @@ describe('ReactTypeScriptClass', function() { } } expect(function() { - act(() => + ReactDOM.flushSync(() => root.render(React.createElement(Foo, {foo: 'foo'})) ); }).toErrorDev( @@ -547,7 +545,7 @@ describe('ReactTypeScriptClass', function() { 'DIV', 'foo' ); - act(() => attachedListener()); + ReactDOM.flushSync(() => attachedListener()); expect(renderedName).toBe('bar'); }); @@ -566,7 +564,7 @@ describe('ReactTypeScriptClass', function() { 'DIV', 'foo' ); - act(() => attachedListener()); + ReactDOM.flushSync(() => attachedListener()); expect(renderedName).toBe('bar'); }); @@ -590,7 +588,7 @@ describe('ReactTypeScriptClass', function() { {}, ]); lifeCycles = []; // reset - act(() => root.unmount(container)); + ReactDOM.flushSync(() => root.unmount(container)); expect(lifeCycles).toEqual(['will-unmount']); }); diff --git a/packages/react/src/__tests__/testDefinitions/ReactDOM.d.ts b/packages/react/src/__tests__/testDefinitions/ReactDOM.d.ts index 0b20d827b4514..eb6ad709efb1a 100644 --- a/packages/react/src/__tests__/testDefinitions/ReactDOM.d.ts +++ b/packages/react/src/__tests__/testDefinitions/ReactDOM.d.ts @@ -16,4 +16,5 @@ declare module 'react-dom' { export function render(element : any, container : any) : any export function unmountComponentAtNode(container : any) : void export function findDOMNode(instance : any) : any + export function flushSync(cb : any) : any }