Remove buggy unstable_deferredUpdates() (#13488)
* Add a failing test for deferredUpdates not being respected * Don't consider deferred updates interactive * Remove now-unnecessary setTimeout hack in the fixture * Remove unstable_deferredUpdates
Dan Abramov committed
Aug 28, 2018 at 18:12 UTC
2967ebdbeabde4aab37b45946b891170c7670164
5 files changed
+90
-34
fixtures/unstable-async/suspense/src/components/App.js
+1
-2
@@ -1,5 +1,4 @@
1
import React, {Placeholder, PureComponent} from 'react';
2
-import {unstable_deferredUpdates} from 'react-dom';
2
import {createResource} from 'simple-cache-provider';
3
import {cache} from '../cache';
4
import Spinner from './Spinner';
@@ -31,7 +30,7 @@ export default class App extends PureComponent {
30
this.setState({
31
currentId: id,
32
});
34
- unstable_deferredUpdates(() => {
33
+ requestIdleCallback(() => {
34
this.setState({
35
showDetail: true,
36
});
fixtures/unstable-async/time-slicing/src/index.js
+25
-28
@@ -1,5 +1,5 @@
1
-import React, {PureComponent, unstable_AsyncMode} from 'react';
2
-import {flushSync, render, unstable_deferredUpdates} from 'react-dom';
1
+import React, {PureComponent} from 'react';
2
+import {flushSync, render} from 'react-dom';
3
import _ from 'lodash';
4
import Charts from './Charts';
5
import Clock from './Clock';
@@ -54,9 +54,11 @@ class App extends PureComponent {
54
return;
55
}
56
if (this.state.strategy !== 'async') {
57
- this.setState(state => ({
58
- showDemo: !state.showDemo,
59
- }));
57
+ flushSync(() => {
58
+ this.setState(state => ({
59
+ showDemo: !state.showDemo,
60
+ }));
61
+ });
62
return;
63
}
64
if (this._ignoreClick) {
@@ -64,12 +66,9 @@ class App extends PureComponent {
66
}
67
this._ignoreClick = true;
68
67
- // TODO: needing setTimeout here seems like a React bug.
68
- setTimeout(() => {
69
- unstable_deferredUpdates(() => {
70
- this.setState({showDemo: true}, () => {
71
- this._ignoreClick = false;
72
- });
69
+ requestIdleCallback(() => {
70
+ this.setState({showDemo: true}, () => {
71
+ this._ignoreClick = false;
72
});
73
});
74
};
@@ -107,11 +106,8 @@ class App extends PureComponent {
106
this.debouncedHandleChange(value);
107
break;
108
case 'async':
110
- // TODO: needing setTimeout here seems like a React bug.
111
- setTimeout(() => {
112
- unstable_deferredUpdates(() => {
113
- this.setState({value});
114
- });
109
+ requestIdleCallback(() => {
110
+ this.setState({value});
111
});
112
break;
113
default:
@@ -120,8 +116,6 @@ class App extends PureComponent {
116
};
117
118
render() {
123
- const Wrapper =
124
- this.state.strategy === 'async' ? unstable_AsyncMode : 'div';
119
const {showClock} = this.state;
120
const data = this.getStreamData(this.state.value);
121
return (
@@ -137,20 +131,23 @@ class App extends PureComponent {
131
defaultValue={this.state.input}
132
onChange={this.handleChange}
133
/>
140
- <Wrapper>
141
- <div className="demo" onClick={this.handleChartClick}>
142
- {this.state.showDemo && (
143
- <Charts data={data} onClick={this.handleChartClick} />
144
- )}
145
- <div style={{display: showClock ? 'block' : 'none'}}>
146
- <Clock />
147
- </div>
134
+ <div className="demo" onClick={this.handleChartClick}>
135
+ {this.state.showDemo && (
136
+ <Charts data={data} onClick={this.handleChartClick} />
137
+ )}
138
+ <div style={{display: showClock ? 'block' : 'none'}}>
139
+ <Clock />
140
</div>
149
- </Wrapper>
141
+ </div>
142
</div>
143
);
144
}
145
}
146
147
const container = document.getElementById('root');
156
-render(<App />, container);
148
+render(
149
+ <React.unstable_AsyncMode>
150
+ <App />
151
+ </React.unstable_AsyncMode>,
152
+ container
153
+);
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js
+61
-2
@@ -16,6 +16,11 @@ let ReactDOM;
16
17
const AsyncMode = React.unstable_AsyncMode;
18
19
+const setUntrackedInputValue = Object.getOwnPropertyDescriptor(
20
+ HTMLInputElement.prototype,
21
+ 'value',
22
+).set;
23
+
24
describe('ReactDOMFiberAsync', () => {
25
let container;
26
@@ -47,6 +52,12 @@ describe('ReactDOMFiberAsync', () => {
52
jest.resetModules();
53
container = document.createElement('div');
54
ReactDOM = require('react-dom');
55
+
56
+ document.body.appendChild(container);
57
+ });
58
+
59
+ afterEach(() => {
60
+ document.body.removeChild(container);
61
});
62
63
it('renders synchronously by default', () => {
@@ -60,11 +71,60 @@ describe('ReactDOMFiberAsync', () => {
71
expect(ops).toEqual(['Hi', 'Bye']);
72
});
73
74
+ it('does not perform deferred updates synchronously', () => {
75
+ let inputRef = React.createRef();
76
+ let asyncValueRef = React.createRef();
77
+ let syncValueRef = React.createRef();
78
+
79
+ class Counter extends React.Component {
80
+ state = {asyncValue: '', syncValue: ''};
81
+
82
+ handleChange = e => {
83
+ const nextValue = e.target.value;
84
+ requestIdleCallback(() => {
85
+ this.setState({
86
+ asyncValue: nextValue,
87
+ });
88
+ });
89
+ this.setState({
90
+ syncValue: nextValue,
91
+ });
92
+ };
93
+
94
+ render() {
95
+ return (
96
+ <div>
97
+ <input
98
+ ref={inputRef}
99
+ onChange={this.handleChange}
100
+ defaultValue=""
101
+ />
102
+ <p ref={asyncValueRef}>{this.state.asyncValue}</p>
103
+ <p ref={syncValueRef}>{this.state.syncValue}</p>
104
+ </div>
105
+ );
106
+ }
107
+ }
108
+ ReactDOM.render(<Counter />, container);
109
+ expect(asyncValueRef.current.textContent).toBe('');
110
+ expect(syncValueRef.current.textContent).toBe('');
111
+
112
+ setUntrackedInputValue.call(inputRef.current, 'hello');
113
+ inputRef.current.dispatchEvent(new MouseEvent('input', {bubbles: true}));
114
+ // Should only flush non-deferred update.
115
+ expect(asyncValueRef.current.textContent).toBe('');
116
+ expect(syncValueRef.current.textContent).toBe('hello');
117
+
118
+ // Should flush both updates now.
119
+ jest.runAllTimers();
120
+ expect(asyncValueRef.current.textContent).toBe('hello');
121
+ expect(syncValueRef.current.textContent).toBe('hello');
122
+ });
123
+
124
describe('with feature flag disabled', () => {
125
beforeEach(() => {
126
jest.resetModules();
127
ReactFeatureFlags = require('shared/ReactFeatureFlags');
67
- container = document.createElement('div');
128
ReactDOM = require('react-dom');
129
});
130
@@ -91,7 +151,6 @@ describe('ReactDOMFiberAsync', () => {
151
beforeEach(() => {
152
jest.resetModules();
153
ReactFeatureFlags = require('shared/ReactFeatureFlags');
94
- container = document.createElement('div');
154
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
155
ReactDOM = require('react-dom');
156
});
packages/react-dom/src/client/ReactDOM.js
-2
@@ -732,8 +732,6 @@ const ReactDOM: Object = {
732
733
unstable_batchedUpdates: DOMRenderer.batchedUpdates,
734
735
- unstable_deferredUpdates: DOMRenderer.deferredUpdates,
736
-
735
unstable_interactiveUpdates: DOMRenderer.interactiveUpdates,
736
737
flushSync: DOMRenderer.flushSync,
packages/react-reconciler/src/ReactFiberScheduler.js
+3
@@ -1568,11 +1568,14 @@ function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) {
1568
function deferredUpdates<A>(fn: () => A): A {
1569
const currentTime = requestCurrentTime();
1570
const previousExpirationContext = expirationContext;
1571
+ const previousIsBatchingInteractiveUpdates = isBatchingInteractiveUpdates;
1572
expirationContext = computeAsyncExpiration(currentTime);
1573
+ isBatchingInteractiveUpdates = false;
1574
try {
1575
return fn();
1576
} finally {
1577
expirationContext = previousExpirationContext;
1578
+ isBatchingInteractiveUpdates = previousIsBatchingInteractiveUpdates;
1579
}
1580
}
1581