Respect 'key' prop for object identity
Now when a `key` prop appears, its value is always honored. This means that in the root component or as an only child, changing key will cause remounting; in a `children` object, the `key` prop will be joined with the object key to form a two-part key. Fixes #590.
Ben Alpert committed
Nov 22, 2013 at 23:43 UTC
b0431a51ca6dd4404c5e19c632b00abe3b3082a1
10 files changed
+152
-57
src/core/ReactComponent.js
-17
@@ -155,23 +155,6 @@ var ReactComponent = {
155
);
156
},
157
158
- /**
159
- * Generate a key string that identifies a component within a set.
160
- *
161
- * @param {*} component A component that could contain a manual key.
162
- * @param {number} index Index that is used if a manual key is not provided.
163
- * @return {string}
164
- * @internal
165
- */
166
- getKey: function(component, index) {
167
- if (component && component.props && component.props.key != null) {
168
- // Explicit key
169
- return '{' + component.props.key + '}';
170
- }
171
- // Implicit key determined by the index in the set
172
- return '[' + index + ']';
173
- },
174
-
158
/**
159
* @internal
160
*/
src/core/__tests__/ReactIdentity-test.js
+3
-3
@@ -55,8 +55,8 @@ describe('ReactIdentity', function() {
55
React.renderComponent(instance, document.createElement('div'));
56
var node = instance.getDOMNode();
57
reactComponentExpect(instance).toBeDOMComponentWithChildCount(2);
58
- checkId(node.childNodes[0], '.r[0].{first}');
59
- checkId(node.childNodes[1], '.r[0].{second}');
58
+ checkId(node.childNodes[0], '.r[0].{first}[0]');
59
+ checkId(node.childNodes[1], '.r[0].{second}[0]');
60
});
61
62
it('should allow key property to express identity', function() {
@@ -117,7 +117,7 @@ describe('ReactIdentity', function() {
117
expect(span2.getDOMNode()).not.toBe(null);
118
119
checkId(span1.getDOMNode(), '.r[0].{' + key + '}');
120
- checkId(span2.getDOMNode(), '.r[0].[1]{' + key + '}');
120
+ checkId(span2.getDOMNode(), '.r[0].[1]{' + key + '}[0]');
121
}
122
123
it('should allow any character as a key, in a detached parent', function() {
src/core/__tests__/ReactMount-test.js
+35
@@ -35,4 +35,39 @@ describe('ReactMount', function() {
35
ReactMount.renderComponent(<span></span>, container);
36
expect(container.firstChild.nodeName).toBe('SPAN');
37
});
38
+
39
+ it('should unmount and remount if the key changes', function() {
40
+ var container = document.createElement('container');
41
+
42
+ var mockMount = mocks.getMockFunction();
43
+ var mockUnmount = mocks.getMockFunction();
44
+
45
+ var Component = React.createClass({
46
+ componentDidMount: mockMount,
47
+ componentWillUnmount: mockUnmount,
48
+ render: function() {
49
+ return <span>{this.props.text}</span>;
50
+ }
51
+ });
52
+
53
+ expect(mockMount.mock.calls.length).toBe(0);
54
+ expect(mockUnmount.mock.calls.length).toBe(0);
55
+
56
+ ReactMount.renderComponent(<Component text="orange" key="A" />, container);
57
+ expect(container.firstChild.innerText).toBe('orange');
58
+ expect(mockMount.mock.calls.length).toBe(1);
59
+ expect(mockUnmount.mock.calls.length).toBe(0);
60
+
61
+ // If we change the key, the component is unmounted and remounted
62
+ ReactMount.renderComponent(<Component text="green" key="B" />, container);
63
+ expect(container.firstChild.innerText).toBe('green');
64
+ expect(mockMount.mock.calls.length).toBe(2);
65
+ expect(mockUnmount.mock.calls.length).toBe(1);
66
+
67
+ // But if we don't change the key, the component instance is reused
68
+ ReactMount.renderComponent(<Component text="blue" key="B" />, container);
69
+ expect(container.firstChild.innerText).toBe('blue');
70
+ expect(mockMount.mock.calls.length).toBe(2);
71
+ expect(mockUnmount.mock.calls.length).toBe(1);
72
+ });
73
});
src/core/__tests__/ReactMultiChild-test.js
+28
@@ -125,6 +125,34 @@ describe('ReactMultiChild', function() {
125
expect(mockMount.mock.calls.length).toBe(2);
126
expect(mockUnmount.mock.calls.length).toBe(1);
127
});
128
+
129
+ it('should replace children with different keys', function() {
130
+ var container = document.createElement('div');
131
+
132
+ var mockMount = mocks.getMockFunction();
133
+ var mockUnmount = mocks.getMockFunction();
134
+
135
+ var MockComponent = React.createClass({
136
+ componentDidMount: mockMount,
137
+ componentWillUnmount: mockUnmount,
138
+ render: function() {
139
+ return <span />;
140
+ }
141
+ });
142
+
143
+ expect(mockMount.mock.calls.length).toBe(0);
144
+ expect(mockUnmount.mock.calls.length).toBe(0);
145
+
146
+ React.renderComponent(<div><MockComponent key="A" /></div>, container);
147
+
148
+ expect(mockMount.mock.calls.length).toBe(1);
149
+ expect(mockUnmount.mock.calls.length).toBe(0);
150
+
151
+ React.renderComponent(<div><MockComponent key="B" /></div>, container);
152
+
153
+ expect(mockMount.mock.calls.length).toBe(2);
154
+ expect(mockUnmount.mock.calls.length).toBe(1);
155
+ });
156
});
157
158
describe('innerHTML', function() {
src/core/__tests__/ReactMultiChildReconcile-test.js
+3
-3
@@ -42,11 +42,11 @@ var stripEmptyValues = function(obj) {
42
};
43
44
/**
45
- * Child names is are wrapped in an prefix and suffix character. We strip those
46
- * out. This relies on a tiny implementation detail of the rendering system.
45
+ * Child key names are wrapped like '{key}[0]'. We strip the extra chars out
46
+ * here. This relies on an implementation detail of the rendering system.
47
*/
48
var getOriginalKey = function(childName) {
49
- return childName.substr(1, childName.length - 2);
49
+ return childName.slice('{'.length, childName.length - '}[0]'.length);
50
};
51
52
/**
src/core/getComponentKey.js
new
+38
@@ -0,0 +1,38 @@
1
+/**
2
+ * Copyright 2013 Facebook, Inc.
3
+ *
4
+ * Licensed under the Apache License, Version 2.0 (the "License");
5
+ * you may not use this file except in compliance with the License.
6
+ * You may obtain a copy of the License at
7
+ *
8
+ * http://www.apache.org/licenses/LICENSE-2.0
9
+ *
10
+ * Unless required by applicable law or agreed to in writing, software
11
+ * distributed under the License is distributed on an "AS IS" BASIS,
12
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13
+ * See the License for the specific language governing permissions and
14
+ * limitations under the License.
15
+ *
16
+ * @providesModule getComponentKey
17
+ */
18
+
19
+"use strict";
20
+
21
+/**
22
+ * Generate a key string that identifies a component within a set.
23
+ *
24
+ * @param {*} component A component that could contain a manual key.
25
+ * @param {number} index Index that is used if a manual key is not provided.
26
+ * @return {string}
27
+ * @internal
28
+ */
29
+function getComponentKey(component, index) {
30
+ if (component && component.props && component.props.key != null) {
31
+ // Explicit key
32
+ return '{' + component.props.key + '}';
33
+ }
34
+ // Implicit key determined by the index in the set
35
+ return '[' + index + ']';
36
+}
37
+
38
+module.exports = getComponentKey;
src/core/shouldUpdateReactComponent.js
+6
-1
@@ -19,6 +19,8 @@
19
20
"use strict";
21
22
+var getComponentKey = require('getComponentKey');
23
+
24
/**
25
* Given a `prevComponent` and `nextComponent`, determines if `prevComponent`
26
* should be updated as opposed to being destroyed or replaced.
@@ -32,7 +34,10 @@ function shouldUpdateReactComponent(prevComponent, nextComponent) {
34
// TODO: Remove warning after a release.
35
if (prevComponent && nextComponent &&
36
prevComponent.constructor === nextComponent.constructor) {
35
- if (prevComponent._owner === nextComponent._owner) {
37
+ if (prevComponent._owner === nextComponent._owner && (
38
+ getComponentKey(prevComponent, /* index: */ 0) ===
39
+ getComponentKey(nextComponent, /* index: */ 0)
40
+ )) {
41
return true;
42
} else {
43
if (__DEV__) {
src/utils/__tests__/ReactChildren-test.js
+13
-14
@@ -124,7 +124,7 @@ describe('ReactChildren', function() {
124
var three = null;
125
var four = <div key="keyFour" />;
126
127
- var zeroMapped = <div key="giraffe" />; // Key should be overridden
127
+ var zeroMapped = <div key="giraffe" />; // Key should be joined to obj key
128
var oneMapped = null; // Key should be added even if we don't supply it!
129
var twoMapped = <div />; // Key should be added even if not supplied!
130
var threeMapped = <span />; // Map from null to something.
@@ -188,13 +188,12 @@ describe('ReactChildren', function() {
188
var two = <div key="keyTwo" />;
189
var three = null;
190
var four = <div key="keyFour" />;
191
- var five = <div key="keyFiveCompletelyIgnored" />;
192
- // five is placed into a JS object with a key that takes precedence over the
191
+ var five = <div key="keyFiveInner" />;
192
+ // five is placed into a JS object with a key that is joined to the
193
// component key attribute.
194
// Precedence is as follows:
195
- // 1. JavaScript Object key if in a JavaScript object:
196
- // 2. If grouped in an Array, the `key` attribute.
197
- // 3. The array index if in a JavaScript Array.
195
+ // 1. If grouped in an Object, the object key combined with `key` prop
196
+ // 2. If grouped in an Array, the `key` prop, falling back to array index
197
198
var zeroMapped = <div key="giraffe" />; // Key should be overridden
199
var oneMapped = null; // Key should be added even if we don't supply it!
@@ -236,12 +235,12 @@ describe('ReactChildren', function() {
235
expect(mappedKeys.length).toBe(6);
236
// Keys default to indices.
237
expect(mappedKeys).toEqual([
239
- '[0]{firstHalfKey}{keyZero}',
240
- '[0]{firstHalfKey}[1]',
241
- '[0]{firstHalfKey}{keyTwo}',
242
- '[0]{secondHalfKey}[0]',
243
- '[0]{secondHalfKey}{keyFour}',
244
- '[0]{keyFive}'
238
+ '[0]{firstHalfKey}[0]{keyZero}',
239
+ '[0]{firstHalfKey}[0][1]',
240
+ '[0]{firstHalfKey}[0]{keyTwo}',
241
+ '[0]{secondHalfKey}[0][0]',
242
+ '[0]{secondHalfKey}[0]{keyFour}',
243
+ '[0]{keyFive}{keyFiveInner}'
244
]);
245
246
expect(callback).toHaveBeenCalledWith(zero, 0);
@@ -267,7 +266,7 @@ describe('ReactChildren', function() {
266
var zeroForceKey = <div key="keyZero" />;
267
var oneForceKey = <div key="keyOne" />;
268
270
- // Key should be overridden
269
+ // Key should be joined to object key
270
var zeroForceKeyMapped = <div key="giraffe" />;
271
// Key should be added even if we don't supply it!
272
var oneForceKeyMapped = <div />;
@@ -289,7 +288,7 @@ describe('ReactChildren', function() {
288
var mappedForcedKeys = Object.keys(mappedChildrenForcedKeys);
289
expect(mappedForcedKeys).toEqual(expectedForcedKeys);
290
292
- var expectedRemappedForcedKeys = ['{{keyZero}}', '{{keyOne}}'];
291
+ var expectedRemappedForcedKeys = ['{{keyZero}}{giraffe}', '{{keyOne}}[0]'];
292
var remappedChildrenForcedKeys =
293
ReactChildren.map(mappedChildrenForcedKeys, mapFn);
294
expect(
src/utils/__tests__/traverseAllChildren-test.js
+22
-13
@@ -141,11 +141,12 @@ describe('traverseAllChildren', function() {
141
var two = <div key="keyTwo" />;
142
var three = null;
143
var four = <div key="keyFour" />;
144
- var five = <div key="keyFiveCompletelyIgnored" />;
145
- // Name precedence is as follows:
146
- // 1. JavaScript Object key if in a JavaScript object:
147
- // 2. If grouped in an Array, the `key` attribute.
148
- // 3. The array index if in a JavaScript Array.
144
+ var five = <div key="keyFiveInner" />;
145
+ // five is placed into a JS object with a key that is joined to the
146
+ // component key attribute.
147
+ // Precedence is as follows:
148
+ // 1. If grouped in an Object, the object key combined with `key` prop
149
+ // 2. If grouped in an Array, the `key` prop, falling back to array index
150
151
152
var traverseContext = [];
@@ -170,32 +171,40 @@ describe('traverseAllChildren', function() {
171
expect(traverseFn).toHaveBeenCalledWith(
172
traverseContext,
173
zero,
173
- '[0]{firstHalfKey}{keyZero}',
174
+ '[0]{firstHalfKey}[0]{keyZero}',
175
0
176
);
177
178
expect(traverseFn)
178
- .toHaveBeenCalledWith(traverseContext, one, '[0]{firstHalfKey}[1]', 1);
179
+ .toHaveBeenCalledWith(traverseContext, one, '[0]{firstHalfKey}[0][1]', 1);
180
181
expect(traverseFn).toHaveBeenCalledWith(
182
traverseContext,
183
two,
183
- '[0]{firstHalfKey}{keyTwo}',
184
+ '[0]{firstHalfKey}[0]{keyTwo}',
185
2
186
);
187
187
- expect(traverseFn)
188
- .toHaveBeenCalledWith(traverseContext, three, '[0]{secondHalfKey}[0]', 3);
188
+ expect(traverseFn).toHaveBeenCalledWith(
189
+ traverseContext,
190
+ three,
191
+ '[0]{secondHalfKey}[0][0]',
192
+ 3
193
+ );
194
195
expect(traverseFn).toHaveBeenCalledWith(
196
traverseContext,
197
four,
193
- '[0]{secondHalfKey}{keyFour}',
198
+ '[0]{secondHalfKey}[0]{keyFour}',
199
4
200
);
201
197
- expect(traverseFn)
198
- .toHaveBeenCalledWith(traverseContext, five, '[0]{keyFive}', 5);
202
+ expect(traverseFn).toHaveBeenCalledWith(
203
+ traverseContext,
204
+ five,
205
+ '[0]{keyFive}{keyFiveInner}',
206
+ 5
207
+ );
208
});
209
210
it('should retain key across two mappings', function() {
src/utils/traverseAllChildren.js
+4
-6
@@ -18,9 +18,9 @@
18
19
"use strict";
20
21
-var ReactComponent = require('ReactComponent');
21
var ReactTextComponent = require('ReactTextComponent');
22
23
+var getComponentKey = require('getComponentKey');
24
var invariant = require('invariant');
25
26
/**
@@ -46,7 +46,7 @@ var traverseAllChildrenImpl =
46
if (Array.isArray(children)) {
47
for (var i = 0; i < children.length; i++) {
48
var child = children[i];
49
- var nextName = nameSoFar + ReactComponent.getKey(child, i);
49
+ var nextName = nameSoFar + getComponentKey(child, i);
50
var nextIndex = indexSoFar + subtreeCount;
51
subtreeCount += traverseAllChildrenImpl(
52
child,
@@ -61,9 +61,7 @@ var traverseAllChildrenImpl =
61
var isOnlyChild = nameSoFar === '';
62
// If it's the only child, treat the name as if it was wrapped in an array
63
// so that it's consistent if the number of children grows
64
- var storageName = isOnlyChild ?
65
- ReactComponent.getKey(children, 0):
66
- nameSoFar;
64
+ var storageName = isOnlyChild ? getComponentKey(children, 0) : nameSoFar;
65
if (children === null || children === undefined || type === 'boolean') {
66
// All of the above are perceived as null.
67
callback(traverseContext, null, storageName, indexSoFar);
@@ -82,7 +80,7 @@ var traverseAllChildrenImpl =
80
if (children.hasOwnProperty(key)) {
81
subtreeCount += traverseAllChildrenImpl(
82
children[key],
85
- nameSoFar + '{' + key + '}',
83
+ nameSoFar + '{' + key + '}' + getComponentKey(children[key], 0),
84
indexSoFar + subtreeCount,
85
callback,
86
traverseContext