jsx transform: "for" should be renamed as "class" is

[piranha]

JSX renames class to className, but is not doing the same for for, which causes a bit of confusion.

[zpao]

I'm going to leave this open for now for some discussion, but (like we talked about on IRC), I don't think we should do this (and I think we should get rid of class as well). Take this:

var Component = React.createClass({
  render: function() {
    return <div class={this.props.className} />;
  }
});

React.renderComponent(<Component class="foo" />, document.body);

You can't actually access this.props.class in your component, you can only access this.props.className. It's fine and simpler down at your actual DOM nodes, but it means you have to actually think about this and recognize that you don't have access to that specific prop like it might be used in the JSX syntax.

Not everybody agrees with me, so let's start that discussion...

[piranha]

Well for me main concern was discoverability of those special cases. Right now compiler just silently eats 'for' and says nothing at all. And while this is more or less acceptable for other properties, React declares that it passes all HTML attributes. This note about className and htmlFor is not exactly easy to discover, so I think having warnings for them is very important for first-time users.

[zpao]

Totally agree. We should probably make #267 know about class and for and give helpful warnings there.

[jeffmo]

I'm a little out of context here, but why is it a problem to have a property name of for?

[chenglou]

@jeffmo because you access it in the child using this.props.htmlFor rather than this.props.for. Might throw people off.

[jeffmo]

I see...and the reason these things are renamed is really just because of issues when dealing with old versions of IE right? (because they don't like it when you access object non-computed properties named the same as a keyword)

Under the assumption that's the reasoning for all this, I think we might need a more general solution than just making up case-by-case magically renamed identifiers for all JS keywords. Maybe we come up with a more standardized renaming scheme for keyword attributes? Like class -> classProp and for -> forProp (where a legit property called forProp would throw an error if there's a collision). At least this way you only need to learn this nuance once and after that you can always remember how to reference the keyword-props... (I'm not tied to this particular renaming scheme...just ad-hoc suggestion)

Also, (still assuming stupid old IE is the only reason for all this) for people who don't care about old IE it would be nice if we set both class and classProp so that "normal people" can do this.props.class and this.props.for.

I'm positive this topic has been discussed before -- I just wasn't around for the discussion, so anyone feel free to point out anything I'm missing.

[sophiebits]

As far as I know, the only thing broken about class and for as property names in old IE is that you need to do ['class'] rather than .for, which doesn't seem like that terrible. Maybe we don't need to change the name at all.

[jeffmo]

In fact, I would agree that's not a terrible thing to do if you care about IE7 -- and anyone who thinks it looks ugly should blame IE7, not us :)

[chenglou]

I recall @zpao saying that this.props.class would break stuff in ES6?

[jeffmo]

I personally would love it if we didn't translate props like this at all though -- I probably should have mentioned that in my previous comment.

ES6 has a class keyword, but I don't believe the spec states that its illegal to use a keyword as a property identifier in a non-computed member expression... (I'll double-check that when I can get a chance -- but I'm fairly sure on that one). So these shouldn't be issues for anything except for old IE which sadly does whatever the hell it wants.

[jeffmo]

Yea ok -- chatted this over with @zpao and @jordwalke and I think we three agreed that it would be best if there were consistency between attribute names and the property identifiers you access them through.

That could mean one of two things: We either stop converting 'class' -> 'className' and just do this.props.class (with a computed property for those who care about IE7) or we start using 'className' as the attribute name and continue doing this.props.className -- but we definitely shouldn't continue adding to this conventional magic-prop-translation insanity.

[andreypopp]

I'm against using className as an attribute in JSX — it feels unnatural and may alienate someone from using JSX. I think it's better to stop converting class to className.

[chenglou]

So this.props.class is fine? One vote for class then.

[andreypopp]

@chenglou well, it is fine. Though I think, I have no voice on this issue cause I use CoffeeScript and it compiles obj.class to obj["class"]. :-)

[petehunt]

I think that the value of copying and pasting HTML and having a (mostly) working component is huge. So +1 to class :)

[sophiebits]

+1 for storing it as class and for in the props object.

[piranha]

a = {for: 1}; a.for also works absolutely fine. So I also vote for not renaming stuff at all.

[piranha]

@zpao @jeffmo @jordwalke @petehunt so what's the decision? Will we have 'class' or is it going to be a className? :-)

[zpao]

We just had a big discussion here. We're going to send out some proposals internally soon and I'll post them on the google group too (with link here in case you aren't signed up).

[zpao]

As promised: https://groups.google.com/forum/#!topic/reactjs/xovHWHGHPCA

[zpao]

We're going in the opposite direction and not supporting any prop transforms. The result of that decision and the ensuing discussion resulted in d83fe78 and the final decision that we will not be supporting class or for but will opt for keeping our DOM props as close to the JS API as we can.

@piranha, I really appreciate you taking the initiative here which forced us to make a decision.