Skip to content

Commit

Permalink
Warn about unresolved function as a child (#10376)
Browse files Browse the repository at this point in the history
* Warn about unresolved function as a child

* Oops
  • Loading branch information
gaearon authored Aug 4, 2017
1 parent 6f28ecf commit efa7148
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 2 deletions.
78 changes: 76 additions & 2 deletions src/renderers/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
var React;
var ReactDOM;
var ReactDOMServer;
var ReactDOMFeatureFlags;
var ReactTestUtils;

var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');

describe('ReactComponent', () => {
function normalizeCodeLocInfo(str) {
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
Expand All @@ -26,7 +27,6 @@ describe('ReactComponent', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
ReactTestUtils = require('react-dom/test-utils');
});

Expand Down Expand Up @@ -493,4 +493,78 @@ describe('ReactComponent', () => {
' in Foo (at **)',
);
});

if (ReactDOMFeatureFlags.useFiber) {
describe('with new features', () => {
beforeEach(() => {
require('ReactFeatureFlags').disableNewFiberFeatures = false;
});

it('warns on function as a return value from a function', () => {
function Foo() {
return Foo;
}
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expectDev(console.error.calls.count()).toBe(1);
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
});

it('warns on function as a return value from a class', () => {
class Foo extends React.Component {
render() {
return Foo;
}
}
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expectDev(console.error.calls.count()).toBe(1);
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
});

it('warns on function as a child to host component', () => {
function Foo() {
return <div><span>{Foo}</span></div>;
}
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expectDev(console.error.calls.count()).toBe(1);
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
);
});

it('does not warn for function-as-a-child that gets resolved', () => {
function Bar(props) {
return props.children();
}
function Foo() {
return <Bar>{() => 'Hello'}</Bar>;
}
spyOn(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<Foo />, container);
expect(container.innerHTML).toBe('Hello');
expectDev(console.error.calls.count()).toBe(0);
});
});
}
});
36 changes: 36 additions & 0 deletions src/renderers/shared/fiber/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,16 @@ function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
}
}

function warnOnFunctionType() {
warning(
false,
'Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.%s',
getCurrentFiberStackAddendum() || '',
);
}

// This wrapper function exists because I expect to clone the code in each path
// to be able to optimize each path individually by branching early. This needs
// a compiler or we can do it manually. Helpers that don't need this branching
Expand Down Expand Up @@ -565,6 +575,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (__DEV__) {
const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
if (!disableNewFiberFeatures && typeof newChild === 'function') {
warnOnFunctionType();
}
}

return null;
}

Expand Down Expand Up @@ -638,6 +655,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (__DEV__) {
const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
if (!disableNewFiberFeatures && typeof newChild === 'function') {
warnOnFunctionType();
}
}

return null;
}

Expand Down Expand Up @@ -697,6 +721,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (__DEV__) {
const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
if (!disableNewFiberFeatures && typeof newChild === 'function') {
warnOnFunctionType();
}
}

return null;
}

Expand Down Expand Up @@ -1418,6 +1449,11 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (__DEV__) {
if (!disableNewFiberFeatures && typeof newChild === 'function') {
warnOnFunctionType();
}
}
if (!disableNewFiberFeatures && typeof newChild === 'undefined') {
// If the new child is undefined, and the return fiber is a composite
// component, throw an error. If Fiber return types are disabled,
Expand Down

0 comments on commit efa7148

Please sign in to comment.