nihey / react-clipboard.js

React wrapper for clipboard.js (flashless clipboard)
https://www.npmjs.com/package/react-clipboard.js
273 stars 38 forks source link

Support Stateless custom components #75

Open nihey opened 5 years ago

nihey commented 5 years ago

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

miloxeon commented 5 years ago

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.

miloxeon commented 5 years ago

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

nihey commented 5 years ago

@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.

miloxeon commented 5 years ago

@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.

nihey commented 5 years ago

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

miloxeon commented 5 years ago

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.

nihey commented 5 years ago

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.

miloxeon commented 5 years ago

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.

derek-adair commented 5 years ago

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.