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

React component #1

Closed
danielvanc opened this Issue Mar 5, 2019 · 13 comments

Comments

Projects
None yet
3 participants
@danielvanc
Copy link

danielvanc commented Mar 5, 2019

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

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 5, 2019

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

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 5, 2019

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 ntkme transferred this issue from ntkme/github-buttons Mar 8, 2019

@ntkme

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 8, 2019

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

@ntkme ntkme closed this Mar 8, 2019

@daniel-vc

This comment has been minimized.

Copy link

daniel-vc commented Mar 8, 2019

Thanks @ntkme! Will try it later 👍

@danielvanc

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 8, 2019

Seems to be working great. Thanks again 👍

@danielvanc

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 12, 2019

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

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 13, 2019

  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

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 13, 2019

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

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 14, 2019

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

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 14, 2019

Can you try if 1.0.3 still has this problem?

@danielvanc

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 14, 2019

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

@ntkme

This comment has been minimized.

Copy link
Owner

ntkme commented Mar 14, 2019

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:

71f5b7b#diff-168726dbe96b3ce427e7fedce31bb0bcR11

@danielvanc

This comment has been minimized.

Copy link
Author

danielvanc commented Mar 14, 2019

Excellent - good work! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.