From ac48085ac9a7a5ac0d183f481a05d7870f34676e Mon Sep 17 00:00:00 2001
From: Eric Matthys <ericmatthys@gmail.com>
Date: Fri, 8 Apr 2016 07:46:33 -0600
Subject: [PATCH] Do not clone key and ref getters

---
 .../classic/element/ReactElement.js           | 38 ++++++---
 .../element/__tests__/ReactElement-test.js    | 80 +++++++++++++++++++
 2 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js
index 307bb4d5c88b3..1b5dc51a5f894 100644
--- a/src/isomorphic/classic/element/ReactElement.js
+++ b/src/isomorphic/classic/element/ReactElement.js
@@ -31,6 +31,19 @@ var RESERVED_PROPS = {
 
 var specialPropKeyWarningShown, specialPropRefWarningShown;
 
+function isValidConfigRefOrKey(config, name) {
+  if (__DEV__) {
+    return config.hasOwnProperty(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
@@ -133,14 +146,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 = !config.hasOwnProperty('ref') ||
-        Object.getOwnPropertyDescriptor(config, 'ref').get ? null : config.ref;
-      key = !config.hasOwnProperty('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
@@ -284,14 +299,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) {
diff --git a/src/isomorphic/classic/element/__tests__/ReactElement-test.js b/src/isomorphic/classic/element/__tests__/ReactElement-test.js
index 8b3900db9f651..79c2ca7eaa13b 100644
--- a/src/isomorphic/classic/element/__tests__/ReactElement-test.js
+++ b/src/isomorphic/classic/element/__tests__/ReactElement-test.js
@@ -164,6 +164,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,