Do not clone key and ref getters

facebook · Apr 8, 2016 · 3549c7d · 3549c7d
1 parent 2b1bd1d
commit 3549c7d
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 11 deletions.
diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js
@@ -31,6 +31,24 @@ var RESERVED_PROPS = {
 
 var specialPropKeyWarningShown, specialPropRefWarningShown;
 
+function getRef(config) {
+  if (__DEV__) {
+    return !config.hasOwnProperty('ref') ||
+      Object.getOwnPropertyDescriptor(config, 'ref').get ? null : config.ref;
+  } else {
+    return config.ref === undefined ? null : config.ref;
+  }
+}
+
+function getKey(config) {
+  if (__DEV__) {
+    return !config.hasOwnProperty('key') ||
+      Object.getOwnPropertyDescriptor(config, 'key').get ? null : '' + config.key;
+  } else {
+    return config.key === undefined ? null : '' + 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 +151,11 @@ 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;
     }
+
+    ref = getRef(config);
+    key = getKey(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,20 @@ ReactElement.cloneElement = function(element, config, children) {
         'Properties defined in its prototype chain will be ignored.'
       );
     }
-    if (config.ref !== undefined) {
+
+    var configRef = getRef(config);
+    var configKey = getKey(config);
+
+    if (configRef != null) {
       // Silently steal the ref from the parent.
-      ref = config.ref;
+      ref = configRef;
       owner = ReactCurrentOwner.current;
     }
-    if (config.key !== undefined) {
-      key = '' + config.key;
+
+    if (configKey != null) {
+      key = configKey;
     }
+
     // 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
@@ -164,6 +164,64 @@ 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('coerces the key to a string', function() {
     var element = React.createFactory(ComponentClass)({
       key: 12,