Skip to content

Commit 4f33288

Browse files
chenesangaearon
authored andcommitted
Fix shallow renderer set instance state after gDSFP before calling sCU (#14613)
* Fix shallow renderer set instance state after gDSFP before calling sCU * Update ReactShallowRenderer.js * Unwind abstraction * Fewer names
1 parent e1cd83e commit 4f33288

File tree

3 files changed

+96
-24
lines changed

3 files changed

+96
-24
lines changed

Diff for: packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js

+34
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,40 @@ describe('ReactComponentLifeCycle', () => {
12011201
expect(log).toEqual([]);
12021202
});
12031203

1204+
it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => {
1205+
const divRef = React.createRef();
1206+
class SimpleComponent extends React.Component {
1207+
constructor(props) {
1208+
super(props);
1209+
this.state = {
1210+
value: props.value,
1211+
};
1212+
}
1213+
1214+
static getDerivedStateFromProps(nextProps, prevState) {
1215+
if (nextProps.value === prevState.value) {
1216+
return null;
1217+
}
1218+
return {value: nextProps.value};
1219+
}
1220+
1221+
shouldComponentUpdate(nextProps, nextState) {
1222+
return nextState.value !== this.state.value;
1223+
}
1224+
1225+
render() {
1226+
return <div ref={divRef}>value: {this.state.value}</div>;
1227+
}
1228+
}
1229+
1230+
const div = document.createElement('div');
1231+
1232+
ReactDOM.render(<SimpleComponent value="initial" />, div);
1233+
expect(divRef.current.textContent).toBe('value: initial');
1234+
ReactDOM.render(<SimpleComponent value="updated" />, div);
1235+
expect(divRef.current.textContent).toBe('value: updated');
1236+
});
1237+
12041238
it('should call getSnapshotBeforeUpdate before mutations are committed', () => {
12051239
const log = [];
12061240

Diff for: packages/react-test-renderer/src/ReactShallowRenderer.js

+26-24
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,20 @@ class ReactShallowRenderer {
529529
this._updater,
530530
);
531531

532-
this._updateStateFromStaticLifecycle(element.props);
532+
if (typeof element.type.getDerivedStateFromProps === 'function') {
533+
const partialState = element.type.getDerivedStateFromProps.call(
534+
null,
535+
element.props,
536+
this._instance.state,
537+
);
538+
if (partialState != null) {
539+
this._instance.state = Object.assign(
540+
{},
541+
this._instance.state,
542+
partialState,
543+
);
544+
}
545+
}
533546

534547
if (element.type.hasOwnProperty('contextTypes')) {
535548
currentlyValidatingElement = element;
@@ -653,10 +666,19 @@ class ReactShallowRenderer {
653666
}
654667
}
655668
}
656-
this._updateStateFromStaticLifecycle(props);
657669

658670
// Read state after cWRP in case it calls setState
659-
const state = this._newState || oldState;
671+
let state = this._newState || oldState;
672+
if (typeof type.getDerivedStateFromProps === 'function') {
673+
const partialState = type.getDerivedStateFromProps.call(
674+
null,
675+
props,
676+
state,
677+
);
678+
if (partialState != null) {
679+
state = Object.assign({}, state, partialState);
680+
}
681+
}
660682

661683
let shouldUpdate = true;
662684
if (this._forcedUpdate) {
@@ -692,34 +714,14 @@ class ReactShallowRenderer {
692714
this._instance.context = context;
693715
this._instance.props = props;
694716
this._instance.state = state;
717+
this._newState = null;
695718

696719
if (shouldUpdate) {
697720
this._rendered = this._instance.render();
698721
}
699722
// Intentionally do not call componentDidUpdate()
700723
// because DOM refs are not available.
701724
}
702-
703-
_updateStateFromStaticLifecycle(props: Object) {
704-
if (this._element === null) {
705-
return;
706-
}
707-
const {type} = this._element;
708-
709-
if (typeof type.getDerivedStateFromProps === 'function') {
710-
const oldState = this._newState || this._instance.state;
711-
const partialState = type.getDerivedStateFromProps.call(
712-
null,
713-
props,
714-
oldState,
715-
);
716-
717-
if (partialState != null) {
718-
const newState = Object.assign({}, oldState, partialState);
719-
this._instance.state = this._newState = newState;
720-
}
721-
}
722-
}
723725
}
724726

725727
let currentlyValidatingElement = null;

Diff for: packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js

+36
Original file line numberDiff line numberDiff line change
@@ -942,6 +942,42 @@ describe('ReactShallowRenderer', () => {
942942
expect(result).toEqual(<div>value:1</div>);
943943
});
944944

945+
it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => {
946+
class SimpleComponent extends React.Component {
947+
constructor(props) {
948+
super(props);
949+
this.state = {
950+
value: props.value,
951+
};
952+
}
953+
954+
static getDerivedStateFromProps(nextProps, prevState) {
955+
if (nextProps.value === prevState.value) {
956+
return null;
957+
}
958+
return {value: nextProps.value};
959+
}
960+
961+
shouldComponentUpdate(nextProps, nextState) {
962+
return nextState.value !== this.state.value;
963+
}
964+
965+
render() {
966+
return <div>{`value:${this.state.value}`}</div>;
967+
}
968+
}
969+
970+
const shallowRenderer = createRenderer();
971+
const initialResult = shallowRenderer.render(
972+
<SimpleComponent value="initial" />,
973+
);
974+
expect(initialResult).toEqual(<div>value:initial</div>);
975+
const updatedResult = shallowRenderer.render(
976+
<SimpleComponent value="updated" />,
977+
);
978+
expect(updatedResult).toEqual(<div>value:updated</div>);
979+
});
980+
945981
it('can setState with an updater function', () => {
946982
let instance;
947983

0 commit comments

Comments
 (0)