diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 2e56a911a0c38..ba3118bc651b0 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -261,29 +261,17 @@ describe('ReactCompositeComponent', () => { await act(() => { root.render(); }); - if (gate(flags => flags.enableRefAsProp)) { - expect(instance1.props).toEqual({prop: 'testKey', ref: refFn1}); - } else { - expect(instance1.props).toEqual({prop: 'testKey'}); - } + expect(instance1.props).toEqual({prop: 'testKey'}); await act(() => { root.render(); }); - if (gate(flags => flags.enableRefAsProp)) { - expect(instance2.props).toEqual({prop: 'testKey', ref: refFn2}); - } else { - expect(instance2.props).toEqual({prop: 'testKey'}); - } + expect(instance2.props).toEqual({prop: 'testKey'}); await act(() => { root.render(); }); - if (gate(flags => flags.enableRefAsProp)) { - expect(instance3.props).toEqual({prop: null, ref: refFn3}); - } else { - expect(instance3.props).toEqual({prop: null}); - } + expect(instance3.props).toEqual({prop: null}); }); it('should not mutate passed-in props object', async () => { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index bd745e02d12a2..8c7ba485856f2 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -245,6 +245,7 @@ import { mountClassInstance, resumeMountClassInstance, updateClassInstance, + resolveClassComponentProps, } from './ReactFiberClassComponent'; import {resolveDefaultProps} from './ReactFiberLazyComponent'; import { @@ -1762,9 +1763,9 @@ function mountLazyComponent( // Store the unwrapped component in the type. workInProgress.type = Component; - const resolvedProps = resolveDefaultProps(Component, props); if (typeof Component === 'function') { if (isFunctionClassComponent(Component)) { + const resolvedProps = resolveClassComponentProps(Component, props, false); workInProgress.tag = ClassComponent; if (__DEV__) { workInProgress.type = Component = @@ -1778,6 +1779,7 @@ function mountLazyComponent( renderLanes, ); } else { + const resolvedProps = resolveDefaultProps(Component, props); workInProgress.tag = FunctionComponent; if (__DEV__) { validateFunctionComponentInDev(workInProgress, Component); @@ -1795,6 +1797,7 @@ function mountLazyComponent( } else if (Component !== undefined && Component !== null) { const $$typeof = Component.$$typeof; if ($$typeof === REACT_FORWARD_REF_TYPE) { + const resolvedProps = resolveDefaultProps(Component, props); workInProgress.tag = ForwardRef; if (__DEV__) { workInProgress.type = Component = @@ -1808,6 +1811,7 @@ function mountLazyComponent( renderLanes, ); } else if ($$typeof === REACT_MEMO_TYPE) { + const resolvedProps = resolveDefaultProps(Component, props); workInProgress.tag = MemoComponent; return updateMemoComponent( null, @@ -3938,10 +3942,11 @@ function beginWork( case ClassComponent: { const Component = workInProgress.type; const unresolvedProps = workInProgress.pendingProps; - const resolvedProps = - workInProgress.elementType === Component - ? unresolvedProps - : resolveDefaultProps(Component, unresolvedProps); + const resolvedProps = resolveClassComponentProps( + Component, + unresolvedProps, + workInProgress.elementType === Component, + ); return updateClassComponent( current, workInProgress, @@ -4024,10 +4029,11 @@ function beginWork( } const Component = workInProgress.type; const unresolvedProps = workInProgress.pendingProps; - const resolvedProps = - workInProgress.elementType === Component - ? unresolvedProps - : resolveDefaultProps(Component, unresolvedProps); + const resolvedProps = resolveClassComponentProps( + Component, + unresolvedProps, + workInProgress.elementType === Component, + ); return mountIncompleteClassComponent( current, workInProgress, @@ -4042,10 +4048,11 @@ function beginWork( } const Component = workInProgress.type; const unresolvedProps = workInProgress.pendingProps; - const resolvedProps = - workInProgress.elementType === Component - ? unresolvedProps - : resolveDefaultProps(Component, unresolvedProps); + const resolvedProps = resolveClassComponentProps( + Component, + unresolvedProps, + workInProgress.elementType === Component, + ); return mountIncompleteFunctionComponent( current, workInProgress, diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 47d1c3cfee476..697be25eee678 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -23,6 +23,7 @@ import { enableDebugTracing, enableSchedulingProfiler, enableLazyContextPropagation, + enableRefAsProp, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings'; import {isMounted} from './ReactFiberTreeReflection'; @@ -34,7 +35,6 @@ import assign from 'shared/assign'; import isArray from 'shared/isArray'; import {REACT_CONTEXT_TYPE, REACT_CONSUMER_TYPE} from 'shared/ReactSymbols'; -import {resolveDefaultProps} from './ReactFiberLazyComponent'; import { DebugTracingMode, NoMode, @@ -904,7 +904,12 @@ function resumeMountClassInstance( ): boolean { const instance = workInProgress.stateNode; - const oldProps = workInProgress.memoizedProps; + const unresolvedOldProps = workInProgress.memoizedProps; + const oldProps = resolveClassComponentProps( + ctor, + unresolvedOldProps, + workInProgress.type === workInProgress.elementType, + ); instance.props = oldProps; const oldContext = instance.context; @@ -926,6 +931,13 @@ function resumeMountClassInstance( typeof getDerivedStateFromProps === 'function' || typeof instance.getSnapshotBeforeUpdate === 'function'; + // When comparing whether props changed, we should compare using the + // unresolved props object that is stored on the fiber, rather than the + // one that gets assigned to the instance, because that object may have been + // cloned to resolve default props and/or remove `ref`. + const unresolvedNewProps = workInProgress.pendingProps; + const didReceiveNewProps = unresolvedNewProps !== unresolvedOldProps; + // Note: During these life-cycles, instance.props/instance.state are what // ever the previously attempted to render - not the "current". However, // during componentDidUpdate we pass the "current" props. @@ -937,7 +949,7 @@ function resumeMountClassInstance( (typeof instance.UNSAFE_componentWillReceiveProps === 'function' || typeof instance.componentWillReceiveProps === 'function') ) { - if (oldProps !== newProps || oldContext !== nextContext) { + if (didReceiveNewProps || oldContext !== nextContext) { callComponentWillReceiveProps( workInProgress, instance, @@ -955,7 +967,7 @@ function resumeMountClassInstance( suspendIfUpdateReadFromEntangledAsyncAction(); newState = workInProgress.memoizedState; if ( - oldProps === newProps && + !didReceiveNewProps && oldState === newState && !hasContextChanged() && !checkHasForceUpdateAfterProcessing() @@ -1052,10 +1064,11 @@ function updateClassInstance( cloneUpdateQueue(current, workInProgress); const unresolvedOldProps = workInProgress.memoizedProps; - const oldProps = - workInProgress.type === workInProgress.elementType - ? unresolvedOldProps - : resolveDefaultProps(workInProgress.type, unresolvedOldProps); + const oldProps = resolveClassComponentProps( + ctor, + unresolvedOldProps, + workInProgress.type === workInProgress.elementType, + ); instance.props = oldProps; const unresolvedNewProps = workInProgress.pendingProps; @@ -1225,6 +1238,42 @@ function updateClassInstance( return shouldUpdate; } +export function resolveClassComponentProps( + Component: any, + baseProps: Object, + // Only resolve default props if this is a lazy component. Otherwise, they + // would have already been resolved by the JSX runtime. + // TODO: We're going to remove default prop resolution from the JSX runtime + // and keep it only for class components. As part of that change, we should + // remove this extra check. + alreadyResolvedDefaultProps: boolean, +): Object { + let newProps = baseProps; + + // Resolve default props. Taken from old JSX runtime, where this used to live. + const defaultProps = Component.defaultProps; + if (defaultProps && !alreadyResolvedDefaultProps) { + newProps = assign({}, newProps, baseProps); + for (const propName in defaultProps) { + if (newProps[propName] === undefined) { + newProps[propName] = defaultProps[propName]; + } + } + } + + if (enableRefAsProp) { + // Remove ref from the props object, if it exists. + if ('ref' in newProps) { + if (newProps === baseProps) { + newProps = assign({}, newProps); + } + delete newProps.ref; + } + } + + return newProps; +} + export { constructClassInstance, mountClassInstance, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index ba11d5b6149b2..213ff26d82e2c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -104,7 +104,7 @@ import { setCurrentFiber as setCurrentDebugFiberInDEV, getCurrentFiber as getCurrentDebugFiberInDEV, } from './ReactCurrentFiber'; -import {resolveDefaultProps} from './ReactFiberLazyComponent'; +import {resolveClassComponentProps} from './ReactFiberClassComponent'; import { isCurrentUpdateNested, getCommitTime, @@ -244,7 +244,11 @@ function shouldProfile(current: Fiber): boolean { } function callComponentWillUnmountWithTimer(current: Fiber, instance: any) { - instance.props = current.memoizedProps; + instance.props = resolveClassComponentProps( + current.type, + current.memoizedProps, + current.elementType === current.type, + ); instance.state = current.memoizedState; if (shouldProfile(current)) { try { @@ -471,7 +475,8 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) { // TODO: revisit this when we implement resuming. if (__DEV__) { if ( - finishedWork.type === finishedWork.elementType && + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && !didWarnAboutReassigningProps ) { if (instance.props !== finishedWork.memoizedProps) { @@ -497,9 +502,11 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) { } } const snapshot = instance.getSnapshotBeforeUpdate( - finishedWork.elementType === finishedWork.type - ? prevProps - : resolveDefaultProps(finishedWork.type, prevProps), + resolveClassComponentProps( + finishedWork.type, + prevProps, + finishedWork.elementType === finishedWork.type, + ), prevState, ); if (__DEV__) { @@ -807,7 +814,8 @@ function commitClassLayoutLifecycles( // TODO: revisit this when we implement resuming. if (__DEV__) { if ( - finishedWork.type === finishedWork.elementType && + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && !didWarnAboutReassigningProps ) { if (instance.props !== finishedWork.memoizedProps) { @@ -848,17 +856,19 @@ function commitClassLayoutLifecycles( } } } else { - const prevProps = - finishedWork.elementType === finishedWork.type - ? current.memoizedProps - : resolveDefaultProps(finishedWork.type, current.memoizedProps); + const prevProps = resolveClassComponentProps( + finishedWork.type, + current.memoizedProps, + finishedWork.elementType === finishedWork.type, + ); const prevState = current.memoizedState; // We could update instance props and state here, // but instead we rely on them being set during last render. // TODO: revisit this when we implement resuming. if (__DEV__) { if ( - finishedWork.type === finishedWork.elementType && + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && !didWarnAboutReassigningProps ) { if (instance.props !== finishedWork.memoizedProps) { @@ -918,7 +928,8 @@ function commitClassCallbacks(finishedWork: Fiber) { const instance = finishedWork.stateNode; if (__DEV__) { if ( - finishedWork.type === finishedWork.elementType && + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && !didWarnAboutReassigningProps ) { if (instance.props !== finishedWork.memoizedProps) { diff --git a/packages/react-reconciler/src/ReactFiberLazyComponent.js b/packages/react-reconciler/src/ReactFiberLazyComponent.js index b66f8efe4ed1b..b6c28f0ed5cce 100644 --- a/packages/react-reconciler/src/ReactFiberLazyComponent.js +++ b/packages/react-reconciler/src/ReactFiberLazyComponent.js @@ -10,6 +10,10 @@ import assign from 'shared/assign'; export function resolveDefaultProps(Component: any, baseProps: Object): Object { + // TODO: Remove support for default props for everything except class + // components, including setting default props on a lazy wrapper around a + // class type. + if (Component && Component.defaultProps) { // Resolve default props. Taken from ReactElement const props = assign({}, baseProps); diff --git a/packages/react-reconciler/src/__tests__/ReactClassComponentPropResolution-test.js b/packages/react-reconciler/src/__tests__/ReactClassComponentPropResolution-test.js new file mode 100644 index 0000000000000..53f3a3d04b564 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactClassComponentPropResolution-test.js @@ -0,0 +1,133 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let act; +let assertLog; + +describe('ReactClassComponentPropResolution', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + act = require('internal-test-utils').act; + assertLog = require('internal-test-utils').assertLog; + }); + + function Text({text}) { + Scheduler.log(text); + return text; + } + + test('resolves ref and default props before calling lifecycle methods', async () => { + const root = ReactNoop.createRoot(); + + function getPropKeys(props) { + return Object.keys(props).join(', '); + } + + class Component extends React.Component { + constructor(props) { + super(props); + Scheduler.log('constructor: ' + getPropKeys(props)); + } + shouldComponentUpdate(props) { + Scheduler.log( + 'shouldComponentUpdate (prev props): ' + getPropKeys(this.props), + ); + Scheduler.log( + 'shouldComponentUpdate (next props): ' + getPropKeys(props), + ); + return true; + } + componentDidUpdate(props) { + Scheduler.log('componentDidUpdate (prev props): ' + getPropKeys(props)); + Scheduler.log( + 'componentDidUpdate (next props): ' + getPropKeys(this.props), + ); + return true; + } + componentDidMount() { + Scheduler.log('componentDidMount: ' + getPropKeys(this.props)); + return true; + } + UNSAFE_componentWillMount() { + Scheduler.log('componentWillMount: ' + getPropKeys(this.props)); + } + UNSAFE_componentWillReceiveProps(nextProps) { + Scheduler.log( + 'componentWillReceiveProps (prev props): ' + getPropKeys(this.props), + ); + Scheduler.log( + 'componentWillReceiveProps (next props): ' + getPropKeys(nextProps), + ); + } + UNSAFE_componentWillUpdate(nextProps) { + Scheduler.log( + 'componentWillUpdate (prev props): ' + getPropKeys(this.props), + ); + Scheduler.log( + 'componentWillUpdate (next props): ' + getPropKeys(nextProps), + ); + } + componentWillUnmount() { + Scheduler.log('componentWillUnmount: ' + getPropKeys(this.props)); + } + render() { + return ; + } + } + + Component.defaultProps = { + default: 'yo', + }; + + // `ref` should never appear as a prop. `default` always should. + + // Mount + const ref = React.createRef(); + await act(async () => { + root.render(); + }); + assertLog([ + 'constructor: text, default', + 'componentWillMount: text, default', + 'render: text, default', + 'componentDidMount: text, default', + ]); + + // Update + await act(async () => { + root.render(); + }); + assertLog([ + 'componentWillReceiveProps (prev props): text, default', + 'componentWillReceiveProps (next props): text, default', + 'shouldComponentUpdate (prev props): text, default', + 'shouldComponentUpdate (next props): text, default', + 'componentWillUpdate (prev props): text, default', + 'componentWillUpdate (next props): text, default', + 'render: text, default', + 'componentDidUpdate (prev props): text, default', + 'componentDidUpdate (next props): text, default', + ]); + + // Unmount + await act(async () => { + root.render(null); + }); + assertLog(['componentWillUnmount: text, default']); + }); +}); diff --git a/packages/react/src/__tests__/ReactCreateElement-test.js b/packages/react/src/__tests__/ReactCreateElement-test.js index cf372a33e35f0..c4a2d66310492 100644 --- a/packages/react/src/__tests__/ReactCreateElement-test.js +++ b/packages/react/src/__tests__/ReactCreateElement-test.js @@ -90,7 +90,7 @@ describe('ReactCreateElement', () => { ); }); - // @gate !enableRefAsProp + // @gate !enableRefAsProp || !__DEV__ it('should warn when `ref` is being accessed', async () => { class Child extends React.Component { render() { diff --git a/packages/react/src/__tests__/ReactJSXRuntime-test.js b/packages/react/src/__tests__/ReactJSXRuntime-test.js index 1c57954b205c4..d250792583271 100644 --- a/packages/react/src/__tests__/ReactJSXRuntime-test.js +++ b/packages/react/src/__tests__/ReactJSXRuntime-test.js @@ -244,7 +244,7 @@ describe('ReactJSXRuntime', () => { ); }); - // @gate !enableRefAsProp + // @gate !enableRefAsProp || !__DEV__ it('should warn when `ref` is being accessed', async () => { const container = document.createElement('div'); class Child extends React.Component {