Skip to content

Commit e6e393b

Browse files
wyzegaearon
authored andcommitted
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
1 parent 77f96ed commit e6e393b

File tree

4 files changed

+62
-8
lines changed

4 files changed

+62
-8
lines changed

packages/react-dom/src/__tests__/ReactCompositeComponent-test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,11 @@ describe('ReactCompositeComponent', () => {
338338
'Change ClassWithRenderNotExtended to extend React.Component instead.',
339339
);
340340
}).toThrow(TypeError);
341+
342+
// Test deduplication
343+
expect(() => {
344+
ReactDOM.render(<ClassWithRenderNotExtended />, container);
345+
}).toThrow(TypeError);
341346
});
342347

343348
it('should warn about `setState` in render', () => {

packages/react-dom/src/__tests__/ReactServerRendering-test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,4 +554,27 @@ describe('ReactDOMServer', () => {
554554
'The experimental Call and Return types are not currently supported by the server renderer.',
555555
);
556556
});
557+
558+
it('should warn when server rendering a class with a render method that does not extend React.Component', () => {
559+
class ClassWithRenderNotExtended {
560+
render() {
561+
return <div />;
562+
}
563+
}
564+
565+
expect(() => {
566+
expect(() =>
567+
ReactDOMServer.renderToString(<ClassWithRenderNotExtended />),
568+
).toWarnDev(
569+
'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
570+
"but doesn't extend React.Component. This is likely to cause errors. " +
571+
'Change ClassWithRenderNotExtended to extend React.Component instead.',
572+
);
573+
}).toThrow(TypeError);
574+
575+
// Test deduplication
576+
expect(() => {
577+
ReactDOMServer.renderToString(<ClassWithRenderNotExtended />);
578+
}).toThrow(TypeError);
579+
});
557580
});

packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ let didWarnDefaultSelectValue = false;
122122
let didWarnDefaultTextareaValue = false;
123123
let didWarnInvalidOptionChildren = false;
124124
const didWarnAboutNoopUpdateForComponent = {};
125+
const didWarnAboutBadClass = {};
125126
const valuePropNames = ['value', 'defaultValue'];
126127
const newlineEatingTags = {
127128
listing: true,
@@ -421,6 +422,25 @@ function resolve(
421422
if (shouldConstruct(Component)) {
422423
inst = new Component(element.props, publicContext, updater);
423424
} else {
425+
if (__DEV__) {
426+
if (
427+
Component.prototype &&
428+
typeof Component.prototype.render === 'function'
429+
) {
430+
const componentName = getComponentName(Component) || 'Unknown';
431+
432+
if (!didWarnAboutBadClass[componentName]) {
433+
warning(
434+
false,
435+
"The <%s /> component appears to have a render method, but doesn't extend React.Component. " +
436+
'This is likely to cause errors. Change %s to extend React.Component instead.',
437+
componentName,
438+
componentName,
439+
);
440+
didWarnAboutBadClass[componentName] = true;
441+
}
442+
}
443+
}
424444
inst = Component(element.props, publicContext, updater);
425445
if (inst == null || inst.render == null) {
426446
child = inst;

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ import {NoWork, Never} from './ReactFiberExpirationTime';
6161
import {AsyncUpdates} from './ReactTypeOfInternalContext';
6262

6363
let warnedAboutStatelessRefs;
64+
let didWarnAboutBadClass;
6465

6566
if (__DEV__) {
6667
warnedAboutStatelessRefs = {};
68+
didWarnAboutBadClass = {};
6769
}
6870

6971
export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
@@ -455,14 +457,18 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
455457

456458
if (__DEV__) {
457459
if (fn.prototype && typeof fn.prototype.render === 'function') {
458-
const componentName = getComponentName(workInProgress);
459-
warning(
460-
false,
461-
"The <%s /> component appears to have a render method, but doesn't extend React.Component. " +
462-
'This is likely to cause errors. Change %s to extend React.Component instead.',
463-
componentName,
464-
componentName,
465-
);
460+
const componentName = getComponentName(workInProgress) || 'Unknown';
461+
462+
if (!didWarnAboutBadClass[componentName]) {
463+
warning(
464+
false,
465+
"The <%s /> component appears to have a render method, but doesn't extend React.Component. " +
466+
'This is likely to cause errors. Change %s to extend React.Component instead.',
467+
componentName,
468+
componentName,
469+
);
470+
didWarnAboutBadClass[componentName] = true;
471+
}
466472
}
467473
ReactCurrentOwner.current = workInProgress;
468474
value = fn(props, context);

0 commit comments

Comments
 (0)