buttons / react-github-btn

:octocat: Unofficial github:button component for React.js
https://buttons.github.io
BSD 2-Clause "Simplified" License
62 stars 8 forks source link

React component #1

Closed danielvanc closed 5 years ago

danielvanc commented 5 years ago

I've just noticed the buttons no longer work on my React site. And i see that the ReadME file has taken out the 'for react component' section. Are you no longer supporting React for this?

ntkme commented 5 years ago

It’s moved to: https://github.com/ntkme/react-github-button

I’m still looking at how exactly this shall be published.

Apparently publish it as ES6 module would have some benefits to the final bundle size as it would deduplicate polyfills on final bundle. However, publish as ES6 meaning users’ babel / webpack configurations need to transpile packages in node_modules, and it looks like not everyone have that configured.

danielvanc commented 5 years ago

Ah okay, I'm so glad it's still being kept alive. I'm using it on a GatsbyJS site, so am happy with carrying on using as it is. When do you think it will be ready to use?

ntkme commented 5 years ago

The official create-react-app has no problem transpiling ES6 class / module syntax to ES5 for packages in node_modules. Other than ES6 class / module, the code here is actually ES3 compatible.

Therefore I decided to just publish it as ES6, and it's available on npm:

https://www.npmjs.com/package/react-github-btn

daniel-vc commented 5 years ago

Thanks @ntkme! Will try it later 👍

danielvanc commented 5 years ago

Seems to be working great. Thanks again 👍

danielvanc commented 5 years ago

Need to re-open this as I'm receiving this error after a hot reload. I'm using GatsbyJS by the way.

Screenshot 2019-03-12 at 23 12 38
ntkme commented 5 years ago
  componentDidMount () {
    this.paint()
  }
  componentWillUpdate () {
    this.reset()
  }
  componentDidUpdate () {
    this.paint()
  }
  componentWillUnmount () {
    this.reset()
  }

The error you see could happen only if this.reset() is called twice in a row without this.paint() in between, which should never happen in a proper lifecycle. Somehow your react life cycle is broken by hot reload.

danielvanc commented 5 years ago

There is nothing to suggest that my react life cycle is broken by hot reload, especially when this is the only component breaking. I'm just running from a standard GatsbyJS v2 install. With a simple GH Button import with 3 GH buttons appearing on the page.

ntkme commented 5 years ago

Well, another possibility is that a component is relocated in DOM without properly unmount and then remount. The current implementation relies on proper unmount and remount when component get moved around in VDOM, if that's not the case, then it's screwed.

ntkme commented 5 years ago

Can you try if 1.0.3 still has this problem?

danielvanc commented 5 years ago

Yeah that seems to have worked! :) What was changed?

ntkme commented 5 years ago

It was my second guess then. The component was moved with in DOM during the life cycle. In order to make sure DOM modification does not interfere with ability to relocate, the root element for the component cannot change. Basically I wrapped another <span> that does not change to be the root element:

https://github.com/ntkme/react-github-btn/commit/71f5b7b264c19e8870230be17bf05b4d452ae44f#diff-168726dbe96b3ce427e7fedce31bb0bcR11

danielvanc commented 5 years ago

Excellent - good work! 👍

yujiangshui commented 5 years ago

I have an awkward situation about ES6 in node_modules. I'm using next.js to develop my application, it's not easy to custom webpack config to transpile ES6 packages in node_modules. If I use the HTML version of GitHub Buttons, the data-xxx on a tag will cause a TypeScript error, so that I have to add a new interface. I don't want to do anything of them. So, I choose to copy this component code and import github-buttons...

But after I try, I find another problem, github-buttons use window object and not compatible with next.js so I can only use HTML version.

ntkme commented 5 years ago

@yujiangshui Can you open a new issue and be explicit about the errors?