Fix issues introduced by createElement() warning

src/isomorphic/classic/element/ReactElement.js

@@ -32,6 +32,26 @@ var RESERVED_PROPS = {
 
 var specialPropKeyWarningShown, specialPropRefWarningShown;
 
+function hasValidRef(config) {
+  if (__DEV__) {
+    if (hasOwnProperty.call(config, 'ref') &&
+        Object.getOwnPropertyDescriptor(config, 'ref').get) {
+      return false;
+    }
+  }
+  return config.ref !== undefined;
+}
+
+function hasValidKey(config) {
+  if (__DEV__) {
+    if (hasOwnProperty.call(config, 'key') &&
+        Object.getOwnPropertyDescriptor(config, 'key').get) {
+      return false;
+    }
+  }
+  return config.key !== undefined;
+}
+
 /**
  * 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 +158,15 @@ 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 (hasValidRef(config)) {
+      ref = config.ref;
+    }
+    if (hasValidKey(config)) {
+      key = '' + config.key;
+    }
+
     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 +318,16 @@ ReactElement.cloneElement = function(element, config, children) {
         'Properties defined in its prototype chain will be ignored.'
       );
     }
-    if (config.ref !== undefined) {
+
+    if (hasValidRef(config)) {
       // Silently steal the ref from the parent.
       ref = config.ref;
       owner = ReactCurrentOwner.current;
     }
-    if (config.key !== undefined) {
+    if (hasValidKey(config)) {
       key = '' + config.key;
     }
+
     // Remaining properties override existing props
     var defaultProps;
     if (element.type && element.type.defaultProps) {

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

@@ -165,7 +165,59 @@ describe('ReactElement', function() {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe('12');
     expect(element.ref).toBe('34');
-    var expectation = {foo:'56'};
+    var expectation = {foo: '56'};
+    Object.freeze(expectation);
+    expect(element.props).toEqual(expectation);
+  });
+
+  it('extracts null key and ref', function() {
+    var element = React.createFactory(ComponentClass)({
+      key: null,
+      ref: null,
+      foo: '12',
+    });
+    expect(element.type).toBe(ComponentClass);
+    expect(element.key).toBe('null');
+    expect(element.ref).toBe(null);
+    var expectation = {foo: '12'};
+    Object.freeze(expectation);
+    expect(element.props).toEqual(expectation);
+  });
+
+  it('ignores undefined key and ref', function() {
+    var props = {
+      foo: '56',
+      key: undefined,
+      ref: undefined,
+    };
+    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('ignores key and ref getters', 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);
   });
@@ -178,7 +230,7 @@ describe('ReactElement', function() {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe('12');
     expect(element.ref).toBe(null);
-    var expectation = {foo:'56'};
+    var expectation = {foo: '56'};
     Object.freeze(expectation);
     expect(element.props).toEqual(expectation);
   });
@@ -229,18 +281,6 @@ describe('ReactElement', function() {
     expect(console.error.calls.count()).toBe(0);
   });
 
-  it('overrides children if undefined is provided as an argument', function() {
-    var element = React.createElement(ComponentClass, {
-      children: 'text',
-    }, undefined);
-    expect(element.props.children).toBe(undefined);
-
-    var element2 = React.cloneElement(React.createElement(ComponentClass, {
-      children: 'text',
-    }), {}, undefined);
-    expect(element2.props.children).toBe(undefined);
-  });
-
   it('merges rest arguments onto the children prop in an array', function() {
     spyOn(console, 'error');
     var a = 1;
@@ -370,29 +410,6 @@ describe('ReactElement', function() {
     expect(inst2.props.prop).toBe(null);
   });
 
-  it('should normalize props with default values in cloning', function() {
-    var Component = React.createClass({
-      getDefaultProps: function() {
-        return {prop: 'testKey'};
-      },
-      render: function() {
-        return <span />;
-      },
-    });
-
-    var instance = React.createElement(Component);
-    var clonedInstance = React.cloneElement(instance, {prop: undefined});
-    expect(clonedInstance.props.prop).toBe('testKey');
-    var clonedInstance2 = React.cloneElement(instance, {prop: null});
-    expect(clonedInstance2.props.prop).toBe(null);
-
-    var instance2 = React.createElement(Component, {prop: 'newTestKey'});
-    var cloneInstance3 = React.cloneElement(instance2, {prop: undefined});
-    expect(cloneInstance3.props.prop).toBe('testKey');
-    var cloneInstance4 = React.cloneElement(instance2, {});
-    expect(cloneInstance4.props.prop).toBe('newTestKey');
-  });
-
   it('throws when changing a prop (in dev) after element creation', function() {
     var Outer = React.createClass({
       render: function() {
@@ -583,4 +600,5 @@ describe('comparing jsx vs .createFactory() vs .createElement()', function() {
       expect(Child.mock.calls[0][0]).toEqual({ foo: 'foo value', children: 'children value' });
     });
   });
+
 });

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

@@ -16,11 +16,20 @@ var ReactDOM;
 var ReactTestUtils;
 
 describe('ReactElementClone', function() {
+  var ComponentClass;
 
   beforeEach(function() {
     React = require('React');
     ReactDOM = require('ReactDOM');
     ReactTestUtils = require('ReactTestUtils');
+
+    // NOTE: We're explicitly not using JSX here. This is intended to test
+    // classic JS without JSX.
+    ComponentClass = React.createClass({
+      render: function() {
+        return React.createElement('div');
+      },
+    });
   });
 
   it('should clone a DOM component with new props', function() {
@@ -155,6 +164,18 @@ describe('ReactElementClone', function() {
     ]);
   });
 
+  it('should override children if undefined is provided as an argument', function() {
+    var element = React.createElement(ComponentClass, {
+      children: 'text',
+    }, undefined);
+    expect(element.props.children).toBe(undefined);
+
+    var element2 = React.cloneElement(React.createElement(ComponentClass, {
+      children: 'text',
+    }), {}, undefined);
+    expect(element2.props.children).toBe(undefined);
+  });
+
   it('should support keys and refs', function() {
     var Parent = React.createClass({
       render: function() {
@@ -208,6 +229,29 @@ describe('ReactElementClone', function() {
     );
   });
 
+  it('should normalize props with default values', function() {
+    var Component = React.createClass({
+      getDefaultProps: function() {
+        return {prop: 'testKey'};
+      },
+      render: function() {
+        return <span />;
+      },
+    });
+
+    var instance = React.createElement(Component);
+    var clonedInstance = React.cloneElement(instance, {prop: undefined});
+    expect(clonedInstance.props.prop).toBe('testKey');
+    var clonedInstance2 = React.cloneElement(instance, {prop: null});
+    expect(clonedInstance2.props.prop).toBe(null);
+
+    var instance2 = React.createElement(Component, {prop: 'newTestKey'});
+    var cloneInstance3 = React.cloneElement(instance2, {prop: undefined});
+    expect(cloneInstance3.props.prop).toBe('testKey');
+    var cloneInstance4 = React.cloneElement(instance2, {});
+    expect(cloneInstance4.props.prop).toBe('newTestKey');
+  });
+
   it('warns for keys for arrays of elements in rest args', function() {
     spyOn(console, 'error');
 
@@ -278,4 +322,72 @@ describe('ReactElementClone', function() {
     );
   });
 
+  it('should ignore key and ref getters', 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 ignore undefined key and ref', function() {
+    var element = React.createFactory(ComponentClass)({
+      key: '12',
+      ref: '34',
+      foo: '56',
+    });
+    var props = {
+      key: undefined,
+      ref: undefined,
+      foo: 'ef',
+    };
+    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 extract null key and ref', 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);
+  });
+
 });