@samitouri / QOS-React / commits / 2ae4c62158

Always set pendingProps to the next props (#11580)

In the current implementation, pendingProps is null if there are no new props since the last commit. When that happens, we bail out and reuse the current props. But it makes more sense to always set pendingProps to whatever the next props will be. In other words, pendingProps is never null: it points to either new props, or to the current props. Modeling it this way lets us delete lots of code branches and is easier to reason about bail outs: just compare the pending props to the current props.

Andrew Clark committed Nov 28, 2017 at 16:50 UTC 2ae4c6215891c67aed17c22261db74c1f078f663
4 files changed +45 -100
packages/react-reconciler/src/ReactFiber.js
+31 -20
@@ -152,6 +152,7 @@ if (__DEV__) {
152
153 function FiberNode(
154 tag: TypeOfWork,
155 + pendingProps: mixed,
156 key: null | string,
157 internalContextTag: TypeOfInternalContext,
158 ) {
@@ -169,7 +170,7 @@ function FiberNode(
170
171 this.ref = null;
172
172 - this.pendingProps = null;
173 + this.pendingProps = pendingProps;
174 this.memoizedProps = null;
175 this.updateQueue = null;
176 this.memoizedState = null;
@@ -213,11 +214,12 @@ function FiberNode(
214 // compatible.
215 var createFiber = function(
216 tag: TypeOfWork,
217 + pendingProps: mixed,
218 key: null | string,
219 internalContextTag: TypeOfInternalContext,
220 ): Fiber {
221 // $FlowFixMe: the shapes are exact here but Flow doesn't like constructors
220 - return new FiberNode(tag, key, internalContextTag);
222 + return new FiberNode(tag, pendingProps, key, internalContextTag);
223 };
224
225 function shouldConstruct(Component) {
@@ -239,6 +241,7 @@ export function createWorkInProgress(
241 // reclaim the extra memory if needed.
242 workInProgress = createFiber(
243 current.tag,
244 + pendingProps,
245 current.key,
246 current.internalContextTag,
247 );
@@ -255,6 +258,8 @@ export function createWorkInProgress(
258 workInProgress.alternate = current;
259 current.alternate = workInProgress;
260 } else {
261 + workInProgress.pendingProps = pendingProps;
262 +
263 // We already have an alternate.
264 // Reset the effect tag.
265 workInProgress.effectTag = NoEffect;
@@ -266,7 +271,6 @@ export function createWorkInProgress(
271 }
272
273 workInProgress.expirationTime = expirationTime;
269 - workInProgress.pendingProps = pendingProps;
274
275 workInProgress.child = current.child;
276 workInProgress.memoizedProps = current.memoizedProps;
@@ -297,17 +301,22 @@ export function createFiberFromElement(
301 }
302
303 let fiber;
300 - const {type, key} = element;
304 + const type = element.type;
305 + const key = element.key;
306 + const pendingProps = element.props;
307 if (typeof type === 'function') {
308 fiber = shouldConstruct(type)
303 - ? createFiber(ClassComponent, key, internalContextTag)
304 - : createFiber(IndeterminateComponent, key, internalContextTag);
309 + ? createFiber(ClassComponent, pendingProps, key, internalContextTag)
310 + : createFiber(
311 + IndeterminateComponent,
312 + pendingProps,
313 + key,
314 + internalContextTag,
315 + );
316 fiber.type = type;
306 - fiber.pendingProps = element.props;
317 } else if (typeof type === 'string') {
308 - fiber = createFiber(HostComponent, key, internalContextTag);
318 + fiber = createFiber(HostComponent, pendingProps, key, internalContextTag);
319 fiber.type = type;
310 - fiber.pendingProps = element.props;
320 } else if (
321 typeof type === 'object' &&
322 type !== null &&
@@ -320,7 +329,7 @@ export function createFiberFromElement(
329 // we don't know if we can reuse that fiber or if we need to clone it.
330 // There is probably a clever way to restructure this.
331 fiber = ((type: any): Fiber);
323 - fiber.pendingProps = element.props;
332 + fiber.pendingProps = pendingProps;
333 } else {
334 let info = '';
335 if (__DEV__) {
@@ -364,8 +373,7 @@ export function createFiberFromFragment(
373 expirationTime: ExpirationTime,
374 key: null | string,
375 ): Fiber {
367 - const fiber = createFiber(Fragment, key, internalContextTag);
368 - fiber.pendingProps = elements;
376 + const fiber = createFiber(Fragment, elements, key, internalContextTag);
377 fiber.expirationTime = expirationTime;
378 return fiber;
379 }
@@ -375,14 +383,13 @@ export function createFiberFromText(
383 internalContextTag: TypeOfInternalContext,
384 expirationTime: ExpirationTime,
385 ): Fiber {
378 - const fiber = createFiber(HostText, null, internalContextTag);
379 - fiber.pendingProps = content;
386 + const fiber = createFiber(HostText, content, null, internalContextTag);
387 fiber.expirationTime = expirationTime;
388 return fiber;
389 }
390
391 export function createFiberFromHostInstanceForDeletion(): Fiber {
385 - const fiber = createFiber(HostComponent, null, NoContext);
392 + const fiber = createFiber(HostComponent, null, null, NoContext);
393 fiber.type = 'DELETED';
394 return fiber;
395 }
@@ -392,9 +399,8 @@ export function createFiberFromCall(
399 internalContextTag: TypeOfInternalContext,
400 expirationTime: ExpirationTime,
401 ): Fiber {
395 - const fiber = createFiber(CallComponent, call.key, internalContextTag);
402 + const fiber = createFiber(CallComponent, call, call.key, internalContextTag);
403 fiber.type = call.handler;
397 - fiber.pendingProps = call;
404 fiber.expirationTime = expirationTime;
405 return fiber;
406 }
@@ -404,7 +410,7 @@ export function createFiberFromReturn(
410 internalContextTag: TypeOfInternalContext,
411 expirationTime: ExpirationTime,
412 ): Fiber {
407 - const fiber = createFiber(ReturnComponent, null, internalContextTag);
413 + const fiber = createFiber(ReturnComponent, null, null, internalContextTag);
414 fiber.expirationTime = expirationTime;
415 return fiber;
416 }
@@ -414,8 +420,13 @@ export function createFiberFromPortal(
420 internalContextTag: TypeOfInternalContext,
421 expirationTime: ExpirationTime,
422 ): Fiber {
417 - const fiber = createFiber(HostPortal, portal.key, internalContextTag);
418 - fiber.pendingProps = portal.children || [];
423 + const pendingProps = portal.children !== null ? portal.children : [];
424 + const fiber = createFiber(
425 + HostPortal,
426 + pendingProps,
427 + portal.key,
428 + internalContextTag,
429 + );
430 fiber.expirationTime = expirationTime;
431 fiber.stateNode = {
432 containerInfo: portal.containerInfo,
packages/react-reconciler/src/ReactFiberBeginWork.js
+11 -48
@@ -142,13 +142,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
142 }
143
144 function updateFragment(current, workInProgress) {
145 - var nextChildren = workInProgress.pendingProps;
145 + const nextChildren = workInProgress.pendingProps;
146 if (hasContextChanged()) {
147 // Normally we can bail out on props equality but if context has changed
148 // we don't do the bailout and we have to reuse existing props instead.
149 - if (nextChildren === null) {
150 - nextChildren = workInProgress.memoizedProps;
151 - }
149 } else if (
150 nextChildren === null ||
151 workInProgress.memoizedProps === nextChildren
@@ -169,18 +166,14 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
166 }
167
168 function updateFunctionalComponent(current, workInProgress) {
172 - var fn = workInProgress.type;
173 - var nextProps = workInProgress.pendingProps;
169 + const fn = workInProgress.type;
170 + const nextProps = workInProgress.pendingProps;
171
175 - const memoizedProps = workInProgress.memoizedProps;
172 if (hasContextChanged()) {
173 // Normally we can bail out on props equality but if context has changed
174 // we don't do the bailout and we have to reuse existing props instead.
179 - if (nextProps === null) {
180 - nextProps = memoizedProps;
181 - }
175 } else {
183 - if (nextProps === null || memoizedProps === nextProps) {
176 + if (workInProgress.memoizedProps === nextProps) {
177 return bailoutOnAlreadyFinishedWork(current, workInProgress);
178 }
179 // TODO: consider bringing fn.shouldComponentUpdate() back.
@@ -380,21 +373,13 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
373
374 const type = workInProgress.type;
375 const memoizedProps = workInProgress.memoizedProps;
383 - let nextProps = workInProgress.pendingProps;
384 - if (nextProps === null) {
385 - nextProps = memoizedProps;
386 - invariant(
387 - nextProps !== null,
388 - 'We should always have pending or current props. This error is ' +
389 - 'likely caused by a bug in React. Please file an issue.',
390 - );
391 - }
376 + const nextProps = workInProgress.pendingProps;
377 const prevProps = current !== null ? current.memoizedProps : null;
378
379 if (hasContextChanged()) {
380 // Normally we can bail out on props equality but if context has changed
381 // we don't do the bailout and we have to reuse existing props instead.
397 - } else if (nextProps === null || memoizedProps === nextProps) {
382 + } else if (memoizedProps === nextProps) {
383 return bailoutOnAlreadyFinishedWork(current, workInProgress);
384 }
385
@@ -436,10 +421,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
421 if (current === null) {
422 tryToClaimNextHydratableInstance(workInProgress);
423 }
439 - let nextProps = workInProgress.pendingProps;
440 - if (nextProps === null) {
441 - nextProps = workInProgress.memoizedProps;
442 - }
424 + const nextProps = workInProgress.pendingProps;
425 memoizeProps(workInProgress, nextProps);
426 // Nothing to do here. This is terminal. We'll do the completion step
427 // immediately after.
@@ -541,19 +523,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
523 }
524
525 function updateCallComponent(current, workInProgress, renderExpirationTime) {
544 - var nextCall = (workInProgress.pendingProps: null | ReactCall);
526 + var nextCall = (workInProgress.pendingProps: ReactCall);
527 if (hasContextChanged()) {
528 // Normally we can bail out on props equality but if context has changed
529 // we don't do the bailout and we have to reuse existing props instead.
548 - if (nextCall === null) {
549 - nextCall = current && current.memoizedProps;
550 - invariant(
551 - nextCall !== null,
552 - 'We should always have pending or current props. This error is ' +
553 - 'likely caused by a bug in React. Please file an issue.',
554 - );
555 - }
556 - } else if (nextCall === null || workInProgress.memoizedProps === nextCall) {
530 + } else if (workInProgress.memoizedProps === nextCall) {
531 nextCall = workInProgress.memoizedProps;
532 // TODO: When bailing out, we might need to return the stateNode instead
533 // of the child. To check it for work.
@@ -592,22 +566,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
566 renderExpirationTime,
567 ) {
568 pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo);
595 - let nextChildren = workInProgress.pendingProps;
569 + const nextChildren = workInProgress.pendingProps;
570 if (hasContextChanged()) {
571 // Normally we can bail out on props equality but if context has changed
572 // we don't do the bailout and we have to reuse existing props instead.
599 - if (nextChildren === null) {
600 - nextChildren = current && current.memoizedProps;
601 - invariant(
602 - nextChildren != null,
603 - 'We should always have pending or current props. This error is ' +
604 - 'likely caused by a bug in React. Please file an issue.',
605 - );
606 - }
607 - } else if (
608 - nextChildren === null ||
609 - workInProgress.memoizedProps === nextChildren
610 - ) {
573 + } else if (workInProgress.memoizedProps === nextChildren) {
574 return bailoutOnAlreadyFinishedWork(current, workInProgress);
575 }
576
packages/react-reconciler/src/ReactFiberClassComponent.js
+2 -19
@@ -450,14 +450,7 @@ export default function(
450
451 const instance = workInProgress.stateNode;
452 const state = instance.state || null;
453 -
454 - let props = workInProgress.pendingProps;
455 - invariant(
456 - props,
457 - 'There must be pending props for an initial mount. This error is ' +
458 - 'likely caused by a bug in React. Please file an issue.',
459 - );
460 -
453 + const props = workInProgress.pendingProps;
454 const unmaskedContext = getUnmaskedContext(workInProgress);
455
456 instance.props = props;
@@ -610,17 +603,7 @@ export default function(
603 resetInputPointers(workInProgress, instance);
604
605 const oldProps = workInProgress.memoizedProps;
613 - let newProps = workInProgress.pendingProps;
614 - if (!newProps) {
615 - // If there aren't any new props, then we'll reuse the memoized props.
616 - // This could be from already completed work.
617 - newProps = oldProps;
618 - invariant(
619 - newProps != null,
620 - 'There should always be pending or memoized props. This error is ' +
621 - 'likely caused by a bug in React. Please file an issue.',
622 - );
623 - }
606 + const newProps = workInProgress.pendingProps;
607 const oldContext = instance.context;
608 const newUnmaskedContext = getUnmaskedContext(workInProgress);
609 const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
packages/react-reconciler/src/ReactFiberCompleteWork.js
+1 -13
@@ -41,7 +41,6 @@ import {
41 popContextProvider,
42 popTopLevelContextObject,
43 } from './ReactFiberContext';
44 -import {Never} from './ReactFiberExpirationTime';
44
45 export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
46 config: HostConfig<T, P, I, TI, HI, PI, C, CC, CX, PL>,
@@ -392,18 +391,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
391 workInProgress: Fiber,
392 renderExpirationTime: ExpirationTime,
393 ): Fiber | null {
395 - // Get the latest props.
396 - let newProps = workInProgress.pendingProps;
397 - if (newProps === null) {
398 - newProps = workInProgress.memoizedProps;
399 - } else if (
400 - workInProgress.expirationTime !== Never ||
401 - renderExpirationTime === Never
402 - ) {
403 - // Reset the pending props, unless this was a down-prioritization.
404 - workInProgress.pendingProps = null;
405 - }
406 -
394 + const newProps = workInProgress.pendingProps;
395 switch (workInProgress.tag) {
396 case FunctionalComponent:
397 return null;