Skip to content

Commit

Permalink
flush work on exiting outermost act(), with nested act()s from differ…
Browse files Browse the repository at this point in the history 
…ent renderers (#16181)

Given this snippet:
```jsx
    TestRenderer.act(() => {
      TestUtils.act(() => {
        TestRenderer.create(<Effecty />);
      });
    });
```
We want to make sure that all work is only flushed on exiting the outermost act().

Now, naively doing this based on actingScopeDepth would work with a mocked scheduler, where flushAll() would flush all work across renderers.

This doesn't work without mocking the scheduler though; and where flushing work only works per renderer. So we disable this behaviour for a non-mocked scenario. This seems like an ok tradeoff.
  • Loading branch information
Sunil Pai authored Jul 23, 2019
1 parent 5098891 commit c73e1f2
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 156 deletions.
308 changes: 182 additions & 126 deletions fixtures/dom/src/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
* @emails react-core
*/

import React from 'react';
import ReactDOM from 'react-dom';
import ReactART from 'react-art';
import ARTSVGMode from 'art/modes/svg';
import ARTCurrentMode from 'art/modes/current';
import TestUtils from 'react-dom/test-utils';
import TestRenderer from 'react-test-renderer';

ARTCurrentMode.setCurrent(ARTSVGMode);
let React;
let ReactDOM;
let ReactART;
let ARTSVGMode;
let ARTCurrentMode;
let TestUtils;
let TestRenderer;
let ARTTest;

global.__DEV__ = process.env.NODE_ENV !== 'production';

Expand All @@ -25,153 +24,210 @@ function App(props) {
return 'hello world';
}

it("doesn't warn when you use the right act + renderer: dom", () => {
TestUtils.act(() => {
TestUtils.renderIntoDocument(<App />);
});
describe('legacy mode', () => {
runTests();
});

it("doesn't warn when you use the right act + renderer: test", () => {
TestRenderer.act(() => {
TestRenderer.create(<App />);
describe('mocked scheduler', () => {
beforeEach(() => {
jest.mock('scheduler', () =>
require.requireActual('scheduler/unstable_mock')
);
});
afterEach(() => {
jest.unmock('scheduler');
});
runTests();
});

it('resets correctly across renderers', () => {
function Effecty() {
React.useEffect(() => {}, []);
return null;
}
TestUtils.act(() => {
TestRenderer.act(() => {});
expect(() => {
TestRenderer.create(<Effecty />);
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
function runTests() {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactART = require('react-art');
ARTSVGMode = require('art/modes/svg');
ARTCurrentMode = require('art/modes/current');
TestUtils = require('react-dom/test-utils');
TestRenderer = require('react-test-renderer');

ARTCurrentMode.setCurrent(ARTSVGMode);

ARTTest = function ARTTest(props) {
return (
<ReactART.Surface width={150} height={200}>
<ReactART.Group>
<ReactART.Shape
d="M0,0l50,0l0,50l-50,0z"
fill={new ReactART.LinearGradient(['black', 'white'])}
key="a"
width={50}
height={50}
x={50}
y={50}
opacity={0.1}
/>
<ReactART.Shape
fill="#3C5A99"
key="b"
scale={0.5}
x={50}
y={50}
title="This is an F"
cursor="pointer">
M64.564,38.583H54l0.008-5.834c0-3.035,0.293-4.666,4.657-4.666
h5.833V16.429h-9.33c-11.213,0-15.159,5.654-15.159,15.16v6.994
h-6.99v11.652h6.99v33.815H54V50.235h9.331L64.564,38.583z
</ReactART.Shape>
</ReactART.Group>
</ReactART.Surface>
);
};
});
it("doesn't warn when you use the right act + renderer: dom", () => {
TestUtils.act(() => {
TestUtils.renderIntoDocument(<App />);
});
});
});

it('warns when using createRoot() + .render', () => {
const root = ReactDOM.unstable_createRoot(document.createElement('div'));
expect(() => {
it("doesn't warn when you use the right act + renderer: test", () => {
TestRenderer.act(() => {
root.render(<App />);
TestRenderer.create(<App />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});

it('warns when using the wrong act version - test + dom: render', () => {
expect(() => {
TestRenderer.act(() => {
TestUtils.renderIntoDocument(<App />);
it('resets correctly across renderers', () => {
function Effecty() {
React.useEffect(() => {}, []);
return null;
}
TestUtils.act(() => {
TestRenderer.act(() => {});
expect(() => {
TestRenderer.create(<Effecty />);
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});

it('warns when using the wrong act version - test + dom: updates', () => {
let setCtr;
function Counter(props) {
const [ctr, _setCtr] = React.useState(0);
setCtr = _setCtr;
return ctr;
}
TestUtils.renderIntoDocument(<Counter />);
expect(() => {
TestRenderer.act(() => {
setCtr(1);
it('warns when using createRoot() + .render', () => {
const root = ReactDOM.unstable_createRoot(document.createElement('div'));
expect(() => {
TestRenderer.act(() => {
root.render(<App />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
}).toWarnDev(["It looks like you're using the wrong act()"]);
});
});

it('warns when using the wrong act version - dom + test: .create()', () => {
expect(() => {
TestUtils.act(() => {
TestRenderer.create(<App />);
it('warns when using the wrong act version - test + dom: render', () => {
expect(() => {
TestRenderer.act(() => {
TestUtils.renderIntoDocument(<App />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});

it('warns when using the wrong act version - dom + test: .update()', () => {
const root = TestRenderer.create(<App key="one" />);
expect(() => {
TestUtils.act(() => {
root.update(<App key="two" />);
it('warns when using the wrong act version - test + dom: updates', () => {
let setCtr;
function Counter(props) {
const [ctr, _setCtr] = React.useState(0);
setCtr = _setCtr;
return ctr;
}
TestUtils.renderIntoDocument(<Counter />);
expect(() => {
TestRenderer.act(() => {
setCtr(1);
});
}).toWarnDev(["It looks like you're using the wrong act()"]);
});

it('warns when using the wrong act version - dom + test: .create()', () => {
expect(() => {
TestUtils.act(() => {
TestRenderer.create(<App />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
});

it('warns when using the wrong act version - dom + test: updates', () => {
let setCtr;
function Counter(props) {
const [ctr, _setCtr] = React.useState(0);
setCtr = _setCtr;
return ctr;
}
const root = TestRenderer.create(<Counter />);
expect(() => {
TestUtils.act(() => {
setCtr(1);
it('warns when using the wrong act version - dom + test: .update()', () => {
const root = TestRenderer.create(<App key="one" />);
expect(() => {
TestUtils.act(() => {
root.update(<App key="two" />);
});
}).toWarnDev(["It looks like you're using the wrong act()"], {
withoutStack: true,
});
}).toWarnDev(["It looks like you're using the wrong act()"]);
});
});

const {Surface, Group, Shape} = ReactART;
function ARTTest(props) {
return (
<Surface width={150} height={200}>
<Group>
<Shape
d="M0,0l50,0l0,50l-50,0z"
fill={new ReactART.LinearGradient(['black', 'white'])}
key="a"
width={50}
height={50}
x={50}
y={50}
opacity={0.1}
/>
<Shape
fill="#3C5A99"
key="b"
scale={0.5}
x={50}
y={50}
title="This is an F"
cursor="pointer">
M64.564,38.583H54l0.008-5.834c0-3.035,0.293-4.666,4.657-4.666
h5.833V16.429h-9.33c-11.213,0-15.159,5.654-15.159,15.16v6.994
h-6.99v11.652h6.99v33.815H54V50.235h9.331L64.564,38.583z
</Shape>
</Group>
</Surface>
);
}
it('warns when using the wrong act version - dom + test: updates', () => {
let setCtr;
function Counter(props) {
const [ctr, _setCtr] = React.useState(0);
setCtr = _setCtr;
return ctr;
}
const root = TestRenderer.create(<Counter />);
expect(() => {
TestUtils.act(() => {
setCtr(1);
});
}).toWarnDev(["It looks like you're using the wrong act()"]);
});

it('does not warn when nesting react-act inside react-dom', () => {
TestUtils.act(() => {
TestUtils.renderIntoDocument(<ARTTest />);
});
});

it('does not warn when nesting react-act inside react-dom', () => {
TestUtils.act(() => {
TestUtils.renderIntoDocument(<ARTTest />);
it('does not warn when nesting react-act inside react-test-renderer', () => {
TestRenderer.act(() => {
TestRenderer.create(<ARTTest />);
});
});
});

it('does not warn when nesting react-act inside react-test-renderer', () => {
TestRenderer.act(() => {
TestRenderer.create(<ARTTest />);
it("doesn't warn if you use nested acts from different renderers", () => {
TestRenderer.act(() => {
TestUtils.act(() => {
TestRenderer.create(<App />);
});
});
});
});

it("doesn't warn if you use nested acts from different renderers", () => {
TestRenderer.act(() => {
it('flushes work only outside the outermost act(), even when nested from different renderers', () => {
const log = [];
function Effecty() {
React.useEffect(() => {
log.push('called');
}, []);
return null;
}
// in legacy mode, this tests whether an act only flushes its own effects
// with a mocked scheduler, this tests whether it flushes all work only on the outermost act
TestRenderer.act(() => {
TestUtils.act(() => {
TestRenderer.create(<Effecty />);
});
expect(log).toEqual([]);
});
expect(log).toEqual(['called']);

log.splice(0);
// for doublechecking, we flip it inside out, and assert on the outermost
TestUtils.act(() => {
TestRenderer.create(<App />);
TestRenderer.act(() => {
TestRenderer.create(<Effecty />);
});
});
expect(log).toEqual(['called']);
});
});
}
Loading

0 comments on commit c73e1f2

Please sign in to comment.