Started setting both props.class and props.className for DOM elements

facebook · Feb 4, 2016 · 1304828 · 1304828
1 parent f2bb015
commit 1304828
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 12 deletions.
diff --git a/src/addons/getCssClassFromProps.js b/src/addons/getCssClassFromProps.js
@@ -0,0 +1,36 @@
+/**
+ * Copyright 2013-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+* @providesModule getCssClassFromProps
+*/
+
+'use strict';
+
+var warning = require('warning');
+
+/**
+ * Takes in an props (or an element) and returns the CSS class prop.
+ */
+function getCssClassFromProps(props) {
+  if (__DEV__) {
+    getCssClassFromProps.isExecuting = true;
+    warning(
+  	  props.className == null || props.class == null || props.class === props.className,
+      'props.className and props.class should have the same values'
+    );
+  }
+  var cls = props.className || props.class;
+  if (__DEV__) {
+    getCssClassFromProps.isExecuting = false;
+  }
+  return cls;
+}
+
+getCssClassFromProps.isExecuting = false;
+
+module.exports = getCssClassFromProps;
diff --git a/src/isomorphic/classic/element/ReactElement.js b/src/isomorphic/classic/element/ReactElement.js
@@ -15,6 +15,8 @@ var ReactCurrentOwner = require('ReactCurrentOwner');
 
 var assign = require('Object.assign');
 var canDefineProperty = require('canDefineProperty');
+var getCssClassFromProps = require('getCssClassFromProps');
+var warning = require('warning');
 
 // The Symbol used to tag the ReactElement type. If there is no native Symbol
 // nor polyfill, then a plain number is used for performance.
@@ -29,6 +31,42 @@ var RESERVED_PROPS = {
   __source: true,
 };
 
