Fix issues introduced by createElement() warning

src/isomorphic/classic/element/ReactElement.js

@@ -32,6 +32,30 @@ var RESERVED_PROPS = {
 
 var specialPropKeyWarningShown, specialPropRefWarningShown;
 
+function hasValidRef(config) {
+  if (__DEV__) {
+    if (hasOwnProperty.call(config, 'ref')) {
+      var getter = Object.getOwnPropertyDescriptor(config, 'ref').get;
+      if (getter && getter.isReactWarning) {
+        return false;
+      }
+    }
+  }
+  return config.ref !== undefined;
+}
+
+function hasValidKey(config) {
+  if (__DEV__) {
+    if (hasOwnProperty.call(config, 'key')) {
+      var getter = Object.getOwnPropertyDescriptor(config, 'key').get;
+      if (getter && getter.isReactWarning) {
+        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 +162,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
@@ -180,45 +205,54 @@ ReactElement.createElement = function(type, config, children) {
     }
   }
   if (__DEV__) {
-    // Create dummy `key` and `ref` property to `props` to warn users
-    // against its use
+    var displayName = typeof type === 'function' ?
+      (type.displayName || type.name || 'Unknown') :
+      type;
+
+    // Create dummy `key` and `ref` property to `props` to warn users against its use
+    function warnAboutAccessingKey() {
+      if (!specialPropKeyWarningShown) {
+        specialPropKeyWarningShown = true;
+        warning(
+          false,
+          '%s: `key` is not a prop. Trying to access it will result ' +
+          'in `undefined` being returned. If you need to access the same ' +
+          'value within the child component, you should pass it as a different ' +
+          'prop. (https://fb.me/react-special-props)',
+          displayName
+        );
+      }
+      return undefined;
+    }
+    warnAboutAccessingKey.isReactWarning = true;
+
+    function warnAboutAccessingRef() {
+      if (!specialPropRefWarningShown) {
+        specialPropRefWarningShown = true;
+        warning(
+          false,
+          '%s: `ref` is not a prop. Trying to access it will result ' +
+          'in `undefined` being returned. If you need to access the same ' +
+          'value within the child component, you should pass it as a different ' +
+          'prop. (https://fb.me/react-special-props)',
+          displayName
+        );
+      }
+      return undefined;
+    }
+    warnAboutAccessingRef.isReactWarning = true;
+
     if (typeof props.$$typeof === 'undefined' ||
         props.$$typeof !== REACT_ELEMENT_TYPE) {
       if (!props.hasOwnProperty('key')) {
         Object.defineProperty(props, 'key', {
-          get: function() {
-            if (!specialPropKeyWarningShown) {
-              specialPropKeyWarningShown = true;
-              warning(
-                false,
-                '%s: `key` is not a prop. Trying to access it will result ' +
-                  'in `undefined` being returned. If you need to access the same ' +
-                  'value within the child component, you should pass it as a different ' +
-                  'prop. (https://fb.me/react-special-props)',
-                typeof type === 'function' && 'displayName' in type ? type.displayName : 'Element'
-              );
-            }
-            return undefined;
-          },
+          get: warnAboutAccessingKey,
           configurable: true,
         });
       }
       if (!props.hasOwnProperty('ref')) {
         Object.defineProperty(props, 'ref', {
-          get: function() {
-            if (!specialPropRefWarningShown) {
-              specialPropRefWarningShown = true;
-              warning(
-                false,
-                '%s: `ref` is not a prop. Trying to access it will result ' +
-                  'in `undefined` being returned. If you need to access the same ' +
-                  'value within the child component, you should pass it as a different ' +
-                  'prop. (https://fb.me/react-special-props)',
-                typeof type === 'function' && 'displayName' in type ? type.displayName : 'Element'
-              );
-            }
-            return undefined;
-          },
+          get: warnAboutAccessingRef,
           configurable: true,
         });
       }
@@ -297,14 +331,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

@@ -52,12 +52,12 @@ describe('ReactElement', function() {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe(null);
     expect(element.ref).toBe(null);
-    var expectation = {};
-    Object.freeze(expectation);
-    expect(element.props).toEqual(expectation);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({});
   });
 
-  it('should warn when `key` is being accessed', function() {
+  it('should warn when `key` is being accessed on createClass element', function() {
     spyOn(console, 'error');
     var container = document.createElement('div');
     var Child = React.createClass({
@@ -87,6 +87,50 @@ describe('ReactElement', function() {
     );
   });
 
+  it('should warn when `key` is being accessed on ES class element', function() {
+    spyOn(console, 'error');
+    var container = document.createElement('div');
+    class Child extends React.Component {
+      render() {
+        return <div> {this.props.key} </div>;
+      }
+    }
+    var Parent = React.createClass({
+      render: function() {
+        return (
+          <div>
+            <Child key="0" />
+            <Child key="1" />
+            <Child key="2" />
+          </div>
+        );
+      },
+    });
+    expect(console.error.calls.count()).toBe(0);
+    ReactDOM.render(<Parent />, container);
+    expect(console.error.calls.count()).toBe(1);
+    expect(console.error.calls.argsFor(0)[0]).toContain(
+      'Child: `key` is not a prop. Trying to access it will result ' +
+      'in `undefined` being returned. If you need to access the same ' +
+      'value within the child component, you should pass it as a different ' +
+      'prop. (https://fb.me/react-special-props)'
+    );
+  });
+
+  it('should warn when `key` is being accessed on a host element', function() {
+    spyOn(console, 'error');
+    var element = <div key="3" />;
+    expect(console.error.calls.count()).toBe(0);
+    void element.props.key;
+    expect(console.error.calls.count()).toBe(1);
+    expect(console.error.calls.argsFor(0)[0]).toContain(
+      'div: `key` is not a prop. Trying to access it will result ' +
+      'in `undefined` being returned. If you need to access the same ' +
+      'value within the child component, you should pass it as a different ' +
+      'prop. (https://fb.me/react-special-props)'
+    );
+  });
+
   it('should warn when `ref` is being accessed', function() {
     spyOn(console, 'error');
     var container = document.createElement('div');
@@ -120,9 +164,9 @@ describe('ReactElement', function() {
     expect(element.type).toBe('div');
     expect(element.key).toBe(null);
     expect(element.ref).toBe(null);
-    var expectation = {};
-    Object.freeze(expectation);
-    expect(element.props).toEqual(expectation);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({});
   });
 
   it('returns an immutable element', function() {
@@ -165,9 +209,45 @@ describe('ReactElement', function() {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe('12');
     expect(element.ref).toBe('34');
-    var expectation = {foo:'56'};
-    Object.freeze(expectation);
-    expect(element.props).toEqual(expectation);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({foo: '56'});
+  });
+
+  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);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({foo: '12'});
+  });
+
+  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);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({foo: '56'});
+  });
+
+  it('ignores key and ref warning getters', function() {
+    var elementA = React.createElement('div');
+    var elementB = React.createElement('div', elementA.props);
+    expect(elementB.key).toBe(null);
+    expect(elementB.ref).toBe(null);
   });
 
   it('coerces the key to a string', function() {
@@ -178,9 +258,9 @@ describe('ReactElement', function() {
     expect(element.type).toBe(ComponentClass);
     expect(element.key).toBe('12');
     expect(element.ref).toBe(null);
-    var expectation = {foo:'56'};
-    Object.freeze(expectation);
-    expect(element.props).toEqual(expectation);
+    expect(Object.isFrozen(element)).toBe(true);
+    expect(Object.isFrozen(element.props)).toBe(true);
+    expect(element.props).toEqual({foo: '56'});
   });
 
   it('preserves the owner on the element', function() {
@@ -229,18 +309,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 +438,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 +628,5 @@ describe('comparing jsx vs .createFactory() vs .createElement()', function() {
       expect(Child.mock.calls[0][0]).toEqual({ foo: 'foo value', children: 'children value' });
     });
   });
+
 });