Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/addons/getCssClassFromProps.js
Original file line number Diff line number Diff line change
@@ -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;
52 changes: 52 additions & 0 deletions src/isomorphic/classic/element/ReactElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -308,6 +356,10 @@ ReactElement.cloneElement = function(element, config, children) {
props.children = childArray;
}

if (__DEV__) {
getCssClassFromProps.isExecuting = false;
}

return ReactElement(
element.type,
key,
Expand Down
8 changes: 5 additions & 3 deletions src/isomorphic/classic/element/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
var React;
var ReactDOM;
var ReactTestUtils;
var getCssClassFromProps;

describe('ReactElement', function() {
var ComponentClass;
Expand All @@ -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');
},
});
Expand Down Expand Up @@ -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;
},
Expand Down
18 changes: 18 additions & 0 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -956,6 +971,9 @@ ReactDOMComponent.Mixin = {
this
);
}
if (__DEV__) {
getCssClassFromProps.isExecuting = false;
}
},

/**
Expand Down
24 changes: 13 additions & 11 deletions src/test/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var React;
var ReactDOM;
var ReactDOMServer;
var ReactTestUtils;
var getCssClassFromProps;

describe('ReactTestUtils', function() {

Expand All @@ -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>
);
},
Expand All @@ -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" />,
]);
});

Expand Down Expand Up @@ -135,8 +137,8 @@ describe('ReactTestUtils', function() {
} else {
return (
<div>
<span className="child1" />
<span className="child2" />
<img src="child1" />
<img src="child2" />
</div>
);
}
Expand All @@ -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" />);
Expand All @@ -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() {
Expand Down Expand Up @@ -215,12 +217,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() {
Expand Down