Skip to content

Commit 6d8cd15

Browse files
committed
Use the memoized props/state from the workInProgress
We don't want to use the "current" props/state because if we have started work, then pause and continue then we'll have the newer props/state on it already. If it is not a resume, it should be the same as current.
1 parent 3d7869a commit 6d8cd15

File tree

2 files changed

+107
-6
lines changed

2 files changed

+107
-6
lines changed

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,24 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>, s
160160
// Account for the possibly of missing pending props by falling back to the
161161
// memoized props.
162162
var props = workInProgress.pendingProps;
163-
if (!props && current) {
164-
props = current.memoizedProps;
163+
if (!props) {
164+
// If there isn't any new props, then we'll reuse the memoized props.
165+
// This could be from already completed work.
166+
props = workInProgress.memoizedProps;
167+
if (!props) {
168+
throw new Error('There should always be pending or memoized props.');
169+
}
165170
}
171+
166172
// Compute the state using the memoized state and the update queue.
167173
var updateQueue = workInProgress.updateQueue;
168-
var previousState = current ? current.memoizedState : null;
169-
var state = updateQueue ?
170-
mergeUpdateQueue(updateQueue, previousState, props) :
171-
previousState;
174+
var previousState = workInProgress.memoizedState;
175+
var state;
176+
if (updateQueue) {
177+
state = mergeUpdateQueue(updateQueue, previousState, props);
178+
} else {
179+
state = previousState;
180+
}
172181

173182
var instance = workInProgress.stateNode;
174183
if (!instance) {

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,4 +835,96 @@ describe('ReactIncremental', () => {
835835
ReactNoop.flush();
836836
expect(ops).toEqual(['Foo', 'Bar', 'Baz', 'Bar', 'Baz']);
837837
});
838+
839+
it('can call sCU while resuming a partly mounted component', () => {
840+
var ops = [];
841+
842+
class Bar extends React.Component {
843+
state = { y: 'A' };
844+
shouldComponentUpdate(newProps, newState) {
845+
return this.props.x !== newProps.x ||
846+
this.state.y !== newState.y;
847+
}
848+
render() {
849+
ops.push('Bar:' + this.props.x);
850+
return <span prop={'' + (this.props.x === this.state.y)} />;
851+
}
852+
}
853+
854+
function Foo(props) {
855+
ops.push('Foo');
856+
return [
857+
<Bar key="a" x="A" />,
858+
<Bar key="b" x="B" />,
859+
<Bar key="c" x="C" />,
860+
];
861+
}
862+
863+
ReactNoop.render(<Foo />);
864+
ReactNoop.flushDeferredPri(30);
865+
expect(ops).toEqual(['Foo', 'Bar:A', 'Bar:B']);
866+
867+
ops = [];
868+
869+
ReactNoop.render(<Foo />);
870+
ReactNoop.flushDeferredPri(40);
871+
expect(ops).toEqual(['Foo', 'Bar:B', 'Bar:C']);
872+
});
873+
874+
it('gets new props when setting state on a partly updated component', () => {
875+
var ops = [];
876+
var instances = [];
877+
878+
class Bar extends React.Component {
879+
state = { y: 'A' };
880+
constructor() {
881+
super();
882+
instances.push(this);
883+
}
884+
performAction() {
885+
this.setState({
886+
y: 'B',
887+
});
888+
}
889+
render() {
890+
ops.push('Bar:' + this.props.x + '-' + this.props.step);
891+
return <span prop={'' + (this.props.x === this.state.y)} />;
892+
}
893+
}
894+
895+
function Baz() {
896+
// This component is used as a sibling to Foo so that we can fully
897+
// complete Foo, without committing.
898+
ops.push('Baz');
899+
return <div />;
900+
}
901+
902+
function Foo(props) {
903+
ops.push('Foo');
904+
return [
905+
<Bar key="a" x="A" step={props.step} />,
906+
<Bar key="b" x="B" step={props.step} />,
907+
];
908+
}
909+
910+
ReactNoop.render(<div><Foo step={0} /><Baz /><Baz /></div>);
911+
ReactNoop.flush();
912+
913+
ops = [];
914+
915+
// Flush part way through with new props, fully completing the first Bar.
916+
// However, it doesn't commit yet.
917+
ReactNoop.render(<div><Foo step={1} /><Baz /><Baz /></div>);
918+
ReactNoop.flushDeferredPri(45);
919+
expect(ops).toEqual(['Foo', 'Bar:A-1', 'Bar:B-1', 'Baz']);
920+
921+
// Make an update to the same Bar.
922+
instances[0].performAction();
923+
924+
ops = [];
925+
926+
ReactNoop.flush();
927+
expect(ops).toEqual(['Bar:A-1', 'Baz', 'Baz']);
928+
});
929+
838930
});

0 commit comments

Comments
 (0)