From 50cbdbc9abd13641f7a5a3ea2bfbcab215f62aa7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 16 Mar 2015 18:05:15 -0700 Subject: [PATCH] Don't use owner to determine statefulness This reverts an early commit that made it so that elements from two different owner in the same slot wouldn't share state. That behavior was helpful, and we did hit a case which was solved by this. However, this pattern is extremely uncommon. I've yet to even find the original case, let alone any existing cases in our codebase. Therefore, we're dropping this to simplify elements and enable new optimizations. --- .../__tests__/ReactCompositeComponent-test.js | 44 ------------ src/core/__tests__/ReactMultiChild-test.js | 6 +- src/core/shouldUpdateReactComponent.js | 67 ++----------------- 3 files changed, 8 insertions(+), 109 deletions(-) diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 4c2c8e3aad..76c8b2c2a8 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -500,50 +500,6 @@ describe('ReactCompositeComponent', function() { ); }); - it('should warn when owner is necessary', function() { - - var Chooser = React.createClass({ - render: function() { - return this.props.selection == 1 ? this.props.child1 : this.props.child2; - } - }); - - var CoolParent1 = React.createClass({ - render: function() { - return } - child2={this.props.child2} />; - } - }); - - var CoolParent2 = React.createClass({ - render: function() { - return } />; - } - }); - - var div = document.createElement('div'); - React.render(, div); - expect(console.warn.argsForCall.length).toBe(0); - React.render(, div); - expect(console.warn.argsForCall.length).toBe(1); - React.render(, div); - React.render(, div); - expect(console.warn.argsForCall.length).toBe(1); - - expect(console.warn.argsForCall[0][0]).toBe( - 'Warning: is being rendered by both CoolParent1 and ' + - 'CoolParent2 using the same key (null) in the same place. Currently, ' + - 'this means that they don\'t preserve state. This behavior should be ' + - 'very rare so we\'re considering deprecating it. Please contact the ' + - 'React team and explain your use case so that we can take that into ' + - 'consideration.' - ); - }); - it('should pass context', function() { var childInstance = null; var grandchildInstance = null; diff --git a/src/core/__tests__/ReactMultiChild-test.js b/src/core/__tests__/ReactMultiChild-test.js index 80dd0f14a9..2d7e1dfe27 100644 --- a/src/core/__tests__/ReactMultiChild-test.js +++ b/src/core/__tests__/ReactMultiChild-test.js @@ -83,7 +83,7 @@ describe('ReactMultiChild', function() { expect(mockUnmount.mock.calls.length).toBe(1); }); - it('should replace children with different owners', function() { + it('should NOT replace children with different owners', function() { var container = document.createElement('div'); var mockMount = mocks.getMockFunction(); @@ -116,8 +116,8 @@ describe('ReactMultiChild', function() { container ); - expect(mockMount.mock.calls.length).toBe(2); - expect(mockUnmount.mock.calls.length).toBe(1); + expect(mockMount.mock.calls.length).toBe(1); + expect(mockUnmount.mock.calls.length).toBe(0); }); it('should replace children with different keys', function() { diff --git a/src/core/shouldUpdateReactComponent.js b/src/core/shouldUpdateReactComponent.js index 7892c86375..f80cdde268 100644 --- a/src/core/shouldUpdateReactComponent.js +++ b/src/core/shouldUpdateReactComponent.js @@ -12,8 +12,6 @@ 'use strict'; -var warning = require('warning'); - /** * Given a `prevElement` and `nextElement`, determines if the existing * instance should be updated as opposed to being destroyed or replaced by a new @@ -32,66 +30,11 @@ function shouldUpdateReactComponent(prevElement, nextElement) { if (prevType === 'string' || prevType === 'number') { return (nextType === 'string' || nextType === 'number'); } else { - if (nextType === 'object' && - prevElement.type === nextElement.type && - prevElement.key === nextElement.key) { - var ownersMatch = prevElement._owner === nextElement._owner; - if (__DEV__) { - if (!ownersMatch) { - var prevName = null; - var nextName = null; - var nextDisplayName = null; - if (prevElement._owner != null && - prevElement._owner.getPublicInstance() != null && - prevElement._owner.getPublicInstance().constructor != null) { - prevName = - prevElement._owner.getPublicInstance().constructor.displayName; - } - if (nextElement._owner != null && - nextElement._owner.getPublicInstance() != null && - nextElement._owner.getPublicInstance().constructor != null) { - nextName = - nextElement._owner.getPublicInstance().constructor.displayName; - } - if (nextElement.type != null && - nextElement.type.displayName != null) { - nextDisplayName = nextElement.type.displayName; - } - if (nextElement.type != null && typeof nextElement.type === 'string') { - nextDisplayName = nextElement.type; - } - if (typeof nextElement.type !== 'string' || - nextElement.type === 'input' || - nextElement.type === 'textarea') { - if ((prevElement._owner != null && - prevElement._owner._isOwnerNecessary === false) || - (nextElement._owner != null && - nextElement._owner._isOwnerNecessary === false)) { - if (prevElement._owner != null) { - prevElement._owner._isOwnerNecessary = true; - } - if (nextElement._owner != null) { - nextElement._owner._isOwnerNecessary = true; - } - warning( - false, - '<%s /> is being rendered by both %s and %s using the same ' + - 'key (%s) in the same place. Currently, this means that ' + - 'they don\'t preserve state. This behavior should be very ' + - 'rare so we\'re considering deprecating it. Please contact ' + - 'the React team and explain your use case so that we can ' + - 'take that into consideration.', - nextDisplayName || 'Unknown Component', - prevName || '[Unknown]', - nextName || '[Unknown]', - prevElement.key - ); - } - } - } - } - return ownersMatch; - } + return ( + nextType === 'object' && + prevElement.type === nextElement.type && + prevElement.key === nextElement.key + ); } } return false;