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

Feature request: Allow className to take an array #3138

Closed
trotzig opened this issue Feb 13, 2015 · 23 comments
Closed

Feature request: Allow className to take an array #3138

trotzig opened this issue Feb 13, 2015 · 23 comments

Comments

@trotzig
Copy link

trotzig commented Feb 13, 2015

I often use components that both have an internal className as well as an optional className passed in as a prop. The way I've been implementing this is either through (A) a local array that I construct and serialize into the className prop, or (B) through using React.addons.classSet.

A:

var classNames = ['my-component'];
if (this.props.className) {
  classNames.push(this.props.className);
}
return <MyComponent className={classNames.join(' ')} />;

B:

var classes = {
  'my-component': true
};
if (this.props.className) {
  classes[this.props.className] = true;
}
return <MyComponent className={React.addons.classSet(classes)} />;

A is slightly less verbose, but harder to use if you have modifier classes as well (e.g. my-component--is-selected).

I'm suggesting allowing the className to be a string or an array. And if it is an array, React internals would compact and serialize the className array into a className string.

return <MyComponent className={['my-component', this.props.className]} />;

If this is a good idea, I could make an attempt at a PR.

@janpaul123
Copy link

Actually you can just do

return <MyComponent className={['my-component', this.props.className].join(' ')} />;

If this.props.className is undefined, you will get an extra space in the className (class="my-component "), but who cares?

@sophiebits
Copy link
Contributor

You can also write className={'my-component ' + (this.props.className || '')}. See #1198 though – we're unlikely to do this due to perf.

@trotzig
Copy link
Author

trotzig commented Feb 13, 2015

Thanks @spicyj for that context! I figured this had already been investigated.

@pgom
Copy link

pgom commented Nov 24, 2016

Just a chance to revisiting this issue. This feature would be extremely useful instead of having to manipulate the classes beforehand. Also, I noticed a strange behaviour

<div className={['a', 'b']} /> compiles to <div className="a,b" /> which is invalid class definition.

Such feature is not being considered for future releases? /cc @spicyj

@BR0kEN-
Copy link

BR0kEN- commented Nov 26, 2016

Completely agree with @pgom. If array stringify, dividing items by comma, then why we can't just a bit rectify this behavior and use space as delimiter?

@BR0kEN-
Copy link

BR0kEN- commented Nov 26, 2016

I was a bit inaccurate. After walking through the code of React I've learned that className property ain't processed via custom logic. The "strange" behavior, that @pgom has mentioned, could be described by this construction: ['a', 'b'].toString() === 'a,b'. So for now React haven't any additional logic for this and implementation could lead to performance leaks (because, might be, we will need some checks for array items to not allow cases like [{}, []], for instance), as @spicyj said.

As a solution I can only propose: ['a', 'b'].join(' ').

@iorrah
Copy link

iorrah commented Feb 8, 2018

If what you want to achieve is something like this:

const myDivClassNames = ['a', 'b'];

...

<div className={myDivClassNames} /> // <div class="a b" />

... then you could possibly use the classnames utility as follows:

import classNames from 'classnames';

...

const myDivClassNames = classnames('a', 'b');

...

<div className={myDivClassNames} /> // <div class="a b" />

... or maybe:

import classNames from 'classnames';

...

const isCTrue = false;

const myDivClassNames = classnames(
  { 'a': true }, 
  { 'b': true }, 
  { 'c': isTrue }, 
);

...

<div className={myDivClassNames} /> // <div class="a b" />

@Jack-Works
Copy link
Contributor

className={["a", "b"]}
className={classnames(["a", "b"])}
className={["a", "b"].join(" ")}

Why can't we have the first way?

@kmbriedis
Copy link

To avoid breaking anything, it could be
classNames={["a", "b"]}

@stowball
Copy link

stowball commented May 2, 2019

I just stumbled in to this issue, so wrote a simple helper function:

const classNames = (arr) => arr.filter(Boolean).join(' ');

then you can do you pass anything to the array that generates a string, and not have to worry about falses etc being output:

