Skip to content

Commit 4266f08

Browse files
acdlitegaearon
authored andcommitted
[Fiber] "Task" priority for error boundaries and batched updates (#8193)
* Refactor scheduling functions and introduce Task priority There was lots of duplication across all the scheduling functions. I think we're far enough along that we can start trying to clean some stuff up. Also introduces a new priority level provisionally called Task priority. This is for work that completes at the end of the current tick, after the current batch of work has been committed. It's different from Synchronous priority, which needs to complete immediately. A full implementation of Task priority will follow. It will replace the current batching solution. * Implement Task priority Task priority is similar to Synchronous priority. Both are flushed in the current tick. Synchronous priority is flushed immediately (e.g. sync work triggered by setState will flush before setState exits), where as Task is flushed after the current batch of work is committed. Currently used for batchedUpdates and nested sync updates. Task should also be used for componentDidUpdate/Mount and error boundary work. I'll add this in a later commit. * Make error boundaries use Task Priority I have all but one error fixed. Not sure how tricky the last one is, but I'm cautiously optimistic. Pushing to show my current progress. * Remove recursion from handleErrors Changed the algorithm of handleErrors a bit to ensure that boundaries are not revisited once they are acknowledged. * Add incremental error boundary test Discovered an edge case: a noop error boundary will break in incremental mode unless you explicitly schedule an update. * Refactor error boundaries in Fiber * Simplify trapError() calls The existing logic was written before we had a proper error handling system. * Remove unnecessary flags * Prevent performTaskWork recursion * Be stricter about preventing recursion
1 parent f4a42b8 commit 4266f08

File tree

7 files changed

+314
-369
lines changed

7 files changed

+314
-369
lines changed

scripts/fiber/tests-failing.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,9 +554,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
554554
src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentNestedState-test.js
555555
* should provide up to date values for props
556556

557-
src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js
558-
* should support setting state
559-
560557
src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js
561558
* should still throw when rendering to undefined
562559
* should be able to switch between rendering null and a normal tag

scripts/fiber/tests-passing.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
791791
* skips will/DidUpdate when bailing unless an update was already in progress
792792
* performs batched updates at the end of the batch
793793
* can nest batchedUpdates
794+
* propagates an error from a noop error boundary
794795

795796
src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
796797
* handles isMounted even when the initial render is deferred
@@ -1016,6 +1017,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentDOMMinima
10161017
* should not render extra nodes for non-interpolated text
10171018

10181019
src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js
1020+
* should support setting state
10191021
* should batch unmounts
10201022

10211023
src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js

src/renderers/shared/fiber/ReactFiberCommitWork.js

Lines changed: 25 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
'use strict';
1414

15-
import type { TrappedError } from 'ReactFiberErrorBoundary';
1615
import type { Fiber } from 'ReactFiber';
1716
import type { FiberRoot } from 'ReactFiberRoot';
1817
import type { HostConfig } from 'ReactFiberReconciler';
@@ -24,7 +23,6 @@ var {
2423
HostComponent,
2524
HostText,
2625
} = ReactTypeOfWork;
27-
var { trapError } = require('ReactFiberErrorBoundary');
2826
var { callCallbacks } = require('ReactFiberUpdateQueue');
2927

3028
var {
@@ -33,7 +31,10 @@ var {
3331
Callback,
3432
} = require('ReactTypeOfSideEffect');
3533

36-
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
34+
module.exports = function<T, P, I, TI, C>(
35+
config : HostConfig<T, P, I, TI, C>,
36+
trapError : (fiber : Fiber, error: Error, isUnmounting : boolean) => void
37+
) {
3738

3839
const updateContainer = config.updateContainer;
3940
const commitUpdate = config.commitUpdate;
@@ -156,94 +157,71 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
156157
}
157158
}
158159

159-
function commitNestedUnmounts(root : Fiber): Array<TrappedError> | null {
160-
// Since errors are rare, we allocate this array on demand.
161-
let trappedErrors = null;
162-
160+
function commitNestedUnmounts(root : Fiber): void {
163161
// While we're inside a removed host node we don't want to call
164162
// removeChild on the inner nodes because they're removed by the top
165163
// call anyway. We also want to call componentWillUnmount on all
166164
// composites before this host node is removed from the tree. Therefore
167165
// we do an inner loop while we're still inside the host node.
168166
let node : Fiber = root;
169167
while (true) {
170-
const error = commitUnmount(node);
171-
if (error) {
172-
trappedErrors = trappedErrors || [];
173-
trappedErrors.push(error);
174-
}
168+
commitUnmount(node);
175169
if (node.child) {
176170
// TODO: Coroutines need to visit the stateNode.
177171
node = node.child;
178172
continue;
179173
}
180174
if (node === root) {
181-
return trappedErrors;
175+
return;
182176
}
183177
while (!node.sibling) {
184178
if (!node.return || node.return === root) {
185-
return trappedErrors;
179+
return;
186180
}
187181
node = node.return;
188182
}
189183
node = node.sibling;
190184
}
191-
return trappedErrors;
192185
}
193186

194-
function unmountHostComponents(parent, current): Array<TrappedError> | null {
195-
// Since errors are rare, we allocate this array on demand.
196-
let trappedErrors = null;
197-
187+
function unmountHostComponents(parent, current): void {
198188
// We only have the top Fiber that was inserted but we need recurse down its
199189
// children to find all the terminal nodes.
200190
let node : Fiber = current;
201191
while (true) {
202192
if (node.tag === HostComponent || node.tag === HostText) {
203-
const errors = commitNestedUnmounts(node);
204-
if (errors) {
205-
if (!trappedErrors) {
206-
trappedErrors = errors;
207-
} else {
208-
trappedErrors.push.apply(trappedErrors, errors);
209-
}
210-
}
193+
commitNestedUnmounts(node);
211194
// After all the children have unmounted, it is now safe to remove the
212195
// node from the tree.
213196
if (parent) {
214197
removeChild(parent, node.stateNode);
215198
}
216199
} else {
217-
const error = commitUnmount(node);
218-
if (error) {
219-
trappedErrors = trappedErrors || [];
220-
trappedErrors.push(error);
221-
}
200+
commitUnmount(node);
222201
if (node.child) {
223202
// TODO: Coroutines need to visit the stateNode.
224203
node = node.child;
225204
continue;
226205
}
227206
}
228207
if (node === current) {
229-
return trappedErrors;
208+
return;
230209
}
231210
while (!node.sibling) {
232211
if (!node.return || node.return === current) {
233-
return trappedErrors;
212+
return;
234213
}
235214
node = node.return;
236215
}
237216
node = node.sibling;
238217
}
239-
return trappedErrors;
240218
}
241219

242-
function commitDeletion(current : Fiber) : Array<TrappedError> | null {
220+
function commitDeletion(current : Fiber) : void {
243221
// Recursively delete all host nodes from the parent.
244222
const parent = getHostParent(current);
245223
// Detach refs and call componentWillUnmount() on the whole subtree.
246-
const trappedErrors = unmountHostComponents(parent, current);
224+
unmountHostComponents(parent, current);
247225

248226
// Cut off the return pointers to disconnect it from the tree. Ideally, we
249227
// should clear the child pointer of the parent alternate to let this
@@ -256,29 +234,24 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
256234
current.alternate.child = null;
257235
current.alternate.return = null;
258236
}
259-
260-
return trappedErrors;
261237
}
262238

263-
function commitUnmount(current : Fiber) : TrappedError | null {
239+
function commitUnmount(current : Fiber) : void {
264240
switch (current.tag) {
265241
case ClassComponent: {
266242
detachRef(current);
267243
const instance = current.stateNode;
268244
if (typeof instance.componentWillUnmount === 'function') {
269245
const error = tryCallComponentWillUnmount(instance);
270246
if (error) {
271-
return trapError(current, error);
247+
trapError(current, error, true);
272248
}
273249
}
274-
return null;
250+
return;
275251
}
276252
case HostComponent: {
277253
detachRef(current);
278-
return null;
279-
}
280-
default: {
281-
return null;
254+
return;
282255
}
283256
}
284257
}
@@ -323,7 +296,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
323296
}
324297
}
325298

326-
function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : TrappedError | null {
299+
function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : void {
327300
switch (finishedWork.tag) {
328301
case ClassComponent: {
329302
const instance = finishedWork.stateNode;
@@ -353,9 +326,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
353326
}
354327
}
355328
if (error) {
356-
return trapError(finishedWork, error);
329+
trapError(finishedWork, error, false);
357330
}
358-
return null;
331+
return;
359332
}
360333
case HostContainer: {
361334
const rootFiber = finishedWork.stateNode;
@@ -364,15 +337,16 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
364337
rootFiber.callbackList = null;
365338
callCallbacks(callbackList, rootFiber.current.child.stateNode);
366339
}
340+
return;
367341
}
368342
case HostComponent: {
369343
const instance : I = finishedWork.stateNode;
370344
attachRef(current, finishedWork, instance);
371-
return null;
345+
return;
372346
}
373347
case HostText: {
374348
// We have no life-cycles associated with text.
375-
return null;
349+
return;
376350
}
377351
default:
378352
throw new Error('This unit of work tag should not have side-effects.');

src/renderers/shared/fiber/ReactFiberErrorBoundary.js

Lines changed: 0 additions & 53 deletions
This file was deleted.

0 commit comments

Comments
 (0)