diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 0788c87115712..c8264410b64ea 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -591,6 +591,7 @@ describe('ReactComponentLifeCycle', () => { } componentDidMount = logger('outer componentDidMount'); shouldComponentUpdate = logger('outer shouldComponentUpdate'); + getSnapshotBeforeUpdate = logger('outer getSnapshotBeforeUpdate'); componentDidUpdate = logger('outer componentDidUpdate'); componentWillUnmount = logger('outer componentWillUnmount'); render() { @@ -610,6 +611,7 @@ describe('ReactComponentLifeCycle', () => { } componentDidMount = logger('inner componentDidMount'); shouldComponentUpdate = logger('inner shouldComponentUpdate'); + getSnapshotBeforeUpdate = logger('inner getSnapshotBeforeUpdate'); componentDidUpdate = logger('inner componentDidUpdate'); componentWillUnmount = logger('inner componentWillUnmount'); render() { @@ -635,6 +637,8 @@ describe('ReactComponentLifeCycle', () => { 'outer shouldComponentUpdate', 'inner getDerivedStateFromProps', 'inner shouldComponentUpdate', + 'inner getSnapshotBeforeUpdate', + 'outer getSnapshotBeforeUpdate', 'inner componentDidUpdate', 'outer componentDidUpdate', ]); @@ -669,10 +673,38 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.', + 'Unsafe legacy lifecycles will not be called for components using new component APIs.', ); }); + it('should not invoke deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => { + class Component extends React.Component { + state = {}; + getSnapshotBeforeUpdate() { + return null; + } + componentWillMount() { + throw Error('unexpected'); + } + componentWillReceiveProps() { + throw Error('unexpected'); + } + componentWillUpdate() { + throw Error('unexpected'); + } + componentDidUpdate() {} + render() { + return null; + } + } + + const container = document.createElement('div'); + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.', + ); + ReactDOM.render(, container); + }); + it('should not invoke new unsafe lifecycles (cWM/cWRP/cWU) if static gDSFP is present', () => { class Component extends React.Component { state = {}; @@ -694,9 +726,10 @@ describe('ReactComponentLifeCycle', () => { } const container = document.createElement('div'); - expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.', + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.', ); + ReactDOM.render(, container); }); it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new static gDSFP is present', () => { @@ -716,7 +749,7 @@ describe('ReactComponentLifeCycle', () => { } expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' UNSAFE_componentWillReceiveProps\n' + @@ -737,7 +770,7 @@ describe('ReactComponentLifeCycle', () => { } expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + @@ -757,7 +790,7 @@ describe('ReactComponentLifeCycle', () => { } expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' UNSAFE_componentWillUpdate\n\n' + @@ -777,7 +810,7 @@ describe('ReactComponentLifeCycle', () => { } expect(() => ReactDOM.render(, container)).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillReceiveProps\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + @@ -785,6 +818,88 @@ describe('ReactComponentLifeCycle', () => { ); }); + it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => { + const container = document.createElement('div'); + + class AllLegacyLifecycles extends React.Component { + state = {}; + getSnapshotBeforeUpdate() {} + componentWillMount() {} + UNSAFE_componentWillReceiveProps() {} + componentWillUpdate() {} + componentDidUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'AllLegacyLifecycles uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', + ); + + class WillMount extends React.Component { + state = {}; + getSnapshotBeforeUpdate() {} + UNSAFE_componentWillMount() {} + componentDidUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillMount uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' UNSAFE_componentWillMount\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', + ); + + class WillMountAndUpdate extends React.Component { + state = {}; + getSnapshotBeforeUpdate() {} + componentWillMount() {} + UNSAFE_componentWillUpdate() {} + componentDidUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillMountAndUpdate uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', + ); + + class WillReceiveProps extends React.Component { + state = {}; + getSnapshotBeforeUpdate() {} + componentWillReceiveProps() {} + componentDidUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillReceiveProps uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillReceiveProps\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', + ); + }); + it('calls effects on module-pattern component', function() { const log = []; @@ -961,4 +1076,117 @@ describe('ReactComponentLifeCycle', () => { ReactTestUtils.Simulate.click(divRef.current); expect(divRef.current.textContent).toBe('remote:2, local:2'); }); + + it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => { + const log = []; + + class MyComponent extends React.Component { + state = { + value: 0, + }; + static getDerivedStateFromProps(nextProps, prevState) { + return { + value: prevState.value + 1, + }; + } + getSnapshotBeforeUpdate(prevProps, prevState) { + log.push( + `getSnapshotBeforeUpdate() prevProps:${prevProps.value} prevState:${ + prevState.value + }`, + ); + return 'abc'; + } + componentDidUpdate(prevProps, prevState, snapshot) { + log.push( + `componentDidUpdate() prevProps:${prevProps.value} prevState:${ + prevState.value + } snapshot:${snapshot}`, + ); + } + render() { + log.push('render'); + return null; + } + } + + const div = document.createElement('div'); + ReactDOM.render( +
+ +
, + div, + ); + expect(log).toEqual(['render']); + log.length = 0; + + ReactDOM.render( +
+ +
, + div, + ); + expect(log).toEqual([ + 'render', + 'getSnapshotBeforeUpdate() prevProps:foo prevState:1', + 'componentDidUpdate() prevProps:foo prevState:1 snapshot:abc', + ]); + log.length = 0; + + ReactDOM.render( +
+ +
, + div, + ); + expect(log).toEqual([ + 'render', + 'getSnapshotBeforeUpdate() prevProps:bar prevState:2', + 'componentDidUpdate() prevProps:bar prevState:2 snapshot:abc', + ]); + log.length = 0; + + ReactDOM.render(
, div); + expect(log).toEqual([]); + }); + + it('should warn if getSnapshotBeforeUpdate returns undefined', () => { + class MyComponent extends React.Component { + getSnapshotBeforeUpdate() {} + componentDidUpdate() {} + render() { + return null; + } + } + + const div = document.createElement('div'); + ReactDOM.render(, div); + expect(() => ReactDOM.render(, div)).toWarnDev( + 'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' + + 'be returned. You have returned undefined.', + ); + + // De-duped + ReactDOM.render(, div); + }); + + it('should warn if getSnapshotBeforeUpdate is defined with no componentDidUpdate', () => { + class MyComponent extends React.Component { + getSnapshotBeforeUpdate() { + return null; + } + render() { + return null; + } + } + + const div = document.createElement('div'); + expect(() => ReactDOM.render(, div)).toWarnDev( + 'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' + + 'This component defines getSnapshotBeforeUpdate() only.', + ); + + // De-duped + ReactDOM.render(, div); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index bec420aeabc97..e8186a79977f8 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -2081,4 +2081,44 @@ describe('ReactErrorBoundaries', () => { }); }).toThrow('foo error'); }); + + it('handles errors that occur in before-mutation commit hook', () => { + const errors = []; + let caughtError; + class Parent extends React.Component { + getSnapshotBeforeUpdate() { + errors.push('parent sad'); + throw new Error('parent sad'); + } + componentDidUpdate() {} + render() { + return ; + } + } + class Child extends React.Component { + getSnapshotBeforeUpdate() { + errors.push('child sad'); + throw new Error('child sad'); + } + componentDidUpdate() {} + render() { + return
; + } + } + + const container = document.createElement('div'); + ReactDOM.render(, container); + try { + ReactDOM.render(, container); + } catch (e) { + if (e.message !== 'parent sad' && e.message !== 'child sad') { + throw e; + } + caughtError = e; + } + + expect(errors).toEqual(['child sad', 'parent sad']); + // Error should be the first thrown + expect(caughtError.message).toBe('child sad'); + }); }); diff --git a/packages/react-reconciler/src/ReactDebugFiberPerf.js b/packages/react-reconciler/src/ReactDebugFiberPerf.js index a23446f3cc48e..da7cae5e26826 100644 --- a/packages/react-reconciler/src/ReactDebugFiberPerf.js +++ b/packages/react-reconciler/src/ReactDebugFiberPerf.js @@ -31,7 +31,8 @@ type MeasurementPhase = | 'componentWillUpdate' | 'componentDidUpdate' | 'componentDidMount' - | 'getChildContext'; + | 'getChildContext' + | 'getSnapshotBeforeUpdate'; // Prefix measurements so that it's possible to filter them. // Longer prefixes are hard to read in DevTools. diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index e8f16d4c9ca4d..bf0baf0d865f3 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -12,7 +12,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {LegacyContext} from './ReactFiberContext'; import type {CapturedValue} from './ReactCapturedValue'; -import {Update} from 'shared/ReactTypeOfSideEffect'; +import {Update, Snapshot} from 'shared/ReactTypeOfSideEffect'; import { enableGetDerivedStateFromCatch, debugRenderPhaseSideEffects, @@ -41,23 +41,26 @@ const isArray = Array.isArray; let didWarnAboutStateAssignmentForComponent; let didWarnAboutUndefinedDerivedState; let didWarnAboutUninitializedState; +let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate; let didWarnAboutLegacyLifecyclesAndDerivedState; let warnOnInvalidCallback; if (__DEV__) { - didWarnAboutStateAssignmentForComponent = {}; - didWarnAboutUndefinedDerivedState = {}; - didWarnAboutUninitializedState = {}; - didWarnAboutLegacyLifecyclesAndDerivedState = {}; + didWarnAboutStateAssignmentForComponent = new Set(); + didWarnAboutUndefinedDerivedState = new Set(); + didWarnAboutUninitializedState = new Set(); + didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate = new Set(); + didWarnAboutLegacyLifecyclesAndDerivedState = new Set(); - const didWarnOnInvalidCallback = {}; + const didWarnOnInvalidCallback = new Set(); warnOnInvalidCallback = function(callback: mixed, callerName: string) { if (callback === null || typeof callback === 'function') { return; } const key = `${callerName}_${(callback: any)}`; - if (!didWarnOnInvalidCallback[key]) { + if (!didWarnOnInvalidCallback.has(key)) { + didWarnOnInvalidCallback.add(key); warning( false, '%s(...): Expected the last optional `callback` argument to be a ' + @@ -65,7 +68,6 @@ if (__DEV__) { callerName, callback, ); - didWarnOnInvalidCallback[key] = true; } }; @@ -364,6 +366,22 @@ export default function( name, name, ); + + if ( + typeof instance.getSnapshotBeforeUpdate === 'function' && + typeof instance.componentDidUpdate !== 'function' && + typeof instance.componentDidUpdate !== 'function' && + !didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.has(type) + ) { + didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.add(type); + warning( + false, + '%s: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' + + 'This component defines getSnapshotBeforeUpdate() only.', + getComponentName(workInProgress), + ); + } + const noInstanceGetDerivedStateFromProps = typeof instance.getDerivedStateFromProps !== 'function'; warning( @@ -435,24 +453,30 @@ export default function( adoptClassInstance(workInProgress, instance); if (__DEV__) { - if (typeof ctor.getDerivedStateFromProps === 'function') { - if (state === null) { - const componentName = getComponentName(workInProgress) || 'Component'; - if (!didWarnAboutUninitializedState[componentName]) { - warning( - false, - '%s: Did not properly initialize state during construction. ' + - 'Expected state to be an object, but it was %s.', - componentName, - instance.state === null ? 'null' : 'undefined', - ); - didWarnAboutUninitializedState[componentName] = true; - } + if ( + typeof ctor.getDerivedStateFromProps === 'function' && + state === null + ) { + const componentName = getComponentName(workInProgress) || 'Component'; + if (!didWarnAboutUninitializedState.has(componentName)) { + didWarnAboutUninitializedState.add(componentName); + warning( + false, + '%s: Did not properly initialize state during construction. ' + + 'Expected state to be an object, but it was %s.', + componentName, + instance.state === null ? 'null' : 'undefined', + ); } + } - // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. - // Warn about these lifecycles if they are present. - // Don't warn about react-lifecycles-compat polyfilled methods though. + // If new component APIs are defined, "unsafe" lifecycles won't be called. + // Warn about these lifecycles if they are present. + // Don't warn about react-lifecycles-compat polyfilled methods though. + if ( + typeof ctor.getDerivedStateFromProps === 'function' || + typeof instance.getSnapshotBeforeUpdate === 'function' + ) { let foundWillMountName = null; let foundWillReceivePropsName = null; let foundWillUpdateName = null; @@ -475,7 +499,10 @@ export default function( ) { foundWillReceivePropsName = 'UNSAFE_componentWillReceiveProps'; } - if (typeof instance.componentWillUpdate === 'function') { + if ( + typeof instance.componentWillUpdate === 'function' && + instance.componentWillUpdate.__suppressDeprecationWarning !== true + ) { foundWillUpdateName = 'componentWillUpdate'; } else if (typeof instance.UNSAFE_componentWillUpdate === 'function') { foundWillUpdateName = 'UNSAFE_componentWillUpdate'; @@ -486,23 +513,26 @@ export default function( foundWillUpdateName !== null ) { const componentName = getComponentName(workInProgress) || 'Component'; - if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { + const newApiName = + typeof ctor.getDerivedStateFromProps === 'function' + ? 'getDerivedStateFromProps()' + : 'getSnapshotBeforeUpdate()'; + if (!didWarnAboutLegacyLifecyclesAndDerivedState.has(componentName)) { + didWarnAboutLegacyLifecyclesAndDerivedState.add(componentName); warning( false, - 'Unsafe legacy lifecycles will not be called for components using ' + - 'the new getDerivedStateFromProps() API.\n\n' + - '%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' + - '%s%s%s\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + '%s uses %s but also contains the following legacy lifecycles:%s%s%s\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, + newApiName, foundWillMountName !== null ? `\n ${foundWillMountName}` : '', foundWillReceivePropsName !== null ? `\n ${foundWillReceivePropsName}` : '', foundWillUpdateName !== null ? `\n ${foundWillUpdateName}` : '', ); - didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; } } } @@ -583,7 +613,8 @@ export default function( if (instance.state !== oldState) { if (__DEV__) { const componentName = getComponentName(workInProgress) || 'Component'; - if (!didWarnAboutStateAssignmentForComponent[componentName]) { + if (!didWarnAboutStateAssignmentForComponent.has(componentName)) { + didWarnAboutStateAssignmentForComponent.add(componentName); warning( false, '%s.componentWillReceiveProps(): Assigning directly to ' + @@ -591,7 +622,6 @@ export default function( 'constructor). Use setState instead.', componentName, ); - didWarnAboutStateAssignmentForComponent[componentName] = true; } } updater.enqueueReplaceState(instance, instance.state, null); @@ -625,14 +655,14 @@ export default function( if (__DEV__) { if (partialState === undefined) { const componentName = getComponentName(workInProgress) || 'Component'; - if (!didWarnAboutUndefinedDerivedState[componentName]) { + if (!didWarnAboutUndefinedDerivedState.has(componentName)) { + didWarnAboutUndefinedDerivedState.add(componentName); warning( false, '%s.getDerivedStateFromProps(): A valid state object (or null) must be returned. ' + 'You have returned undefined.', componentName, ); - didWarnAboutUndefinedDerivedState[componentName] = componentName; } } } @@ -679,11 +709,12 @@ export default function( } // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( + typeof ctor.getDerivedStateFromProps !== 'function' && + typeof instance.getSnapshotBeforeUpdate !== 'function' && (typeof instance.UNSAFE_componentWillMount === 'function' || - typeof instance.componentWillMount === 'function') && - typeof ctor.getDerivedStateFromProps !== 'function' + typeof instance.componentWillMount === 'function') ) { callComponentWillMount(workInProgress, instance); // If we had additional state updates during this life-cycle, let's @@ -719,16 +750,20 @@ export default function( const newUnmaskedContext = getUnmaskedContext(workInProgress); const newContext = getMaskedContext(workInProgress, newUnmaskedContext); + const hasNewLifecycles = + typeof ctor.getDerivedStateFromProps === 'function' || + typeof instance.getSnapshotBeforeUpdate === 'function'; + // 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. // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( + !hasNewLifecycles && (typeof instance.UNSAFE_componentWillReceiveProps === 'function' || - typeof instance.componentWillReceiveProps === 'function') && - typeof ctor.getDerivedStateFromProps !== 'function' + typeof instance.componentWillReceiveProps === 'function') ) { if (oldProps !== newProps || oldContext !== newContext) { callComponentWillReceiveProps( @@ -835,11 +870,11 @@ export default function( if (shouldUpdate) { // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( + !hasNewLifecycles && (typeof instance.UNSAFE_componentWillMount === 'function' || - typeof instance.componentWillMount === 'function') && - typeof ctor.getDerivedStateFromProps !== 'function' + typeof instance.componentWillMount === 'function') ) { startPhaseTimer(workInProgress, 'componentWillMount'); if (typeof instance.componentWillMount === 'function') { @@ -891,16 +926,20 @@ export default function( const newUnmaskedContext = getUnmaskedContext(workInProgress); const newContext = getMaskedContext(workInProgress, newUnmaskedContext); + const hasNewLifecycles = + typeof ctor.getDerivedStateFromProps === 'function' || + typeof instance.getSnapshotBeforeUpdate === 'function'; + // 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. // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( + !hasNewLifecycles && (typeof instance.UNSAFE_componentWillReceiveProps === 'function' || - typeof instance.componentWillReceiveProps === 'function') && - typeof ctor.getDerivedStateFromProps !== 'function' + typeof instance.componentWillReceiveProps === 'function') ) { if (oldProps !== newProps || oldContext !== newContext) { callComponentWillReceiveProps( @@ -917,6 +956,7 @@ export default function( // TODO: Previous state can be null. let newState; let derivedStateFromCatch; + if (workInProgress.updateQueue !== null) { newState = processUpdateQueue( current, @@ -998,6 +1038,14 @@ export default function( workInProgress.effectTag |= Update; } } + if (typeof instance.getSnapshotBeforeUpdate === 'function') { + if ( + oldProps !== current.memoizedProps || + oldState !== current.memoizedState + ) { + workInProgress.effectTag |= Snapshot; + } + } return false; } @@ -1012,11 +1060,11 @@ export default function( if (shouldUpdate) { // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( + !hasNewLifecycles && (typeof instance.UNSAFE_componentWillUpdate === 'function' || - typeof instance.componentWillUpdate === 'function') && - typeof ctor.getDerivedStateFromProps !== 'function' + typeof instance.componentWillUpdate === 'function') ) { startPhaseTimer(workInProgress, 'componentWillUpdate'); if (typeof instance.componentWillUpdate === 'function') { @@ -1030,6 +1078,9 @@ export default function( if (typeof instance.componentDidUpdate === 'function') { workInProgress.effectTag |= Update; } + if (typeof instance.getSnapshotBeforeUpdate === 'function') { + workInProgress.effectTag |= Snapshot; + } } else { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. @@ -1041,6 +1092,14 @@ export default function( workInProgress.effectTag |= Update; } } + if (typeof instance.getSnapshotBeforeUpdate === 'function') { + if ( + oldProps !== current.memoizedProps || + oldState !== current.memoizedState + ) { + workInProgress.effectTag |= Snapshot; + } + } // If shouldComponentUpdate returned false, we should still update the // memoized props/state to indicate that this work can be reused. diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index a34a2c655e5c2..afa5b46d0b0f8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -27,7 +27,12 @@ import { CallComponent, } from 'shared/ReactTypeOfWork'; import ReactErrorUtils from 'shared/ReactErrorUtils'; -import {Placement, Update, ContentReset} from 'shared/ReactTypeOfSideEffect'; +import { + Placement, + Update, + ContentReset, + Snapshot, +} from 'shared/ReactTypeOfSideEffect'; import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; @@ -44,6 +49,11 @@ const { clearCaughtError, } = ReactErrorUtils; +let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; +if (__DEV__) { + didWarnAboutUndefinedSnapshotBeforeUpdate = new Set(); +} + function logError(boundary: Fiber, errorInfo: CapturedValue) { const source = errorInfo.source; let stack = errorInfo.stack; @@ -151,6 +161,63 @@ export default function( } } + function commitBeforeMutationLifeCycles( + current: Fiber | null, + finishedWork: Fiber, + ): void { + switch (finishedWork.tag) { + case ClassComponent: { + if (finishedWork.effectTag & Snapshot) { + if (current !== null) { + const prevProps = current.memoizedProps; + const prevState = current.memoizedState; + startPhaseTimer(finishedWork, 'getSnapshotBeforeUpdate'); + const instance = finishedWork.stateNode; + instance.props = finishedWork.memoizedProps; + instance.state = finishedWork.memoizedState; + const snapshot = instance.getSnapshotBeforeUpdate( + prevProps, + prevState, + ); + if (__DEV__) { + const didWarnSet = ((didWarnAboutUndefinedSnapshotBeforeUpdate: any): Set< + mixed, + >); + if ( + snapshot === undefined && + !didWarnSet.has(finishedWork.type) + ) { + didWarnSet.add(finishedWork.type); + warning( + false, + '%s.getSnapshotBeforeUpdate(): A snapshot value (or null) ' + + 'must be returned. You have returned undefined.', + getComponentName(finishedWork), + ); + } + } + instance.__reactInternalSnapshotBeforeUpdate = snapshot; + stopPhaseTimer(); + } + } + return; + } + case HostRoot: + case HostComponent: + case HostText: + case HostPortal: + // Nothing to do for these component types + return; + default: { + invariant( + false, + 'This unit of work tag should not have side-effects. This error is ' + + 'likely caused by a bug in React. Please file an issue.', + ); + } + } + } + function commitLifeCycles( finishedRoot: FiberRoot, current: Fiber | null, @@ -174,7 +241,11 @@ export default function( startPhaseTimer(finishedWork, 'componentDidUpdate'); instance.props = finishedWork.memoizedProps; instance.state = finishedWork.memoizedState; - instance.componentDidUpdate(prevProps, prevState); + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); stopPhaseTimer(); } } @@ -492,6 +563,7 @@ export default function( commitContainer(finishedWork); }, commitLifeCycles, + commitBeforeMutationLifeCycles, commitErrorLogging, commitAttachRef, commitDetachRef, @@ -814,6 +886,7 @@ export default function( if (enableMutatingReconciler) { return { + commitBeforeMutationLifeCycles, commitResetTextContent, commitPlacement, commitDeletion, diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index ea5df6dc84128..ec4c554182eab 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -21,6 +21,7 @@ import { PerformedWork, Placement, Update, + Snapshot, PlacementAndUpdate, Deletion, ContentReset, @@ -197,6 +198,7 @@ export default function( isAlreadyFailedLegacyErrorBoundary, ); const { + commitBeforeMutationLifeCycles, commitResetTextContent, commitPlacement, commitDeletion, @@ -319,6 +321,12 @@ export default function( recordEffect(); const effectTag = nextEffect.effectTag; + + if (effectTag & Snapshot) { + const current = nextEffect.alternate; + commitBeforeMutationLifeCycles(current, nextEffect); + } + if (effectTag & ContentReset) { commitResetTextContent(nextEffect); } diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js index 0afaa74d520b4..de37ab620336c 100644 --- a/packages/react-reconciler/src/ReactStrictModeWarnings.js +++ b/packages/react-reconciler/src/ReactStrictModeWarnings.js @@ -213,7 +213,10 @@ if (__DEV__) { ) { pendingComponentWillReceivePropsWarnings.push(fiber); } - if (typeof instance.componentWillUpdate === 'function') { + if ( + typeof instance.componentWillUpdate === 'function' && + instance.componentWillUpdate.__suppressDeprecationWarning !== true + ) { pendingComponentWillUpdateWarnings.push(fiber); } }; diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 8d80221cbbd1e..131db2d417511 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -139,20 +139,18 @@ class ReactShallowRenderer { ) { const beforeState = this._newState; - if (typeof this._instance.componentWillMount === 'function') { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - if (typeof element.type.getDerivedStateFromProps !== 'function') { - this._instance.componentWillMount(); - } - } + // In order to support react-lifecycles-compat polyfilled components, + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( - typeof this._instance.UNSAFE_componentWillMount === 'function' && - typeof element.type.getDerivedStateFromProps !== 'function' + typeof element.type.getDerivedStateFromProps !== 'function' && + typeof this._instance.getSnapshotBeforeUpdate !== 'function' ) { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - this._instance.UNSAFE_componentWillMount(); + if (typeof this._instance.componentWillMount === 'function') { + this._instance.componentWillMount(); + } + if (typeof this._instance.UNSAFE_componentWillMount === 'function') { + this._instance.UNSAFE_componentWillMount(); + } } // setState may have been called during cWM @@ -173,20 +171,20 @@ class ReactShallowRenderer { const oldProps = this._instance.props; if (oldProps !== props) { - if (typeof this._instance.componentWillReceiveProps === 'function') { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - if (typeof element.type.getDerivedStateFromProps !== 'function') { - this._instance.componentWillReceiveProps(props, context); - } - } + // In order to support react-lifecycles-compat polyfilled components, + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( - typeof this._instance.UNSAFE_componentWillReceiveProps === 'function' && - typeof element.type.getDerivedStateFromProps !== 'function' + typeof element.type.getDerivedStateFromProps !== 'function' && + typeof this._instance.getSnapshotBeforeUpdate !== 'function' ) { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - this._instance.UNSAFE_componentWillReceiveProps(props, context); + if (typeof this._instance.componentWillReceiveProps === 'function') { + this._instance.componentWillReceiveProps(props, context); + } + if ( + typeof this._instance.UNSAFE_componentWillReceiveProps === 'function' + ) { + this._instance.UNSAFE_componentWillReceiveProps(props, context); + } } this._updateStateFromStaticLifecycle(props); @@ -211,20 +209,18 @@ class ReactShallowRenderer { } if (shouldUpdate) { - if (typeof this._instance.componentWillUpdate === 'function') { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - if (typeof type.getDerivedStateFromProps !== 'function') { - this._instance.componentWillUpdate(props, state, context); - } - } + // In order to support react-lifecycles-compat polyfilled components, + // Unsafe lifecycles should not be invoked for components using the new APIs. if ( - typeof this._instance.UNSAFE_componentWillUpdate === 'function' && - typeof type.getDerivedStateFromProps !== 'function' + typeof element.type.getDerivedStateFromProps !== 'function' && + typeof this._instance.getSnapshotBeforeUpdate !== 'function' ) { - // In order to support react-lifecycles-compat polyfilled components, - // Unsafe lifecycles should not be invoked for any component with the new gDSFP. - this._instance.UNSAFE_componentWillUpdate(props, state, context); + if (typeof this._instance.componentWillUpdate === 'function') { + this._instance.componentWillUpdate(props, state, context); + } + if (typeof this._instance.UNSAFE_componentWillUpdate === 'function') { + this._instance.UNSAFE_componentWillUpdate(props, state, context); + } } } diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 2c6618d28b729..7e60bde00367f 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -128,6 +128,48 @@ describe('ReactShallowRenderer', () => { shallowRenderer.render(); }); + it('should not invoke deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => { + class Component extends React.Component { + getSnapshotBeforeUpdate() { + return null; + } + componentWillMount() { + throw Error('unexpected'); + } + componentWillReceiveProps() { + throw Error('unexpected'); + } + componentWillUpdate() { + throw Error('unexpected'); + } + render() { + return null; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + shallowRenderer.render(); + }); + + it('should not call getSnapshotBeforeUpdate or componentDidUpdate when updating since refs wont exist', () => { + class Component extends React.Component { + getSnapshotBeforeUpdate() { + throw Error('unexpected'); + } + componentDidUpdate() { + throw Error('unexpected'); + } + render() { + return null; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + shallowRenderer.render(); + }); + it('should only render 1 level deep', () => { function Parent() { return ( diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index f762b6b21b6b3..d3b9e5b240b24 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -119,43 +119,6 @@ describe('ReactContextValidator', () => { expect(actualComponentWillUpdate).toEqual({foo: 'def'}); }); - it('should not pass previous context to lifecycles', () => { - let actualComponentDidUpdate; - - class Parent extends React.Component { - getChildContext() { - return { - foo: this.props.foo, - }; - } - - render() { - return ; - } - } - Parent.childContextTypes = { - foo: PropTypes.string.isRequired, - }; - - class Component extends React.Component { - componentDidUpdate(...args) { - actualComponentDidUpdate = args; - } - - render() { - return
; - } - } - Component.contextTypes = { - foo: PropTypes.string, - }; - - const container = document.createElement('div'); - ReactDOM.render(, container); - ReactDOM.render(, container); - expect(actualComponentDidUpdate).toHaveLength(2); - }); - it('should check context types', () => { class Component extends React.Component { render() { diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index eedc2df740438..a1dadaef4b0a2 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -510,7 +510,7 @@ describe('create-react-class-integration', () => { expect(() => { ReactDOM.render(, document.createElement('div')); }).toWarnDev( - 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'Component uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' componentWillReceiveProps\n' + @@ -521,6 +521,40 @@ describe('create-react-class-integration', () => { ReactDOM.render(, document.createElement('div')); }); + it('should not invoke deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => { + const Component = createReactClass({ + getSnapshotBeforeUpdate: function() { + return null; + }, + componentWillMount: function() { + throw Error('unexpected'); + }, + componentWillReceiveProps: function() { + throw Error('unexpected'); + }, + componentWillUpdate: function() { + throw Error('unexpected'); + }, + componentDidUpdate: function() {}, + render: function() { + return null; + }, + }); + + expect(() => { + ReactDOM.render(, document.createElement('div')); + }).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'Component uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', + ); + ReactDOM.render(, document.createElement('div')); + }); + it('should invoke both deprecated and new lifecycles if both are present', () => { const log = []; diff --git a/packages/shared/ReactTypeOfSideEffect.js b/packages/shared/ReactTypeOfSideEffect.js index 9c59502f596a6..82e8c3342fc70 100644 --- a/packages/shared/ReactTypeOfSideEffect.js +++ b/packages/shared/ReactTypeOfSideEffect.js @@ -10,22 +10,23 @@ export type TypeOfSideEffect = number; // Don't change these two values. They're used by React Dev Tools. -export const NoEffect = /* */ 0b00000000000; -export const PerformedWork = /* */ 0b00000000001; +export const NoEffect = /* */ 0b000000000000; +export const PerformedWork = /* */ 0b000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b00000000010; -export const Update = /* */ 0b00000000100; -export const PlacementAndUpdate = /* */ 0b00000000110; -export const Deletion = /* */ 0b00000001000; -export const ContentReset = /* */ 0b00000010000; -export const Callback = /* */ 0b00000100000; -export const DidCapture = /* */ 0b00001000000; -export const Ref = /* */ 0b00010000000; -export const ErrLog = /* */ 0b00100000000; +export const Placement = /* */ 0b000000000010; +export const Update = /* */ 0b000000000100; +export const PlacementAndUpdate = /* */ 0b000000000110; +export const Deletion = /* */ 0b000000001000; +export const ContentReset = /* */ 0b000000010000; +export const Callback = /* */ 0b000000100000; +export const DidCapture = /* */ 0b000001000000; +export const Ref = /* */ 0b000010000000; +export const ErrLog = /* */ 0b000100000000; +export const Snapshot = /* */ 0b100000000000; // Union of all host effects -export const HostEffectMask = /* */ 0b00111111111; +export const HostEffectMask = /* */ 0b100111111111; -export const Incomplete = /* */ 0b01000000000; -export const ShouldCapture = /* */ 0b10000000000; +export const Incomplete = /* */ 0b001000000000; +export const ShouldCapture = /* */ 0b010000000000;