diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 2ff6e03e4d290..6a88845751cd4 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -250,6 +250,9 @@ function updateMemoComponent( // to a SimpleMemoComponent to allow fast path updates. workInProgress.tag = SimpleMemoComponent; workInProgress.type = type; + if (__DEV__) { + validateFunctionComponentInDev(workInProgress, type); + } return updateSimpleMemoComponent( current, workInProgress, @@ -990,68 +993,72 @@ function mountIndeterminateComponent( // Proceed under the assumption that this is a function component workInProgress.tag = FunctionComponent; value = finishHooks(Component, props, value, context); + reconcileChildren(null, workInProgress, value, renderExpirationTime); if (__DEV__) { - if (Component) { - warningWithoutStack( - !Component.childContextTypes, - '%s(...): childContextTypes cannot be defined on a function component.', - Component.displayName || Component.name || 'Component', - ); - } - if (workInProgress.ref !== null) { - let info = ''; - const ownerName = ReactCurrentFiber.getCurrentFiberOwnerNameInDevOrNull(); - if (ownerName) { - info += '\n\nCheck the render method of `' + ownerName + '`.'; - } + validateFunctionComponentInDev(workInProgress, Component); + } + return workInProgress.child; + } +} - let warningKey = ownerName || workInProgress._debugID || ''; - const debugSource = workInProgress._debugSource; - if (debugSource) { - warningKey = debugSource.fileName + ':' + debugSource.lineNumber; - } - if (!didWarnAboutFunctionRefs[warningKey]) { - didWarnAboutFunctionRefs[warningKey] = true; - warning( - false, - 'Function components cannot be given refs. ' + - 'Attempts to access this ref will fail.%s', - info, - ); - } - } +function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) { + if (Component) { + warningWithoutStack( + !Component.childContextTypes, + '%s(...): childContextTypes cannot be defined on a function component.', + Component.displayName || Component.name || 'Component', + ); + } + if (workInProgress.ref !== null) { + let info = ''; + const ownerName = ReactCurrentFiber.getCurrentFiberOwnerNameInDevOrNull(); + if (ownerName) { + info += '\n\nCheck the render method of `' + ownerName + '`.'; + } + + let warningKey = ownerName || workInProgress._debugID || ''; + const debugSource = workInProgress._debugSource; + if (debugSource) { + warningKey = debugSource.fileName + ':' + debugSource.lineNumber; + } + if (!didWarnAboutFunctionRefs[warningKey]) { + didWarnAboutFunctionRefs[warningKey] = true; + warning( + false, + 'Function components cannot be given refs. ' + + 'Attempts to access this ref will fail.%s', + info, + ); + } + } - if (typeof Component.getDerivedStateFromProps === 'function') { - const componentName = getComponentName(Component) || 'Unknown'; + if (typeof Component.getDerivedStateFromProps === 'function') { + const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) { - warningWithoutStack( - false, - '%s: Function components do not support getDerivedStateFromProps.', - componentName, - ); - didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true; - } - } + if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) { + warningWithoutStack( + false, + '%s: Function components do not support getDerivedStateFromProps.', + componentName, + ); + didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true; + } + } - if ( - typeof Component.contextType === 'object' && - Component.contextType !== null - ) { - const componentName = getComponentName(Component) || 'Unknown'; + if ( + typeof Component.contextType === 'object' && + Component.contextType !== null + ) { + const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) { - warningWithoutStack( - false, - '%s: Function components do not support contextType.', - componentName, - ); - didWarnAboutContextTypeOnFunctionComponent[componentName] = true; - } - } + if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) { + warningWithoutStack( + false, + '%s: Function components do not support contextType.', + componentName, + ); + didWarnAboutContextTypeOnFunctionComponent[componentName] = true; } - reconcileChildren(null, workInProgress, value, renderExpirationTime); - return workInProgress.child; } } diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js index 8105d392bd6a0..a7abe9dff3720 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js @@ -15,6 +15,7 @@ let React; let ReactFeatureFlags; let ReactNoop; +let Suspense; describe('memo', () => { beforeEach(() => { @@ -23,6 +24,7 @@ describe('memo', () => { ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); + ({Suspense} = React); }); function span(prop) { @@ -38,6 +40,42 @@ describe('memo', () => { return {default: result}; } + it('warns when giving a ref (simple)', async () => { + // This test lives outside sharedTests because the wrappers don't forward + // refs properly, and they end up affecting the current owner which is used + // by the warning (making the messages not line up). + function App() { + return null; + } + App = React.memo(App); + function Outer() { + return {}} />; + } + ReactNoop.render(); + expect(ReactNoop.flush).toWarnDev([ + 'Warning: Function components cannot be given refs. Attempts to access ' + + 'this ref will fail.', + ]); + }); + + it('warns when giving a ref (complex)', async () => { + // defaultProps means this won't use SimpleMemoComponent (as of this writing) + // SimpleMemoComponent is unobservable tho, so we can't check :) + function App() { + return null; + } + App.defaultProps = {}; + App = React.memo(App); + function Outer() { + return {}} />; + } + ReactNoop.render(); + expect(ReactNoop.flush).toWarnDev([ + 'Warning: Function components cannot be given refs. Attempts to access ' + + 'this ref will fail.', + ]); + }); + // Tests should run against both the lazy and non-lazy versions of `memo`. // To make the tests work for both versions, we wrap the non-lazy version in // a lazy function component. @@ -56,8 +94,6 @@ describe('memo', () => { function sharedTests(label, memo) { describe(`${label}`, () => { it('bails out on props equality', async () => { - const {Suspense} = React; - function Counter({count}) { return ; } @@ -93,8 +129,6 @@ describe('memo', () => { }); it("does not bail out if there's a context change", async () => { - const {Suspense} = React; - const CountContext = React.createContext(0); function readContext(Context) { @@ -142,8 +176,6 @@ describe('memo', () => { }); it('accepts custom comparison function', async () => { - const {Suspense} = React; - function Counter({count}) { return ; } @@ -184,8 +216,6 @@ describe('memo', () => { }); it('supports non-pure class components', async () => { - const {Suspense} = React; - class CounterInner extends React.Component { static defaultProps = {suffix: '!'}; render() {