Remove beginWork shortcut
This shortcut had a bug associated with it. If beginWork on this child returns null, then we don't call completeWork on that fiber. Since removing this short cut adds another time check, we have to add a single unit of time in tests to account for the top level call now taking one more unit. This was also the only recursive call in all of fiber so it's nice to get rid of it to guarantee that a flat stack is possible.
Sebastian Markbage committed
Oct 5, 2016 at 18:04 UTC
1d437dc9ad11567e8da785534f1111755aff9e3e
2 files changed
+10
-38
src/renderers/shared/fiber/ReactFiberBeginWork.js
+2
-30
@@ -441,14 +441,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
441
reconcileChildren(current, workInProgress, workInProgress.pendingProps);
442
// A yield component is just a placeholder, we can just run through the
443
// next one immediately.
444
- if (workInProgress.child) {
445
- return beginWork(
446
- workInProgress.child.alternate,
447
- workInProgress.child,
448
- priorityLevel
449
- );
450
- }
451
- return null;
444
+ return workInProgress.child;
445
case HostComponent:
446
if (workInProgress.stateNode && config.beginUpdate) {
447
config.beginUpdate(workInProgress.stateNode);
@@ -466,35 +459,14 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, g
459
updateCoroutineComponent(current, workInProgress);
460
// This doesn't take arbitrary time so we could synchronously just begin
461
// eagerly do the work of workInProgress.child as an optimization.
469
- if (workInProgress.child) {
470
- return beginWork(
471
- workInProgress.child.alternate,
472
- workInProgress.child,
473
- priorityLevel
474
- );
475
- }
462
return workInProgress.child;
463
case YieldComponent:
464
// A yield component is just a placeholder, we can just run through the
465
// next one immediately.
480
- if (workInProgress.sibling) {
481
- return beginWork(
482
- workInProgress.sibling.alternate,
483
- workInProgress.sibling,
484
- priorityLevel
485
- );
486
- }
466
return null;
467
case Fragment:
468
updateFragment(current, workInProgress);
490
- if (workInProgress.child) {
491
- return beginWork(
492
- workInProgress.child.alternate,
493
- workInProgress.child,
494
- priorityLevel
495
- );
496
- }
497
- return null;
469
+ return workInProgress.child;
470
default:
471
throw new Error('Unknown unit of work tag');
472
}
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+8
-8
@@ -56,7 +56,7 @@ describe('ReactIncremental', () => {
56
expect(fooCalled).toBe(false);
57
expect(barCalled).toBe(false);
58
// Do one step of work.
59
- ReactNoop.flushDeferredPri(7);
59
+ ReactNoop.flushDeferredPri(7 + 5);
60
expect(fooCalled).toBe(true);
61
expect(barCalled).toBe(false);
62
// Do the rest of the work.
@@ -143,7 +143,7 @@ describe('ReactIncremental', () => {
143
144
ReactNoop.render(<Foo text="bar" />);
145
// Flush part of the work
146
- ReactNoop.flushDeferredPri(20);
146
+ ReactNoop.flushDeferredPri(20 + 5);
147
148
expect(ops).toEqual(['Foo', 'Bar']);
149
@@ -153,7 +153,7 @@ describe('ReactIncremental', () => {
153
ReactNoop.render(<Foo text="baz" />);
154
155
// Flush part of the new work
156
- ReactNoop.flushDeferredPri(20);
156
+ ReactNoop.flushDeferredPri(20 + 5);
157
158
expect(ops).toEqual(['Foo', 'Bar']);
159
@@ -328,7 +328,7 @@ describe('ReactIncremental', () => {
328
329
// We're now rendering an update that will bail out on updating middle.
330
ReactNoop.render(<Foo text="bar" />);
331
- ReactNoop.flushDeferredPri(45);
331
+ ReactNoop.flushDeferredPri(45 + 5);
332
333
expect(ops).toEqual(['Foo', 'Bar', 'Bar']);
334
@@ -395,7 +395,7 @@ describe('ReactIncremental', () => {
395
396
// Init
397
ReactNoop.render(<Foo text="foo" />);
398
- ReactNoop.flushDeferredPri(52);
398
+ ReactNoop.flushDeferredPri(52 + 5);
399
400
expect(ops).toEqual(['Foo', 'Bar', 'Tester', 'Bar']);
401
@@ -467,7 +467,7 @@ describe('ReactIncremental', () => {
467
468
// Init
469
ReactNoop.render(<Foo text="foo" text2="foo" step={0} />);
470
- ReactNoop.flushDeferredPri(55 + 25);
470
+ ReactNoop.flushDeferredPri(55 + 25 + 5);
471
472
// We only finish the higher priority work. So the low pri content
473
// has not yet finished mounting.
@@ -489,7 +489,7 @@ describe('ReactIncremental', () => {
489
// Make a quick update which will schedule low priority work to
490
// update the middle content.
491
ReactNoop.render(<Foo text="bar" text2="bar" step={1} />);
492
- ReactNoop.flushDeferredPri(30 + 25);
492
+ ReactNoop.flushDeferredPri(30 + 25 + 5);
493
494
expect(ops).toEqual(['Foo', 'Bar']);
495
@@ -583,7 +583,7 @@ describe('ReactIncremental', () => {
583
ops = [];
584
585
// The middle content is now pending rendering...
586
- ReactNoop.flushDeferredPri(30 + 25);
586
+ ReactNoop.flushDeferredPri(30 + 25 + 5);
587
expect(ops).toEqual(['Content', 'Middle', 'Bar']); // One more Middle left.
588
589
ops = [];