From 757756f682ab11961479e3c10adb0444271fdb9e Mon Sep 17 00:00:00 2001 From: jim Date: Tue, 19 Jan 2016 13:11:06 -0800 Subject: [PATCH] Enable null return values in plain functions --- .../class/__tests__/ReactES6Class-test.js | 4 +- .../__tests__/ReactTypeScriptClass-test.ts | 4 +- .../reconciler/ReactCompositeComponent.js | 38 +++++-------- .../__tests__/ReactCompositeComponent-test.js | 44 -------------- .../__tests__/ReactStatelessComponent-test.js | 57 +++++++++++++------ 5 files changed, 56 insertions(+), 91 deletions(-) diff --git a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js index dfd235f22ec23..17a2414c53d22 100644 --- a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js +++ b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js @@ -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`.' ); }); diff --git a/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts b/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts index a1d1470cd5ffa..0078265bcb25c 100644 --- a/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts +++ b/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts @@ -319,9 +319,7 @@ describe('ReactTypeScriptClass', function() { expect((console.error).argsForCall.length).toBe(1); expect((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`.' ); }); diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index 45401f499a82a..f453538916806 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -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; }; /** @@ -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 { @@ -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); } @@ -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' ); } diff --git a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js index e39184c44b6ad..696786bea19e9 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js @@ -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 [
,
]; - } - expect(function() { - ReactTestUtils.renderIntoDocument(
); - }).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: { @@ -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 ; - } - } - - function Bar() { } - Bar.prototype = Object.create(React.Component.prototype); - Bar.prototype.render = function() { - return ; - }; - - expect(console.error.calls.length).toBe(0); - - ReactDOM.render(, container); - - expect(console.error.calls.length).toBe(0); - - ReactDOM.render(, 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'); diff --git a/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js index 8771f6ebcbe5e..ff0c8628290c4 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js @@ -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
{test}
; - }, - }; + it('should warn when stateless component returns array', function() { + spyOn(console, 'error'); + function NotAComponent() { + return [
,
]; } - - var el = document.createElement('div'); - ReactDOM.render(, el); - - expect(el.textContent).toBe('test'); + expect(function() { + ReactTestUtils.renderIntoDocument(
); + }).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() { @@ -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
; }; @@ -217,4 +213,33 @@ describe('ReactStatelessComponent', function() { expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); }); + + it('should allow simple functions to return null', function() { + var Child = function() { + return null; + }; + expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); + }); + + it('should allow simple functions to return false', function() { + function Child() { + return false; + }; + expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); + }); + + it('should warn when using non-React functions in JSX', function() { + spyOn(console, 'error'); + function NotAComponent() { + return [
,
]; + } + expect(function() { + ReactTestUtils.renderIntoDocument(
); + }).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.' + ); + }); });