Skip to content

Commit

Permalink
Merge pull request #5884 from jimfb/component-extends-react-component
Browse files Browse the repository at this point in the history
Enable null return values in plain functions
  • Loading branch information
jimfb committed Jan 29, 2016
2 parents 9d5825c + 757756f commit 188e8cd
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 91 deletions.
4 changes: 1 addition & 3 deletions src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ describe('ReactES6Class', function() {
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Foo(...): No `render` method found on the returned component ' +
'instance: you may have forgotten to define `render`, returned ' +
'null/false from a stateless component, or tried to render an element ' +
'whose type is a function that isn\'t a React component.'
'instance: you may have forgotten to define `render`.'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,7 @@ describe('ReactTypeScriptClass', function() {
expect((<any>console.error).argsForCall.length).toBe(1);
expect((<any>console.error).argsForCall[0][0]).toBe(
'Warning: Empty(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`, ' +
'returned null/false from a stateless component, or tried to render an ' +
'element whose type is a function that isn\'t a React component.'
'component instance: you may have forgotten to define `render`.'
);
});

Expand Down
38 changes: 13 additions & 25 deletions src/renderers/shared/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,16 @@ function StatelessComponent(Component) {
}
StatelessComponent.prototype.render = function() {
var Component = ReactInstanceMap.get(this)._currentElement.type;
return Component(this.props, this.context, this.updater);
var element = Component(this.props, this.context, this.updater);
if (__DEV__) {
warning(
element === null || element === false || ReactElement.isValidElement(element),
'%s must be a class extending React.Component or be a stateless ' +
'function that returns a valid React element.',
Component.displayName || Component.name || 'Component'
);
}
return element;
};

/**
Expand Down Expand Up @@ -147,13 +156,7 @@ var ReactCompositeComponentMixin = {
var inst;
var renderedElement;

// This is a way to detect if Component is a stateless arrow function
// component, which is not newable. It might not be 100% reliable but is
// something we can do until we start detecting that Component extends
// React.Component. We already assume that typeof Component === 'function'.
var canInstantiate = 'prototype' in Component;

if (canInstantiate) {
if (Component.prototype && Component.prototype.isReactComponent) {
if (__DEV__) {
ReactCurrentOwner.current = this;
try {
Expand All @@ -164,10 +167,7 @@ var ReactCompositeComponentMixin = {
} else {
inst = new Component(publicProps, publicContext, ReactUpdateQueue);
}
}

if (!canInstantiate || inst === null || inst === false || ReactElement.isValidElement(inst)) {
renderedElement = inst;
} else {
inst = new StatelessComponent(Component);
}

Expand All @@ -178,19 +178,7 @@ var ReactCompositeComponentMixin = {
warning(
false,
'%s(...): No `render` method found on the returned component ' +
'instance: you may have forgotten to define `render`, returned ' +
'null/false from a stateless component, or tried to render an ' +
'element whose type is a function that isn\'t a React component.',
Component.displayName || Component.name || 'Component'
);
} else {
// We support ES6 inheriting from React.Component, the module pattern,
// and stateless components, but not ES6 classes that don't extend
warning(
(Component.prototype && Component.prototype.isReactComponent) ||
!canInstantiate ||
!(inst instanceof Component),
'%s(...): React component classes must extend React.Component.',
'instance: you may have forgotten to define `render`.',
Component.displayName || Component.name || 'Component'
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1103,19 +1103,6 @@ describe('ReactCompositeComponent', function() {
expect(a).toBe(b);
});

it('should warn when using non-React functions in JSX', function() {
function NotAComponent() {
return [<div />, <div />];
}
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrow(); // has no method 'render'
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'NotAComponent(...): No `render` method found'
);
});

it('context should be passed down from the parent', function() {
var Parent = React.createClass({
childContextTypes: {
Expand Down Expand Up @@ -1247,37 +1234,6 @@ describe('ReactCompositeComponent', function() {
expect(console.error.calls.length).toBe(0);
});

it('should warn when a class does not extend React.Component', function() {

var container = document.createElement('div');

class Foo {
render() {
return <span />;
}
}

function Bar() { }
Bar.prototype = Object.create(React.Component.prototype);
Bar.prototype.render = function() {
return <span />;
};

expect(console.error.calls.length).toBe(0);

ReactDOM.render(<Bar />, container);

expect(console.error.calls.length).toBe(0);

ReactDOM.render(<Foo />, container);

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'React component classes must extend React.Component'
);

});

it('should warn when mutated props are passed', function() {

var container = document.createElement('div');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,19 @@ describe('ReactStatelessComponent', function() {
expect(el.textContent).toBe('mest');
});

it('should support module pattern components', function() {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
it('should warn when stateless component returns array', function() {
spyOn(console, 'error');
function NotAComponent() {
return [<div />, <div />];
}

var el = document.createElement('div');
ReactDOM.render(<Child test="test" />, el);

expect(el.textContent).toBe('test');
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrow();
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'NotAComponent must be a class extending React.Component or be a stateless ' +
'function that returns a valid React element.'
);
});

it('should throw on string refs in pure functions', function() {
Expand Down Expand Up @@ -204,10 +204,6 @@ describe('ReactStatelessComponent', function() {
});

it('should work with arrow functions', function() {
// TODO: actually use arrow functions, probably need node v4 and maybe
// a separate file that we blacklist from the arrow function transform.
// We can't actually test this without native arrow functions since the
// issues (non-newable) don't apply to any other functions.
var Child = function() {
return <div />;
};
Expand All @@ -217,4 +213,33 @@ describe('ReactStatelessComponent', function() {

expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
});

it('should allow simple functions to return null', function() {
var Child = function() {
return null;
};
expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
});

it('should allow simple functions to return false', function() {
function Child() {
return false;
};
expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
});

it('should warn when using non-React functions in JSX', function() {
spyOn(console, 'error');
function NotAComponent() {
return [<div />, <div />];
}
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrow(); // has no method 'render'
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'Warning: NotAComponent must be a class extending React.Component or be a stateless ' +
'function that returns a valid React element.'
);
});
});

0 comments on commit 188e8cd

Please sign in to comment.