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

Use 'class' and 'for' for DOM property names #1223

Closed
wants to merge 1 commit into from

Conversation

sophiebits
Copy link
Collaborator

React fixes inconsistencies in the DOM and avoids being held back by older browsers (to the extent possible). The only reason that the DOM authors chose className and htmlFor was because the JS engines of the time didn't support using class and for as unquoted property names.

Changing React to use class and for agrees with our principles of improving the DOM and using modern JS, and is what most people expect when first starting with React.

React fixes inconsistencies in the DOM and avoids being held back by older browsers (to the extent possible). The only reason that the DOM authors chose `className` and `htmlFor` was because the JS engines of the time didn't support using `class` and `for` as unquoted property names.

Changing React to use `class` and `for` agrees with our principles of improving the DOM and using modern JS, and is what most people expect when first starting with React.
@@ -162,7 +162,7 @@ var DOMPropertyOperations = {
var propName = DOMProperty.getPropertyName[name];
var defaultValue = DOMProperty.getDefaultValueForProperty(
node.nodeName,
name
propName
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(A bug that no one noticed before.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix this independently? Also a new test? I'm not convinced yet that we'll take the rest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so. #1225

@chenglou
Copy link
Contributor

chenglou commented Mar 6, 2014

That sure is a valid argument

sophiebits added a commit to sophiebits/react that referenced this pull request Mar 6, 2014
This would make facebook#1223 easier to deal with in older browsers.

With this change,

    <div class="orange" />

compiles into

    React.DOM.div( {"class":"orange"} );

and `this.props.class` turns into `this.props["class"]`.
@joecritch
Copy link
Contributor

+1

@akre54
Copy link
Contributor

akre54 commented Mar 10, 2014

Don't the reserved words (class and for) still need to be quoted for pre-ES5 environments?

@chenglou
Copy link
Contributor

@akre54 that's what #1224 is for

@akre54
Copy link
Contributor

akre54 commented Mar 10, 2014

Oh sweet. Good stuff.

@lrowe
Copy link
Contributor

lrowe commented Mar 20, 2014

Finding a way to make HTML attribute names work would definitely make React with JSX more approachable for new developers. But it has to be consistent - non-camelCased attribute names need to work as would attribute names with hyphens.

@sophiebits
Copy link
Collaborator Author

Unfortunately this won't work with object destructuring, like

var {class, color} = this.props;

@sophiebits sophiebits closed this Jun 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants