From 0b45aa2aa540eba815b3085e3249343e24c1d556 Mon Sep 17 00:00:00 2001 From: Jim Date: Wed, 3 Feb 2016 05:02:07 -0800 Subject: [PATCH] Started setting both props.class and props.className for DOM elements --- src/addons/getCssClassFromProps.js | 36 +++++++++++++ .../classic/element/ReactElement.js | 52 +++++++++++++++++++ .../element/__tests__/ReactElement-test.js | 8 +-- src/renderers/dom/shared/ReactDOMComponent.js | 18 +++++++ src/test/__tests__/ReactTestUtils-test.js | 24 +++++---- 5 files changed, 124 insertions(+), 14 deletions(-) create mode 100644 src/addons/getCssClassFromProps.js diff --git a/src/addons/getCssClassFromProps.js b/src/addons/getCssClassFromProps.js new file mode 100644 index 0000000000000..4b29ff573d912 --- /dev/null +++ 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 index 6e371596d06d1..19023879beafa 100644 --- a/src/isomorphic/classic/element/ReactElement.js +++ b/src/isomorphic/classic/element/ReactElement.js @@ -16,6 +16,8 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var assign = require('Object.assign'); var warning = require('warning'); 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. @@ -32,6 +34,42 @@ var RESERVED_PROPS = { var specialPropKeyWarningShown, specialPropRefWarningShown; +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 @@ -144,6 +182,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 @@ -250,6 +291,9 @@ ReactElement.cloneAndReplaceKey = function(oldElement, newKey) { }; ReactElement.cloneElement = function(element, config, children) { + if (__DEV__) { + getCssClassFromProps.isExecuting = true; + } var propName; // Original props are copied @@ -295,6 +339,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; @@ -308,6 +356,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 index 402c9b95eddef..dca74216805d0 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElement-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElement-test.js @@ -14,6 +14,7 @@ var React; var ReactDOM; var ReactTestUtils; +var getCssClassFromProps; describe('ReactElement', function() { var ComponentClass; @@ -30,10 +31,11 @@ describe('ReactElement', 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. + getCssClassFromProps = require('getCssClassFromProps'); ComponentClass = React.createClass({ render: function() { + // NOTE: We're explicitly not using JSX here. This is intended to test + // classic JS without JSX. return React.createElement('div'); }, }); @@ -371,7 +373,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 index 244d80b94fa35..f310ea774b869 100644 --- 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'); @@ -54,6 +55,7 @@ var registrationNameModules = EventPluginRegistry.registrationNameModules; var CONTENT_TYPES = {'string': true, 'number': true}; var STYLE = keyOf({style: null}); +var CLASS = keyOf({'class': null}); var HTML = keyOf({__html: null}); var RESERVED_PROPS = { children: null, @@ -612,6 +614,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) { @@ -655,6 +660,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) { @@ -832,6 +841,9 @@ ReactDOMComponent.Mixin = { * @param {?DOMElement} node */ _updateDOMProperties: function(lastProps, nextProps, transaction) { + if (__DEV__) { + getCssClassFromProps.isExecuting = true; + } var propKey; var styleName; var styleUpdates; @@ -841,6 +853,9 @@ ReactDOMComponent.Mixin = { lastProps[propKey] == null) { continue; } + if (propKey === CLASS) { + continue; + } if (propKey === STYLE) { var lastStyle = this._previousStyleCopy; for (styleName in lastStyle) { @@ -956,6 +971,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 index a7cf4ab846536..95cef500f311f 100644 --- 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,6 +24,7 @@ describe('ReactTestUtils', function() { ReactDOM = require('ReactDOM'); ReactDOMServer = require('ReactDOMServer'); ReactTestUtils = require('ReactTestUtils'); + getCssClassFromProps = require('getCssClassFromProps'); }); it('should have shallow rendering', function() { @@ -30,8 +32,8 @@ describe('ReactTestUtils', function() { render: function() { return (
- - + +
); }, @@ -42,8 +44,8 @@ describe('ReactTestUtils', function() { expect(result.type).toBe('div'); expect(result.props.children).toEqual([ - , - , + , + , ]); }); @@ -135,8 +137,8 @@ describe('ReactTestUtils', function() { } else { return (
- - + +
); } @@ -147,8 +149,8 @@ describe('ReactTestUtils', function() { var result = shallowRenderer.render(); expect(result.type).toBe('div'); expect(result.props.children).toEqual([ - , - , + , + , ]); var updatedResult = shallowRenderer.render(); @@ -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() { @@ -215,12 +217,12 @@ describe('ReactTestUtils', function() { shallowRenderer.render(); 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() {