Do not clone key and ref getters
Eric Matthys committed
Apr 8, 2016 at 07:46 UTC
94d0dc68c89c0292cd430976a7dafb95f8d13cf4
2 files changed
+108
-10
src/isomorphic/classic/element/ReactElement.js
+28
-10
@@ -32,6 +32,19 @@ var RESERVED_PROPS = {
32
33
var specialPropKeyWarningShown, specialPropRefWarningShown;
34
35
+function isValidConfigRefOrKey(config, name) {
36
+ if (__DEV__) {
37
+ return hasOwnProperty.call(config, name) &&
38
+ !Object.getOwnPropertyDescriptor(config, name).get;
39
+ }
40
+
41
+ return config[name] !== undefined;
42
+}
43
+
44
+function getConfigKey(config) {
45
+ return '' + config.key;
46
+}
47
+
48
/**
49
* Factory method to create a new React element. This no longer adheres to
50
* the class pattern, so do not use new to call it. Also, no instanceof check
@@ -138,14 +151,16 @@ ReactElement.createElement = function(type, config, children) {
151
'React.createElement(...): Expected props argument to be a plain object. ' +
152
'Properties defined in its prototype chain will be ignored.'
153
);
141
- ref = !hasOwnProperty.call(config, 'ref') ||
142
- Object.getOwnPropertyDescriptor(config, 'ref').get ? null : config.ref;
143
- key = !hasOwnProperty.call(config, 'key') ||
144
- Object.getOwnPropertyDescriptor(config, 'key').get ? null : '' + config.key;
145
- } else {
146
- ref = config.ref === undefined ? null : config.ref;
147
- key = config.key === undefined ? null : '' + config.key;
154
}
155
+
156
+ if (isValidConfigRefOrKey(config, 'ref')) {
157
+ ref = config.ref;
158
+ }
159
+
160
+ if (isValidConfigRefOrKey(config, 'key')) {
161
+ key = getConfigKey(config);
162
+ }
163
+
164
self = config.__self === undefined ? null : config.__self;
165
source = config.__source === undefined ? null : config.__source;
166
// Remaining properties are added to a new props object
@@ -297,14 +312,17 @@ ReactElement.cloneElement = function(element, config, children) {
312
'Properties defined in its prototype chain will be ignored.'
313
);
314
}
300
- if (config.ref !== undefined) {
315
+
316
+ if (isValidConfigRefOrKey(config, 'ref')) {
317
// Silently steal the ref from the parent.
318
ref = config.ref;
319
owner = ReactCurrentOwner.current;
320
}
305
- if (config.key !== undefined) {
306
- key = '' + config.key;
321
+
322
+ if (isValidConfigRefOrKey(config, 'key')) {
323
+ key = getConfigKey(config);
324
}
325
+
326
// Remaining properties override existing props
327
var defaultProps;
328
if (element.type && element.type.defaultProps) {
src/isomorphic/classic/element/__tests__/ReactElement-test.js
+80
@@ -170,6 +170,86 @@ describe('ReactElement', function() {
170
expect(element.props).toEqual(expectation);
171
});
172
173
+ it('should not extract key and ref getters from the config when creating an element', function() {
174
+ var props = {
175
+ foo: '56',
176
+ };
177
+
178
+ Object.defineProperty(props, 'key', {
179
+ get: function() {
180
+ return '12';
181
+ },
182
+ });
183
+
184
+ Object.defineProperty(props, 'ref', {
185
+ get: function() {
186
+ return '34';
187
+ },
188
+ });
189
+
190
+ var element = React.createFactory(ComponentClass)(props);
191
+ expect(element.type).toBe(ComponentClass);
192
+ expect(element.key).toBe(null);
193
+ expect(element.ref).toBe(null);
194
+ var expectation = {foo:'56'};
195
+ Object.freeze(expectation);
196
+ expect(element.props).toEqual(expectation);
197
+ });
198
+
199
+ it('should not extract key and ref getters from the config when cloning an element', function() {
200
+ var element = React.createFactory(ComponentClass)({
201
+ key: '12',
202
+ ref: '34',
203
+ foo: '56',
204
+ });
205
+
206
+ var props = {
207
+ foo: 'ef',
208
+ };
209
+
210
+ Object.defineProperty(props, 'key', {
211
+ get: function() {
212
+ return 'ab';
213
+ },
214
+ });
215
+
216
+ Object.defineProperty(props, 'ref', {
217
+ get: function() {
218
+ return 'cd';
219
+ },
220
+ });
221
+
222
+ var clone = React.cloneElement(element, props);
223
+ expect(clone.type).toBe(ComponentClass);
224
+ expect(clone.key).toBe('12');
225
+ expect(clone.ref).toBe('34');
226
+ var expectation = {foo:'ef'};
227
+ Object.freeze(expectation);
228
+ expect(clone.props).toEqual(expectation);
229
+ });
230
+
231
+ it('should allow null key and ref values when cloning an element', function() {
232
+ var element = React.createFactory(ComponentClass)({
233
+ key: '12',
234
+ ref: '34',
235
+ foo: '56',
236
+ });
237
+
238
+ var props = {
239
+ key: null,
240
+ ref: null,
241
+ foo: 'ef',
242
+ };
243
+
244
+ var clone = React.cloneElement(element, props);
245
+ expect(clone.type).toBe(ComponentClass);
246
+ expect(clone.key).toBe('null');
247
+ expect(clone.ref).toBe(null);
248
+ var expectation = {foo:'ef'};
249
+ Object.freeze(expectation);
250
+ expect(clone.props).toEqual(expectation);
251
+ });
252
+
253
it('coerces the key to a string', function() {
254
var element = React.createFactory(ComponentClass)({
255
key: 12,