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

Allow className and for in JSX element declaration #4433

Closed
gajus opened this issue Jul 20, 2015 · 22 comments
Closed

Allow className and for in JSX element declaration #4433

gajus opened this issue Jul 20, 2015 · 22 comments

Comments

@gajus
Copy link
Contributor

gajus commented Jul 20, 2015

Since JSX is JavaScript, identifiers such as class and for are discouraged as XML attribute names. Instead, React DOM components expect DOM property names like className and htmlFor, respectively.

from: https://facebook.github.io/react/docs/jsx-in-depth.html

  • Instead of <div className='foo'> we should be able to have <div class='foo'>.
  • Instead of <div htmlFor='bar'> we should be able to have <div for='bar'>.

It is true that class and for are reserved keywords in ECMAScript. However, JSX is not part of the ECMAScript. JSX is being transpiled to JavaScript. Just because JSX can be inlined with JavaScript, it should not be restricted by the same limitations. The transpiler should carry the weight of the incompatibilities.

Babel

Babel transpiler is able to read class and for attributes for what they are and convert them to:

React.createElement('div', { 'class': 'foo' });
React.createElement('div', { 'for': 'bar' });

Refer to: babel/babel#2039

React should support class and for element properties and not throw a warning (Warning: Unknown DOM property class. Did you mean className?).

@gajus
Copy link
Contributor Author

gajus commented Jul 20, 2015

While it is true that we are writing virtual DOM, the markup is identical to HTML. Developer does not need to be aware that in DOM API HTMLElement has a property htmlFor and not for because of JavaScript reserved keywords. The fact is that developer is trying to represent DOM structure described using HTML/XML like markup.

@StoneCypher
Copy link

this issue has already been raised and declined many times

@gajus
Copy link
Contributor Author

gajus commented Jul 20, 2015

I have look through 10 pages of issues and found only this issue discussing this very problem. The latter issue did not give any arguments to support the use of class attribute. Whats the name of the original discussion thread?

@gajus
Copy link
Contributor Author

gajus commented Jul 20, 2015

Found: #2781

We're already years deep on this commitment so we're not backing out, sorry.

@gajus gajus closed this as completed Jul 20, 2015
@sophiebits
Copy link
Collaborator

Copying from my Quora answer:

We certainly could have done it the other way around. I argued for that for a while. There are transforms that convert this.props.class to this.props['class'] and so using the names class and for would be nearly seamless. Babel includes one. We're sticking with className and htmlFor for a couple of reasons:

First, we tend to look at HTML properties (like el.className = ...) when possible, as opposed to attributes (like el.setAttribute('class', ...)). Attributes are always string-typed, while properties can have any JavaScript object value, which gives more flexibility in some circumstances. One example is the .classList property, which arguably is a better fit for the data model than .className is. React doesn't support classList right now, but it certainly could. Given that React's className behaves like the HTML property of the same name, it makes sense to keep that name.

Another reason is more forward-thinking. In the future, idiomatic React may use object destructuring to pick apart this.props. The react-future repo shows one example of how this could work. Even in modern browsers, this wouldn't work with class and for which are keywords and can't appear as standalone identifiers even though they can appear as property names.

Third, our thinking is that JSX's primary advantage is the symmetry of matching closing tags which make code easier to read, not the direct resemblance to HTML or XML. It's convenient to copy/paste HTML directly, but other minor differences (in self-closing tags, for example) make this a losing battle and we have a HTML to JSX converter to help you anyway. Finally, to translate HTML to idiomatic React code, a fair amount of work is usually involved in breaking up the markup into components that make sense, so changing class to className is only a small part of that anyway.

@gajus
Copy link
Contributor Author

gajus commented Jul 20, 2015

Thank you @spicyj

@dantman
Copy link
Contributor

dantman commented Aug 18, 2015

@spicyj Ok, I guess object destructuring and how class and for cannot be variable names does make sense. Though keep in mind that this is only relevant in the context of making it "convenient" for developers, in the sense that they can use the same variable names as prop names. Not in any way a case of it being invalid or not working with destructuring. Because {className, style} is only the shorthand sugar of object destructuring, you are not forced to use the same variable names as property names. You just use the full object destructuring syntax {class: className, style}, {class: cls, style}, etc.

