From 1faf194d91260b42239d3981f8abc5eaafd1a9da Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Aug 2017 02:20:20 +0100 Subject: [PATCH 1/2] Warn about unresolved function as a child --- .../__tests__/ReactComponent-test.js | 78 ++++++++++++++++++- src/renderers/shared/fiber/ReactChildFiber.js | 36 +++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/src/renderers/__tests__/ReactComponent-test.js b/src/renderers/__tests__/ReactComponent-test.js index dbea2bd3dcd..f5fee581d9a 100644 --- a/src/renderers/__tests__/ReactComponent-test.js +++ b/src/renderers/__tests__/ReactComponent-test.js @@ -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 **)'); @@ -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'); }); @@ -493,4 +493,78 @@ describe('ReactComponent', () => { ' in Foo (at **)', ); }); + + if (ReactDOMFeatureFlags.useFiber) { + fdescribe('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(, 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 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(, 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 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
{Foo}
; + } + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(, 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 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 {() => 'Hello'}; + } + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(, container); + expect(container.innerHTML).toBe('Hello'); + expectDev(console.error.calls.count()).toBe(0); + }); + }); + } }); diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 461ddd79ea3..9a4157b13e9 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -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 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 @@ -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; } @@ -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; } @@ -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; } @@ -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, From b38b294b506c5e62b2fb527bf1823c3afef2b967 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Aug 2017 11:37:26 +0100 Subject: [PATCH 2/2] Oops --- src/renderers/__tests__/ReactComponent-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/__tests__/ReactComponent-test.js b/src/renderers/__tests__/ReactComponent-test.js index f5fee581d9a..a5f5ce287f6 100644 --- a/src/renderers/__tests__/ReactComponent-test.js +++ b/src/renderers/__tests__/ReactComponent-test.js @@ -495,7 +495,7 @@ describe('ReactComponent', () => { }); if (ReactDOMFeatureFlags.useFiber) { - fdescribe('with new features', () => { + describe('with new features', () => { beforeEach(() => { require('ReactFeatureFlags').disableNewFiberFeatures = false; });