react-clipboard.js icon indicating copy to clipboard operation
react-clipboard.js copied to clipboard

Support Stateless custom components

Open nihey opened this issue 6 years ago • 9 comments

This could be done by wrapping the custom component in a stateful component. (Suggestion by @uyouthe)

nihey avatar Jun 04 '19 21:06 nihey

This is my code:

class WrappedButton extends Component {
  render () {
    return (
      <Button { ...this.props }>
        { this.props.children }
      </Button>
    )
  }
}

...

component={ WrappedButton }

This fails miserably:

Uncaught TypeError: First argument must be a String, HTMLElement, HTMLCollection, or NodeList

I'm using the latest version of your library.

textpain avatar Jun 05 '19 15:06 textpain

@nihey I strongly suggest you not to control children. Let them be PropTypes.any.

textpain avatar Jun 05 '19 15:06 textpain

@uyouthe I believe it was actually caused by specifying the propTypes of component. I've just made a release changing that (2.0.11), it should work sucessfully.

nihey avatar Jun 05 '19 17:06 nihey

@nihey checked this out. I'm still getting this error

Warning: Failed prop type: Invalid prop `children` supplied to `ClipboardButton`.

no matter stateless or stateful component I pass.

textpain avatar Jun 05 '19 17:06 textpain

@uyouthe I've just updated to use propTypes.any on children too on 2.0.12, could you try it out?

nihey avatar Jun 05 '19 18:06 nihey

It works. Great job so far.

Now you clearly can see how much of unusual complexity do PropTypes offer. Please, don't use some tool just because "it's a de-facto standard" or because "everyone use this, it's a best practice to use this".

Please, consider tooling responsibly. I'd once launched a cluster Docker/CouchDB/ClojureScript startup without a single type-checking tool or even a single line of code that check types and encounter only 3 (three) bugs in 8 months, all of them JS-related.

Maybe you don't always need PropTypes or explicit types at all.

textpain avatar Jun 05 '19 18:06 textpain

It clearly does, one thing that I should eventually implement are the unit and/or e2e tests. Even though I maintain the library, I do not use all of its features, so not all of it is tested on every release :/.

Maybe adding some unit tests to cover most of the documented features could improve the quality assurance of each new version.

nihey avatar Jun 05 '19 18:06 nihey

I personally think that you need neither unit tests nor proptypes. Consider investing time in decent specs though.

Code problems aren't always solved by code. The more code you have the more bugs are to come so maybe we should solve as much problems as possible without any code. Maybe we should try other things: plain-text docs, community chats, decent FAQs and other things like this.

textpain avatar Jun 05 '19 18:06 textpain

I'm confused as to how I may achieve this. In fact I can't figure out how to get anything to render at all, even in a fully connected component.

I'm a bit new to react so thats probably the issue. Either way I tried my best to follow the examples on the readme.

derek-adair avatar Aug 14 '19 19:08 derek-adair