Skip to content

Commit cdb508a

Browse files
committed
Always set pendingProps to the next props
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.
1 parent 4a924a2 commit cdb508a

File tree

4 files changed

+45
-99
lines changed

4 files changed

+45
-99
lines changed

packages/react-reconciler/src/ReactFiber.js

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ if (__DEV__) {
152152

153153
function FiberNode(
154154
tag: TypeOfWork,
155+
pendingProps: mixed,
155156
key: null | string,
156157
internalContextTag: TypeOfInternalContext,
157158
) {
@@ -169,7 +170,7 @@ function FiberNode(
169170

170171
this.ref = null;
171172

172-
this.pendingProps = null;
173+
this.pendingProps = pendingProps;
173174
this.memoizedProps = null;
174175
this.updateQueue = null;
175176
this.memoizedState = null;
@@ -213,11 +214,12 @@ function FiberNode(
213214
// compatible.
214215
var createFiber = function(
215216
tag: TypeOfWork,
217+
pendingProps: mixed,
216218
key: null | string,
217219
internalContextTag: TypeOfInternalContext,
218220
): Fiber {
219221
// $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);
221223
};
222224

223225
function shouldConstruct(Component) {
@@ -239,6 +241,7 @@ export function createWorkInProgress(
239241
// reclaim the extra memory if needed.
240242
workInProgress = createFiber(
241243
current.tag,
244+
pendingProps,
242245
current.key,
243246
current.internalContextTag,
244247
);
@@ -255,6 +258,8 @@ export function createWorkInProgress(
255258
workInProgress.alternate = current;
256259
current.alternate = workInProgress;
257260
} else {
261+
workInProgress.pendingProps = pendingProps;
262+
258263
// We already have an alternate.
259264
// Reset the effect tag.
260265
workInProgress.effectTag = NoEffect;
@@ -266,7 +271,6 @@ export function createWorkInProgress(
266271
}
267272

268273
workInProgress.expirationTime = expirationTime;
269-
workInProgress.pendingProps = pendingProps;
270274

271275
workInProgress.child = current.child;
272276
workInProgress.memoizedProps = current.memoizedProps;
@@ -297,17 +301,22 @@ export function createFiberFromElement(
297301
}
298302

299303
let fiber;
300-
const {type, key} = element;
304+
const type = element.type;
305+
const key = element.key;
306+
const pendingProps = element.props;
301307
if (typeof type === 'function') {
302308
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+
);
305316
fiber.type = type;
306-
fiber.pendingProps = element.props;
307317
} else if (typeof type === 'string') {
308-
fiber = createFiber(HostComponent, key, internalContextTag);
318+
fiber = createFiber(HostComponent, pendingProps, key, internalContextTag);
309319
fiber.type = type;
310-
fiber.pendingProps = element.props;
311320
} else if (
312321
typeof type === 'object' &&
313322
type !== null &&
@@ -320,7 +329,7 @@ export function createFiberFromElement(
320329
// we don't know if we can reuse that fiber or if we need to clone it.
321330
// There is probably a clever way to restructure this.
322331
fiber = ((type: any): Fiber);
323-
fiber.pendingProps = element.props;
332+
fiber.pendingProps = pendingProps;
324333
} else {
325334
let info = '';
326335
if (__DEV__) {
@@ -364,8 +373,7 @@ export function createFiberFromFragment(
364373
expirationTime: ExpirationTime,
365374
key: null | string,
366375
): Fiber {
367-
const fiber = createFiber(Fragment, key, internalContextTag);
368-
fiber.pendingProps = elements;
376+
const fiber = createFiber(Fragment, elements, key, internalContextTag);
369377
fiber.expirationTime = expirationTime;
370378
return fiber;
371379
}
@@ -375,14 +383,13 @@ export function createFiberFromText(
375383
internalContextTag: TypeOfInternalContext,
376384
expirationTime: ExpirationTime,
377385
): Fiber {
378-
const fiber = createFiber(HostText, null, internalContextTag);
379-
fiber.pendingProps = content;
386+
const fiber = createFiber(HostText, content, null, internalContextTag);
380387
fiber.expirationTime = expirationTime;
381388
return fiber;
382389
}
383390

384391
export function createFiberFromHostInstanceForDeletion(): Fiber {
385-
const fiber = createFiber(HostComponent, null, NoContext);
392+
const fiber = createFiber(HostComponent, null, null, NoContext);
386393
fiber.type = 'DELETED';
387394
return fiber;
388395
}
@@ -392,9 +399,8 @@ export function createFiberFromCall(
392399
internalContextTag: TypeOfInternalContext,
393400
expirationTime: ExpirationTime,
394401
): Fiber {
395-
const fiber = createFiber(CallComponent, call.key, internalContextTag);
402+
const fiber = createFiber(CallComponent, call, call.key, internalContextTag);
396403
fiber.type = call.handler;
397-
fiber.pendingProps = call;
398404
fiber.expirationTime = expirationTime;
399405
return fiber;
400406
}
@@ -404,7 +410,7 @@ export function createFiberFromReturn(
404410
internalContextTag: TypeOfInternalContext,
405411
expirationTime: ExpirationTime,
406412
): Fiber {
407-
const fiber = createFiber(ReturnComponent, null, internalContextTag);
413+
const fiber = createFiber(ReturnComponent, null, null, internalContextTag);
408414
fiber.expirationTime = expirationTime;
409415
return fiber;
410416
}
@@ -414,8 +420,13 @@ export function createFiberFromPortal(
414420
internalContextTag: TypeOfInternalContext,
415421
expirationTime: ExpirationTime,
416422
): 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+
);
419430
fiber.expirationTime = expirationTime;
420431
fiber.stateNode = {
421432
containerInfo: portal.containerInfo,

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
141141
}
142142

143143
function updateFragment(current, workInProgress) {
144-
var nextChildren = workInProgress.pendingProps;
144+
const nextChildren = workInProgress.pendingProps;
145145
if (hasContextChanged()) {
146146
// Normally we can bail out on props equality but if context has changed
147147
// we don't do the bailout and we have to reuse existing props instead.
148-
if (nextChildren === null) {
149-
nextChildren = workInProgress.memoizedProps;
150-
}
151148
} else if (
152149
nextChildren === null ||
153150
workInProgress.memoizedProps === nextChildren
@@ -168,18 +165,14 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
168165
}
169166

170167
function updateFunctionalComponent(current, workInProgress) {
171-
var fn = workInProgress.type;
172-
var nextProps = workInProgress.pendingProps;
168+
const fn = workInProgress.type;
169+
const nextProps = workInProgress.pendingProps;
173170

174-
const memoizedProps = workInProgress.memoizedProps;
175171
if (hasContextChanged()) {
176172
// Normally we can bail out on props equality but if context has changed
177173
// we don't do the bailout and we have to reuse existing props instead.
178-
if (nextProps === null) {
179-
nextProps = memoizedProps;
180-
}
181174
} else {
182-
if (nextProps === null || memoizedProps === nextProps) {
175+
if (workInProgress.memoizedProps === nextProps) {
183176
return bailoutOnAlreadyFinishedWork(current, workInProgress);
184177
}
185178
// TODO: consider bringing fn.shouldComponentUpdate() back.
@@ -373,21 +366,13 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
373366

374367
const type = workInProgress.type;
375368
const memoizedProps = workInProgress.memoizedProps;
376-
let nextProps = workInProgress.pendingProps;
377-
if (nextProps === null) {
378-
nextProps = memoizedProps;
379-
invariant(
380-
nextProps !== null,
381-
'We should always have pending or current props. This error is ' +
382-
'likely caused by a bug in React. Please file an issue.',
383-
);
384-
}
369+
const nextProps = workInProgress.pendingProps;
385370
const prevProps = current !== null ? current.memoizedProps : null;
386371

387372
if (hasContextChanged()) {
388373
// Normally we can bail out on props equality but if context has changed
389374
// we don't do the bailout and we have to reuse existing props instead.
390-
} else if (nextProps === null || memoizedProps === nextProps) {
375+
} else if (memoizedProps === nextProps) {
391376
return bailoutOnAlreadyFinishedWork(current, workInProgress);
392377
}
393378

@@ -429,10 +414,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
429414
if (current === null) {
430415
tryToClaimNextHydratableInstance(workInProgress);
431416
}
432-
let nextProps = workInProgress.pendingProps;
433-
if (nextProps === null) {
434-
nextProps = workInProgress.memoizedProps;
435-
}
417+
const nextProps = workInProgress.pendingProps;
436418
memoizeProps(workInProgress, nextProps);
437419
// Nothing to do here. This is terminal. We'll do the completion step
438420
// immediately after.
@@ -534,19 +516,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
534516
}
535517

536518
function updateCallComponent(current, workInProgress, renderExpirationTime) {
537-
var nextCall = (workInProgress.pendingProps: null | ReactCall);
519+
var nextCall = (workInProgress.pendingProps: ReactCall);
538520
if (hasContextChanged()) {
539521
// Normally we can bail out on props equality but if context has changed
540522
// we don't do the bailout and we have to reuse existing props instead.
541-
if (nextCall === null) {
542-
nextCall = current && current.memoizedProps;
543-
invariant(
544-
nextCall !== null,
545-
'We should always have pending or current props. This error is ' +
546-
'likely caused by a bug in React. Please file an issue.',
547-
);
548-
}
549-
} else if (nextCall === null || workInProgress.memoizedProps === nextCall) {
523+
} else if (workInProgress.memoizedProps === nextCall) {
550524
nextCall = workInProgress.memoizedProps;
551525
// TODO: When bailing out, we might need to return the stateNode instead
552526
// of the child. To check it for work.
@@ -585,22 +559,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
585559
renderExpirationTime,
586560
) {
587561
pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo);
588-
let nextChildren = workInProgress.pendingProps;
562+
const nextChildren = workInProgress.pendingProps;
589563
if (hasContextChanged()) {
590564
// Normally we can bail out on props equality but if context has changed
591565
// we don't do the bailout and we have to reuse existing props instead.
592-
if (nextChildren === null) {
593-
nextChildren = current && current.memoizedProps;
594-
invariant(
595-
nextChildren != null,
596-
'We should always have pending or current props. This error is ' +
597-
'likely caused by a bug in React. Please file an issue.',
598-
);
599-
}
600-
} else if (
601-
nextChildren === null ||
602-
workInProgress.memoizedProps === nextChildren
603-
) {
566+
} else if (workInProgress.memoizedProps === nextChildren) {
604567
return bailoutOnAlreadyFinishedWork(current, workInProgress);
605568
}
606569

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -433,14 +433,7 @@ export default function(
433433

434434
const instance = workInProgress.stateNode;
435435
const state = instance.state || null;
436-
437-
let props = workInProgress.pendingProps;
438-
invariant(
439-
props,
440-
'There must be pending props for an initial mount. This error is ' +
441-
'likely caused by a bug in React. Please file an issue.',
442-
);
443-
436+
const props = workInProgress.pendingProps;
444437
const unmaskedContext = getUnmaskedContext(workInProgress);
445438

446439
instance.props = props;
@@ -593,17 +586,7 @@ export default function(
593586
resetInputPointers(workInProgress, instance);
594587

595588
const oldProps = workInProgress.memoizedProps;
596-
let newProps = workInProgress.pendingProps;
597-
if (!newProps) {
598-
// If there aren't any new props, then we'll reuse the memoized props.
599-
// This could be from already completed work.
600-
newProps = oldProps;
601-
invariant(
602-
newProps != null,
603-
'There should always be pending or memoized props. This error is ' +
604-
'likely caused by a bug in React. Please file an issue.',
605-
);
606-
}
589+
const newProps = workInProgress.pendingProps;
607590
const oldContext = instance.context;
608591
const newUnmaskedContext = getUnmaskedContext(workInProgress);
609592
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
392392
workInProgress: Fiber,
393393
renderExpirationTime: ExpirationTime,
394394
): 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-
395+
const newProps = workInProgress.pendingProps;
407396
switch (workInProgress.tag) {
408397
case FunctionalComponent:
409398
return null;

0 commit comments

Comments
 (0)