+function setupClassProp(type, config, props) {
+  if (config != null) {
+    if (config.className !== config.class) {
+      if (config.class == null) {
+        props.class = config.className;
+      } else if (config.className == null) {
+        props.className = config.class;
+      } else {
+        warning(
+          false,
+          'class and className values (`%s` and `%s` respectively) ' +
+          'differ for element type: %s',
+          config.class,
+          config.className,
+          type
+        );
+      }
+    }
+
+    if (__DEV__ && props.className !== undefined) {
+      var className = props.className;
+      Object.defineProperty(props, 'className', {
+        get: function() {
+          warning(
+            getCssClassFromProps.isExecuting,
+            'Reading `className` prop directly from `%s` element is deprecated, ' +
+            'use `getCssClassFromProps(props)` instead.',
+            type
+          );
+          return className;
+        },
+      });
+    }
+  }
+}
+
 /**
  * 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
@@ -134,6 +172,9 @@ ReactElement.createElement = function(type, config, children) {
         props[propName] = config[propName];
       }
     }
+    if (typeof type === 'string') {
+      setupClassProp(type, config, props);
+    }
   }
 
   // Children can be more than one argument, and those are transferred onto
@@ -215,6 +256,9 @@ ReactElement.cloneAndReplaceProps = function(oldElement, newProps) {
 };
 
 ReactElement.cloneElement = function(element, config, children) {
+  if (__DEV__) {
+    getCssClassFromProps.isExecuting = true;
+  }
   var propName;
 
   // Original props are copied
@@ -251,6 +295,10 @@ ReactElement.cloneElement = function(element, config, children) {
     }
   }
 
+  if (typeof element.type === 'string') {
+    setupClassProp(element.type, config, props);
+  }
+
   // Children can be more than one argument, and those are transferred onto
   // the newly allocated props object.
   var childrenLength = arguments.length - 2;
@@ -264,6 +312,10 @@ ReactElement.cloneElement = function(element, config, children) {
     props.children = childArray;
   }
 
+  if (__DEV__) {
+    getCssClassFromProps.isExecuting = false;
+  }
+
   return ReactElement(
     element.type,
     key,

diff --git a/src/isomorphic/classic/element/__tests__/ReactElement-test.js b/src/isomorphic/classic/element/__tests__/ReactElement-test.js
@@ -17,6 +17,7 @@
 var React;
 var ReactDOM;
 var ReactTestUtils;
+var getCssClassFromProps;
 
 describe('ReactElement', function() {
   var ComponentClass;
@@ -33,6 +34,7 @@ describe('ReactElement', function() {
     React = require('React');
     ReactDOM = require('ReactDOM');
     ReactTestUtils = require('ReactTestUtils');
+    getCssClassFromProps = require('getCssClassFromProps');
     ComponentClass = React.createClass({
       render: function() {
         return React.createElement('div');
@@ -281,7 +283,7 @@ describe('ReactElement', function() {
         expect(function() {
           el.props.className = 'quack';
         }).toThrow();
-        expect(el.props.className).toBe('moo');
+        expect(getCssClassFromProps(el.props)).toBe('moo');
 
         return el;
       },

diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -37,6 +37,7 @@ var ReactPerf = require('ReactPerf');
 
 var assign = require('Object.assign');
 var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
+var getCssClassFromProps = require('getCssClassFromProps');
 var invariant = require('invariant');
 var isEventSupported = require('isEventSupported');
 var keyOf = require('keyOf');
@@ -55,6 +56,8 @@ var CONTENT_TYPES = {'string': true, 'number': true};
 
 var CHILDREN = keyOf({children: null});
 var STYLE = keyOf({style: null});
+var CLASSNAME = keyOf({className: null});
+var CLASS = keyOf({'class': null});
 var HTML = keyOf({__html: null});
 
 function getDeclarationErrorAddendum(internalInstance) {
@@ -606,6 +609,9 @@ ReactDOMComponent.Mixin = {
    * @return {string} Markup of opening tag.
    */
   _createOpenTagMarkupAndPutListeners: function(transaction, props) {
+    if (__DEV__) {
+      getCssClassFromProps.isExecuting = true;
+    }
     var ret = '<' + this._currentElement.type;
 
     for (var propKey in props) {
@@ -647,6 +653,10 @@ ReactDOMComponent.Mixin = {
       }
     }
 
+    if (__DEV__) {
+      getCssClassFromProps.isExecuting = false;
+    }
+
     // For static pages, no need to put React ID and checksum. Saves lots of
     // bytes.
     if (transaction.renderToStaticMarkup) {
@@ -824,6 +834,9 @@ ReactDOMComponent.Mixin = {
    * @param {?DOMElement} node
    */
   _updateDOMProperties: function(lastProps, nextProps, transaction) {
+    if (__DEV__) {
+      getCssClassFromProps.isExecuting = true;
+    }
     var propKey;
     var styleName;
     var styleUpdates;
@@ -833,6 +846,9 @@ ReactDOMComponent.Mixin = {
          lastProps[propKey] == null) {
         continue;
       }
+      if (propKey === CLASS) {
+        continue;
+      }
       if (propKey === STYLE) {
         var lastStyle = this._previousStyleCopy;
         for (styleName in lastStyle) {
@@ -947,6 +963,9 @@ ReactDOMComponent.Mixin = {
         this
       );
     }
+    if (__DEV__) {
+      getCssClassFromProps.isExecuting = false;
+    }
   },
 
   /**

diff --git a/src/test/__tests__/ReactTestUtils-test.js b/src/test/__tests__/ReactTestUtils-test.js
@@ -15,6 +15,7 @@ var React;
 var ReactDOM;
 var ReactDOMServer;
 var ReactTestUtils;
+var getCssClassFromProps;
 
 describe('ReactTestUtils', function() {
 
@@ -23,15 +24,16 @@ describe('ReactTestUtils', function() {
     ReactDOM = require('ReactDOM');
     ReactDOMServer = require('ReactDOMServer');
     ReactTestUtils = require('ReactTestUtils');
+    getCssClassFromProps = require('getCssClassFromProps');
   });
 
   it('should have shallow rendering', function() {
     var SomeComponent = React.createClass({
       render: function() {
         return (
           <div>
-            <span className="child1" />
-            <span className="child2" />
+            <img src="child1" />
+            <img src="child2" />
           </div>
         );
       },
@@ -42,8 +44,8 @@ describe('ReactTestUtils', function() {
 
     expect(result.type).toBe('div');
     expect(result.props.children).toEqual([
-      <span className="child1" />,
-      <span className="child2" />,
+      <img src="child1" />,
+      <img src="child2" />,
     ]);
   });
 
@@ -135,8 +137,8 @@ describe('ReactTestUtils', function() {
         } else {
           return (
             <div>
-              <span className="child1" />
-              <span className="child2" />
+              <img src="child1" />
+              <img src="child2" />
             </div>
           );
         }
@@ -147,8 +149,8 @@ describe('ReactTestUtils', function() {
     var result = shallowRenderer.render(<SomeComponent />);
     expect(result.type).toBe('div');
     expect(result.props.children).toEqual([
-      <span className="child1" />,
-      <span className="child2" />,
+      <img src="child1" />,
+      <img src="child2" />,
     ]);
 
     var updatedResult = shallowRenderer.render(<SomeComponent aNew="prop" />);
@@ -159,7 +161,7 @@ describe('ReactTestUtils', function() {
 
     var updatedResultCausedByClick = shallowRenderer.getRenderOutput();
     expect(updatedResultCausedByClick.type).toBe('a');
-    expect(updatedResultCausedByClick.props.className).toBe('was-clicked');
+    expect(getCssClassFromProps(updatedResultCausedByClick.props)).toBe('was-clicked');
   });
 
   it('can access the mounted component instance', function() {
@@ -209,12 +211,12 @@ describe('ReactTestUtils', function() {
     shallowRenderer.render(<SimpleComponent />);
     var result = shallowRenderer.getRenderOutput();
     expect(result.type).toEqual('div');
-    expect(result.props.className).toEqual('');
+    expect(getCssClassFromProps(result.props)).toEqual('');
     result.props.onClick();
 
     result = shallowRenderer.getRenderOutput();
     expect(result.type).toEqual('div');
-    expect(result.props.className).toEqual('clicked');
+    expect(getCssClassFromProps(result.props)).toEqual('clicked');
   });
 
   it('can setState in componentWillMount when shallow rendering', function() {