Skip to content

Commit 2336c85

Browse files
acdliteacdlite
committed
Call getDerivedStateFromProps every render, even if props did not change
Rather than enqueue a new setState updater for every props change, we can skip the update queue entirely and merge the result into state at the end. This makes more sense, since "receiving props" is not an event that should be observed. It's still a bit weird, since eventually we do persist the derived state (in other words, it accumulates).
1 parent 77ddeab commit 2336c85

File tree

5 files changed

+84
-215
lines changed

5 files changed

+84
-215
lines changed

‎packages/create-subscription/src/__tests__/createSubscription-test.internal.js‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ describe('createSubscription', () => {
264264

265265
it('should ignore values emitted by a new subscribable until the commit phase', () => {
266266
const log = [];
267-
let parentInstance;
268267

269268
function Child({value}) {
270269
ReactNoop.yield('Child: ' + value);
@@ -301,8 +300,6 @@ describe('createSubscription', () => {
301300
}
302301

303302
render() {
304-
parentInstance = this;
305-
306303
return (
307304
<Subscription source={this.state.observed}>
308305
{(value = 'default') => {
@@ -331,8 +328,8 @@ describe('createSubscription', () => {
331328
observableB.next('b-2');
332329
observableB.next('b-3');
333330

334-
// Mimic a higher-priority interruption
335-
parentInstance.setState({observed: observableA});
331+
// Update again
332+
ReactNoop.render(<Parent observed={observableA} />);
336333

337334
// Flush everything and ensure that the correct subscribable is used
338335
// We expect the last emitted update to be rendered (because of the commit phase value check)
@@ -354,7 +351,6 @@ describe('createSubscription', () => {
354351

355352
it('should not drop values emitted between updates', () => {
356353
const log = [];
357-
let parentInstance;
358354

359355
function Child({value}) {
360356
ReactNoop.yield('Child: ' + value);
@@ -391,8 +387,6 @@ describe('createSubscription', () => {
391387
}
392388

393389
render() {
394-
parentInstance = this;
395-
396390
return (
397391
<Subscription source={this.state.observed}>
398392
{(value = 'default') => {
@@ -420,8 +414,8 @@ describe('createSubscription', () => {
420414
observableA.next('a-1');
421415
observableA.next('a-2');
422416

423-
// Mimic a higher-priority interruption
424-
parentInstance.setState({observed: observableA});
417+
// Update again
418+
ReactNoop.render(<Parent observed={observableA} />);
425419

426420
// Flush everything and ensure that the correct subscribable is used
427421
// We expect the new subscribable to finish rendering,

‎packages/react-reconciler/src/ReactFiberBeginWork.js‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@ import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';
5555
import {cancelWorkTimer} from './ReactDebugFiberPerf';
5656

5757
import ReactFiberClassComponent, {
58-
createGetDerivedStateFromPropsUpdate,
58+
applyDerivedStateFromProps,
5959
} from './ReactFiberClassComponent';
6060
import {
6161
mountChildFibers,
6262
reconcileChildFibers,
6363
cloneChildFibers,
6464
} from './ReactChildFiber';
65-
import {enqueueRenderPhaseUpdate, processUpdateQueue} from './ReactUpdateQueue';
65+
import {processUpdateQueue} from './ReactUpdateQueue';
6666
import {NoWork, Never} from './ReactFiberExpirationTime';
6767
import {AsyncMode, StrictMode} from './ReactTypeOfMode';
6868
import MAX_SIGNED_31_BIT_INT from './maxSigned31BitInt';
@@ -592,15 +592,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
592592

593593
const getDerivedStateFromProps = Component.getDerivedStateFromProps;
594594
if (typeof getDerivedStateFromProps === 'function') {
595-
const update = createGetDerivedStateFromPropsUpdate(
595+
applyDerivedStateFromProps(
596+
workInProgress,
596597
getDerivedStateFromProps,
597-
renderExpirationTime,
598+
props,
598599
);
599-
enqueueRenderPhaseUpdate(workInProgress, update, renderExpirationTime);
600-
const updateQueue = workInProgress.updateQueue;
601-
if (updateQueue !== null) {
602-
processUpdateQueue(workInProgress, updateQueue, renderExpirationTime);
603-
}
604600
}
605601

606602
// Push context providers early to prevent context stack mismatches.

‎packages/react-reconciler/src/ReactFiberClassComponent.js‎

Lines changed: 61 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
3030
import {StrictMode} from './ReactTypeOfMode';
3131
import {
3232
enqueueUpdate,
33-
enqueueRenderPhaseUpdate,
3433
processUpdateQueue,
3534
createUpdate,
3635
} from './ReactUpdateQueue';
36+
import {NoWork} from './ReactFiberExpirationTime';
3737

3838
const fakeInternalInstance = {};
3939
const isArray = Array.isArray;
@@ -109,32 +109,40 @@ if (__DEV__) {
109109
Object.freeze(fakeInternalInstance);
110110
}
111111

112-
export function createGetDerivedStateFromPropsUpdate(
112+
export function applyDerivedStateFromProps(
113+
workInProgress: Fiber,
113114
getDerivedStateFromProps: (props: any, state: any) => any,
114-
renderExpirationTime: ExpirationTime,
115+
nextProps: any,
115116
) {
116-
const update = createUpdate(renderExpirationTime);
117-
update.process = (nextWorkInProgress, prevState) => {
118-
const nextProps = nextWorkInProgress.pendingProps;
117+
const prevState = workInProgress.memoizedState;
119118

120-
if (
121-
debugRenderPhaseSideEffects ||
122-
(debugRenderPhaseSideEffectsForStrictMode &&
123-
nextWorkInProgress.mode & StrictMode)
124-
) {
125-
// Invoke the function an extra time to help detect side-effects.
126-
getDerivedStateFromProps(nextProps, prevState);
127-
}
119+
if (
120+
debugRenderPhaseSideEffects ||
121+
(debugRenderPhaseSideEffectsForStrictMode &&
122+
workInProgress.mode & StrictMode)
123+
) {
124+
// Invoke the function an extra time to help detect side-effects.
125+
getDerivedStateFromProps(nextProps, prevState);
126+
}
128127

129-
const partialState = getDerivedStateFromProps(nextProps, prevState);
128+
const partialState = getDerivedStateFromProps(nextProps, prevState);
130129

131-
if (__DEV__) {
132-
warnOnUndefinedDerivedState(nextWorkInProgress, partialState);
133-
}
134-
// Merge the partial state and the previous state.
135-
return Object.assign({}, prevState, partialState);
136-
};
137-
return update;
130+
if (__DEV__) {
131+
warnOnUndefinedDerivedState(workInProgress, partialState);
132+
}
133+
// Merge the partial state and the previous state.
134+
const memoizedState =
135+
partialState === null || partialState === undefined
136+
? prevState
137+
: Object.assign({}, prevState, partialState);
138+
workInProgress.memoizedState = memoizedState;
139+
140+
// Once the update queue is empty, persist the derived state onto the
141+
// base state.
142+
const updateQueue = workInProgress.updateQueue;
143+
if (updateQueue !== null && updateQueue.expirationTime === NoWork) {
144+
updateQueue.baseState = memoizedState;
145+
}
138146
}
139147

140148
export default function(
@@ -742,19 +750,20 @@ export default function(
742750
}
743751
}
744752

753+
let updateQueue = workInProgress.updateQueue;
754+
if (updateQueue !== null) {
755+
processUpdateQueue(workInProgress, updateQueue, renderExpirationTime);
756+
instance.state = workInProgress.memoizedState;
757+
}
758+
745759
const getDerivedStateFromProps =
746760
workInProgress.type.getDerivedStateFromProps;
747761
if (typeof getDerivedStateFromProps === 'function') {
748-
const update = createGetDerivedStateFromPropsUpdate(
762+
applyDerivedStateFromProps(
763+
workInProgress,
749764
getDerivedStateFromProps,
750-
renderExpirationTime,
765+
props,
751766
);
752-
enqueueRenderPhaseUpdate(workInProgress, update, renderExpirationTime);
753-
}
754-
755-
let updateQueue = workInProgress.updateQueue;
756-
if (updateQueue !== null) {
757-
processUpdateQueue(workInProgress, updateQueue, renderExpirationTime);
758767
instance.state = workInProgress.memoizedState;
759768
}
760769

@@ -796,8 +805,9 @@ export default function(
796805
const newUnmaskedContext = getUnmaskedContext(workInProgress);
797806
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
798807

808+
const getDerivedStateFromProps = ctor.getDerivedStateFromProps;
799809
const hasNewLifecycles =
800-
typeof ctor.getDerivedStateFromProps === 'function' ||
810+
typeof getDerivedStateFromProps === 'function' ||
801811
typeof instance.getSnapshotBeforeUpdate === 'function';
802812

803813
// Note: During these life-cycles, instance.props/instance.state are what
@@ -821,19 +831,6 @@ export default function(
821831
}
822832
}
823833

824-
// Only call getDerivedStateFromProps if the props have changed
825-
if (oldProps !== newProps) {
826-
const getDerivedStateFromProps =
827-
workInProgress.type.getDerivedStateFromProps;
828-
if (typeof getDerivedStateFromProps === 'function') {
829-
const update = createGetDerivedStateFromPropsUpdate(
830-
getDerivedStateFromProps,
831-
renderExpirationTime,
832-
);
833-
enqueueRenderPhaseUpdate(workInProgress, update, renderExpirationTime);
834-
}
835-
}
836-
837834
const oldState = workInProgress.memoizedState;
838835
let newState = (instance.state = oldState);
839836
let updateQueue = workInProgress.updateQueue;
@@ -842,6 +839,15 @@ export default function(
842839
newState = workInProgress.memoizedState;
843840
}
844841

842+
if (typeof getDerivedStateFromProps === 'function') {
843+
applyDerivedStateFromProps(
844+
workInProgress,
845+
getDerivedStateFromProps,
846+
newProps,
847+
);
848+
newState = workInProgress.memoizedState;
849+
}
850+
845851
if (
846852
oldProps === newProps &&
847853
oldState === newState &&
@@ -927,8 +933,9 @@ export default function(
927933
const newUnmaskedContext = getUnmaskedContext(workInProgress);
928934
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
929935

936+
const getDerivedStateFromProps = ctor.getDerivedStateFromProps;
930937
const hasNewLifecycles =
931-
typeof ctor.getDerivedStateFromProps === 'function' ||
938+
typeof getDerivedStateFromProps === 'function' ||
932939
typeof instance.getSnapshotBeforeUpdate === 'function';
933940

934941
// Note: During these life-cycles, instance.props/instance.state are what
@@ -952,19 +959,6 @@ export default function(
952959
}
953960
}
954961

955-
// Only call getDerivedStateFromProps if the props have changed
956-
if (oldProps !== newProps) {
957-
const getDerivedStateFromProps =
958-
workInProgress.type.getDerivedStateFromProps;
959-
if (typeof getDerivedStateFromProps === 'function') {
960-
const update = createGetDerivedStateFromPropsUpdate(
961-
getDerivedStateFromProps,
962-
renderExpirationTime,
963-
);
964-
enqueueRenderPhaseUpdate(workInProgress, update, renderExpirationTime);
965-
}
966-
}
967-
968962
const oldState = workInProgress.memoizedState;
969963
let newState = (instance.state = oldState);
970964
let updateQueue = workInProgress.updateQueue;
@@ -973,6 +967,15 @@ export default function(
973967
newState = workInProgress.memoizedState;
974968
}
975969

970+
if (typeof getDerivedStateFromProps === 'function') {
971+
applyDerivedStateFromProps(
972+
workInProgress,
973+
getDerivedStateFromProps,
974+
newProps,
975+
);
976+
newState = workInProgress.memoizedState;
977+
}
978+
976979
if (
977980
oldProps === newProps &&
978981
oldState === newState &&

0 commit comments

Comments
 (0)