Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable null return values in plain functions #5884

Merged
merged 1 commit into from
Jan 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.'
);
});
});