Fix ignored sync work in passive effects (#14799)
* Fix ignored sync work in passive effects * Fix batching
Dan Abramov committed
Feb 12, 2019 at 20:18 UTC
3ae94e1885b673543a30a05906c4f9a0e4b682cb
2 files changed
+82
-1
packages/react-dom/src/__tests__/ReactDOMHooks-test.js
+78
-1
@@ -12,7 +12,7 @@
12
let React;
13
let ReactDOM;
14
15
-describe('ReactDOMSuspensePlaceholder', () => {
15
+describe('ReactDOMHooks', () => {
16
let container;
17
18
beforeEach(() => {
@@ -29,6 +29,83 @@ describe('ReactDOMSuspensePlaceholder', () => {
29
document.body.removeChild(container);
30
});
31
32
+ it('can ReactDOM.render() from useEffect', () => {
33
+ let container2 = document.createElement('div');
34
+ let container3 = document.createElement('div');
35
+
36
+ function Example1({n}) {
37
+ React.useEffect(() => {
38
+ ReactDOM.render(<Example2 n={n} />, container2);
39
+ });
40
+ return 1 * n;
41
+ }
42
+
43
+ function Example2({n}) {
44
+ React.useEffect(() => {
45
+ ReactDOM.render(<Example3 n={n} />, container3);
46
+ });
47
+ return 2 * n;
48
+ }
49
+
50
+ function Example3({n}) {
51
+ return 3 * n;
52
+ }
53
+
54
+ ReactDOM.render(<Example1 n={1} />, container);
55
+ expect(container.textContent).toBe('1');
56
+ expect(container2.textContent).toBe('');
57
+ expect(container3.textContent).toBe('');
58
+ jest.runAllTimers();
59
+ expect(container.textContent).toBe('1');
60
+ expect(container2.textContent).toBe('2');
61
+ expect(container3.textContent).toBe('3');
62
+
63
+ ReactDOM.render(<Example1 n={2} />, container);
64
+ expect(container.textContent).toBe('2');
65
+ expect(container2.textContent).toBe('2'); // Not flushed yet
66
+ expect(container3.textContent).toBe('3'); // Not flushed yet
67
+ jest.runAllTimers();
68
+ expect(container.textContent).toBe('2');
69
+ expect(container2.textContent).toBe('4');
70
+ expect(container3.textContent).toBe('6');
71
+ });
72
+
73
+ it('can batch synchronous work inside effects with other work', () => {
74
+ let otherContainer = document.createElement('div');
75
+
76
+ let calledA = false;
77
+ function A() {
78
+ calledA = true;
79
+ return 'A';
80
+ }
81
+
82
+ let calledB = false;
83
+ function B() {
84
+ calledB = true;
85
+ return 'B';
86
+ }
87
+
88
+ let _set;
89
+ function Foo() {
90
+ _set = React.useState(0)[1];
91
+ React.useEffect(() => {
92
+ ReactDOM.render(<A />, otherContainer);
93
+ });
94
+ return null;
95
+ }
96
+
97
+ ReactDOM.render(<Foo />, container);
98
+ ReactDOM.unstable_batchedUpdates(() => {
99
+ _set(0); // Forces the effect to be flushed
100
+ expect(otherContainer.textContent).toBe('');
101
+ ReactDOM.render(<B />, otherContainer);
102
+ expect(otherContainer.textContent).toBe('');
103
+ });
104
+ expect(otherContainer.textContent).toBe('B');
105
+ expect(calledA).toBe(false); // It was in a batch
106
+ expect(calledB).toBe(true);
107
+ });
108
+
109
it('should not bail out when an update is scheduled from within an event handler', () => {
110
const {createRef, useCallback, useState} = React;
111
packages/react-reconciler/src/ReactFiberScheduler.js
+4
@@ -575,6 +575,10 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
575
if (rootExpirationTime !== NoWork) {
576
requestWork(root, rootExpirationTime);
577
}
578
+ // Flush any sync work that was scheduled by effects
579
+ if (!isBatchingUpdates && !isRendering) {
580
+ performSyncWork();
581
+ }
582
}
583
584
function isAlreadyFailedLegacyErrorBoundary(instance: mixed): boolean {