Do not clone key and ref getters

Author: ericmatthys

src/isomorphic/classic/element/ReactElement.js

@@ -32,6 +32,19 @@ var RESERVED_PROPS = {
 
 var specialPropKeyWarningShown, specialPropRefWarningShown;
 
+function isValidConfigRefOrKey(config, name) {
+  if (__DEV__) {
+    return hasOwnProperty.call(config, name) &&
+      !Object.getOwnPropertyDescriptor(config, name).get;
+  }
+
+  return config[name] !== undefined;
+}
+
+function getConfigKey(config) {
+  return '' + config.key;
+}
+
 /**
  * Factory method to create a new React element. This no longer adheres to
  * 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) {
         'React.createElement(...): Expected props argument to be a plain object. ' +
         'Properties defined in its prototype chain will be ignored.'
       );
-      ref = !hasOwnProperty.call(config, 'ref') ||
-        Object.getOwnPropertyDescriptor(config, 'ref').get ? null : config.ref;
-      key = !hasOwnProperty.call(config, 'key') ||
-        Object.getOwnPropertyDescriptor(config, 'key').get ? null : '' + config.key;
-    } else {
-      ref = config.ref === undefined ? null : config.ref;
-      key = config.key === undefined ? null : '' + config.key;
     }
+
+    if (isValidConfigRefOrKey(config, 'ref')) {
+      ref = config.ref;
+    }
+
+    if (isValidConfigRefOrKey(config, 'key')) {
+      key = getConfigKey(config);
+    }
+
     self = config.__self === undefined ? null : config.__self;
     source = config.__source === undefined ? null : config.__source;
     // Remaining properties are added to a new props object
@@ -297,14 +312,17 @@ ReactElement.cloneElement = function(element, config, children) {
         'Properties defined in its prototype chain will be ignored.'
       );
     }
-    if (config.ref !== undefined) {
+
+    if (isValidConfigRefOrKey(config, 'ref')) {
       // Silently steal the ref from the parent.
       ref = config.ref;
       owner = ReactCurrentOwner.current;
     }
-    if (config.key !== undefined) {
-      key = '' + config.key;
+
+    if (isValidConfigRefOrKey(config, 'key')) {
+      key = getConfigKey(config);
     }
+
     // Remaining properties override existing props
     var defaultProps;
     if (element.type && element.type.defaultProps) {

src/isomorphic/classic/element/__tests__/ReactElement-test.js

@@ -170,6 +170,86 @@ describe('ReactElement', function() {
     expect(element.props).toEqual(expectation);
   });
 
+  it('should not extract key and ref getters from the config when creating an element', function() {
+    var props = {
+      foo: '56',
+    };
+
+    Object.defineProperty(props, 'key', {
+      get: function() {
+        return '12';
+      },
+    });
+
+    Object.defineProperty(props, 'ref', {
+      get: function() {
+        return '34';
+      },
+    });
+
+    var element = React.createFactory(ComponentClass)(props);
+    expect(element.type).toBe(ComponentClass);
+    expect(element.key).toBe(null);
+    expect(element.ref).toBe(null);
+    var expectation = {foo:'56'};
+    Object.freeze(expectation);
+    expect(element.props).toEqual(expectation);
+  });
+
+  it('should not extract key and ref getters from the config when cloning an element', function() {
+    var element = React.createFactory(ComponentClass)({
+      key: '12',
+      ref: '34',
+      foo: '56',
+    });
+
+    var props = {
+      foo: 'ef',
+    };
+
+    Object.defineProperty(props, 'key', {
+      get: function() {
+        return 'ab';
+      },
+    });
+
+    Object.defineProperty(props, 'ref', {
+      get: function() {
+        return 'cd';
+      },
+    });
+
+    var clone = React.cloneElement(element, props);
+    expect(clone.type).toBe(ComponentClass);
+    expect(clone.key).toBe('12');
+    expect(clone.ref).toBe('34');
+    var expectation = {foo:'ef'};
+    Object.freeze(expectation);
+    expect(clone.props).toEqual(expectation);
+  });
+
+  it('should allow null key and ref values when cloning an element', function() {
+    var element = React.createFactory(ComponentClass)({
+      key: '12',
+      ref: '34',
+      foo: '56',
+    });
+
+    var props = {
+      key: null,
+      ref: null,
+      foo: 'ef',
+    };
+
+    var clone = React.cloneElement(element, props);
+    expect(clone.type).toBe(ComponentClass);
+    expect(clone.key).toBe('null');
+    expect(clone.ref).toBe(null);
+    var expectation = {foo:'ef'};
+    Object.freeze(expectation);
+    expect(clone.props).toEqual(expectation);
+  });
+
   it('coerces the key to a string', function() {
     var element = React.createFactory(ComponentClass)({
       key: 12,