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

Merge className when using spread operator #2440

Closed
smussell opened this issue Oct 31, 2014 · 5 comments
Closed

Merge className when using spread operator #2440

smussell opened this issue Oct 31, 2014 · 5 comments

Comments

@smussell
Copy link

When using transferPropsTo the className property would be merged, this is not true when using the new spread operator. This was one of the nicest features in transferPropsTo, it made extending existing components much easier. I created a couple of fiddles to illustrate:

0.12:
http://jsfiddle.net/kb3gN/7005/

0.11.2:
http://jsfiddle.net/qjohd7ut/

I'm not sure if this would be considered a bug, I can understand there would be reasons not to do it, but it is incredibly convenient. If this is not possible it would be nice to have some utility method or something to do className merging, maybe as part of add-ons.

@andreypopp
Copy link
Contributor

There's React.addons.classSet which is often abbreviated as cx:

var cx = React.addons.classSet
...
render() {
  var {className, ...props} = this.props
  return <div {...props} className={cx(className, 'someClassName')} />
}

@syranide
Copy link
Contributor

The spread operator is part of the ES6/7 spec, JSX shouldn't change implementation details (which is also specific to DOM components). You'll have to do something like {...this.props, className: mergeClassNames('...', this.props.className)}.

Closing as this is intended behavior.

@sebmarkbage
Copy link
Collaborator

joinClasses is an internal helper that we could expose as an addon. You can also just use simple concatentation:

var { className = '', ...props } = this.props;
return <div {...props} classList={className + ' someClassName'} />;

I was hoping on being able to do something like:

var { classList, ...props } = this.props;
return <div {...props} classList={[ ...classList, 'someClassName' ]} />;

But that might not be very performant.

Either way, it's more explicit and less magical. It should make it easier to understand what's going on. That tends to be more convenient than saving the typing. The problem with magic is that you don't know which props have magic behavior and what exactly it might do.

@browniefed
Copy link

Is there someway to call this out in the release notes for .12, this wasn't made very explicit

@teophilus
Copy link

teophilus commented Mar 25, 2020

You can also try:

className={`custom-class ${className}`}

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

6 participants