React: Pass "className" through classnames module

Created on 25 Jul 2017  Â·  18Comments  Â·  Source: facebook/react

Hi!

I usually am in the situation where I have to write code like this:

return (
  <div className={classnames(['...', '...'])}>
    Children
  </div>
);

This seems simple, but when you have more elements that need dynamic classes, it gets tedious and pretty hard to read. I propose the following thing:

When className attribute is evaluated, how about running it through classnames before rendering the HTML?

This means that the className attribute should allow Array<string>, {[string]: boolean} and string types and would become much simpler to write code.

What do you guys think?

Most helpful comment

I have written a babel plugin (babel-plugin-transform-jsx-classnames) to automatically add classnames:

Code:

<button
  className={{
    'btn-active': active,
    'btn-disabled': disabled,
  }}>
</button>

Will transform to:

import classNames from 'classnames'

<button
  className={classNames({
    'btn-active': active,
    'btn-disabled': disabled,
  })}>
</button>

You may need it.

All 18 comments

So, you basically want to get rid of the classnames library and move the functionality to react? If things are getting hard to read, then why not just declare the component class name before creating the component?

const xClassName = classnames(
  {  'foo': bar },
  ...bars,
  'foo'
);
return (
  <div className={xClassName}>
    Children
  </div>
);

Hi!
This works when you have one, maybe two elements that need this, but when
you have more than two, the code gets hard to read as well not to mention
that you have to find names for variables with basically no meaning.

So does React currently support adding such a "middleware"? Or does the
core need to be modified?

On Thu, 27 Jul 2017 at 00:16, Ian Howell notifications@github.com wrote:

So, you basically want to get rid of the classnames library and move the
functionality to react? If things are getting hard to read, then why not
just declare the component class name before creating the component?

const xClassName = classNames(
{ 'foo': bar },
...bars,
'foo'
);
return (

Children

);

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/10271#issuecomment-318184841,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AG73y0rp1cZnWPTWr2cjSy2NgMynsYlVks5sR6ypgaJpZM4OiQRK
.

>

Victor Barbu

Could you give an example of how this middleware could work?

Well, I honestly don't know the codebase of React so I couldn't have any opinion on this, I just know I need it 😃

I meant, could you give an example of how you would use it, would what you want be implemented?

Well, I mentioned the types that className should accept, so here it is:

<component className="myClass" />
<component className={['myClass', 'yourClass']} />
<component className={{myClass: true, yourClass: false}} />

The problem with baking it into React is that now all third party components need to understand this. For example:

<BootstrapButton className={{myClass: true, yourClass: false}} />

but what if it wants to add .BootstrapButton to the class?

Keeping the helper separate keeps your app in control over when to turn it into a string. Baking it into ReactDOM <div>s makes it hard for libraries to manipulate className while it is still being passed around.

The solution I am proposing also accepts className to be a string so such a
change would only add behavior, not change the current one.

On Thu, 27 Jul 2017 at 18:02, Dan Abramov notifications@github.com wrote:

The problem with baking it into React is that now all third party
components need to understand this. For example:

but what if it wants to add .BootstrapButton to the class?

Keeping the helper separate keeps your app in control over when to turn
it into a string. Baking it into ReactDOM

s makes it hard for
libraries to manipulate className while it is still being passed around.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/10271#issuecomment-318388581,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AG73y10DZY1q9dndautsAGOI7dELkEwvks5sSKZygaJpZM4OiQRK
.

>

Victor Barbu

...change would only add behavior, not change the current one.

It doesn't change the behavior for className on DOM nodes, but it does for intermediary components, which is what @gaearon is pointing out.

<Button className={{myClass: true, yourClass: false}} />

In the above example lets look at how the implementation would look now.

const Button = ({ className, ...props }) => (
  <button className={className + ' default-button-class'} {...props} />
)

This is safe to do currently because everyone assumes className is a string. Your proposal would mean that className in this case _may_ be a string, or an object, or an array of objects or strings (assuming we copy the classNames api completely). Now me, the Button owner, needs to check what type className is before adding my own class to it.

Ok I see your point. But would it be possible (for the convenience of
writing less code when you instance a component like I'm proposing) to
convert the className to a string before the component'a constructor sees
it?

On Thu, 27 Jul 2017 at 18:25, Jason Quense notifications@github.com wrote:

...change would only add behavior, not change the current one.

It doesn't change the behavior for className on DOM nodes, but it does for
intermediary components, which is what @gaearon
https://github.com/gaearon is pointing out.

Victor Barbu

Where would the conversion happen?

@barbu110 If you find yourself adding classnames to multiple elements in a single component, then that might be a good indicator that you should split them into smaller components.

Using small components as visual primitives will make your app more composable down the road :smile:

Perhaps a generic way of doing this so that it can be configured on a global scale. I'm thinking that maybe there should be an API that looks something like:

React.addPropTranslator("className", classnames)

which then causes React.createElement to scan apply the function classnames to any prop in any element that's called className before the value is sent to the component's constructor. This would seem to satisfy any backwards compatibility requirements, because all components would receive the translated class name string, rather than any non-standard forms that may be passed as an argument to the classnames module itself, so everything would continue working the same way as today from the component's perspective -- only the component creator would need to be aware that the translation was happening.

I have written a babel plugin (babel-plugin-transform-jsx-classnames) to automatically add classnames:

Code:

<button
  className={{
    'btn-active': active,
    'btn-disabled': disabled,
  }}>
</button>

Will transform to:

import classNames from 'classnames'

<button
  className={classNames({
    'btn-active': active,
    'btn-disabled': disabled,
  })}>
</button>

You may need it.

@meowtec If you just give it a normal class name, does it not add the className?

<button
  className={'my-class'}
>
  Button text
</button>

@ianshowell
It should not, but I did not deal with this case, so that anything inside braces will be add classnames().

The only excepted case is

<button className='my-class'></button>

It will stay the same.

I appreciate the discussion but I don’t think there’s any way to fulfil the feature request without significant downsides.

In general I suggest to not worry so much about a little extra typing. It is not going to make or break your application. I think we should strive for clear data flow, and keeping components easy to compose, rather than avoid a few extra keystrokes here and there but compromise on those goals.

Perhaps a generic way of doing this so that it can be configured on a global scale. I'm thinking that maybe there should be an API that looks something like:

Global APIs have pretty bad effects on the ecosystem because components written assuming one configuration won’t work with the other global configuration. This is exactly why React essentially doesn’t have any global configuration at all.

Was this page helpful?
0 / 5 - 0 ratings