Skip to content

Commit

Permalink
Convert class equivlance tests to flushSync (#26333)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@acdlite
acdlite authored Mar 7, 2023
1 parent 8f812e7 commit b380c24
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 35 deletions.
22 changes: 11 additions & 11 deletions packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -50,7 +49,7 @@ describe 'ReactCoffeeScriptClass', ->
class Foo extends React.Component
expect(->
expect(->
act ->
ReactDOM.flushSync ->
root.render React.createElement(Foo)
).toThrow()
).toErrorDev([
Expand Down Expand Up @@ -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', ->
Expand All @@ -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.'
Expand All @@ -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.'
Expand All @@ -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.'
Expand All @@ -170,7 +170,7 @@ describe 'ReactCoffeeScriptClass', ->
bar: 'bar'
}
expect(->
act ->
ReactDOM.flushSync ->
root.render React.createElement(Foo, foo: 'foo')
return
).toErrorDev (
Expand Down Expand Up @@ -303,7 +303,7 @@ describe 'ReactCoffeeScriptClass', ->
)

test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
act ->
ReactDOM.flushSync ->
attachedListener()
expect(renderedName).toBe 'bar'

Expand Down Expand Up @@ -340,7 +340,7 @@ describe 'ReactCoffeeScriptClass', ->
)

test React.createElement(Foo, initialValue: 'foo'), 'DIV', 'foo'
act ->
ReactDOM.flushSync ->
attachedListener()
expect(renderedName).toBe 'bar'

Expand Down Expand Up @@ -391,7 +391,7 @@ describe 'ReactCoffeeScriptClass', ->
'did-update', { value: 'foo' }, {}
]
lifeCycles = [] # reset
act ->
ReactDOM.flushSync ->
root.unmount()
expect(lifeCycles).toEqual ['will-unmount']

Expand Down
22 changes: 10 additions & 12 deletions packages/react/src/__tests__/ReactES6Class-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ let PropTypes;
let React;
let ReactDOM;
let ReactDOMClient;
let act;

describe('ReactES6Class', () => {
let container;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -63,7 +61,7 @@ describe('ReactES6Class', () => {
it('throws if no render function is defined', () => {
class Foo extends React.Component {}
expect(() => {
expect(() => act(() => root.render(<Foo />))).toThrow();
expect(() => ReactDOM.flushSync(() => root.render(<Foo />))).toThrow();
}).toErrorDev([
// A failed component renders four times in DEV in concurrent mode
'Warning: Foo(...): No `render` method found on the returned component ' +
Expand Down Expand Up @@ -118,7 +116,7 @@ describe('ReactES6Class', () => {
}
const ref = React.createRef();
test(<Foo initialValue="foo" ref={ref} />, 'DIV', 'foo');
act(() => ref.current.changeState());
ReactDOM.flushSync(() => ref.current.changeState());
test(<Foo />, 'SPAN', 'bar');
});

Expand Down Expand Up @@ -148,7 +146,7 @@ describe('ReactES6Class', () => {
}
}
expect(() => {
act(() => root.render(<Foo foo="foo" />));
ReactDOM.flushSync(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getDerivedStateFromProps() is defined as an instance method ' +
'and will be ignored. Instead, declare it as a static method.',
Expand All @@ -165,7 +163,7 @@ describe('ReactES6Class', () => {
}
}
expect(() => {
act(() => root.render(<Foo foo="foo" />));
ReactDOM.flushSync(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getDerivedStateFromError() is defined as an instance method ' +
'and will be ignored. Instead, declare it as a static method.',
Expand All @@ -180,7 +178,7 @@ describe('ReactES6Class', () => {
}
}
expect(() => {
act(() => root.render(<Foo foo="foo" />));
ReactDOM.flushSync(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'Foo: getSnapshotBeforeUpdate() is defined as a static method ' +
'and will be ignored. Instead, declare it as an instance method.',
Expand All @@ -200,7 +198,7 @@ describe('ReactES6Class', () => {
}
}
expect(() => {
act(() => root.render(<Foo foo="foo" />));
ReactDOM.flushSync(() => root.render(<Foo foo="foo" />));
}).toErrorDev(
'`Foo` uses `getDerivedStateFromProps` but its initial state is ' +
'undefined. This is not recommended. Instead, define the initial state by ' +
Expand Down Expand Up @@ -347,7 +345,7 @@ describe('ReactES6Class', () => {
}
test(<Foo initialValue="foo" />, 'DIV', 'foo');

act(() => attachedListener());
ReactDOM.flushSync(() => attachedListener());
expect(renderedName).toBe('bar');
});

Expand Down Expand Up @@ -388,7 +386,7 @@ describe('ReactES6Class', () => {
}
}
test(<Foo initialValue="foo" />, 'DIV', 'foo');
act(() => attachedListener());
ReactDOM.flushSync(() => attachedListener());
expect(renderedName).toBe('bar');
});

Expand Down Expand Up @@ -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']);
});

Expand Down
22 changes: 10 additions & 12 deletions packages/react/src/__tests__/ReactTypeScriptClass-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ 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

let container;
let root;
let attachedListener = null;
let renderedName = null;
let act = internalAct.act;

class Inner extends React.Component {
getName() {
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -366,7 +364,7 @@ describe('ReactTypeScriptClass', function() {
'DIV',
'foo'
);
act(() => ref.current.changeState());
ReactDOM.flushSync(() => ref.current.changeState());
test(React.createElement(StateBasedOnProps), 'SPAN', 'bar');
});

Expand Down Expand Up @@ -401,7 +399,7 @@ describe('ReactTypeScriptClass', function() {
}
}
expect(function() {
act(() =>
ReactDOM.flushSync(() =>
root.render(React.createElement(Foo, {foo: 'foo'}))
);
}).toErrorDev(
Expand All @@ -420,7 +418,7 @@ describe('ReactTypeScriptClass', function() {
}
}
expect(function() {
act(() =>
ReactDOM.flushSync(() =>
root.render(React.createElement(Foo, {foo: 'foo'}))
);
}).toErrorDev(
Expand All @@ -437,7 +435,7 @@ describe('ReactTypeScriptClass', function() {
}
}
expect(function() {
act(() =>
ReactDOM.flushSync(() =>
root.render(React.createElement(Foo, {foo: 'foo'}))
);
}).toErrorDev(
Expand All @@ -461,7 +459,7 @@ describe('ReactTypeScriptClass', function() {
}
}
expect(function() {
act(() =>
ReactDOM.flushSync(() =>
root.render(React.createElement(Foo, {foo: 'foo'}))
);
}).toErrorDev(
Expand Down Expand Up @@ -547,7 +545,7 @@ describe('ReactTypeScriptClass', function() {
'DIV',
'foo'
);
act(() => attachedListener());
ReactDOM.flushSync(() => attachedListener());
expect(renderedName).toBe('bar');
});

Expand All @@ -566,7 +564,7 @@ describe('ReactTypeScriptClass', function() {
'DIV',
'foo'
);
act(() => attachedListener());
ReactDOM.flushSync(() => attachedListener());
expect(renderedName).toBe('bar');
});

Expand All @@ -590,7 +588,7 @@ describe('ReactTypeScriptClass', function() {
{},
]);
lifeCycles = []; // reset
act(() => root.unmount(container));
ReactDOM.flushSync(() => root.unmount(container));
expect(lifeCycles).toEqual(['will-unmount']);
});

Expand Down
1 change: 1 addition & 0 deletions packages/react/src/__tests__/testDefinitions/ReactDOM.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit b380c24

Please sign in to comment.