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.
Sebastian Markbage committed
Mar 16, 2015 at 18:05 UTC
50cbdbc9abd13641f7a5a3ea2bfbcab215f62aa7
3 files changed
+8
-109
src/core/__tests__/ReactCompositeComponent-test.js
-44
@@ -500,50 +500,6 @@ describe('ReactCompositeComponent', function() {
500
);
501
});
502
503
- it('should warn when owner is necessary', function() {
504
-
505
- var Chooser = React.createClass({
506
- render: function() {
507
- return this.props.selection == 1 ? this.props.child1 : this.props.child2;
508
- }
509
- });
510
-
511
- var CoolParent1 = React.createClass({
512
- render: function() {
513
- return <Chooser
514
- selection={this.props.selection}
515
- child1={<input value='foo' readOnly='true' />}
516
- child2={this.props.child2} />;
517
- }
518
- });
519
-
520
- var CoolParent2 = React.createClass({
521
- render: function() {
522
- return <CoolParent1
523
- selection={this.props.selection}
524
- child2={<input value='foo' readOnly='true' />} />;
525
- }
526
- });
527
-
528
- var div = document.createElement('div');
529
- React.render(<CoolParent2 selection={1} />, div);
530
- expect(console.warn.argsForCall.length).toBe(0);
531
- React.render(<CoolParent2 selection={2} />, div);
532
- expect(console.warn.argsForCall.length).toBe(1);
533
- React.render(<CoolParent2 selection={1} />, div);
534
- React.render(<CoolParent2 selection={2} />, div);
535
- expect(console.warn.argsForCall.length).toBe(1);
536
-
537
- expect(console.warn.argsForCall[0][0]).toBe(
538
- 'Warning: <input /> is being rendered by both CoolParent1 and ' +
539
- 'CoolParent2 using the same key (null) in the same place. Currently, ' +
540
- 'this means that they don\'t preserve state. This behavior should be ' +
541
- 'very rare so we\'re considering deprecating it. Please contact the ' +
542
- 'React team and explain your use case so that we can take that into ' +
543
- 'consideration.'
544
- );
545
- });
546
-
503
it('should pass context', function() {
504
var childInstance = null;
505
var grandchildInstance = null;
src/core/__tests__/ReactMultiChild-test.js
+3
-3
@@ -83,7 +83,7 @@ describe('ReactMultiChild', function() {
83
expect(mockUnmount.mock.calls.length).toBe(1);
84
});
85
86
- it('should replace children with different owners', function() {
86
+ it('should NOT replace children with different owners', function() {
87
var container = document.createElement('div');
88
89
var mockMount = mocks.getMockFunction();
@@ -116,8 +116,8 @@ describe('ReactMultiChild', function() {
116
container
117
);
118
119
- expect(mockMount.mock.calls.length).toBe(2);
120
- expect(mockUnmount.mock.calls.length).toBe(1);
119
+ expect(mockMount.mock.calls.length).toBe(1);
120
+ expect(mockUnmount.mock.calls.length).toBe(0);
121
});
122
123
it('should replace children with different keys', function() {
src/core/shouldUpdateReactComponent.js
+5
-62
@@ -12,8 +12,6 @@
12
13
'use strict';
14
15
-var warning = require('warning');
16
-
15
/**
16
* Given a `prevElement` and `nextElement`, determines if the existing
17
* instance should be updated as opposed to being destroyed or replaced by a new
@@ -32,66 +30,11 @@ function shouldUpdateReactComponent(prevElement, nextElement) {
30
if (prevType === 'string' || prevType === 'number') {
31
return (nextType === 'string' || nextType === 'number');
32
} else {
35
- if (nextType === 'object' &&
36
- prevElement.type === nextElement.type &&
37
- prevElement.key === nextElement.key) {
38
- var ownersMatch = prevElement._owner === nextElement._owner;
39
- if (__DEV__) {
40
- if (!ownersMatch) {
41
- var prevName = null;
42
- var nextName = null;
43
- var nextDisplayName = null;
44
- if (prevElement._owner != null &&
45
- prevElement._owner.getPublicInstance() != null &&
46
- prevElement._owner.getPublicInstance().constructor != null) {
47
- prevName =
48
- prevElement._owner.getPublicInstance().constructor.displayName;
49
- }
50
- if (nextElement._owner != null &&
51
- nextElement._owner.getPublicInstance() != null &&
52
- nextElement._owner.getPublicInstance().constructor != null) {
53
- nextName =
54
- nextElement._owner.getPublicInstance().constructor.displayName;
55
- }
56
- if (nextElement.type != null &&
57
- nextElement.type.displayName != null) {
58
- nextDisplayName = nextElement.type.displayName;
59
- }
60
- if (nextElement.type != null && typeof nextElement.type === 'string') {
61
- nextDisplayName = nextElement.type;
62
- }
63
- if (typeof nextElement.type !== 'string' ||
64
- nextElement.type === 'input' ||
65
- nextElement.type === 'textarea') {
66
- if ((prevElement._owner != null &&
67
- prevElement._owner._isOwnerNecessary === false) ||
68
- (nextElement._owner != null &&
69
- nextElement._owner._isOwnerNecessary === false)) {
70
- if (prevElement._owner != null) {
71
- prevElement._owner._isOwnerNecessary = true;
72
- }
73
- if (nextElement._owner != null) {
74
- nextElement._owner._isOwnerNecessary = true;
75
- }
76
- warning(
77
- false,
78
- '<%s /> is being rendered by both %s and %s using the same ' +
79
- 'key (%s) in the same place. Currently, this means that ' +
80
- 'they don\'t preserve state. This behavior should be very ' +
81
- 'rare so we\'re considering deprecating it. Please contact ' +
82
- 'the React team and explain your use case so that we can ' +
83
- 'take that into consideration.',
84
- nextDisplayName || 'Unknown Component',
85
- prevName || '[Unknown]',
86
- nextName || '[Unknown]',
87
- prevElement.key
88
- );
89
- }
90
- }
91
- }
92
- }
93
- return ownersMatch;
94
- }
33
+ return (
34
+ nextType === 'object' &&
35
+ prevElement.type === nextElement.type &&
36
+ prevElement.key === nextElement.key
37
+ );
38
}
39
}
40
return false;