Skip to content

Commit

Permalink
Constructor error message (facebook#11395)
Browse files Browse the repository at this point in the history
* Constructor test and fix complete

* Linters and prettier run

* Remove unnecessary checks

* Update error message

* Updat unit test

* prettier

* Tweak the check to be more specific

* Move tests to ReactCompositeComponent-test

* add error call count and remove line
  • Loading branch information
dylanapplegate authored and Ethan-Arrowood committed Dec 8, 2017
1 parent 853c8ba commit 7036e39
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 6 deletions.
39 changes: 39 additions & 0 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1533,4 +1533,43 @@ describe('ReactCompositeComponent', () => {
ReactTestUtils.renderIntoDocument(<Component />);
expect(mockArgs.length).toEqual(0);
});

it('should return a meaningful warning when constructor is returned', () => {
spyOn(console, 'error');
class RenderTextInvalidConstructor extends React.Component {
constructor(props) {
super(props);
return {something: false};
}

render() {
return <div />;
}
}

expect(function() {
ReactTestUtils.renderIntoDocument(<RenderTextInvalidConstructor />);
}).toThrow();

expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.mostRecent().args[0]).toBe(
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
'did you accidentally return an object from the constructor?',
);
});

it('should return error if render is not defined', () => {
spyOn(console, 'error');
class RenderTestUndefinedRender extends React.Component {}

expect(function() {
ReactTestUtils.renderIntoDocument(<RenderTestUndefinedRender />);
}).toThrow();

expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.mostRecent().args[0]).toBe(
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`.',
);
});
});
25 changes: 19 additions & 6 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,25 @@ module.exports = function(
if (__DEV__) {
const name = getComponentName(workInProgress);
const renderPresent = instance.render;
warning(
renderPresent,
'%s(...): No `render` method found on the returned component ' +
'instance: you may have forgotten to define `render`.',
name,
);

if (!renderPresent) {
if (type.prototype && typeof type.prototype.render === 'function') {
warning(
false,
'%s(...): No `render` method found on the returned component ' +
'instance: did you accidentally return an object from the constructor?',
name,
);
} else {
warning(
false,
'%s(...): No `render` method found on the returned component ' +
'instance: you may have forgotten to define `render`.',
name,
);
}
}

const noGetInitialStateOnES6 =
!instance.getInitialState ||
instance.getInitialState.isReactClassApproved ||
Expand Down

0 comments on commit 7036e39

Please sign in to comment.