Skip to content

Commit

Permalink
Add warning in server renderer if class doesn't extend React.Component (
Browse files Browse the repository at this point in the history
facebook#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
  • Loading branch information
wyze authored and ManasJayanth committed Jan 12, 2018
1 parent 4473898 commit e9338da
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ describe('ReactCompositeComponent', () => {
'Change ClassWithRenderNotExtended to extend React.Component instead.',
);
}).toThrow(TypeError);

// Test deduplication
expect(() => {
ReactDOM.render(<ClassWithRenderNotExtended />, container);
}).toThrow(TypeError);
});

it('should warn about `setState` in render', () => {
Expand Down
23 changes: 23 additions & 0 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <div />;
}
}

expect(() => {
expect(() =>
ReactDOMServer.renderToString(<ClassWithRenderNotExtended />),
).toWarnDev(
'Warning: The <ClassWithRenderNotExtended /> 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(<ClassWithRenderNotExtended />);
}).toThrow(TypeError);
});
});
20 changes: 20 additions & 0 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 14 additions & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ import {NoWork, Never} from './ReactFiberExpirationTime';
import {AsyncUpdates} from './ReactTypeOfInternalContext';

let warnedAboutStatelessRefs;
let didWarnAboutBadClass;

if (__DEV__) {
warnedAboutStatelessRefs = {};
didWarnAboutBadClass = {};
}

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

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);
Expand Down

0 comments on commit e9338da

Please sign in to comment.