From 3ec9f8657308ebba7ce5f76206ce7eeaec9aa8bd Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Fri, 7 Aug 2015 18:53:42 -0700 Subject: [PATCH 1/2] Deduplicate logic in ReactElementValidator Shouldn't be much change. Notably, this calls `.getName()` instead of trying to derive it manually, which is more consistent. --- .../classic/element/ReactElementValidator.js | 54 +++++-------------- .../__tests__/ReactElementValidator-test.js | 2 +- .../ReactJSXElementValidator-test.js | 2 +- 3 files changed, 14 insertions(+), 44 deletions(-) diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 2eb888ed081..203bab1ee22 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -49,37 +49,6 @@ var loggedTypeFailures = {}; var NUMERIC_PROPERTY_REGEX = /^\d+$/; -/** - * Gets the instance's name for use in warnings. - * - * @internal - * @return {?string} Display name or undefined - */ -function getName(instance) { - var publicInstance = instance && instance.getPublicInstance(); - if (!publicInstance) { - return undefined; - } - var constructor = publicInstance.constructor; - if (!constructor) { - return undefined; - } - return constructor.displayName || constructor.name || undefined; -} - -/** - * Gets the current owner's displayName for use in warnings. - * - * @internal - * @return {?string} Display name or undefined - */ -function getCurrentOwnerDisplayName() { - var current = ReactCurrentOwner.current; - return ( - current && getName(current) || undefined - ); -} - /** * Warn if the element doesn't have an explicit key assigned to it. * This element is in an array. The array could grow and shrink or be @@ -150,24 +119,25 @@ function validatePropertyKey(name, element, parentType) { * if the warning has already been shown before (and shouldn't be shown again). */ function getAddendaForKeyUse(messageType, element, parentType) { - var ownerName = getCurrentOwnerDisplayName(); - var parentName = typeof parentType === 'string' ? - parentType : parentType.displayName || parentType.name; + var addendum = getDeclarationErrorAddendum(); + if (!addendum) { + var parentName = typeof parentType === 'string' ? + parentType : parentType.displayName || parentType.name; + if (parentName) { + addendum = ` Check the React.render call using <${parentName}>.`; + } + } - var useName = ownerName || parentName; var memoizer = ownerHasKeyUseWarning[messageType] || ( ownerHasKeyUseWarning[messageType] = {} ); - if (memoizer[useName]) { + if (memoizer[addendum]) { return null; } - memoizer[useName] = true; + memoizer[addendum] = true; var addenda = { - parentOrOwner: - ownerName ? ` Check the render method of ${ownerName}.` : - parentName ? ` Check the React.render call using <${parentName}>.` : - null, + parentOrOwner: addendum, url: ' See https://fb.me/react-warning-keys for more information.', childOwner: null, }; @@ -180,7 +150,7 @@ function getAddendaForKeyUse(messageType, element, parentType) { element._owner !== ReactCurrentOwner.current) { // Give the component that originally created this child. addenda.childOwner = - ` It was passed a child from ${getName(element._owner)}.`; + ` It was passed a child from ${element._owner.getName()}.`; } return addenda; diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index fe3d1159cc1..acf925a9df3 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -77,7 +77,7 @@ describe('ReactElementValidator', function() { expect(console.error.argsForCall.length).toBe(1); expect(console.error.argsForCall[0][0]).toContain( 'Each child in an array or iterator should have a unique "key" prop. ' + - 'Check the render method of InnerClass. ' + + 'Check the render method of `InnerClass`. ' + 'It was passed a child from ComponentWrapper. ' ); }); diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js index 93fe872e092..2ad1ec5f77f 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -83,7 +83,7 @@ describe('ReactJSXElementValidator', function() { expect(console.error.argsForCall.length).toBe(1); expect(console.error.argsForCall[0][0]).toContain( 'Each child in an array or iterator should have a unique "key" prop. ' + - 'Check the render method of InnerComponent. ' + + 'Check the render method of `InnerComponent`. ' + 'It was passed a child from ComponentWrapper. ' ); }); From 5a7bd964b4265a0e0e87e453fa0d1a4ccb604fe1 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Fri, 7 Aug 2015 19:06:44 -0700 Subject: [PATCH 2/2] Minimal implementation of stateless components Stateless pure-function components give us more opportunity to make performance optimizations. For now, we'll do a minimal implementation which has similar performance characteristics to other components in the interests of shipping 0.14 and allowing people to begin writing code using this pattern; in the future we can refactor to allocate less and avoid other unnecessary work. --- .../reconciler/ReactCompositeComponent.js | 39 +++- .../shared/reconciler/ReactUpdateQueue.js | 20 +- .../__tests__/ReactStatelessComponent-test.js | 184 ++++++++++++++++++ 3 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index 617aa91ed83..de2530637d6 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -38,6 +38,13 @@ function getDeclarationErrorAddendum(component) { return ''; } +function StatelessComponent(Component) { +} +StatelessComponent.prototype.render = function() { + var Component = ReactInstanceMap.get(this)._currentElement.type; + return new Component(this.props, this.context, this.updater); +}; + /** * ------------------ The Life-Cycle of a Composite Component ------------------ * @@ -126,7 +133,24 @@ var ReactCompositeComponentMixin = { var Component = this._currentElement.type; // Initialize the public class - var inst = new Component(publicProps, publicContext, ReactUpdateQueue); + var inst; + var renderedElement; + + if (__DEV__) { + ReactCurrentOwner.current = this; + try { + inst = new Component(publicProps, publicContext, ReactUpdateQueue); + } finally { + ReactCurrentOwner.current = null; + } + } else { + inst = new Component(publicProps, publicContext, ReactUpdateQueue); + } + + if (inst === null || inst === false || ReactElement.isValidElement(inst)) { + renderedElement = inst; + inst = new StatelessComponent(Component); + } if (__DEV__) { // This will throw later in _renderValidatedComponent, but add an early @@ -231,7 +255,10 @@ var ReactCompositeComponentMixin = { } } - var renderedElement = this._renderValidatedComponent(); + // If not a stateless component, we now render + if (renderedElement === undefined) { + renderedElement = this._renderValidatedComponent(); + } this._renderedComponent = this._instantiateReactComponent( renderedElement @@ -265,6 +292,7 @@ var ReactCompositeComponentMixin = { ReactReconciler.unmountComponent(this._renderedComponent); this._renderedComponent = null; + this._instance = null; // Reset pending fields // Even if this component is scheduled for another update in ReactUpdates, @@ -759,6 +787,7 @@ var ReactCompositeComponentMixin = { */ attachRef: function(ref, component) { var inst = this.getPublicInstance(); + invariant(inst != null, 'Stateless function components cannot have refs.'); var refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs; refs[ref] = component.getPublicInstance(); }, @@ -800,7 +829,11 @@ var ReactCompositeComponentMixin = { * @internal */ getPublicInstance: function() { - return this._instance; + var inst = this._instance; + if (inst instanceof StatelessComponent) { + return null; + } + return inst; }, // Stub diff --git a/src/renderers/shared/reconciler/ReactUpdateQueue.js b/src/renderers/shared/reconciler/ReactUpdateQueue.js index d406fe20dfb..0c5e325a8d3 100644 --- a/src/renderers/shared/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/reconciler/ReactUpdateQueue.js @@ -25,16 +25,6 @@ function enqueueUpdate(internalInstance) { } function getInternalInstanceReadyForUpdate(publicInstance, callerName) { - if (__DEV__) { - warning( - ReactCurrentOwner.current == null, - '%s(...): Cannot update during an existing state transition ' + - '(such as within `render`). Render methods should be a pure function ' + - 'of props and state.', - callerName - ); - } - var internalInstance = ReactInstanceMap.get(publicInstance); if (!internalInstance) { if (__DEV__) { @@ -54,6 +44,16 @@ function getInternalInstanceReadyForUpdate(publicInstance, callerName) { return null; } + if (__DEV__) { + warning( + ReactCurrentOwner.current == null, + '%s(...): Cannot update during an existing state transition ' + + '(such as within `render`). Render methods should be a pure function ' + + 'of props and state.', + callerName + ); + } + return internalInstance; } diff --git a/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js new file mode 100644 index 00000000000..772d7cdff4f --- /dev/null +++ b/src/renderers/shared/reconciler/__tests__/ReactStatelessComponent-test.js @@ -0,0 +1,184 @@ +/** + * Copyright 2013-2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactTestUtils; + +function StatelessComponent(props) { + return
{props.name}
; +} + +describe('ReactStatelessComponent', function() { + + beforeEach(function() { + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + }); + + it('should render stateless component', function() { + var el = document.createElement('div'); + React.render(, el); + + expect(el.textContent).toBe('A'); + }); + + it('should update stateless component', function() { + var Parent = React.createClass({ + render() { + return ; + }, + }); + + var el = document.createElement('div'); + React.render(, el); + expect(el.textContent).toBe('A'); + + React.render(, el); + expect(el.textContent).toBe('B'); + }); + + it('should unmount stateless component', function() { + var container = document.createElement('div'); + + React.render(, container); + expect(container.textContent).toBe('A'); + + React.unmountComponentAtNode(container); + expect(container.textContent).toBe(''); + }); + + it('should pass context thru stateless component', function() { + var Child = React.createClass({ + contextTypes: { + test: React.PropTypes.string.isRequired, + }, + + render: function() { + return
{this.context.test}
; + }, + }); + + function Parent() { + return ; + } + + var GrandParent = React.createClass({ + childContextTypes: { + test: React.PropTypes.string.isRequired, + }, + + getChildContext() { + return {test: this.props.test}; + }, + + render: function() { + return ; + }, + }); + + var el = document.createElement('div'); + React.render(, el); + + expect(el.textContent).toBe('test'); + + React.render(, el); + + expect(el.textContent).toBe('mest'); + }); + + it('should support module pattern components', function() { + function Child({test}) { + return { + render() { + return
{test}
; + }, + }; + } + + var el = document.createElement('div'); + React.render(, el); + + expect(el.textContent).toBe('test'); + }); + + it('should throw on string refs in pure functions', function() { + function Child() { + return
; + } + + expect(function() { + ReactTestUtils.renderIntoDocument(); + }).toThrow( + 'Invariant Violation: Stateless function components cannot have refs.' + ); + }); + + it('should provide a null ref', function() { + function Child() { + return
; + } + + var comp = ReactTestUtils.renderIntoDocument(); + expect(comp).toBe(null); + }); + + it('should use correct name in key warning', function() { + function Child() { + return
{[]}
; + } + + spyOn(console, 'error'); + ReactTestUtils.renderIntoDocument(); + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain('a unique "key" prop'); + expect(console.error.argsForCall[0][0]).toContain('Child'); + }); + + it('should support default props and prop types', function() { + function Child(props) { + return
{props.test}
; + } + Child.defaultProps = {test: 2}; + Child.propTypes = {test: React.PropTypes.string}; + + spyOn(console, 'error'); + ReactTestUtils.renderIntoDocument(); + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toBe( + 'Warning: Failed propType: Invalid prop `test` of type `number` ' + + 'supplied to `Child`, expected `string`.' + ); + }); + + it('should receive context', function() { + var Parent = React.createClass({ + childContextTypes: { + lang: React.PropTypes.string, + }, + getChildContext: function() { + return {lang: 'en'}; + }, + render: function() { + return ; + }, + }); + function Child(props, context) { + return
{context.lang}
; + } + Child.contextTypes = {lang: React.PropTypes.string}; + + var el = document.createElement('div'); + React.render(, el); + expect(el.textContent).toBe('en'); + }); +});