<a className={classNames([linkStyle, props.selectedIndex === index && 'is-selected'])}>

@bashkovpd
Copy link

I'm using destructuring for that:
className={classNames(...["a", "b"])}

@shakepompom
Copy link

shakepompom commented Aug 21, 2019

I use template string. Works fine for me.
<Component className={`${firstClassName} ${secondClassName}`} />

@jackHedaya
Copy link

I know there are a couple of ways to do this but I think it would result in much cleaner syntax for className to accept an array.

@modiase
Copy link

modiase commented Jun 8, 2020

Can anyone explain the difficulty in implementing this? Vue seems to have managed it with no challenge.

@mattxo
Copy link

mattxo commented Jun 23, 2020

I'm doing...

toClassNames.ts
export default (values: string[]) => values.join(' ');

button.tsx

import toClassNames from '../functions/toClassNames';

classes = 
[
    "btn",
    "btn--large"
];    

render = () => <a className={toClassNames(this.classes)}>{this.props.children}</a>

would be mighty nice for className just to take an array tho :)

@YasirHasn9
Copy link

does the className read the arr of classes one by one ? so there a dynamic loop that iterates over the arr of classes and executes all the classes inside it ? if so , it means loop keeps working and allocates space in the memory and that would slow the performance or I'm just blah blah ... and don't know what I'm talking about :) ?

@artginzburg
Copy link

toClassNames.js gist looks like the best we've got for now...

mpcarolin added a commit to simpleviewinc/keg-hub that referenced this issue Sep 3, 2021
- className was being passed an array
- this is actually not accepted in React, and has been an open issue for awhile: facebook/react#3138
- I am now just joining the class list array as a space-separated string, which will always work
mpcarolin added a commit to simpleviewinc/keg-hub that referenced this issue Sep 9, 2021
* Add flexGrow shortcut: flG

* Update reStyle to use restructureTheme

* Address edges with useCompiledStyles + refactor

* Refactor, fix bugs

* Fix hook bug

* Add more rules

* Fixing some inaccurate jsdocs

* Fixing eslinter issue

* Fix misplaced comments

* Fix issue with size keys remaining in object

* Update unit tests to be more useful

* Refactor useCompiledStyles to be simpler

* Rename function

* Extract useCompiledStyles to dedicated file

* Add compat w/ nested styles to compileStyles

* Update reThemeProvider to use useCompiledStyles

* Make meta props configurable; Add docs

* Add tests and fix bugs

* Fix estlinting

* Move compile phase to StyleInjector for broad use

* Remove log

* Remove unused import

* Remove unused code

* Remove commented code

* Remove obsolete mock

* Improve border aliases to be more intuitive

* Add capability for default props to reStyle

* Add unit tests; Update docs

* Fix StyleInjector bug preventing css classes from rendering
- className was being passed an array
- this is actually not accepted in React, and has been an open issue for awhile: facebook/react#3138
- I am now just joining the class list array as a space-separated string, which will always work

* Remove unused imports

* Update docs

* Update useStyleTag & apply PR feedback
- useStyleTag: now returns string of classes space-separated
@Sodj
Copy link

Sodj commented Dec 14, 2022

This has been implemented in ReactNative ages ago, I don't understand why it's not the case in React

@90dy
Copy link

90dy commented Apr 5, 2023

@sophiebits to improve dev experience, I think you should implement https://npmjs.com/package/classnames natively in react, it's commonly used and having to import it everytime seems not something good subjectively

@artginzburg
Copy link

Is it really that big of a deal with performance? I never thought a check like if (typeof className === 'string') { ... proceed as always ...} else { classnames(className) } is that costly, and it keeps everything as it was for the cases when className is a string

@charlestbell
Copy link

+1 Coming from React Native, this is sorely missed.

@asos-dominicjomaa
Copy link

Would personally love this, so many hand rolled or external dependency based concatenators just so we can glue classnames together!

@Maxime-p
Copy link

Why this feature have been closed? This is a game changer

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