hasClassName throws Error if component has property className which is not a string

[trmjoa]

Hi,

We use a utility like JedWatsons classnames(https://github.com/JedWatson/classnames) for setting classnames on our components. Syntax is like this:

<MyComponent className={ { classA : true, classB : false } } />

This makes the internal hasClassName and all functions in the api blow up with the exception TypeError: classes.replace is not a function since the property className is a object.

In my view the hasClassName function should not throw a exception if it encounters a custom component with a className attribute that is not a string. From my usage of enzyme I have the assumption that you are generally looking for native components when the hasClassName function is used. If that assumption is considered true I am tempted to suggest the following patch:

• If node.type is a non-native component(function) and className is not a string return false.
• If node.type is a non-native component and className is a string, run the check as today (for backwards compatibility)

It could probably also be possible to take the defined proptype into consideration.

/Joakim

[aweary]

This is documented in #375, you can watch that issue for updates! Thanks!

[aweary]

Also, classnames doesn't return an object. It takes an object and returns a string based on the key's value.

import classNames from 'classnames'

const className = classNames({
  foo: true,
  bar: true,
  baz: false
});
// returns "foo bar"

[trmjoa]

Hi,

Thanks for your input. Might be that I misunderstand the proposed fix in the referenced issue, but after I looked through the PR that is connected with the ticket you referenced, it seems to me that it will not solve the issue.

They are discussing a SVG specific solution(using baseVal) which does not solve the general naming collision where a custom created component has a className attribute. You are right that the classnames utility returns a string, and how we use it, is something like this:

import classNames from 'classnames';
class MyComponent extends React.Component{
  render() {
    <div className={ classNames( this.props.className ) }> { this.props.children} </div>
  }
}

ReactDOM.render(
  React.createElement(
    MyComponent, 
    { className : { classA : true, classB : shouldIAppendClassB(), classC : false }}
  )
);

Wouldn't it be more appropriate in the hasClassName method to assume the check is not applicable if the component has no className attribute that should be a string(defined by propTypes on custom components)? The problem comes in the find methods, because they iterate all components, also non-native.

Also, in the issue you linked, it seems to me that the proposed fix only affects the full rendering functionality, not the shallow part.

Best regards
Joakim

[aweary]

Thanks for expanding, we can re-open this as I agree that shallow shouldn't fail if a className prop is not a string.

[ljharb]

Is this still an issue in v3?