Skip to content

Commit 72e8cf4

Browse files
committed
Fix places where class props aren't resolved
Noticed these once I enabled class prop resolution in more places. Technically this was already observable if you wrapped a class with `React.lazy` and gave that wrapper default props, but since that was so rare it was never reported.
1 parent 3c21710 commit 72e8cf4

File tree

3 files changed

+62
-5
lines changed

3 files changed

+62
-5
lines changed

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,12 @@ function resumeMountClassInstance(
908908
): boolean {
909909
const instance = workInProgress.stateNode;
910910

911-
const oldProps = workInProgress.memoizedProps;
911+
const unresolvedOldProps = workInProgress.memoizedProps;
912+
const oldProps = resolveClassComponentProps(
913+
ctor,
914+
unresolvedOldProps,
915+
workInProgress.type === workInProgress.elementType,
916+
);
912917
instance.props = oldProps;
913918

914919
const oldContext = instance.context;
@@ -930,6 +935,13 @@ function resumeMountClassInstance(
930935
typeof getDerivedStateFromProps === 'function' ||
931936
typeof instance.getSnapshotBeforeUpdate === 'function';
932937

938+
// When comparing whether props changed, we should compare using the
939+
// unresolved props object that is stored on the fiber, rather than the
940+
// one that gets assigned to the instance, because that object may have been
941+
// cloned to resolve default props and/or remove `ref`.
942+
const unresolvedNewProps = workInProgress.pendingProps;
943+
const didReceiveNewProps = unresolvedNewProps !== unresolvedOldProps;
944+
933945
// Note: During these life-cycles, instance.props/instance.state are what
934946
// ever the previously attempted to render - not the "current". However,
935947
// during componentDidUpdate we pass the "current" props.
@@ -941,7 +953,7 @@ function resumeMountClassInstance(
941953
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
942954
typeof instance.componentWillReceiveProps === 'function')
943955
) {
944-
if (oldProps !== newProps || oldContext !== nextContext) {
956+
if (didReceiveNewProps || oldContext !== nextContext) {
945957
callComponentWillReceiveProps(
946958
workInProgress,
947959
instance,
@@ -959,7 +971,7 @@ function resumeMountClassInstance(
959971
suspendIfUpdateReadFromEntangledAsyncAction();
960972
newState = workInProgress.memoizedState;
961973
if (
962-
oldProps === newProps &&
974+
didReceiveNewProps &&
963975
oldState === newState &&
964976
!hasContextChanged() &&
965977
!checkHasForceUpdateAfterProcessing()

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,11 @@ function shouldProfile(current: Fiber): boolean {
243243
}
244244

245245
function callComponentWillUnmountWithTimer(current: Fiber, instance: any) {
246-
instance.props = current.memoizedProps;
246+
instance.props = resolveClassComponentProps(
247+
current.type,
248+
current.memoizedProps,
249+
current.elementType === current.type,
250+
);
247251
instance.state = current.memoizedState;
248252
if (shouldProfile(current)) {
249253
try {

packages/react-reconciler/src/__tests__/ReactClassComponentPropResolution-test.js

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ describe('ReactClassComponentPropResolution', () => {
3939
}
4040

4141
class Component extends React.Component {
42+
constructor(props) {
43+
super(props);
44+
Scheduler.log('constructor: ' + getPropKeys(props));
45+
}
4246
shouldComponentUpdate(props) {
4347
Scheduler.log(
4448
'shouldComponentUpdate (prev props): ' + getPropKeys(this.props),
@@ -59,6 +63,28 @@ describe('ReactClassComponentPropResolution', () => {
5963
Scheduler.log('componentDidMount: ' + getPropKeys(this.props));
6064
return true;
6165
}
66+
UNSAFE_componentWillMount() {
67+
Scheduler.log('componentWillMount: ' + getPropKeys(this.props));
68+
}
69+
UNSAFE_componentWillReceiveProps(nextProps) {
70+
Scheduler.log(
71+
'componentWillReceiveProps (prev props): ' + getPropKeys(this.props),
72+
);
73+
Scheduler.log(
74+
'componentWillReceiveProps (next props): ' + getPropKeys(nextProps),
75+
);
76+
}
77+
UNSAFE_componentWillUpdate(nextProps) {
78+
Scheduler.log(
79+
'componentWillUpdate (prev props): ' + getPropKeys(this.props),
80+
);
81+
Scheduler.log(
82+
'componentWillUpdate (next props): ' + getPropKeys(nextProps),
83+
);
84+
}
85+
componentWillUnmount() {
86+
Scheduler.log('componentWillUnmount: ' + getPropKeys(this.props));
87+
}
6288
render() {
6389
return <Text text={'render: ' + getPropKeys(this.props)} />;
6490
}
@@ -75,18 +101,33 @@ describe('ReactClassComponentPropResolution', () => {
75101
await act(async () => {
76102
root.render(<Component text="Yay" ref={ref} />);
77103
});
78-
assertLog(['render: text, default', 'componentDidMount: text, default']);
104+
assertLog([
105+
'constructor: text, default',
106+
'componentWillMount: text, default',
107+
'render: text, default',
108+
'componentDidMount: text, default',
109+
]);
79110

80111
// Update
81112
await act(async () => {
82113
root.render(<Component text="Yay (again)" ref={ref} />);
83114
});
84115
assertLog([
116+
'componentWillReceiveProps (prev props): text, default',
117+
'componentWillReceiveProps (next props): text, default',
85118
'shouldComponentUpdate (prev props): text, default',
86119
'shouldComponentUpdate (next props): text, default',
120+
'componentWillUpdate (prev props): text, default',
121+
'componentWillUpdate (next props): text, default',
87122
'render: text, default',
88123
'componentDidUpdate (prev props): text, default',
89124
'componentDidUpdate (next props): text, default',
90125
]);
126+
127+
// Unmount
128+
await act(async () => {
129+
root.render(null);
130+
});
131+
assertLog(['componentWillUnmount: text, default']);
91132
});
92133
});

0 commit comments

Comments
 (0)