From e9338da1fece767c0806dead9cc94293e2b7fda4 Mon Sep 17 00:00:00 2001 From: Neil Kistner Date: Tue, 9 Jan 2018 10:24:49 -0600 Subject: [PATCH] Add warning in server renderer if class doesn't extend React.Component (#11993) * Add warning in server renderer if class doesn't extend React.Component In dev mode, while server rendering, a warning will be thrown if there is a class that doesn't extend React.Component. * Use `.toWarnDev` matcher and deduplicate warnings * Deduplicate client-side warning if class doesn't extend React.Component * Default componentName to Unknown if null --- .../__tests__/ReactCompositeComponent-test.js | 5 ++++ .../__tests__/ReactServerRendering-test.js | 23 +++++++++++++++++++ .../src/server/ReactPartialRenderer.js | 20 ++++++++++++++++ .../src/ReactFiberBeginWork.js | 22 +++++++++++------- 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index a707a8df6cd1f..330b613644908 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -338,6 +338,11 @@ describe('ReactCompositeComponent', () => { 'Change ClassWithRenderNotExtended to extend React.Component instead.', ); }).toThrow(TypeError); + + // Test deduplication + expect(() => { + ReactDOM.render(, container); + }).toThrow(TypeError); }); it('should warn about `setState` in render', () => { diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js index cfba21e0523ef..cfc17f7e28aab 100644 --- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js @@ -554,4 +554,27 @@ describe('ReactDOMServer', () => { 'The experimental Call and Return types are not currently supported by the server renderer.', ); }); + + it('should warn when server rendering a class with a render method that does not extend React.Component', () => { + class ClassWithRenderNotExtended { + render() { + return
; + } + } + + expect(() => { + expect(() => + ReactDOMServer.renderToString(), + ).toWarnDev( + 'Warning: The component appears to have a render method, ' + + "but doesn't extend React.Component. This is likely to cause errors. " + + 'Change ClassWithRenderNotExtended to extend React.Component instead.', + ); + }).toThrow(TypeError); + + // Test deduplication + expect(() => { + ReactDOMServer.renderToString(); + }).toThrow(TypeError); + }); }); diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index cc939b691b073..526771f49ca5e 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -122,6 +122,7 @@ let didWarnDefaultSelectValue = false; let didWarnDefaultTextareaValue = false; let didWarnInvalidOptionChildren = false; const didWarnAboutNoopUpdateForComponent = {}; +const didWarnAboutBadClass = {}; const valuePropNames = ['value', 'defaultValue']; const newlineEatingTags = { listing: true, @@ -421,6 +422,25 @@ function resolve( if (shouldConstruct(Component)) { inst = new Component(element.props, publicContext, updater); } else { + if (__DEV__) { + if ( + Component.prototype && + typeof Component.prototype.render === 'function' + ) { + const componentName = getComponentName(Component) || 'Unknown'; + + if (!didWarnAboutBadClass[componentName]) { + warning( + false, + "The <%s /> component appears to have a render method, but doesn't extend React.Component. " + + 'This is likely to cause errors. Change %s to extend React.Component instead.', + componentName, + componentName, + ); + didWarnAboutBadClass[componentName] = true; + } + } + } inst = Component(element.props, publicContext, updater); if (inst == null || inst.render == null) { child = inst; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index d97beed6ca370..f2c058d11a194 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -61,9 +61,11 @@ import {NoWork, Never} from './ReactFiberExpirationTime'; import {AsyncUpdates} from './ReactTypeOfInternalContext'; let warnedAboutStatelessRefs; +let didWarnAboutBadClass; if (__DEV__) { warnedAboutStatelessRefs = {}; + didWarnAboutBadClass = {}; } export default function( @@ -455,14 +457,18 @@ export default function( if (__DEV__) { if (fn.prototype && typeof fn.prototype.render === 'function') { - const componentName = getComponentName(workInProgress); - warning( - false, - "The <%s /> component appears to have a render method, but doesn't extend React.Component. " + - 'This is likely to cause errors. Change %s to extend React.Component instead.', - componentName, - componentName, - ); + const componentName = getComponentName(workInProgress) || 'Unknown'; + + if (!didWarnAboutBadClass[componentName]) { + warning( + false, + "The <%s /> component appears to have a render method, but doesn't extend React.Component. " + + 'This is likely to cause errors. Change %s to extend React.Component instead.', + componentName, + componentName, + ); + didWarnAboutBadClass[componentName] = true; + } } ReactCurrentOwner.current = workInProgress; value = fn(props, context);