@cmmartin
Copy link

If you just want it to work, it's pretty simple to write a babel transform to convert them for you. Here is an example...

var transform = new babel.Transformer('class-to-classname', {
    JSXAttribute: {
      exit: function (node, parent, scope, file) {
        if (node.name.name === 'class') {
          node.name.name = 'className';
        } else if (node.name.name === 'for') {
          node.name.name = 'htmlFor';
        }
        return node;
      }
   }
})

@dantman
Copy link
Contributor

dantman commented Aug 19, 2015

@cmmartin That sounds like a bad idea. That makes class in JSX work. However the prop is still className and still accessed via this.props.className. As a result you'll end up with confused developers using class="" in their code but scratching their heads when this.props.class doesn't contain the classes they passed.

@cmmartin
Copy link

Correct. Although, I'd argue that your developers should know that class is a reserved word in javascript. The workaround shouldn't be a surprise either given that the browser already works exactly this way. The html attribute is class and the corresponding javascript property of DOM objects is className

@sophiebits
Copy link
Collaborator

@cmmartin Your suggestion might make sense for DOM components but would be utterly surprising for a user-defined component if you write <Foo class={...} /> and that means className. A very old version of the JSX transform did do this transformation and it was scrapped because it is way too surprising.

@cmmartin
Copy link

Agreed. I only explored this solution as a means to paste chunks of DOM written by designers into high level React components. I'm not so much advocating it as suggesting a possibility for those in similar situations. Aside from these special cases, I think the current solution works best.

@sophiebits
Copy link
Collaborator

Maybe use https://facebook.github.io/react/html-jsx.html instead.

@ericclemmons
Copy link
Contributor

Posting for future readers:

https://github.com/insin/babel-plugin-react-html-attrs

@sophiebits
Copy link
Collaborator

The JSX transform used to do that, but we don't recommend it because it's confusing if you ever try to pass class= or for= as a prop to your own components – you wouldn't expect to access them with a different name.

@ericclemmons
Copy link
Contributor

⬆️ That's correct & very valid. Just adding some flava flave to the thread. 🕐

@DrewWarrenTIY
Copy link

I believe there is a typo in your example @gajus . In the second bullet point of your original post you say "instead of htmlFor=bar we should be able to use foo=bar." Should that say for=bar?

If I'm wrong, sorry for the distraction! It was confusing to me until I realized, though.

@StoneCypher
Copy link

@DrewWarrenTIY - the standard placeholders for things in code are, in order, "foo, bar, baz, quux, corge, grault, flarp."

@DrewWarrenTIY
Copy link

@StoneCypher - Feel free to correct me if I am wrong, but in his example in the first post of this conversation, @gajus gives us two examples of how this plugin is used to simplify writing our JSX code.

Instead of having to type 'className' in our code, with this plug-in we can simply type 'class'
Instead of having to type 'htmlFor' in our code, with this plug-in we can simply type 'for'

However, in the example above, he shortens 'htmlFor' to 'foo' . I believe it is supposed to be 'for'.

Thanks for your reply.

@gajus
Copy link
Contributor Author

gajus commented Nov 2, 2016

However, in the example above, he shortens 'htmlFor' to 'foo' . I believe it is supposed to be 'for'.

That was a typo.

For the record, I'd like to withdraw my support for this proposal. I have been working with React now for couple of years. There is hardly ever a use case for this in practice.

@DrewWarrenTIY
Copy link

Thank you @gajus
I'm in a coding bootcamp and just learning the basics of React. This was helpful for me so I'm glad I found this article. Even if it's deprecated now it helped me in my beginner exercises. :)

I appreciate your response!

@ericclemmons
Copy link
Contributor

Interestingly enough, https://github.com/insin/babel-plugin-react-html-attrs continues to be one of the first installs when we convert a project to React.

Apparently adding .eslintrc is more divisive ;)

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

No branches or pull requests

7 participants