Fallback to current props if memoizedProps is null
If work has progressed on a state update that gets resumed because of another state up, then we won't have an new pendingProps, and we also won't have any memoizedProps because it got aborted before completing. In that case, we can just fallback to the current props. I think that they can't have diverged because the only way they diverge is if there is new props. This lets us bail out on state only updates in more cases which the unit tests reflect.
Sebastian Markbage committed
Oct 20, 2016 at 18:12 UTC
c862ba719e26764473eb9edd798429600ca746ab
3 files changed
+6
-6
src/renderers/shared/fiber/ReactFiberClassComponent.js
+3
-3
@@ -165,12 +165,12 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
165
function updateClassInstance(current : Fiber, workInProgress : Fiber) : boolean {
166
const instance = workInProgress.stateNode;
167
168
- const oldProps = current.memoizedProps;
168
+ const oldProps = workInProgress.memoizedProps || current.memoizedProps;
169
let newProps = workInProgress.pendingProps;
170
if (!newProps) {
171
// If there aren't any new props, then we'll reuse the memoized props.
172
// This could be from already completed work.
173
- newProps = workInProgress.memoizedProps;
173
+ newProps = oldProps;
174
if (!newProps) {
175
throw new Error('There should always be pending or memoized props.');
176
}
@@ -199,7 +199,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
199
200
if (typeof instance.shouldComponentUpdate === 'function' &&
201
!(updateQueue && updateQueue.isForced) &&
202
- workInProgress.memoizedProps !== null &&
202
+ oldProps !== null &&
203
!instance.shouldComponentUpdate(newProps, newState)) {
204
// TODO: Should this get the new props/state updated regardless?
205
return false;
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+2
-2
@@ -625,10 +625,10 @@ describe('ReactIncremental', () => {
625
// Normally shouldComponentUpdate->false is not enough to determine that we
626
// can safely reuse the old props, but I think in this case it would be ok,
627
// since it is a resume of already started work.
628
- // Because of the above we can also not reuse the work of Bar because the
628
+ // Because of the above we can not reuse the work of Bar because the
629
// rerender of Content will generate a new element which will mean we don't
630
// auto-bail out from Bar.
631
- expect(ops).toEqual(['Content', 'Bar', 'Middle']);
631
+ expect(ops).toEqual(['Bar', 'Middle']);
632
633
});
634
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+1
-1
@@ -598,7 +598,7 @@ describe('ReactIncrementalSideEffects', () => {
598
),
599
]);
600
601
- expect(ops).toEqual(['Baz', 'Bar', 'Baz', 'Bar', 'Bar']);
601
+ expect(ops).toEqual(['Bar', 'Baz', 'Bar', 'Bar']);
602
});
603
604
it('deprioritizes setStates that happens within a deprioritized tree', () => {